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
63 changes: 49 additions & 14 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
# Bootstrap each target and run the consolidated behavioral suite against
# the stage 0 and stage 2 compilers.
host-x86:
name: ${{ matrix.architecture }}/${{ matrix.link_mode }} (${{ matrix.compiler }})
name: ${{ matrix.architecture }}/${{ matrix.link_mode }}/${{ matrix.link_mode == 'dynamic' && (matrix.binding_mode == 'immediate' && 'immediate' || 'default') || 'none' }} (${{ matrix.compiler }})

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Naming:

  • static linking mode: <arch>/static/none.
  • dynamic linking mode: <arch>/dynamic/<default or immediate>

The default binding mode for each architecture is as follows:

  • Arm32: lazy binding
  • RISC-V: lazy binding
  • x64: immediate binding
  • Arm64: immediate binding

runs-on: ubuntu-24.04
timeout-minutes: 30
strategy:
Expand All @@ -32,10 +32,28 @@ jobs:
compiler: [gcc, clang]
architecture: [arm, arm64, riscv, x64]
link_mode: [static, dynamic]
include:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

An include entry whose keys all match an existing combination is merged into that combination instead of creating a new job, so these four entries convert the existing arm/dynamic and riscv/dynamic jobs into immediate-binding runs rather than adding jobs. The default lazy path then has no dynamic coverage on Arm32 or RV32 at all, and the include blocks in sanitizer, host-arm and host-arm-sanitizer behave the same way. Adding binding_mode: [lazy] to the matrix makes the immediate entries conflict, so they expand into separate jobs and the lazy jobs survive.

