-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
macho: allow unaligned offsets in object files #12398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
That's what I meant actually. Just wanted for you to locally test it with |
andrewrk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that it solves the issue for me locally 👍
Excellent! |
| @alignCast(@alignOf(macho.nlist_64), &self.contents[symtab.symoff]), | ||
| )[0..symtab.nsyms]; | ||
| // Sadly, SYMTAB may be at an unaligned offset within the object file. | ||
| self.in_symtab = @alignCast(@alignOf(macho.nlist_64), @ptrCast( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I don't know how I missed this before but these @alignCast are doing the exact same assertion they were before. This patch effectively does nothing. I have no idea why the stage3 build completed successfully for me, but anyway now it is panicking again. I'll push the fix soon 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm surprised as this fixed it for me locally. I'll re-do the tests then.
|
btw could it be perhaps that the objects in question are aligned properly, however, the readToEndAlloc call is only using 8 bytes alignment instead of page alignment? edit: nope, I tested this hypothesis: --- a/src/link/MachO.zig
+++ b/src/link/MachO.zig
@@ -56,6 +56,10 @@ pub const SearchStrategy = enum {
pub const N_DESC_GCED: u16 = @bitCast(u16, @as(i16, -1));
+// x86_64 is 4 KB
+// aarch64 is 16 KB
+const max_page_size = 16 * 1024;
+
const SystemLib = struct {
needed: bool = false,
weak: bool = false,
@@ -1384,7 +1388,7 @@ fn parseObject(self: *MachO, path: []const u8) !bool {
};
const file_stat = try file.stat();
const file_size = math.cast(usize, file_stat.size) orelse return error.Overflow;
- const contents = try file.readToEndAllocOptions(gpa, file_size, file_size, @alignOf(u64), null);
+ const contents = try file.readToEndAllocOptions(gpa, file_size, file_size, max_page_size, null);
var object = Object{
.name = name,
@@ -1495,7 +1499,7 @@ pub fn parseDylib(
try file.seekTo(fat_offset);
file_size -= fat_offset;
- const contents = try file.readToEndAllocOptions(gpa, file_size, file_size, @alignOf(u64), null);
+ const contents = try file.readToEndAllocOptions(gpa, file_size, file_size, max_page_size, null);
defer gpa.free(contents);
const dylib_id = @intCast(u16, self.dylibs.items.len);same issue: |
|
As you discovered, it's not the problem with alignment of the entire object file, but with misaligned symtab that is not always aligned to 8 bytes for some reason. |
When parsing dylibs (binary files as opposed to text-based stubs), every
__LINKEDITdata section has to be aligned to a minimum of@alignOf(u64)as otherwisedyldwill refuse to load it, which means we don't have to relax the requirements for dylib parsing.Closes #12387