Re: [PATCH v2] sh: Make protected symbols local for FDPIC -shared

[Fangrui Song <maskray@google.com>]

On 2024-03-06, Alexandre Oliva wrote:
>On Mar  4, 2024, Fangrui Song <maskray@google.com> wrote:
>
>>     __attribute__((visibility("protected"))) void protected_fun() {}
>>     void *get_protected_fun() { return protected_fun; }
>
>> links fine in the non-FDPIC mode but not in the FDPIC mode:
>
>>     R_SH_GOTOFFFUNCDESC relocation against external symbol "protected_fun"
>
>This appears to be in line with the comment over SYMBOL_FUNCDESC_LOCAL:
>
>>  /* Decide whether a reference to a symbol can be resolved locally or
>>     not.  If the symbol is protected, we want the local address, but
>>     its function descriptor must be assigned by the dynamic linker.  */
>
>If the comment is wrong as your proposed change suggests, it ought to be
>fixed along with the patch that changes this behavior.

The comment looks correct to me, so I did not update it.
However, I may be confused by the meaning of "local address"... See
below.

>But I'd also consider the possibility that it is wrong for the compiler
>to expect a GOTOFF-accessible FD, when the FD is only expected to be
>created at run time by the DL.
>
>A protected symbol is visible by name outside its loadable object, so
>other objects may refer to it and get a dynamic linker-created function
>descriptor that needs to be the canonical one for that symbol.  Which
>implies that the relocation to the function descriptor cannot be at a
>link-time constant GOT offset.
>
>OTOH, it might seem to make sense to create a local noncanonical FD for
>the locally-bound function, to be used only to call the function rather
>than as the function address, like a non-canonical PLT entry, but then,
>if it's only for local calls, we don't need a FD for calls, we could
>just call the function directly.
>
>
>But the quoted C testcase above makes it clear that what is expected is
>indeed the canonical function descriptor, so I think this patch is
>papering over a bug in the compiler, that should be generating code to
>load the canonical FD address from the GOT rather than assuming the
>canonical FD is local to the module just because the symbol binds
>locally.
>
>
>Am I making any sense?

I have some notes about the protected visibility at
https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected#stv_protected
There were some old x86 discussions around GCC 5 and GNU ld 2.26. The
gist is that people use protected visibility for performance. The use
case is typically about -fvisibility=protected that applies to
definitions, serving as a superior alternative to -Wl,-Bsymbolic. If
taking the address of a protected visibility symbol uses GOT-indirect
addressing in -fpic mode (GCC flag_shlib), as slow as the default
visibility, then there would be no point to use the protected visibility
in the first place. (Therefore, I don't know what purpose
`local_protected==0` serves. I think all ports should just use 1.)

There have been some debates what to do when copy relocations/canonical
PLT entries are used together with protected symbol. gold, lld, and
certain BFD ports' interpretation is that copy relocations are simply
incompatible with protected visibility. Thankfully, FDPIC codegen
just forces PIC and avoids copy relocations.

I think __attribute__((visibility("protected"))) is for a definition
within a component, not for a cross-DSO API. Let's say a.so defines
protected `foo`, which is referenced by b.so using the default
visibility symbol. The references within a.so use GOTOFF relocations,
which are resolved to the canonical function descriptor. The references
from b.so use GOT relocations, which will be resolved by the dynamic loader
to a.so's canonical function descriptor. Everything should just work.
If b.so tries a protected undefined symbol, it will lead to a linker
error.

     ld.bfd: a.out: protected symbol `foo' isn't defined
     ld.lld: error: undefined protected symbol: foo