Add support for R_AARCH64_TLSDESC with a symbol, and R_AARCH64_TLS_TPREL64

[andrew]

Summary

These are needed to allocate memory, and when accessing thread local data
across different ELF files.

[kib]

libexec/rtld-elf/aarch64/reloc.c

136 ↗

(On Diff #4549)

Use xmalloc() wrapper to avoid manuall testing the allocation failure and provide standard error message.

175 ↗

(On Diff #4549)

Please use _rtld_error().

177 ↗

(On Diff #4549)

Could you, please, clarify the rules for sharing the tls_data items between threads ? Is it thread-private, or could it be that two threads try to perform rtld_tlsdesc_handle() simultaneously on the same tlsdesc ?

I am mostly wondering about read-lock, is it enough or the case needs write-lock due to modifications to tlsdesc.

Note that it is possible that symbol lookup returns different results for separate invocations.

256 ↗

(On Diff #4549)

Hmm, I think you should pass lockstate down to tld_tlsdesc_handle() and then for find_symdef(), instead of recursively locking the bind lock. In fact,what you do could deadlock.

[andrew]

libexec/rtld-elf/aarch64/reloc.c

175 ↗

(On Diff #4549)

I think we want to print the message set by _rtld_error in find_symdef and exit, something like die, but that's a static function in rtld.c.

177 ↗

(On Diff #4549)

We need to look up the symbol to find it's offset from the thread pointer. The details are in [1], and in this case we are interested in TPREL(S+A) on page 23.

I will need to add the offset of the objects tls data. Currently it returns the offset within the objects data, but this will lead two objects pointing to the same data.

Two threads could call this if they both access the same thread local data concurrently. I'll change it to use a write lock to handle this case. Is the reloc_jmpslots case safe if I use the lock provided, or will I need to upgrade it to a write lock?

[1] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056c/IHI0056C_beta_aaelf64.pdf

205 ↗

(On Diff #4549)

I think this will need to change to:
where[1] = obj->tlsindex + rela->r_addend;

256 ↗

(On Diff #4549)

Ok

[emaste]

libexec/rtld-elf/aarch64/reloc.c

175 ↗

(On Diff #4549)

I think I wanted something like die() in MIPS reloc.c too.

[kib]

libexec/rtld-elf/aarch64/reloc.c

175 ↗

(On Diff #4549)

AFAIR, in MIPS case the use of die() is bogus.

Indeed, https://reviews.freebsd.org/D661

As usual, MIPS people do not care.

177 ↗

(On Diff #4549)

Re-using the provided lockstate should be fine. reloc_jmpslots() is called either by _rtld(), when the process is guaranteed to be single-threaded, or from the dlopen(). In the later case, rtld_bind_lock is write-locked.

What worries me somewhat, is the use of rtld_tlsdesc_handle() from rtld_tlsdesc_dynamic(). I suppose you would need one more wrapper.

[emaste]

libexec/rtld-elf/aarch64/reloc.c

175 ↗

(On Diff #4549)

Ah, indeed that is the change I was thinking about.

[andrew]

I have an update that should fix all the issues, but need to handle the symbol lookup failure case before pushing it here.

libexec/rtld-elf/aarch64/reloc.c

175 ↗

(On Diff #4549)

Is the above patch going in or do I need to do something similar here?

177 ↗

(On Diff #4549)

I have a fix for this, but an unable to push it as I need to decide how best to handle the case above where I need something like die().

[kib]

libexec/rtld-elf/aarch64/reloc.c

175 ↗

(On Diff #4549)

I think you need to expose die(). Now, with the -fvisibility=hidden being used on x86, I am much less sensitive to making functions non-static.

You could consider setting hidden for aarch64.

[emaste]

libexec/rtld-elf/aarch64/reloc.c

175 ↗

(On Diff #4549)

I still think there's some value in renaming it _rtld_die too, from a grepability perspective. NetBSD's rtld for example calls it _rtld_die.

[andrew]

Update based on comments

[emaste]

I'm fine with this change but would like to have @kostikbel's sign-off.

[kib]

No, please just expose die, probably as rtld_die(), without #ifdef hacks. If doing this, please make it a separate commit, which should be merged to stable/10 and 9.

That said, what I do not like right now is that dlopen() of somewhat incorrect dso would kill the process, instead of returning an error.

[andrew]

In D2183#16, @kostikbel wrote:

That said, what I do not like right now is that dlopen() of somewhat incorrect dso would kill the process, instead of returning an error.

How? We only look up the symbol when either we try to access it through _rtld_tlsdesc_dynamic, or when LD_BIND_NOW is set.

[kib]

In D2183#17, @andrew wrote:

In D2183#16, @kostikbel wrote:

That said, what I do not like right now is that dlopen() of somewhat incorrect dso would kill the process, instead of returning an error.

How? We only look up the symbol when either we try to access it through _rtld_tlsdesc_dynamic, or when LD_BIND_NOW is set.

Never mind, I was confused.

[andrew]

Update for r281005

[andrew]

Closed by commit rS281014 (authored by @andrew).