commit b4e0ba6840da790cb42f4ed63b684bbc3959ba55
parent f415cd6cd9859e7ec49c82cf5ac4c368b073d242
Author: Ryan Sepassi <rsepassi@gmail.com>
Date: Tue, 16 Jun 2026 15:13:45 -0700
link: fix scripted-layout coalescing after bss tail + perms union
Two correctness bugs in link_layout_sections_scripted's PT_LOAD
coalescing (commit 45dece38):
Bug 1 (vaddr<->file-offset skew): the can_coalesce gate checked only
vaddr/mem contiguity, not that the open segment's file_size equals its
mem_size. When the open output section ends in an internal NOBITS (bss)
tail, mem_size > file_size, and the coalesce path packs the next
section's file bytes file-contiguously while it keeps a mem-contiguous
vaddr -- skewing the follower's loaded bytes down by the tail size.
Fix: gate coalescing on file_size == mem_size; otherwise fall through
to the existing else branch (a correct, separate page-aligned PT_LOAD).
Bug 2 (dropped permission flags): the coalesce branch never OR'd the
coalescing section's perms into seg->flags (set once at creation). For
a FLAGS-less PHDRS PT_LOAD, the ELF emitter maps seg->flags -> p_flags,
so two :load-shared sections of differing perms (RX .text + RW .data)
emitted a PT_LOAD with only the first section's perms -- dropping W or X
by order. Fix: `seg->flags |= SF_ALLOC | perms` in the coalesce branch
(GNU-ld union semantics).
Tests: check_bss_tail_no_coalesce_skew (Bug 1, asserts _start's bytes
stay file-resident at its vaddr) and check_coalesce_perms_union (Bug 2,
both section orders, asserts the coalesced PT_LOAD is R|W|X).
Diffstat:
2 files changed, 305 insertions(+), 0 deletions(-)
diff --git a/src/link/link_layout.c b/src/link/link_layout.c
@@ -1487,6 +1487,14 @@ static void link_layout_sections_scripted(Linker* l, LinkImage* img,
slice_eq(seg_phdr_name[open_seg], primary_phdr) &&
sec_start_dot == img->segments[open_seg].vaddr +
img->segments[open_seg].mem_size &&
+ /* Bug 1: never coalesce onto an open segment that ends in an internal
+ * NOBITS (bss) tail. Coalescing packs the follower's file bytes
+ * file-contiguously (after oseg->file_size) while it keeps its
+ * mem-contiguous vaddr; with file_size < mem_size that skews the
+ * follower's loaded bytes down by the tail size. Falling through to
+ * the else branch opens a correct, separate page-aligned PT_LOAD. */
+ img->segments[open_seg].file_size ==
+ img->segments[open_seg].mem_size &&
!os->lma && !os->flags;
/* Per-section file geometry (used by both the primary placement and the
@@ -1527,6 +1535,12 @@ static void link_layout_sections_scripted(Linker* l, LinkImage* img,
}
seg->file_size = new_file;
seg->nsections += nsec_in_seg;
+ /* Bug 2: a FLAGS-less PHDRS PT_LOAD maps p_flags from seg->flags
+ * (perms_to_pflags); a coalesced PT_LOAD must carry the UNION of all its
+ * mapped sections' perms (GNU-ld behavior), not just the first section's
+ * (set once at segment creation). OR in this section's perms here. The
+ * load-bearing part is `perms` (SF_EXEC/SF_WRITE/SF_TLS). */
+ seg->flags |= SF_ALLOC | perms;
} else {
seg = &img->segments[img->nsegments];
memset(seg, 0, sizeof(*seg));
diff --git a/test/link/link_script_test.c b/test/link/link_script_test.c
@@ -576,6 +576,295 @@ done:
emit_cleanup(&fx);
}
+/* Build an input object whose .data output-bound content is followed by a
+ * NOBITS .bss tail, so that an output section collecting both gets a PROGBITS
+ * head + NOBITS tail (mem_size > file_size). Defines _start (.text),
+ * data_sym (.data), and bss_sym (.bss) so a linked image exposes their vaddrs. */
+static KitObjBuilder* build_bss_tail_input(KitCompiler* c) {
+ static const uint8_t text_bytes[4] = {0xc0, 0x03, 0x5f, 0xd6};
+ static const uint8_t data_bytes[4] = {1, 2, 3, 4};
+ KitObjBuilder* ob = NULL;
+ KitObjSection text = KIT_SECTION_NONE;
+ KitObjSection data = KIT_SECTION_NONE;
+ KitObjSection bss = KIT_SECTION_NONE;
+ KitObjSymbol sym = KIT_OBJ_SYMBOL_NONE;
+ KitObjSectionDesc desc;
+ KitObjSymbolDesc sym_desc;
+
+ if (kit_obj_builder_new(c, &ob) != KIT_OK || !ob) {
+ EXPECT(0, "obj builder allocation");
+ return NULL;
+ }
+
+ memset(&desc, 0, sizeof(desc));
+ desc.name = kit_sym_intern(c, KIT_SLICE_LIT(".text"));
+ desc.kind = KIT_SEC_TEXT;
+ desc.flags = KIT_SF_ALLOC | KIT_SF_EXEC;
+ desc.align = 4;
+ if (kit_obj_builder_section(ob, &desc, &text) != KIT_OK ||
+ kit_obj_builder_write(ob, text, text_bytes, sizeof(text_bytes)) !=
+ KIT_OK) {
+ EXPECT(0, "create+write .text");
+ goto fail;
+ }
+
+ memset(&desc, 0, sizeof(desc));
+ desc.name = kit_sym_intern(c, KIT_SLICE_LIT(".data"));
+ desc.kind = KIT_SEC_DATA;
+ desc.flags = KIT_SF_ALLOC | KIT_SF_WRITE;
+ desc.align = 4;
+ if (kit_obj_builder_section(ob, &desc, &data) != KIT_OK ||
+ kit_obj_builder_write(ob, data, data_bytes, sizeof(data_bytes)) !=
+ KIT_OK) {
+ EXPECT(0, "create+write .data");
+ goto fail;
+ }
+
+ memset(&desc, 0, sizeof(desc));
+ desc.name = kit_sym_intern(c, KIT_SLICE_LIT(".bss"));
+ desc.kind = KIT_SEC_BSS;
+ desc.flags = KIT_SF_ALLOC | KIT_SF_WRITE;
+ desc.align = 8;
+ if (kit_obj_builder_section(ob, &desc, &bss) != KIT_OK ||
+ kit_obj_builder_reserve_bss(ob, bss, 0x40u, 8u) != KIT_OK) {
+ EXPECT(0, "create+reserve .bss");
+ goto fail;
+ }
+
+ memset(&sym_desc, 0, sizeof(sym_desc));
+ sym_desc.name = kit_sym_intern(c, KIT_SLICE_LIT("_start"));
+ sym_desc.bind = KIT_SB_GLOBAL;
+ sym_desc.kind = KIT_SK_FUNC;
+ sym_desc.section = text;
+ sym_desc.size = sizeof(text_bytes);
+ if (kit_obj_builder_symbol(ob, &sym_desc, &sym) != KIT_OK) {
+ EXPECT(0, "define _start");
+ goto fail;
+ }
+
+ sym = KIT_OBJ_SYMBOL_NONE;
+ memset(&sym_desc, 0, sizeof(sym_desc));
+ sym_desc.name = kit_sym_intern(c, KIT_SLICE_LIT("data_sym"));
+ sym_desc.bind = KIT_SB_GLOBAL;
+ sym_desc.kind = KIT_SK_OBJ;
+ sym_desc.section = data;
+ sym_desc.size = sizeof(data_bytes);
+ if (kit_obj_builder_symbol(ob, &sym_desc, &sym) != KIT_OK) {
+ EXPECT(0, "define data_sym");
+ goto fail;
+ }
+
+ sym = KIT_OBJ_SYMBOL_NONE;
+ memset(&sym_desc, 0, sizeof(sym_desc));
+ sym_desc.name = kit_sym_intern(c, KIT_SLICE_LIT("bss_sym"));
+ sym_desc.bind = KIT_SB_GLOBAL;
+ sym_desc.kind = KIT_SK_OBJ;
+ sym_desc.section = bss;
+ sym_desc.size = 0x40u;
+ if (kit_obj_builder_symbol(ob, &sym_desc, &sym) != KIT_OK) {
+ EXPECT(0, "define bss_sym");
+ goto fail;
+ }
+
+ if (kit_obj_builder_finalize(ob) != KIT_OK) {
+ EXPECT(0, "finalize bss-tail input");
+ goto fail;
+ }
+ return ob;
+
+fail:
+ kit_obj_builder_free(ob);
+ return NULL;
+}
+
+/* Like emit_layout, but uses build_bss_tail_input as the link input. */
+static int emit_layout_with(const char* script_text, size_t script_len,
+ KitObjBuilder* (*build)(KitCompiler*),
+ EmitFixture* fx) {
+ KitTargetSpec target =
+ kit_unit_target(KIT_ARCH_ARM_64, KIT_OS_LINUX, KIT_OBJ_ELF);
+ KitLinkSessionOptions opts;
+ KitSlice script;
+ KitSlice bytes;
+ size_t len = 0;
+
+ memset(fx, 0, sizeof(*fx));
+ script.s = script_text;
+ script.len = script_len;
+ EXPECT(kit_link_script_parse(&g_u.ctx, script, &fx->s) == KIT_OK && fx->s,
+ "layout script parses");
+ if (!fx->s) return 0;
+ EXPECT(kit_unit_compiler_new(&g_u, target, &fx->c) == KIT_OK && fx->c,
+ "compiler allocation");
+ if (!fx->c) return 0;
+ fx->ob = build(fx->c);
+ if (!fx->ob) return 0;
+ memset(&opts, 0, sizeof(opts));
+ opts.output_kind = KIT_LINK_OUTPUT_EXE;
+ opts.linker_script = fx->s;
+ EXPECT(kit_link_session_new(fx->c, &opts, &fx->sess) == KIT_OK && fx->sess,
+ "link session allocation");
+ if (!fx->sess) return 0;
+ EXPECT(kit_link_session_add_obj(fx->sess, fx->ob) == KIT_OK,
+ "add input object");
+ EXPECT(kit_writer_mem(&g_u.heap, &fx->w) == KIT_OK && fx->w, "memory writer");
+ if (!fx->w) return 0;
+ EXPECT(kit_link_session_emit(fx->sess, fx->w) == KIT_OK,
+ "emit scripted image");
+ bytes.data = kit_writer_mem_bytes(fx->w, &len);
+ bytes.len = len;
+ EXPECT(bytes.data && bytes.len > 0, "scripted image bytes");
+ if (!bytes.data || bytes.len == 0) return 0;
+ EXPECT(kit_obj_open(&g_u.ctx, KIT_SLICE_LIT("<scripted-layout>"), &bytes,
+ &fx->f) == KIT_OK &&
+ fx->f,
+ "open scripted image");
+ return fx->f != NULL;
+}
+
+/* Bug 1: an output section with a PROGBITS head + NOBITS (bss) tail must NOT
+ * coalesce a following same-:phdr, vaddr-contiguous output section into the
+ * same PT_LOAD by packing its file bytes file-contiguously — that would skew
+ * the follower's loaded bytes down by the bss-tail size relative to its vaddr.
+ *
+ * .first collects .data (4 PROGBITS bytes) + .bss (0x40 NOBITS bytes) so its
+ * mem_size (0x44) > file_size (4). .second collects .text and shares :ld and is
+ * vaddr-contiguous. We assert no vaddr<->file-offset skew: the segment carrying
+ * .second's bytes must place them so (file_off - seg_file_off) matches
+ * (vaddr - seg_vaddr). The safe fix splits into two PT_LOADs. */
+static void check_bss_tail_no_coalesce_skew(void) {
+ static const char script_text[] =
+ "OUTPUT_FORMAT(\"elf64-littleaarch64\")\n"
+ "OUTPUT_ARCH(aarch64)\n"
+ "ENTRY(_start)\n"
+ "PHDRS {\n"
+ " ld PT_LOAD FLAGS(6);\n"
+ "}\n"
+ "SECTIONS {\n"
+ " . = 0x10000;\n"
+ " .first : { *(.data .data.*) *(.bss .bss.*) } :ld\n"
+ " .second : { *(.text .text.*) } :ld\n"
+ "}\n";
+ EmitFixture fx;
+ KitObjSegIter* it = NULL;
+ KitObjSymInfo text_sym;
+ uint64_t text_vaddr = 0;
+ int saw_text_load = 0;
+ int got_text_sym = 0;
+
+ if (!emit_layout_with(script_text, sizeof(script_text) - 1u,
+ build_bss_tail_input, &fx))
+ goto done;
+
+ got_text_sym = kit_obj_symbol_by_name(fx.f, KIT_SLICE_LIT("_start"),
+ &text_sym) == KIT_OK;
+ EXPECT(got_text_sym, "_start resolves in linked image");
+ if (!got_text_sym) goto done;
+ text_vaddr = text_sym.value;
+
+ EXPECT(kit_obj_segiter_new(fx.f, &it) == KIT_OK && it, "segment iterator");
+ if (!it) goto done;
+ for (;;) {
+ KitObjSegInfo seg;
+ KitIterResult r = kit_obj_segiter_next(it, &seg);
+ if (r == KIT_ITER_END) break;
+ if (r != KIT_ITER_ITEM) break;
+ if (!kit_slice_eq_cstr(seg.name, "LOAD")) continue;
+ /* Find the LOAD segment that contains _start's vaddr. */
+ if (text_vaddr >= seg.vaddr && text_vaddr < seg.vaddr + seg.vsize) {
+ uint64_t voff = text_vaddr - seg.vaddr;
+ saw_text_load = 1;
+ /* The byte for _start must be file-resident at the matching offset:
+ * its file delta must equal its vaddr delta (no NOBITS-tail skew). */
+ EXPECT(voff < seg.file_size,
+ "_start's vaddr maps to a file-resident byte (no bss-tail skew)");
+ /* And the segment's own geometry must be self-consistent: a PROGBITS
+ * follower coalesced after a bss tail would have file_size < the offset
+ * at which its bytes sit. The split-PT_LOAD fix keeps file_size==vsize
+ * for the .text-only segment. */
+ EXPECT(seg.file_size >= voff + 4u,
+ "_start's 4 bytes lie within the segment file image");
+ }
+ }
+ EXPECT(saw_text_load, "found the LOAD segment carrying _start");
+
+done:
+ if (it) kit_obj_segiter_free(it);
+ emit_cleanup(&fx);
+}
+
+/* Bug 2: a FLAGS-less PHDRS PT_LOAD that coalesces an RX section and an RW
+ * section must emit p_flags = R|W|X (the union of mapped sections' perms),
+ * matching GNU ld. Without the perms-union, the single PT_LOAD would carry only
+ * the FIRST section's perms. Two order variants pin this down. */
+static void check_coalesce_perms_union(void) {
+ static const char script_rx_first[] =
+ "OUTPUT_FORMAT(\"elf64-littleaarch64\")\n"
+ "OUTPUT_ARCH(aarch64)\n"
+ "ENTRY(_start)\n"
+ "PHDRS {\n"
+ " ld PT_LOAD;\n"
+ "}\n"
+ "SECTIONS {\n"
+ " . = 0x10000;\n"
+ " .text : { *(.text .text.*) } :ld\n"
+ " .data : { *(.data .data.*) } :ld\n"
+ "}\n";
+ static const char script_rw_first[] =
+ "OUTPUT_FORMAT(\"elf64-littleaarch64\")\n"
+ "OUTPUT_ARCH(aarch64)\n"
+ "ENTRY(_start)\n"
+ "PHDRS {\n"
+ " ld PT_LOAD;\n"
+ "}\n"
+ "SECTIONS {\n"
+ " . = 0x10000;\n"
+ " .data : { *(.data .data.*) } :ld\n"
+ " .text : { *(.text .text.*) } :ld\n"
+ "}\n";
+ const char* variants[2];
+ size_t lens[2];
+ unsigned v;
+ variants[0] = script_rx_first;
+ lens[0] = sizeof(script_rx_first) - 1u;
+ variants[1] = script_rw_first;
+ lens[1] = sizeof(script_rw_first) - 1u;
+
+ for (v = 0; v < 2; ++v) {
+ EmitFixture fx;
+ KitObjSegIter* it = NULL;
+ uint32_t nload = 0;
+ int saw_rwx = 0;
+
+ if (!emit_layout(variants[v], lens[v], &fx)) {
+ emit_cleanup(&fx);
+ continue;
+ }
+ EXPECT(kit_obj_segiter_new(fx.f, &it) == KIT_OK && it, "segment iterator");
+ if (!it) {
+ emit_cleanup(&fx);
+ continue;
+ }
+ for (;;) {
+ KitObjSegInfo seg;
+ KitIterResult r = kit_obj_segiter_next(it, &seg);
+ if (r == KIT_ITER_END) break;
+ if (r != KIT_ITER_ITEM) break;
+ if (!kit_slice_eq_cstr(seg.name, "LOAD")) continue;
+ ++nload;
+ if (seg.perms == (KIT_SEG_R | KIT_SEG_W | KIT_SEG_X)) ++saw_rwx;
+ }
+ EXPECT(nload == 1,
+ v == 0 ? "RX-then-RW coalesces into one PT_LOAD"
+ : "RW-then-RX coalesces into one PT_LOAD");
+ EXPECT(saw_rwx == 1,
+ v == 0 ? "RX-first coalesced PT_LOAD has R|W|X perms (union)"
+ : "RW-first coalesced PT_LOAD has R|W|X perms (union)");
+ if (it) kit_obj_segiter_free(it);
+ emit_cleanup(&fx);
+ }
+}
+
/* Item 5: deeply nested parentheses must fail cleanly (diagnostic) rather than
* overflow the parser/eval stack. */
static void check_recursion_depth_guard(void) {
@@ -610,6 +899,8 @@ int main(void) {
check_intersection_dot_align();
check_multibyte_fill();
check_phdr_coalesce_and_multi();
+ check_bss_tail_no_coalesce_skew();
+ check_coalesce_perms_union();
check_recursion_depth_guard();
kit_unit_summary(&g_u, "link_script_test");
return kit_unit_status(&g_u);
