Skip to content

link.MachO.UnwindInfo: Handle u24 overflow for CU records pointing to DWARF. #23011

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

Merged
merged 2 commits into from
Feb 25, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/link/MachO/UnwindInfo.zig
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,18 @@ pub fn generate(info: *UnwindInfo, macho_file: *MachO) !void {
for (info.records.items) |ref| {
const rec = ref.getUnwindRecord(macho_file);
if (rec.getFde(macho_file)) |fde| {
rec.enc.setDwarfSectionOffset(@intCast(fde.out_offset));
// The unwinder will look for the DWARF entry starting at the hint,
// assuming the hint points to a valid CFI record start. If it
// fails to find the record, it proceeds in a linear search through
// the contiguous CFI records from the hint until the end of the
// section. Ideally, in the case where the offset is too large to
// be encoded, we would instead encode the largest possible offset
// to a valid CFI record, but since we don't keep track of that,
// just encode zero -- the start of the section is always the start
// of a CFI record.
const hint = std.math.cast(u24, fde.out_offset) orelse 0;
rec.enc.setDwarfSectionOffset(hint);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find! Is there a test case demonstrating the issue in action I wonder?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have an easy test case, but the following PRs hit it at various points: #21498, #22605, #22488

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


if (fde.getLsdaAtom(macho_file)) |lsda| {
rec.lsda = lsda.atom_index;
rec.lsda_offset = fde.lsda_offset;
Expand Down
Loading