[lld] [ELF] Make start/stop symbols retain associated discardable output sections (PR #96343)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 24 10:33:51 PDT 2024
MaskRay wrote:
> I can see the benefit of the simplification and reducing the amount of code but I still feel a bit uneasy with keeping the empty sections. The actual somewhat unusual use case that showed up this issue was with `ASAN` and an empty `asan_globals` section which produced `__start_asan_globals` and `__stop_asan_globals` with invalid section indices which then caused an error in `llvm-objcopy`.
I agree invalid section indexes should be fixed.
> This patch fixes the invalid section indices but also introduces another minor issue in that the value of the `__start_asan_globals` symbol may no longer be aligned according to `ASAN`'s requirements and can cause a run-time `ASAN` assertion. Normally, the alignment would be correct because of the input section alignment. This can be addressed in the linker script or perhaps the `ASAN` runtime shouldn't validate the start alignment if the size is `0`. It's also highly unlikely that `asan_globals` would be empty, but it does show that there could be side-effects due to this change in behaviour.
I understand the concern and the preference to make a stronger guarantee `start == 0 && stop == 0 || start%sizeof(T)==0 && stop%sizeof(T)==0`.
But I think we can only provide a weaker guarantee: `start == stop || start%sizeof(T)==0 && stop%sizeof(T)==0`.
There are multiple scenarios when lld don't discard an empty output section. For example, with the manual start/stop symbols, we retain the empty output section.
```
foo : { __start_foo = .; *(foo) __stop_foo = .; }
```
This patch unifies this scenario and the auto start/stop symbol scenario.
GNU ld discards empty sections more aggressively and I am convinced that we are not going to support all its rules.
If the asan issue is about passing an unaligned start/stop to `__asan_register_globals`, it seems that the following code should really be changed to `if (start == stop) return;`.
Even without this patch, the change is necessary to support
```
asan_globals : { __start_asan_globals = .; *(asan_globals) __stop_asan_globals = .; }
```
```c
void __asan_register_elf_globals(uptr *flag, void *start, void *stop) {
if (*flag) return;
if (!start) return;
CHECK_EQ(0, ((uptr)stop - (uptr)start) % sizeof(__asan_global));
__asan_global *globals_start = (__asan_global*)start;
__asan_global *globals_stop = (__asan_global*)stop;
__asan_register_globals(globals_start, globals_stop - globals_start);
*flag = 1;
}
```
https://github.com/llvm/llvm-project/pull/96343
More information about the llvm-commits
mailing list