Page MenuHomeFreeBSD

mips: report missing symbol rather than segfaulting
ClosedPublic

Authored by emaste on Aug 21 2014, 12:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 7:08 AM
Unknown Object (File)
Sat, Nov 2, 5:44 PM
Unknown Object (File)
Sun, Oct 27, 10:09 AM
Unknown Object (File)
Sat, Oct 26, 2:56 PM
Unknown Object (File)
Oct 22 2024, 11:08 AM
Unknown Object (File)
Oct 22 2024, 2:12 AM
Unknown Object (File)
Oct 10 2024, 3:56 PM
Unknown Object (File)
Oct 4 2024, 6:11 PM
Subscribers
None

Details

Summary

Export die() as _rtld_die(), and use it in _mips_rtld_bind upon failure to resolve a symbol. Previously we would just crash.

Current:
root@erl:/nfs/erl/src/shlib # ./a.out
Segmentation fault (core dumped)

With patch:
root@erl:/nfs/erl/src/shlib # ./a.out
./liblib.so: Undefined symbol "missing_lib_fn"

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

emaste retitled this revision from to mips: report missing symbol rather than segfaulting.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added reviewers: imp, kib.

The code which is fixed looks wrong to me in much more worrying sense. Why mips' plt bind code decided to be so special ? All other architectures use asm trampoline to call _rtld_bind(), which takes care of unresolved symbols, but also it implements correct locking to protect rtld structures in the multithreaded processes, handle ifunc relocations, and do correct logging. All this is missed from mips-specific rewrite of it. If you change the binder to call the proper MI code, the problem you care about should disappear.

Another thing which I noted WRT mips version of _rtld_bind_start, is that it seemingly does not save the full CPU state before calling into C. Look at the comment at the beginning of amd64 version of _rtld_bind_start for explanation why should it be handled this way. I may be wrong there because I have very light understanding of mips asm.

That said, you do not need to rename die(), since it is not exported by your patch either.

Why mips' plt bind code decided to be so special ?

I don't know the answer to that - perhaps @imp can comment on the history?

That said, you do not need to rename die(), since it is not exported by your patch either.

That's fair; I was just following the example set by NetBSD, which has several architecture-specific cases that have to terminate.

emaste edited edge metadata.

Don't rename die().

The proper fix is to migrate to the common _rtld_bind, which will make this change unnecessary. I don't expect to have time to look at that for the next while, so for now just make die() available for _mips_rtld_bind.

imp edited edge metadata.

This looks awesome to me.

This revision is now accepted and ready to land.Apr 4 2015, 7:54 PM
In D661#9, @imp wrote:

This looks awesome to me.

Please read my comment from Aug 21, 2014 above.

In D661#12, @kostikbel wrote:

Please read my comment from Aug 21, 2014 above.

Yes, this part of the MIPS rtld is bogus at the moment, and needs to be redone as mentioned in August. But it's still better to have this change in than not. I just forgot about this change until the arm64 MD rtld_die() topic came up recently.

emaste updated this revision to Diff 4678.

Closed by commit rS281107 (authored by @emaste).