Skip to content
Open
Show file tree
Hide file tree
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
5 changes: 3 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ BUILTIN_LIBC_HEADER := c.h
STAGE0_FLAGS ?= --dump-ir
STAGE1_FLAGS ?=
DYNLINK ?= 0
BINDING ?= lazy

COMMENTFLOW ?= commentflow
SHFMT ?= shfmt
ifeq ($(DYNLINK),1)
STAGE0_FLAGS += --dynlink
STAGE1_FLAGS += --dynlink
STAGE0_FLAGS += --dynlink -z $(BINDING)
STAGE1_FLAGS += --dynlink -z $(BINDING)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When a BINDING=now build runs make check, the test suite still exercises lazy binding. The check-stage0/check-stage2/check-abi-* targets forward only $(DYNLINK) to the test drivers, and tests/driver.sh compiles its test programs with a plain --dynlink (no -z now), so on arm/riscv those programs default to lazy binding. With this change BINDING becomes a build knob that stage1/stage2 honor, but the tests built by make check never verify the immediate-binding path this PR adds. Forward $(BINDING) (or a corresponding -z flag) to the driver invocations so immediate binding can actually be validated, matching the PR's TODO about CI validation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Makefile, line 62:

<comment>When a `BINDING=now` build runs `make check`, the test suite still exercises lazy binding. The `check-stage0`/`check-stage2`/`check-abi-*` targets forward only `$(DYNLINK)` to the test drivers, and `tests/driver.sh` compiles its test programs with a plain `--dynlink` (no `-z now`), so on arm/riscv those programs default to lazy binding. With this change `BINDING` becomes a build knob that stage1/stage2 honor, but the tests built by `make check` never verify the immediate-binding path this PR adds. Forward `$(BINDING)` (or a corresponding `-z` flag) to the driver invocations so immediate binding can actually be validated, matching the PR's TODO about CI validation.</comment>

<file context>
@@ -53,12 +53,13 @@ BUILTIN_LIBC_HEADER := c.h
-    STAGE0_FLAGS += --dynlink
-    STAGE1_FLAGS += --dynlink
+    STAGE0_FLAGS += --dynlink -z $(BINDING)
+    STAGE1_FLAGS += --dynlink -z $(BINDING)
 endif
 
</file context>

endif

SRCS := $(wildcard $(patsubst %,%/main.c, $(SRCDIR)))
Expand Down
1 change: 0 additions & 1 deletion mk/arm64.mk
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ ARCH_DEFS = \
\#define PLT_ENT_SIZE 16\n$\
\#define RESERVED_GOT_NUM 3\n$\
\#define R_ARCH_JUMP_SLOT 1026 /* R_AARCH64_JUMP_SLOT */\n$\
\#define DYN_BIND_NOW 1\n$\
"

# An Arm64 Linux host runs this target's output itself, so nothing has to stand
Expand Down
1 change: 0 additions & 1 deletion mk/x64.mk
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ ARCH_DEFS = \
\#define RESERVED_GOT_NUM 3\n$\
\#define R_ARCH_JUMP_SLOT 7 /* R_X86_64_JUMP_SLOT */\n$\
\#define REG_CNT 11 /* rdi rsi rdx rcx r8 r9 rax rbx r14 r12 r13 */\n$\
\#define DYN_BIND_NOW 1 /* this PLT has no lazy-resolution path */\n$\
\#define HAVE_COND_MOVE 1 /* CMOVcc */\n$\
\#define CALLEE_SAVED_REGS 4 /* the file ends rbx r14 r12 r13 */\n$\
"
7 changes: 0 additions & 7 deletions src/defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,6 @@
#define ALIGN_UP(val, align) (((val) + (align) - 1) & ~((align) - 1))
#endif

/* Targets whose PLT has no lazy-resolution path ask the loader to bind every
* PLT entry at load time.
*/
#ifndef DYN_BIND_NOW
#define DYN_BIND_NOW 0
#endif

#define ELF_MACHINE_ARM32 0x28
#define ELF_MACHINE_RV32 0xf3
#define ELF_MACHINE_X86_64 0x3e
Expand Down
11 changes: 7 additions & 4 deletions src/elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1086,14 +1086,17 @@ void elf_generate_dynamic_sections(void)
elf_write_dyn(dynamic_sections.elf_dynamic, 0x3,
dynamic_sections.elf_got_start);
elf_write_dyn(dynamic_sections.elf_dynamic, 0x1, 0x1);
#if DYN_BIND_NOW == 1

