MaskRay added a comment.

In D101873#2743987 <https://reviews.llvm.org/D101873#2743987>, @peter.smith 
wrote:

> Thanks for the update.
>
> With the clarification that this isn't breaking aarch64 long range thunks 
> now, and we are not considering Arm then I'm happy for this to happen if the 
> user opts in with -fno-semantic-interposition. I think the longer term 
> question, outside of the scope of this review, about whether 
> -fno-semantic-interposition should be the default is probably one for 
> llvm-dev.
>
> In D101873#2742660 <https://reviews.llvm.org/D101873#2742660>, @MaskRay wrote:
>
>> In D101873#2741903 <https://reviews.llvm.org/D101873#2741903>, @peter.smith 
>> wrote:
>>
>>> In D101873#2741299 <https://reviews.llvm.org/D101873#2741299>, @MaskRay 
>>> wrote:
>>>
>>>> https://gist.github.com/MaskRay/2d4dfcfc897341163f734afb59f689c6 has more 
>>>> information about -fno-semantic-interposition.
>>>>
>>>>> Can Clang default to -fno-semantic-interposition?
>>>>
>>>> I think we can probably make non-x86 default to 
>>>> -fno-semantic-interposition (dso_local inference, given D72197 
>>>> <https://reviews.llvm.org/D72197>. x86 may find default 
>>>> -fno-semantic-interposition too aggressive.
>>>
>>> Thanks for the link, and the explanation that -fno-semantic-interposition 
>>> is not the default.
>>>
>>> I'm not (yet) convinced that we could make -fno-semantic-interposition the 
>>> default, primarily due to data and not functions, I agree that 
>>> interpositioning functions is rarely used. For data the classic example for 
>>> symbol-interposition was errno, a shared library can't know if any other 
>>> library or executable will define it so it must define, but it must use 
>>> only one value for the definition. I'm not sure if that still holds in 
>>> today's environment with shared C libraries used by practically everything 
>>> but I think the principle still applies.
>>
>> errno needs to be thread-local. C11 7.5.2 says "and errno which expands to a 
>> modifiable lvalue that has type int and thread local storage duration, the 
>> value of which is set to a positive error number by several library 
>> functions."
>> Do you mean that in some environment it may be defined in more than one 
>> shared object?
>
> In the general case it is multiple shared libraries include the same static 
> library that has a global variable, in the normal rules only one of these 
> globals will be used, wheras with -fno-semantic-interposition they will all 
> use individual copies. I don't think that this is common as it is not 
> considered good design, it is just an example of how some programs could be 
> broken in subtle ways if the default were changed.

I had been aware that there could be data preemption though I could not find an 
example.
Being aware of potentially such programs I don't intend to flip the default for 
any target without a wider discussion.

Does this patch look good since no default is flipped?

>>> Looking at the gist I've got one concern for AArch64 and Arm. The ABI 
>>> relies on thunks which are only defined for symbols of type STT_FUNC. 
>>> Changing branches to STT_FUNC to STT_SECTION will break long range thunks 
>>> on AArch64 and interworking for Arm (there is a possibility that the bottom 
>>> bit for STT_FUNC may get used in the future for AArch64 as well). This is 
>>> solvable by keeping the local label and setting STT_FUNC on it.
>>
>> I'll unlikely touch 32-bit arm.
>>
>> For aarch64, aaelf64/aaelf64.rst says
>>
>>   A linker may use a veneer (a sequence of instructions) to implement a 
>> relocated branch if the relocation is either
>>   
>>   ``R_<CLS>_CALL26``, ``R_<CLS>_JUMP26`` or ``R_<CLS>_PLT32`` and:
>>   
>>   - The target symbol has type ``STT_FUNC``.
>>   
>>   - Or, the target symbol and relocated place are in separate sections input 
>> to the linker.
>>   
>>   - Or, the target symbol is undefined (external to the link unit).
>>
>> If `bl .Lhigh_target$local` and `.Lhigh_target$local` are in the same 
>> section, the fixup is resolved at assembly time;
>> otherwise, they are in separate sections and satisfy the ABI requirement.
>>
>> I just changed `bl high_target` in `test/lld/ELF/aarch64-thunk-script.s` and 
>> noticed that both GNU ld and ld.lld can produce a thunk, regardless of the 
>> symbol type.
>
> OK, so it looks like the "Or, the target symbol and relocated place are in 
> separate sections input to the linker." can cover AArch64.
>
> An area I didn't want to mention earlier as there is no guarantee it will be 
> part of the architecture, or the ABI is Morello. This introduces capabilities 
> into AArch64 
> https://github.com/ARM-software/abi-aa/blob/main/aaelf64-morello/aaelf64-morello.rst#414symbol-values
>  with an eye to the future where this might be significant. I realise that we 
> can't be hostage to a future that might not come to pass and there can always 
> be "turn fno-semantic-interposition off when Morello is selected" but my 
> instinct is to be cautious as I don't want to make Morello even more 
> difficult than it already is.

The Morello code is not in the upstream. I have no idea how its thunk may 
behave when the symbol type is STT_NOTYPE or STT_FUNC, but we can either treat 
"-fno-semantic-interposition" as a no-op like other non-x86 non-normal-aarch64 
targets, or refine the dso_local logic to make it workable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101873/new/

https://reviews.llvm.org/D101873

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to