Skip to content

Commit

Permalink
link/Elf: ensure we always sort all relocations by r_offset in -r mode
Browse files Browse the repository at this point in the history
According to a comment in mold, this is the expected (and desired)
condition by the linkers, except for some architectures (RISCV and
Loongarch) where this condition does not have to upheld.

If you follow the changes in this patch and in particular doc comments
I have linked the comment/code in mold that explains and implements
this.

I have also modified `testEhFrameRelocatable` test to now test both
cases such that `zig ld -r a.o b.o -o c.o` and `zig ld -r b.o a.o -o
d.o`. In both cases, `c.o` and `d.o` should produce valid object
files which was not the case before this patch.
  • Loading branch information
kubkon authored and andrewrk committed Oct 30, 2024
1 parent 4661705 commit 6ff267d
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 45 deletions.
4 changes: 4 additions & 0 deletions src/link/Elf/Object.zig
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,10 @@ fn parseEhFrame(
defer gpa.free(relocs);
const rel_start: u32 = @intCast(self.relocs.items.len);
try self.relocs.appendUnalignedSlice(gpa, relocs);

// We expect relocations to be sorted by r_offset as per this comment in mold linker:
// https://github.com/rui314/mold/blob/8e4f7b53832d8af4f48a633a8385cbc932d1944e/src/input-files.cc#L653
// Except for RISCV and Loongarch which do not seem to be uphold this convention.
if (target.cpu.arch == .riscv64) {
sortRelocs(self.relocs.items[rel_start..][0..relocs.len]);
}
Expand Down
11 changes: 4 additions & 7 deletions src/link/Elf/eh_frame.zig
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ fn emitReloc(elf_file: *Elf, r_offset: u64, sym: *const Symbol, rel: elf.Elf64_R
};
}

