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(-)
>>
>

Reply via email to