/* Resolve every PLT entry at load time. This target's PLT[0] does not
* arrange the GOT[1]/GOT[2] hand-off the lazy resolver needs, so the loader
* writes the final addresses straight into the GOT instead.
*/
elf_write_dyn(dynamic_sections.elf_dynamic, 0x18, 0x0); /* DT_BIND_NOW */
elf_write_dyn(dynamic_sections.elf_dynamic, 0x1e, 0x8); /* DF_BIND_NOW */
#endif
if (imm_binding) {
elf_write_dyn(dynamic_sections.elf_dynamic, 0x18,
0x0); /* DT_BIND_NOW */
elf_write_dyn(dynamic_sections.elf_dynamic, 0x1e,
0x8); /* DF_BIND_NOW */
}
elf_write_dyn(dynamic_sections.elf_dynamic, 0x0, 0x0);
}

Expand Down
1 change: 1 addition & 0 deletions src/globals.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ dynamic_sections_t dynamic_sections;

/* Command line compilation flags */
bool dynlink = false;
bool imm_binding = false;
bool libc = true;
bool expand_only = false;
bool dump_ir = false;
Expand Down
25 changes: 23 additions & 2 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,17 @@ int main(int argc, char *argv[])
libc = false;
else if (!strcmp(argv[i], "--dynlink"))
dynlink = true;
else if (!strcmp(argv[i], "-E"))
else if (!strcmp(argv[i], "-z")) {
if (i + 1 >= argc)
usage_error("-z requires \"lazy\" or \"now\"");

if (!strcmp(argv[i + 1], "lazy"))
imm_binding = false;
else if (!strcmp(argv[i + 1], "now"))
imm_binding = true;
else
usage_error("-z requires \"lazy\" or \"now\"");
} else if (!strcmp(argv[i], "-E"))
Comment on lines +120 to +130

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The -z handler reads argv[i + 1] but never advances i, so its value lazy/now is not consumed and is treated as a positional input on the next loop iteration. Unlike the neighboring -o case (which does i++), this leaves argv[i+1] to hit the trailing else ... in = argv[i] branch. As a result, shecc in.c -z now first sets in = "in.c" and then overwrites it with "now", so the compiler tries to open a file named now and fails; and shecc -z now with no real input never triggers the "Missing source file" error because in is set to "now". Add i++; after the value is consumed, matching the -o handling.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/main.c, line 120:

<comment>The `-z` handler reads `argv[i + 1]` but never advances `i`, so its value `lazy`/`now` is not consumed and is treated as a positional input on the next loop iteration. Unlike the neighboring `-o` case (which does `i++`), this leaves `argv[i+1]` to hit the trailing `else ... in = argv[i]` branch. As a result, `shecc in.c -z now` first sets `in = "in.c"` and then overwrites it with `"now"`, so the compiler tries to open a file named `now` and fails; and `shecc -z now` with no real input never triggers the "Missing source file" error because `in` is set to `"now"`. Add `i++;` after the value is consumed, matching the `-o` handling.</comment>

<file context>
@@ -117,7 +117,17 @@ int main(int argc, char *argv[])
         else if (!strcmp(argv[i], "--dynlink"))
             dynlink = true;
-        else if (!strcmp(argv[i], "-E"))
+        else if (!strcmp(argv[i], "-z")) {
+            if (i + 1 >= argc)
+                usage_error("-z requires \"lazy\" or \"now\"");
</file context>
Suggested change
else if (!strcmp(argv[i], "-z")) {
if (i + 1 >= argc)
usage_error("-z requires \"lazy\" or \"now\"");
if (!strcmp(argv[i + 1], "lazy"))
imm_binding = false;
else if (!strcmp(argv[i + 1], "now"))
imm_binding = true;
else
usage_error("-z requires \"lazy\" or \"now\"");
} else if (!strcmp(argv[i], "-E"))
else if (!strcmp(argv[i], "-z")) {
if (i + 1 >= argc)
usage_error("-z requires \"lazy\" or \"now\"");
if (!strcmp(argv[i + 1], "lazy"))
imm_binding = false;
else if (!strcmp(argv[i + 1], "now"))
imm_binding = true;
else
usage_error("-z requires \"lazy\" or \"now\"");
i++;
}

expand_only = true;
else if (!strcmp(argv[i], "-o")) {
if (i + 1 < argc) {
Expand All @@ -131,10 +141,21 @@ int main(int argc, char *argv[])
in = argv[i];
}

if (dynlink) {
switch (ELF_MACHINE) {
/* The following 64-bit targets have no lazy-resolution path, so
* immediate binding must be used.
*/
case ELF_MACHINE_X86_64:
case ELF_MACHINE_AARCH64:
imm_binding = true;
}
}

if (!in) {
printf(
"Usage: shecc [-o output] [+m] [--dot] [--dump-ir] [--no-libc] "
"[--dynlink] [-E] <input.c>\n");
"[--dynlink] [-z <lazy | now>] [-E] <input.c>\n");
usage_error("Missing source file");
}

Expand Down