pub fn writeEhFrameRelocs(elf_file: *Elf, writer: anytype) !void {
pub fn writeEhFrameRelocs(elf_file: *Elf, relocs: *std.ArrayList(elf.Elf64_Rela)) !void {
relocs_log.debug("{x}: .eh_frame", .{
elf_file.sections.items(.shdr)[elf_file.section_indexes.eh_frame.?].sh_addr,
});
Expand All @@ -466,8 +466,7 @@ pub fn writeEhFrameRelocs(elf_file: *Elf, writer: anytype) !void {
for (atom_ptr.relocs(elf_file)) |rel| {
const ref = zo.resolveSymbol(rel.r_sym(), elf_file);
const target = elf_file.symbol(ref).?;
const out_rel = emitReloc(elf_file, rel.r_offset, target, rel);
try writer.writeStruct(out_rel);
try relocs.append(emitReloc(elf_file, rel.r_offset, target, rel));
}
}

Expand All @@ -480,8 +479,7 @@ pub fn writeEhFrameRelocs(elf_file: *Elf, writer: anytype) !void {
const ref = object.resolveSymbol(rel.r_sym(), elf_file);
const sym = elf_file.symbol(ref).?;
const r_offset = cie.address(elf_file) + rel.r_offset - cie.offset;
const out_rel = emitReloc(elf_file, r_offset, sym, rel);
try writer.writeStruct(out_rel);
try relocs.append(emitReloc(elf_file, r_offset, sym, rel));
}
}

Expand All @@ -491,8 +489,7 @@ pub fn writeEhFrameRelocs(elf_file: *Elf, writer: anytype) !void {
const ref = object.resolveSymbol(rel.r_sym(), elf_file);
const sym = elf_file.symbol(ref).?;
const r_offset = fde.address(elf_file) + rel.r_offset - fde.offset;
const out_rel = emitReloc(elf_file, r_offset, sym, rel);
try writer.writeStruct(out_rel);
try relocs.append(emitReloc(elf_file, r_offset, sym, rel));
}
}
}
Expand Down
36 changes: 21 additions & 15 deletions src/link/Elf/relocatable.zig
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,14 @@ fn writeSyntheticSections(elf_file: *Elf) !void {
const gpa = elf_file.base.comp.gpa;
const slice = elf_file.sections.slice();

const SortRelocs = struct {
pub fn lessThan(ctx: void, lhs: elf.Elf64_Rela, rhs: elf.Elf64_Rela) bool {
_ = ctx;
assert(lhs.r_offset != rhs.r_offset);
return lhs.r_offset < rhs.r_offset;
}
};

for (slice.items(.shdr), slice.items(.atom_list), 0..) |shdr, atom_list, shndx| {
if (shdr.sh_type != elf.SHT_RELA) continue;
if (atom_list.items.len == 0) continue;
Expand All @@ -378,15 +386,8 @@ fn writeSyntheticSections(elf_file: *Elf) !void {
try atom_ptr.writeRelocs(elf_file, &relocs);
}
assert(relocs.items.len == num_relocs);

const SortRelocs = struct {
pub fn lessThan(ctx: void, lhs: elf.Elf64_Rela, rhs: elf.Elf64_Rela) bool {
_ = ctx;
assert(lhs.r_offset != rhs.r_offset);
return lhs.r_offset < rhs.r_offset;
}
};

// Sort output relocations by r_offset which is usually an expected (and desired) condition
// by the linkers.
mem.sortUnstable(elf.Elf64_Rela, relocs.items, {}, SortRelocs.lessThan);

log.debug("writing {s} from 0x{x} to 0x{x}", .{
Expand Down Expand Up @@ -418,16 +419,21 @@ fn writeSyntheticSections(elf_file: *Elf) !void {
}
if (elf_file.section_indexes.eh_frame_rela) |shndx| {
const shdr = slice.items(.shdr)[shndx];
const sh_size = math.cast(usize, shdr.sh_size) orelse return error.Overflow;
var buffer = try std.ArrayList(u8).initCapacity(gpa, sh_size);
defer buffer.deinit();
try eh_frame.writeEhFrameRelocs(elf_file, buffer.writer());
assert(buffer.items.len == sh_size);
const num_relocs = math.cast(usize, @divExact(shdr.sh_size, shdr.sh_entsize)) orelse
return error.Overflow;
var relocs = try std.ArrayList(elf.Elf64_Rela).initCapacity(gpa, num_relocs);
defer relocs.deinit();
try eh_frame.writeEhFrameRelocs(elf_file, &relocs);
assert(relocs.items.len == num_relocs);
// Sort output relocations by r_offset which is usually an expected (and desired) condition
// by the linkers.
mem.sortUnstable(elf.Elf64_Rela, relocs.items, {}, SortRelocs.lessThan);

log.debug("writing .rela.eh_frame from 0x{x} to 0x{x}", .{
shdr.sh_offset,
shdr.sh_offset + shdr.sh_size,
});
try elf_file.base.file.?.pwriteAll(buffer.items, shdr.sh_offset);
try elf_file.base.file.?.pwriteAll(mem.sliceAsBytes(relocs.items), shdr.sh_offset);
}

try writeComdatGroups(elf_file);
Expand Down
66 changes: 43 additions & 23 deletions test/link/elf.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2744,32 +2744,52 @@ fn testRelocatableEhFrame(b: *Build, opts: Options) *Step {
,
});
obj2.linkLibCpp();
const obj3 = addObject(b, opts, .{ .name = "obj3", .cpp_source_bytes =
\\#include <iostream>
\\#include <stdexcept>
\\extern int try_again();
\\int main() {
\\ try {
\\ try_again();
\\ } catch (const std::exception &e) {
\\ std::cout << "exception=" << e.what();
\\ }
\\ return 0;
\\}
});
obj3.linkLibCpp();

const obj = addObject(b, opts, .{ .name = "obj" });
obj.addObject(obj1);
obj.addObject(obj2);
obj.linkLibCpp();
{
const obj = addObject(b, opts, .{ .name = "obj" });
obj.addObject(obj1);
obj.addObject(obj2);
obj.linkLibCpp();

const exe = addExecutable(b, opts, .{ .name = "test1" });
addCppSourceBytes(exe,
\\#include <iostream>
\\#include <stdexcept>
\\extern int try_again();
\\int main() {
\\ try {
\\ try_again();
\\ } catch (const std::exception &e) {
\\ std::cout << "exception=" << e.what();
\\ }
\\ return 0;
\\}
, &.{});
exe.addObject(obj);
exe.linkLibCpp();
const exe = addExecutable(b, opts, .{ .name = "test1" });
exe.addObject(obj3);
exe.addObject(obj);
exe.linkLibCpp();

const run = addRunArtifact(exe);
run.expectStdOutEqual("exception=Oh no!");
test_step.dependOn(&run.step);
const run = addRunArtifact(exe);
run.expectStdOutEqual("exception=Oh no!");
test_step.dependOn(&run.step);
}
{
// Flipping the order should not influence the end result.
const obj = addObject(b, opts, .{ .name = "obj" });
obj.addObject(obj2);
obj.addObject(obj1);
obj.linkLibCpp();

const exe = addExecutable(b, opts, .{ .name = "test2" });
exe.addObject(obj3);
exe.addObject(obj);
exe.linkLibCpp();

const run = addRunArtifact(exe);
run.expectStdOutEqual("exception=Oh no!");
test_step.dependOn(&run.step);
}

return test_step;
}
Expand Down

0 comments on commit 6ff267d

Please sign in to comment.