- compiler: gcc
architecture: arm
link_mode: dynamic
binding_mode: immediate
- compiler: clang
architecture: arm
link_mode: dynamic
binding_mode: immediate
- compiler: gcc
architecture: riscv
link_mode: dynamic
binding_mode: immediate
- compiler: clang
architecture: riscv
link_mode: dynamic
binding_mode: immediate
env:
CC: ${{ matrix.compiler }}
ARCH: ${{ matrix.architecture }}
DYNLINK: ${{ matrix.link_mode == 'dynamic' && '1' || '0' }}
BINDING: ${{ matrix.binding_mode == 'immediate' && 'now' || 'lazy' }}
steps:
- name: Checkout code
uses: actions/checkout@v7
Expand All @@ -49,14 +67,14 @@ jobs:
# environment: the build session the tree records assigns ARCH outright,
# and a makefile assignment wins over the environment.
- name: Build artifacts
run: make ARCH="$ARCH" DYNLINK="$DYNLINK"
run: make ARCH="$ARCH" DYNLINK="$DYNLINK" BINDING="$BINDING"
- name: Unit tests
run: make check ARCH="$ARCH" DYNLINK="$DYNLINK"
run: make check ARCH="$ARCH" DYNLINK="$DYNLINK" BINDING="$BINDING"
- name: Upload the test logs
if: failure()
uses: actions/upload-artifact@v7
with:
name: logs-${{ matrix.compiler }}-${{ matrix.architecture }}-${{ matrix.link_mode }}
name: logs-${{ matrix.compiler }}-${{ matrix.architecture }}-${{ matrix.link_mode }}-${{ matrix.link_mode == 'dynamic' && (matrix.binding_mode == 'immediate' && 'immediate' || 'default') || 'none' }}
path: |
out/*.log
if-no-files-found: ignore
Expand All @@ -68,7 +86,7 @@ jobs:
# sanitizer step of host-x86 used to do. Building "sanitizer" first, in a
# job that builds nothing else, is what puts the instrumentation in.
sanitizer:
name: Sanitizers ${{ matrix.architecture }}/${{ matrix.link_mode }}
name: Sanitizers ${{ matrix.architecture }}/${{ matrix.link_mode }}/${{ matrix.link_mode == 'dynamic' && (matrix.binding_mode == 'immediate' && 'immediate' || 'default') || 'none' }}
runs-on: ubuntu-24.04
timeout-minutes: 30
strategy:
Expand All @@ -82,9 +100,17 @@ jobs:
# why first.
architecture: [arm, arm64, riscv, x64]
link_mode: [static, dynamic]
include:
- architecture: arm
link_mode: dynamic
binding_mode: immediate
- architecture: riscv
link_mode: dynamic
binding_mode: immediate
env:
ARCH: ${{ matrix.architecture }}
DYNLINK: ${{ matrix.link_mode == 'dynamic' && '1' || '0' }}
BINDING: ${{ matrix.binding_mode == 'immediate' && 'now' || 'lazy' }}
steps:
- name: Checkout code
uses: actions/checkout@v7
Expand All @@ -95,9 +121,9 @@ jobs:
link-mode: ${{ matrix.link_mode }}
github-token: ${{ github.token }}
- name: Build the stage 0 compiler with sanitizers
run: make sanitizer ARCH="$ARCH" DYNLINK="$DYNLINK"
run: make sanitizer ARCH="$ARCH" DYNLINK="$DYNLINK" BINDING="$BINDING"
- name: Sanitizer-enabled stage 0 tests
run: make check-sanitizer ARCH="$ARCH" DYNLINK="$DYNLINK"
run: make check-sanitizer ARCH="$ARCH" DYNLINK="$DYNLINK" BINDING="$BINDING"

# Preprocess shecc with itself, then compile the result: the stage 1 source
# is the largest input the preprocessor gets, and the only one that exercises
Expand Down Expand Up @@ -135,17 +161,22 @@ jobs:
# AArch64 that is the only place the real loader is exercised: QEMU-user maps
# the image itself, and on an x86-64 host it always presents 4 KiB pages.
host-arm:
name: ${{ matrix.architecture }}/${{ matrix.link_mode }} on Arm64
name: ${{ matrix.architecture }}/${{ matrix.link_mode }}/${{ matrix.link_mode == 'dynamic' && (matrix.binding_mode == 'immediate' && 'immediate' || 'default') || 'none' }} on Arm64
runs-on: ubuntu-24.04-arm
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
architecture: [arm, arm64]
link_mode: [static, dynamic]
include:
- architecture: arm
link_mode: dynamic
binding_mode: immedaite

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

immedaite never matches the matrix.binding_mode == 'immediate' test that computes BINDING, so this entry merges into the existing arm/dynamic job and leaves it on lazy binding. The native Arm64 runner is the only place a real loader resolves these relocations, so immediate binding ends up untested where it matters most.

Suggested change
binding_mode: immedaite
binding_mode: immediate

env:
ARCH: ${{ matrix.architecture }}
DYNLINK: ${{ matrix.link_mode == 'dynamic' && '1' || '0' }}
BINDING: ${{ matrix.binding_mode == 'immediate' && 'now' || 'lazy' }}
steps:
- name: Checkout code
uses: actions/checkout@v7
Expand All @@ -156,14 +187,14 @@ jobs:
link-mode: ${{ matrix.link_mode }}
github-token: ${{ github.token }}
- name: Build artifacts
run: make ARCH="$ARCH" DYNLINK="$DYNLINK"
run: make ARCH="$ARCH" DYNLINK="$DYNLINK" BINDING="$BINDING"
- name: Unit tests
run: make check ARCH="$ARCH" DYNLINK="$DYNLINK"
run: make check ARCH="$ARCH" DYNLINK="$DYNLINK" BINDING="$BINDING"
- name: Upload the test logs
if: failure()
uses: actions/upload-artifact@v7
with:
name: logs-arm64-host-${{ matrix.architecture }}-${{ matrix.link_mode }}
name: logs-arm64-host-${{ matrix.architecture }}-${{ matrix.link_mode }}-${{ matrix.link_mode == 'dynamic' && (matrix.binding_mode == 'immediate' && 'immediate' || 'default') || 'none' }}
path: |
out/*.log
out/tests/*.log
Expand All @@ -174,15 +205,19 @@ jobs:
# build that produces the object files, and that job has already made them
# without instrumentation.
host-arm-sanitizer:
name: Sanitizers arm/${{ matrix.link_mode }} on Arm64
name: Sanitizers arm/${{ matrix.link_mode }}/${{ matrix.link_mode == 'dynamic' && (matrix.binding_mode == 'immediate' && 'immediate' || 'default') || 'none' }} on Arm64
runs-on: ubuntu-24.04-arm
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
link_mode: [static, dynamic]
include:
- link_mode: dynamic
binding_mode: immediate
env:
DYNLINK: ${{ matrix.link_mode == 'dynamic' && '1' || '0' }}
BINDING: ${{ matrix.binding_mode == 'immediate' && 'now' || 'lazy' }}
steps:
- name: Checkout code
uses: actions/checkout@v7
Expand All @@ -193,9 +228,9 @@ jobs:
link-mode: ${{ matrix.link_mode }}
github-token: ${{ github.token }}
- name: Build the stage 0 compiler with sanitizers
run: make sanitizer ARCH=arm DYNLINK="$DYNLINK"
run: make sanitizer ARCH=arm DYNLINK="$DYNLINK" BINDING="$BINDING"
- name: Sanitizer-enabled stage 0 tests
run: make check-sanitizer ARCH=arm DYNLINK="$DYNLINK"
run: make check-sanitizer ARCH=arm DYNLINK="$DYNLINK" BINDING="$BINDING"

coding-style:
name: Coding style
Expand Down
13 changes: 7 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,13 @@ TRANSLATION_DEFS = "\#define SHECC_TRANSLATION_DATE \"$(TRANSLATION_DATE)\"\n\#d
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)
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
endif

SRCS := $(wildcard $(patsubst %,%/main.c, $(SRCDIR)))
Expand Down Expand Up @@ -236,11 +237,11 @@ uninstall-hooks:

check-stage0: $(OUT)/$(STAGE0) tests/driver.sh
$(VECHO) " TEST STAGE 0\n"
tests/driver.sh 0 $(DYNLINK)
tests/driver.sh 0 $(DYNLINK) $(BINDING)

check-stage2: $(OUT)/$(STAGE2) tests/driver.sh
$(VECHO) " TEST STAGE 2\n"
tests/driver.sh 2 $(DYNLINK)
tests/driver.sh 2 $(DYNLINK) $(BINDING)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

check-stage0 and check-stage2 forward $(BINDING), but check-sanitizer below still calls tests/driver.sh 0 $(DYNLINK), so the sanitizer jobs that set BINDING=now run the suite with lazy binding. Pass $(BINDING) there as well.


check-sanitizer: $(OUT)/$(STAGE0)-sanitizer tests/driver.sh
$(VECHO) " TEST STAGE 0 (with sanitizers)\n"
Expand All @@ -249,10 +250,10 @@ check-sanitizer: $(OUT)/$(STAGE0)-sanitizer tests/driver.sh
$(Q)rm $(OUT)/shecc

check-abi-stage0: $(OUT)/$(STAGE0)
tests/$(ARCH)-abi.sh 0 $(DYNLINK);
tests/$(ARCH)-abi.sh 0 $(DYNLINK) $(BINDING);

check-abi-stage2: $(OUT)/$(STAGE2)
tests/$(ARCH)-abi.sh 2 $(DYNLINK);
tests/$(ARCH)-abi.sh 2 $(DYNLINK) $(BINDING);

# Both prerequisites are order-only, and both exist because "make -j" would
# otherwise let a compile start beside the thing it reads. Selecting a target
Expand Down
1 change: 0 additions & 1 deletion mk/arm64.mk
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,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 @@ -17,7 +17,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 @@ -192,13 +192,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 @@ -1112,14 +1112,17 @@ void elf_generate_dynamic_sections(void)
elf_write_dyn(dynamic_sections.elf_dynamic, 0x1, 0x1);
if (libdl_name)
elf_write_dyn(dynamic_sections.elf_dynamic, 0x1, libdl_name);
#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) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The comment above this branch still states the old compile-time rule. On Arm32 and RV32 the PLT does arrange the GOT[1]/GOT[2] hand-off, and these entries are emitted because the user asked for -z now, not because lazy resolution is impossible. Reword it to say the binding mode is selected by imm_binding and forced on the targets whose PLT has no lazy path.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@jserv I would like to ask about the x64 and Arm64 implementations before improving this pull request.

In the x64.mk and arm64.mk, DYN_BIND_NOW is always enabled, forcing immediate binding. However, if I understand correctly, both of these 64-bit architectures also support lazy binding.

Since I wasn't involved in the review of the related pull requests, could you clarify why these targets are forced to use immediate binding?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Since I wasn't involved in the review of the related pull requests, could you clarify why these targets are forced to use immediate binding?

By the way, the x64 and Arm64 are supported, but the documentation hasn't been updated to explain the implementation considerations. I can help update the documentation in a separate pull request.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In the x64.mk and arm64.mk, DYN_BIND_NOW is always enabled, forcing immediate binding. However, if I understand correctly, both of these 64-bit architectures also support lazy binding.

I was standardizing the behavior across all backends. Feel free to consolidate the linkage logic as well.

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 @@ -116,6 +116,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
26 changes: 24 additions & 2 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,18 @@ 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\"");
i++;
} else if (!strcmp(argv[i], "-E"))
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
expand_only = true;
else if (!strcmp(argv[i], "-I")) {
if (i + 1 >= argc)
Expand All @@ -145,11 +156,22 @@ 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;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

An explicit -z lazy is parsed, accepted, and then silently overridden here, so the user gets immediate binding with no diagnostic, unlike the other incompatible option pairs that usage_error rejects. -z without --dynlink is likewise accepted and does nothing. Recording whether -z was given, then rejecting -z lazy on these targets and -z without --dynlink, keeps the option contract honest.

}
}

if (!in) {
printf(
"Usage: shecc [-I directory] [-o output] [+m] [--dot] [--dump-ir] "
"[--warn-string-literals] [--std=c99] [--no-libc] "
"[--dynlink] [-E] <input.c>\n");
"[--dynlink] [-z <lazy | now>] [-E] <input.c>\n");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The usage string gains -z, but the usage line and option list in README.md still document the old set, and docs/dynamic-linking.md still lists "DT_BIND_NOW (force immediate binding) is not set" under Limitations, which is wrong for every target once this lands. Both belong in this change.

usage_error("Missing source file");
}

Expand Down
6 changes: 4 additions & 2 deletions tests/arm-abi.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ fi

# Command Line Arguments
if [ "$#" -lt 1 ]; then
echo "Usage: $0 <stage> [<dynlink>]"
echo "Usage: $0 <stage> [<dynlink> [<binding>]]"
echo " stage: 0 (host compiler), 1 (stage1), or 2 (stage2)"
echo " dynlink: 0 (static linking), 1 (dynamic linking)"
echo " binding: lazy, now"
echo ""
echo "Environment Variables:"
echo " VERBOSE=1 Enable verbose output"
Expand Down Expand Up @@ -73,6 +74,7 @@ case "$1" in
esac

DYNLINK="${2:-0}"
BINDING="${3:-lazy}"

# Banner
echo -e "${BLUE}${BOLD}========================================${NC}"
Expand Down Expand Up @@ -144,7 +146,7 @@ run_abi_test()
# Compile
local compile_cmd="$SHECC"
if [[ "$DYNLINK" == "1" ]]; then
compile_cmd="$compile_cmd --dynlink"
compile_cmd="$compile_cmd --dynlink -z $BINDING"
Comment thread
DrXiao marked this conversation as resolved.
fi
compile_cmd="$compile_cmd -o /tmp/shecc_abi_test_$$.elf $test_file"

Expand Down
4 changes: 2 additions & 2 deletions tests/arm64-abi.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
set -eu

if [ "$#" -lt 1 ]; then
echo "Usage: $0 <stage> [<dynlink>]" >&2
echo "Usage: $0 <stage> [<dynlink> [<binding>]]" >&2
exit 2
fi

Expand All @@ -23,7 +23,7 @@ case "$1" in
esac

if [ "${2:-0}" = 1 ]; then
shecc+=(--dynlink)
shecc+=(--dynlink -z "${3:-lazy}")
link_mode=dynamic
else
link_mode=static
Expand Down
2 changes: 1 addition & 1 deletion tests/driver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ case "$1" in
esac

if [ $# -ge 2 ] && [ "$2" = "1" ]; then
readonly SHECC_CFLAGS="--dynlink"
readonly SHECC_CFLAGS="--dynlink ${3:-lazy}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

SHECC_CFLAGS gets the binding mode without the -z that introduces it, so shecc sees a bare lazy or now operand. That operand is parsed as an input file and then overwritten by the real source that follows it, so make check BINDING=now runs the whole driver suite with lazy binding and reports success.

Suggested change
readonly SHECC_CFLAGS="--dynlink ${3:-lazy}"
readonly SHECC_CFLAGS="--dynlink -z ${3:-lazy}"

readonly LINK_MODE="dynamic"
else
readonly SHECC_CFLAGS=""
Expand Down
Loading