link/Elf: ensure we always sort all relocations by r_offset in -r mode

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.

src/link/Elf/Object.zig

@@ -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]);
     }

src/link/Elf/eh_frame.zig

@@ -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,
     });
@@ -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 relocs.append(emitReloc(elf_file, rel.r_offset, target, rel));
-            try writer.writeStruct(out_rel);
         }
     }
 
@@ -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 relocs.append(emitReloc(elf_file, r_offset, sym, rel));
-                try writer.writeStruct(out_rel);
             }
         }
 
@@ -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 relocs.append(emitReloc(elf_file, r_offset, sym, rel));
-                try writer.writeStruct(out_rel);
             }
         }
     }

src/link/Elf/relocatable.zig

@@ -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;
@@ -378,15 +386,8 @@ fn writeSyntheticSections(elf_file: *Elf) !void {
             try atom_ptr.writeRelocs(elf_file, &relocs);
         }
         assert(relocs.items.len == num_relocs);
-
+        // Sort output relocations by r_offset which is usually an expected (and desired) condition
-        const SortRelocs = struct {
+        // by the linkers.
-            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;
-            }
-        };
-
         mem.sortUnstable(elf.Elf64_Rela, relocs.items, {}, SortRelocs.lessThan);
 
         log.debug("writing {s} from 0x{x} to 0x{x}", .{
@@ -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;
+        const num_relocs = math.cast(usize, @divExact(shdr.sh_size, shdr.sh_entsize)) orelse
-        var buffer = try std.ArrayList(u8).initCapacity(gpa, sh_size);
+            return error.Overflow;
-        defer buffer.deinit();
+        var relocs = try std.ArrayList(elf.Elf64_Rela).initCapacity(gpa, num_relocs);
-        try eh_frame.writeEhFrameRelocs(elf_file, buffer.writer());
+        defer relocs.deinit();
-        assert(buffer.items.len == sh_size);
+        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);

test/link/elf.zig

@@ -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);
+        const obj = addObject(b, opts, .{ .name = "obj" });
-    obj.addObject(obj2);
+        obj.addObject(obj1);
-    obj.linkLibCpp();
+        obj.addObject(obj2);
+        obj.linkLibCpp();
 
-    const exe = addExecutable(b, opts, .{ .name = "test1" });
+        const exe = addExecutable(b, opts, .{ .name = "test1" });
-    addCppSourceBytes(exe,
+        exe.addObject(obj3);
-        \\#include <iostream>
+        exe.addObject(obj);
-        \\#include <stdexcept>
+        exe.linkLibCpp();
-        \\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 run = addRunArtifact(exe);
+        const run = addRunArtifact(exe);
-    run.expectStdOutEqual("exception=Oh no!");
+        run.expectStdOutEqual("exception=Oh no!");
-    test_step.dependOn(&run.step);
+        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;
 }