Repository navigation
Fully support lazy and immediate binding - #335
Conversation
0d1e98b to
3c3998d
Compare
c0c2484 to
e75d599
Compare
These conditions make the workflow modifications somewhat complex. |
e75d599 to
c4fbf69
Compare
| 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) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
In the
x64.mkandarm64.mk,DYN_BIND_NOWis 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.
There was a problem hiding this comment.
6a14a5c implemented the necessary PLT[0], which calls the DL resolver, for the AArch64 backend.
All backends now have their own PLT[0] implementation, so all of them support both lazy binding and immediate binding.
| "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"); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
44553c2 and 5efdd2a have updated the related documents.
However, commit 5efdd2a only updated the use of the -z option and the BINDING variable in the build system for docs/dynamic-linking.md. This document still lacks descriptions of x64 and AArch64 implementations, and I think it would be better to add them in a separate pull request.
These changes remove the use of the DYN_BIND_NOW macro from the codebase. Later commits will introduce a binding option that fully supports both lazy binding and immediate binding for all targets.
c4fbf69 to
5efdd2a
Compare
|
I extend this pull request to fully support lazy and immediate binding:
|
| ifeq ($(DYNLINK),1) | ||
| STAGE0_FLAGS += --dynlink | ||
| STAGE1_FLAGS += --dynlink | ||
| STAGE0_FLAGS += --dynlink -z $(BINDING) |
There was a problem hiding this comment.
Nothing records BINDING, so make DYNLINK=1 followed by make DYNLINK=1 BINDING=now keeps the lazy stage 1 and stage 2 binaries, and the bootstrap comparison still passes because both stages were built the same way. DYNLINK already has this gap. A follow-up could write both values into a stamp file that stage 1 depends on, so switching either one rebuilds the bootstrap.
There was a problem hiding this comment.
This has not yet been resolved. I will think of a proper approach to address it later.
There was a problem hiding this comment.
Since this pull request has been merged, I will submit another one later to address the concern mentioned above.
5efdd2a to
535fd16
Compare
The original AArch64 backend did not generate the PLT stub that invokes _dl_runtime_resolve() so that the dynamic linking with lazy binding was not fully functional. This commit introduces the necessary PLT entry to support dynamic linking and lazy binding for AArch64.
In the previous codebase, the compiler had implemented immediate binding for 64-bit backends using DT_BIND_NOW and DF_BIND_NOW, which are entries in the dynamic section. This commit reintroduces them and extends this binding feature to all backends. When the command-line arguments contain "-z now", the compiler sets its internal flag (imm_binding) to true and generates the necessary Elf32_Dyn/Elf64_Dyn objects so that executables can perform immediate binding at runtime. If "-z lazy" is given, it is still accepted but ignored. However, the compiler will reject compilation if the "-z" option is given in static linking mode.
Since the compiler supports the "-z" option for lazy or immediate binding, a new "BINDING" variable has benn added to the Makefile so that users can choose which binding mode to use. For example, users can build dynamically linked compilers with immediate binding as follows: $ make DYNLINK=1 BINDING=now
Since the compiler has implemented dynamic linking with immediate binding, the test suites have been enhance to validate immediate binding mode. The approach is to add an additional command-line argument to determine which mode, lazy or immediate, is enabled during the tests.
Since the test scripts can validate either lazy binding or immediate binding via an additional argument, the CI workflows are improved so that dynamic linking with immediate binding can be validated on the all architectures. Primary changes and considerations: - Add an additional 'binding_mode' parameter so that the GitHub Actions workflows can verify immediate binding. - When CI tests the static linking mode, the 'binding_mode' parameter is 'none' because static linking involves neither lazy nor immediate binding.
Since the '-z' option has been implemented to enable lazy binding or immediate binding, the README has been revised to document the usage of this new compiler option and the new 'BINDING' variable in the build system.
The '-z' option has been added to enable lazy binding or immediate binding. The dynamic linking document is updated to revise the following descriptions: - Document the usage of the '-z' compiler option and the 'BINDING' variable in the build system. - The original document states that 'DT_BIND_NOW' has not yet been implemented. Remove this statement because this dynamic tag has been introduced.
535fd16 to
9fbbe1b
Compare
|
Thank @DrXiao for contributing! |
Since the immediate binding has already been implemented for the x86-64 and AArch64 architectures, the proposed changes extend its use by introducing a new "-z" command-line option, allowing the Arm32 and RV32 targets to also generate dynamically linked executables with immediate binding.
For the two 64-bit targets, immediate binding is always used under the dynamic linking mode as is.
The update usage:
TODO:
Summary by cubic
Adds a
-z <lazy|now>option (defaultlazy) so all four backends can select lazy or immediate binding in dynamic mode.DYN_BIND_NOWflag is gone;-zdecides binding, and using it under static linking is rejected.BINDINGvariable (defaultlazy) wires the choice through builds and both test scripts; CI exercises dynamic lazy and immediate builds for every architecture.Written for commit 9fbbe1b. Summary will update on new commits.