Committed, thanks. Nelson
On Mon, Mar 3, 2025 at 11:51 AM Nelson Chu <nel...@rivosinc.com> wrote: > Looks like we could give it a try and see if it works and won't affect > current projects. I will commit it before this weekend if there are no > objections. > > Thanks > Nelson > > On Tue, Feb 11, 2025 at 1:53 AM Palmer Dabbelt <pal...@rivosinc.com> > wrote: > >> On Sat, 08 Feb 2025 00:33:37 PST (-0800), Nelson Chu wrote: >> > I got an request about the undefined behaviors, considering the >> following case, >> > >> > $ cat test.c >> > void main () >> > { >> > foo(); >> > } >> > $ cat lib.h >> > void foo(void); >> > $ riscv64-unknown-linux-gnu-gcc test.c >> > riscv64-unknown-linux-gnu/bin/ld: /tmp/ccRO8fJl.o: in function `main': >> > test.c:(.text+0x8): undefined reference to `foo' >> > collect2: error: ld returned 1 exit status >> > $ riscv64-unknown-linux-gnu-gcc test.c >> -Wl,--unresolved-symbols=ignore-in-object-files >> > $ qemu-riscv64 a.out >> > Segmentation fault (core dumped) >> > >> > Testing with x86 and aarch64, they won't get the segfault since they go >> plt >> > for the undefined foo symbol. So, after applying this patch, I can get >> the >> > following too, >> > >> > $ qemu-riscv64 a.out >> > a.out: symbol lookup error: a.out: undefined symbol: foo >> > >> > The change of this patch should only affect the call behavior, which >> refer >> > to an undefined (weak) symbol, when building an dynamic executable. I >> think >> > the pic/pie behavior won't be affected as usual. I am not sure if the >> change >> > will cause trouble or not for other projects, so please feels free to >> cc people >> > that you think they will be affected, thanks. >> >> Thanks for doing this. For some more context, there's a handful of Go >> plugins that seem to want `-Wl,--export-dynamic >> -Wl,--unresolved-symbols=ignore-in-object-files` to result in >> executables that have PLT-indirect calls to these undefined symbols, >> which they'll then later LD_PRELOAD or dlopen() to resolve. Here's an >> example >> < >> https://github.com/NVIDIA/nvidia-container-toolkit/blob/b19f5d8f7d76f8ec05534aadfc0c3641bd281d55/internal/dxcore/dxcore.go#L20 >> >. >> >> IMO that's pretty far into the realm of undefined behavior, and from >> poking around a bit it looks like that's the case even on x86 -- >> basically if my symbol gets linked before something has triggered a PLT >> to be created, then I end up with a direct reference that isn't >> dynamically resolvable. >> >> It also looks like arm64 generates NOPs (rather than calls to absolute >> 0) on these undefined symbols, so it's possible some instances of these >> are just crashing lazily. THere might be some context floating around >> in 7cd2917227 ("aarch64: Return an error on conditional branch to an >> undefined symbol"), it's a bit hard to follow so I'm not sure if that's >> an intentional side-effect or just the easiest arbitrary thing to >> generate for this flavor of undefined behavior. >> >> So I'm kind of split on what we should do here: in general I like to >> have undefined behavior crash eagerly, as otherwise we're just making >> these issues harder to debug. That said, this has blown up internally >> and making it crash lazily will make the fire go out, and it'd be really >> nice to start a Monday morning with more fires going out than >> starting... >> >> Maybe there's some more context floating around in someone's brain about >> this? >> >> > --- >> > bfd/elfnn-riscv.c | 84 +++++++++++++++++++++++++---------------------- >> > 1 file changed, 44 insertions(+), 40 deletions(-) >> >