[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

Doing this on 32-bit Arm would make me nervous as STT_FUNC symbols encode the 
state of Arm/Thumb in the bottom bit, but STT_NOTYPE symbols do not. In 
principle it could be done but extra care would have to be taken to make sure 
no state changes were required. For example caller and callee would need to be 
in the same state. I'm not entirely sure that LLD's current range-extension 
thunks would work to STT_NOTYPE symbols as they use BX IP, which would always 
state change to Arm due to the STT_NOTYPE symbol having the bottom bit clear. 
This is fixable but is the additional complexity and possible fragility of 
older tools worth it?

https://github.com/ARM-software/abi-aa/blob/main/aaelf32/aaelf32.rst#554symbol-names


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D101873#2804669 , @lanza wrote:

> Hey Fangrui, is there any reason this couldn't extend to armv7?

@lanza Always happy when more folks are interested in such kind of stuff:)
This needs backend work. See D101872 . I 
don't have bandwidth working on 32-bit arm :)

Now that we don't optimize variables, the value of -fno-semantic-interposition 
is small.
-fno-semantic-interposition can save a PLT entry (and associated 
`R_*_JUMP_SLOT` dynamic relocation) if a default visibility STB_GLOBAL function 
is only called in its defining TU, not by other TUs linked into the shared 
object.
Its benefit is subsumed by ld -Bsymbolic-non-weak-functions (seems that 
binutils isn't enthusiastic 
https://sourceware.org/pipermail/binutils/2021-May/116753.html)

I asked whether GCC could provide a configure option defaulting 
-fno-semantic-interposition https://gcc.gnu.org/PR100937 and I even sent a 
patch. 
Oops it was immediately closed as a wontfix.
It is so unfortunate that so few people pay attention on performance.
If I still want to try something, my angle has to be ***security hardening** :  
https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572103.html


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

Hey Fangrui, is there any reason this couldn't extend to armv7?


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG68a20c7f36d1: [clang] Support -fpic 
-fno-semantic-interposition for AArch64 (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101873

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsemantic-interposition.c


Index: clang/test/Driver/fsemantic-interposition.c
===
--- clang/test/Driver/fsemantic-interposition.c
+++ clang/test/Driver/fsemantic-interposition.c
@@ -10,6 +10,7 @@
 
 /// If -fno-semantic-interposition is specified and the target supports local
 /// aliases, neither CC1 option is set.
+// RUN: %clang -target aarch64 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target i386 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target x86_64 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=NO %s
 // NO-NOT: "-fsemantic-interposition"
@@ -20,8 +21,8 @@
 /// local aliases, use the traditional half-baked behavor: interprocedural
 /// optimizations are allowed but local aliases are not used. If references are
 /// not optimized out, semantic interposition at runtime is possible.
-// RUN: %clang -target aarch64 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=HALF %s
 // RUN: %clang -target ppc64le %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=HALF %s
+// RUN: %clang -target riscv64 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=HALF %s
 
 // RUN: %clang -target x86_64 %s -Werror -fPIC -c -### 2>&1 | FileCheck 
--check-prefix=HALF %s
 //
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4722,7 +4722,7 @@
  options::OPT_fno_semantic_interposition);
 if (RelocationModel != llvm::Reloc::Static && !IsPIE) {
   // The supported targets need to call AsmPrinter::getSymbolPreferLocal.
-  bool SupportsLocalAlias = Triple.isX86();
+  bool SupportsLocalAlias = Triple.isAArch64() || Triple.isX86();
   if (!A)
 CmdArgs.push_back("-fhalf-no-semantic-interposition");
   else if (A->getOption().matches(options::OPT_fsemantic_interposition))


Index: clang/test/Driver/fsemantic-interposition.c
===
--- clang/test/Driver/fsemantic-interposition.c
+++ clang/test/Driver/fsemantic-interposition.c
@@ -10,6 +10,7 @@
 
 /// If -fno-semantic-interposition is specified and the target supports local
 /// aliases, neither CC1 option is set.
+// RUN: %clang -target aarch64 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target i386 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target x86_64 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
 // NO-NOT: "-fsemantic-interposition"
@@ -20,8 +21,8 @@
 /// local aliases, use the traditional half-baked behavor: interprocedural
 /// optimizations are allowed but local aliases are not used. If references are
 /// not optimized out, semantic interposition at runtime is possible.
-// RUN: %clang -target aarch64 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=HALF %s
 // RUN: %clang -target ppc64le %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=HALF %s
+// RUN: %clang -target riscv64 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=HALF %s
 
 // RUN: %clang -target x86_64 %s -Werror -fPIC -c -### 2>&1 | FileCheck --check-prefix=HALF %s
 //
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4722,7 +4722,7 @@
  options::OPT_fno_semantic_interposition);
 if (RelocationModel != llvm::Reloc::Static && !IsPIE) {
   // The supported targets need to call AsmPrinter::getSymbolPreferLocal.
-  bool SupportsLocalAlias = Triple.isX86();
+  bool SupportsLocalAlias = Triple.isAArch64() || Triple.isX86();
   if (!A)
 CmdArgs.push_back("-fhalf-no-semantic-interposition");
   else if (A->getOption().matches(options::OPT_fsemantic_interposition))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith accepted this revision.
peter.smith added a comment.
This revision is now accepted and ready to land.

LGTM as this is opt in with a command line option.


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In 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 , @MaskRay wrote:
>
>> In D101873#2741903 , @peter.smith 
>> wrote:
>>
>>> In 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 
 . 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__CALL26``, ``R__JUMP26`` or ``R__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 

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

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 , @MaskRay wrote:

> In D101873#2741903 , @peter.smith 
> wrote:
>
>> In 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 . 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.

>> 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__CALL26``, ``R__JUMP26`` or ``R__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.


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D101873#2741903 , @peter.smith 
wrote:

> In 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 . 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?

> 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__CALL26``, ``R__JUMP26`` or ``R__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.


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D101873#2741903 , @peter.smith 
wrote:

> 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.

Ooh, I missed this.  Yes, we need the symbol attributes.  On 32-bit Arm, that 
includes a .thumb_func directive (MCStreamer::emitThumbFunc) in addition to the 
STT_FUNC attribute.


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

In 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 . 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.

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.


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

https://gist.github.com/MaskRay/2d4dfcfc897341163f734afb59f689c6 has more 
information about -fno-semantic-interposition.


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D101873#2738643 , @peter.smith 
wrote:

> I've no comments on the code in D101872  , 
> and D10873  they look reasonable to me. I 
> guess it is down to whether this is the right thing to do or not.
>
> Just to check my understanding:
>
> - Clang defaults to -fno-semantic-interposition (GCC I believe has the 
> opposite default fsemantic-interposition)

Clang defaults to a state between -fno-semantic-interposition and 
-fsemantic-interposition (cc1 -fhalf-no-semantic-interposition): 
interprocedural optimizations are not disabled for default visibility 
definitions, but dso_local is not added.
This has been the traditional behavior for years.

Adding dso_local (-fno-semantic-interposition) will use `.Lfoo$local`, which 
can break a small amount of applications, so I don't want to change the default.

> - With this change code in the same translation unit that defines the global 
> will use a local alias for the global rather than accessing via the GOT, but 
> the global will still be defined with default visibility.

Only when -fno-semantic-interpoistion is explicitly specified, i.e. -fpic 
-fno-semantic-interpoistion

> - Symbol interposing is still permitted at link time so the global can be 
> interposed, but as the code is using a local alias it will still use the 
> original value.
> - Without this chang clang would use fhalf-no-semantic-interposition which I 
> believe permits some assumptions about symbol interpositioning such as 
> resolving some short range assembly pc-relative references to a local alias. 
> These would be out of range if the symbol were interposed anyway.
>
> If I've got this right, particularly the default  then this makes me nervous 
> about the default behaviour as it could silently break some existing code. If 
> a user had to opt in explicitly with -fno-semantic-interposition then fair 
> enough.
>
> Can you let me know, if I'm being overly cautious here? For example are 
> programs that would be affected by this already broken by the 
> half-no-semantic-interpositioning anyway? Is symbol interpositioning so rare 
> that the X86 version of this didn't break anything? Have I got the default of 
> -fno-semantic-interpositioning wrong?

Yes, the first step:) -fno-semantic-interposition is not the default.


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

I've no comments on the code in D101872  , 
and D10873  they look reasonable to me. I 
guess it is down to whether this is the right thing to do or not.

Just to check my understanding:

- Clang defaults to -fno-semantic-interposition (GCC I believe has the opposite 
default fsemantic-interposition)
- With this change code in the same translation unit that defines the global 
will use a local alias for the global rather than accessing via the GOT, but 
the global will still be defined with default visibility.
- Symbol interposing is still permitted at link time so the global can be 
interposed, but as the code is using a local alias it will still use the 
original value.
- Without this chang clang would use fhalf-no-semantic-interposition which I 
believe permits some assumptions about symbol interpositioning such as 
resolving some short range assembly pc-relative references to a local alias. 
These would be out of range if the symbol were interposed anyway.

If I've got this right, particularly the default  then this makes me nervous 
about the default behaviour as it could silently break some existing code. If a 
user had to opt in explicitly with -fno-semantic-interposition then fair enough.

Can you let me know, if I'm being overly cautious here? For example are 
programs that would be affected by this already broken by the 
half-no-semantic-interpositioning anyway? Is symbol interpositioning so rare 
that the X86 version of this didn't break anything? Have I got the default of 
-fno-semantic-interpositioning wrong?


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

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64 Depends on D101872

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: dmgreen, efriedma, peter.smith.
Herald added subscribers: danielkiss, s.egerton, simoncook, kristof.beyls.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101873

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fsemantic-interposition.c


Index: clang/test/Driver/fsemantic-interposition.c
===
--- clang/test/Driver/fsemantic-interposition.c
+++ clang/test/Driver/fsemantic-interposition.c
@@ -10,6 +10,7 @@
 
 /// If -fno-semantic-interposition is specified and the target supports local
 /// aliases, neither CC1 option is set.
+// RUN: %clang -target aarch64 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target i386 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target x86_64 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=NO %s
 // NO-NOT: "-fsemantic-interposition"
@@ -20,8 +21,8 @@
 /// local aliases, use the traditional half-baked behavor: interprocedural
 /// optimizations are allowed but local aliases are not used. If references are
 /// not optimized out, semantic interposition at runtime is possible.
-// RUN: %clang -target aarch64 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=HALF %s
 // RUN: %clang -target ppc64le %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=HALF %s
+// RUN: %clang -target riscv64 %s -Werror -fPIC -fno-semantic-interposition -c 
-### 2>&1 | FileCheck --check-prefix=HALF %s
 
 // RUN: %clang -target x86_64 %s -Werror -fPIC -c -### 2>&1 | FileCheck 
--check-prefix=HALF %s
 //
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4718,7 +4718,7 @@
  options::OPT_fno_semantic_interposition);
 if (RelocationModel != llvm::Reloc::Static && !IsPIE) {
   // The supported targets need to call AsmPrinter::getSymbolPreferLocal.
-  bool SupportsLocalAlias = Triple.isX86();
+  bool SupportsLocalAlias = Triple.isAArch64() || Triple.isX86();
   if (!A)
 CmdArgs.push_back("-fhalf-no-semantic-interposition");
   else if (A->getOption().matches(options::OPT_fsemantic_interposition))


Index: clang/test/Driver/fsemantic-interposition.c
===
--- clang/test/Driver/fsemantic-interposition.c
+++ clang/test/Driver/fsemantic-interposition.c
@@ -10,6 +10,7 @@
 
 /// If -fno-semantic-interposition is specified and the target supports local
 /// aliases, neither CC1 option is set.
+// RUN: %clang -target aarch64 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target i386 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
 // RUN: %clang -target x86_64 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=NO %s
 // NO-NOT: "-fsemantic-interposition"
@@ -20,8 +21,8 @@
 /// local aliases, use the traditional half-baked behavor: interprocedural
 /// optimizations are allowed but local aliases are not used. If references are
 /// not optimized out, semantic interposition at runtime is possible.
-// RUN: %clang -target aarch64 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=HALF %s
 // RUN: %clang -target ppc64le %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=HALF %s
+// RUN: %clang -target riscv64 %s -Werror -fPIC -fno-semantic-interposition -c -### 2>&1 | FileCheck --check-prefix=HALF %s
 
 // RUN: %clang -target x86_64 %s -Werror -fPIC -c -### 2>&1 | FileCheck --check-prefix=HALF %s
 //
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4718,7 +4718,7 @@
  options::OPT_fno_semantic_interposition);
 if (RelocationModel != llvm::Reloc::Static && !IsPIE) {
   // The supported targets need to call AsmPrinter::getSymbolPreferLocal.
-  bool SupportsLocalAlias = Triple.isX86();
+  bool SupportsLocalAlias = Triple.isAArch64() || Triple.isX86();
   if (!A)
 CmdArgs.push_back("-fhalf-no-semantic-interposition");
   else if (A->getOption().matches(options::OPT_fsemantic_interposition))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits