[PATCH] D158822: [Fuchsia] Make __start_* / __stop_* symbols hidden

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158822

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

[PATCH] D157164: [clang-tidy] Add fix-it support to `llvmlibc-inline-function-decl`

[Roland McGrath via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d4162ff28b4: [clang-tidy] Add fix-it support to 
`llvmlibc-inline-function-decl` (authored by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157164

Files:
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
===
--- clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
@@ -17,11 +17,13 @@
 
 constexpr long long addll(long long a, long long b) {
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: LIBC_INLINE constexpr long long addll(long long a, long long b) {
   return a + b;
 }
 
 inline unsigned long addul(unsigned long a, unsigned long b) {
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addul' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: LIBC_INLINE inline unsigned long addul(unsigned long a, unsigned long b) {
   return a + b;
 }
 
@@ -30,11 +32,13 @@
 public:
   MyClass() : A(123) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'MyClass' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  // CHECK-FIXES:   LIBC_INLINE MyClass() : A(123) {}
 
   LIBC_INLINE MyClass(int V) : A(V) {}
 
   constexpr operator int() const { return A; }
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator int' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  // CHECK-FIXES:   LIBC_INLINE constexpr operator int() const { return A; }
 
   LIBC_INLINE bool operator==(const MyClass &RHS) {
 return RHS.A == A;
@@ -42,6 +46,7 @@
 
   static int getVal(const MyClass &V) {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'getVal' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  // CHECK-FIXES:   LIBC_INLINE static int getVal(const MyClass &V) {
 return V.A;
   }
 
@@ -51,6 +56,7 @@
 
   constexpr static int addInt(MyClass &V, int A) {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'addInt' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  // CHECK-FIXES:   LIBC_INLINE constexpr static int addInt(MyClass &V, int A) {
 return V.A += A;
   }
 
@@ -78,6 +84,7 @@
 
 inline void badSimpleFunction() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'badSimpleFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: LIBC_INLINE inline void badSimpleFunction() {}
 
 void LIBC_INLINE badSimpleFunctionWrongLocation() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'badSimpleFunctionWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
@@ -93,6 +100,7 @@
 
 template  inline void badTemplateFunction() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badTemplateFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: template  LIBC_INLINE inline void badTemplateFunction() {}
 
 template  void LIBC_INLINE badTemplateFunctionWrongLocation() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badTemplateFunctionWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
@@ -108,9 +116,11 @@
 
 template  inline void badVariadicFunction() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: template  LIBC_INLINE inline void badVariadicFunction() {}
 
 template  void LIBC_INLINE badVariadicFunctionWrongLocation() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicFunctionWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: template  LIBC_INLINE void LIBC_INLINE badVariadicFunctionWrongLocation() {}
 

[PATCH] D157164: [clang-tidy] Add fix-it support to `llvmlibc-inline-function-decl`

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: abrachet, Caslyn, sivachandra, michaelrj.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
mcgrathr requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This is very simplistic and could be more thorough by replacing
an existing `LIBC_INLINE` in the wrong location or a redunant
`inline` when inserting the right macro use.  But as is this
suffices to automatically apply fixes for most or all of the
instances in the libc tree today and get working results (despite
some superfluous `inline` keywords left behind).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157164

Files:
  clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
===
--- clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
+++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc/inline-function-decl.hpp
@@ -17,11 +17,13 @@
 
 constexpr long long addll(long long a, long long b) {
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addll' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: LIBC_INLINE constexpr long long addll(long long a, long long b) {
   return a + b;
 }
 
 inline unsigned long addul(unsigned long a, unsigned long b) {
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'addul' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: LIBC_INLINE inline unsigned long addul(unsigned long a, unsigned long b) {
   return a + b;
 }
 
@@ -30,11 +32,13 @@
 public:
   MyClass() : A(123) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'MyClass' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  // CHECK-FIXES:   LIBC_INLINE MyClass() : A(123) {}
 
   LIBC_INLINE MyClass(int V) : A(V) {}
 
   constexpr operator int() const { return A; }
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator int' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  // CHECK-FIXES:   LIBC_INLINE constexpr operator int() const { return A; }
 
   LIBC_INLINE bool operator==(const MyClass &RHS) {
 return RHS.A == A;
@@ -42,6 +46,7 @@
 
   static int getVal(const MyClass &V) {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'getVal' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  // CHECK-FIXES:   LIBC_INLINE static int getVal(const MyClass &V) {
 return V.A;
   }
 
@@ -51,6 +56,7 @@
 
   constexpr static int addInt(MyClass &V, int A) {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'addInt' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+  // CHECK-FIXES:   LIBC_INLINE constexpr static int addInt(MyClass &V, int A) {
 return V.A += A;
   }
 
@@ -78,6 +84,7 @@
 
 inline void badSimpleFunction() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'badSimpleFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: LIBC_INLINE inline void badSimpleFunction() {}
 
 void LIBC_INLINE badSimpleFunctionWrongLocation() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'badSimpleFunctionWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
@@ -93,6 +100,7 @@
 
 template  inline void badTemplateFunction() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badTemplateFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CHECK-FIXES: template  LIBC_INLINE inline void badTemplateFunction() {}
 
 template  void LIBC_INLINE badTemplateFunctionWrongLocation() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'badTemplateFunctionWrongLocation' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
@@ -108,9 +116,11 @@
 
 template  inline void badVariadicFunction() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'badVariadicFunction' must be tagged with the LIBC_INLINE macro; the macro should be placed at the beginning of the declaration [llvmlibc-inline-function-decl]
+// CH

[PATCH] D155337: [CMake] Include riscv32-unknown-elf target in Fuchsia toolchain

[Roland McGrath via Phabricator via cfe-commits]

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

It would be a better summary to say "include ... runtimes ...".




Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:10
 set(_FUCHSIA_ENABLE_PROJECTS "bolt;clang;clang-tools-extra;lld;llvm;polly")
-set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING 
"")
+set(LLVM_ENABLE_RUNTIMES "compiler-rt;libc;libcxx;libcxxabi;libunwind" CACHE 
STRING "")
 

Does this have any effect on what is included in any of the other target lib 
directories?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155337

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

[PATCH] D152405: [WIP][clang] Add experimental option to omit the RTTI component from the vtable when -fno-rtti is used

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In D152405#4404616 , @leonardchan 
wrote:

> Oh this is completely independent from relative vtables. I'll update the 
> wording.

Great. I'd like to see us try some experiments with enabling both together in 
places like the Fuchsia kernel where we get benefit from relative-vtables today 
but it's all a single fungible internal ABI domain and there's no problem 
having a one-off ABI. It's also interesting of course for true embedded 
contexts, but those tend to be ILP32 where relative-vtables isn't a size 
savings anyway. In the Fuchsia kernel we should get some size savings from this 
and it would be nice to see what that is (probably not all that big, but 
interesting).

Can we extend the test cases to exercise both with and without relative-vtables 
also enabled?

>> Does any runtime code (libc++abi) ever need to know the vtable layout 
>> details when actually making use of RTTI, such that it would need to adapt 
>> to this ABI change?
>
> `__dynamic_cast` is the only thing that comes to mind, but since this is 
> gated on `-fno-rtti` where dynamic_cast is disabled, then I think libc++abi 
> doesn't need to worry about this.

Perfect. This makes this a good free-floating option for any context like 
kernel or embedded where all code (even if libc++/libc++abi are included) are 
built from scratch using the same switches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152405

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

[PATCH] D152405: [WIP][clang] Add experimental option to omit the RTTI component from the vtable when -fno-rtti is used

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

It's not clear from the change description if this can be enabled orthogonally 
to relative-vtables. I think it should be possible to choose each switch 
independently, thus generating 4 variants of the vtable layout ABI.

Does any runtime code (libc++abi) ever need to know the vtable layout details 
when actually making use of RTTI, such that it would need to adapt to this ABI 
change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152405

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

[PATCH] D151186: [Driver] Properly handle -pie and -nopie on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In GCC the spelling is `-no-pie`. `-nopie` is not documented as an alias, and I 
don't think it works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151186

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

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.

lgtm with some nits and a few more test cases.




Comment at: clang/docs/ShadowCallStack.rst:61
+The instrumentation makes use of the platform register ``x18`` on AArch64 and
+``x3`` on RISC-V. For simplicity we will refer to this as the ``SCSReg`` On
+some platforms, ``SCSReg`` is reserved, and on others, it is designated as a

I think it would be worthwhile to say `x3 (gp)` here and perhaps everywhere 
that `x3` is mentioned.
Since `gp` is always a reserved register from the compiler point of view, I 
think some different verbiage here about the RISC-V case and the interactions 
with `-msmall-data-limit` and linker behavior make sense.





Comment at: clang/lib/Driver/SanitizerArgs.cpp:307
 
+bool shadowCallStackDSLConflicts(const llvm::opt::ArgList &Args,
+ const llvm::Triple &TT) {

I'm guessing that "DSL" stands for "data small limit" or something, which is 
confusing since the thing is called small-data-limit, not data-small-limit. 
Anyway, this is the first use I've seen of DSL as an abbreviation related to 
any of this. I think it would be clear enough to just call it 
`shadowCallStackConflicts` since it's responsible for detecting conflicts 
between shadow-call-stack and whatever is relevant to that in the particular 
target.




Comment at: clang/lib/Driver/SanitizerArgs.cpp:314
+  if (Arg *A = Args.getLastArg(options::OPT_G))
+if (0 != strcmp("0", A->getValue()))
+  return true;

This seems like it ought to parse the integer and then compare it to zero as an 
integer, rather than relying on one exact string representation of zero.




Comment at: clang/lib/Driver/SanitizerArgs.cpp:560
+  if (Kinds & SanitizerKind::ShadowCallStack) {
+if (TC.getTriple().isAArch64() &&
+!llvm::AArch64::isX18ReservedByDefault(TC.getTriple()) &&

I'd just move all this per-target logic inside `shadowCallStackConflicts` and 
maybe just make it a void function called `checkShadowCallStackConflcits` or 
something.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2063
+  // Default small data limitation on some platforms(Android and Fuchsia) is
+  // zero, otherwise its eight.
+  const char *SmallDataLimit =

typo: "it's"



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2065
+  const char *SmallDataLimit =
+  llvm::RISCV::isSmallDataLimitZeroByDefault(Triple) ? "0" : "8";
   // Get small data limitation.

I wonder if it wouldn't be more natural for the target hook to be 
`defaultSmallDataLimit` and return an integer where the base class returns 8 
and the android/fuchsia subclasses return 0. It seems quite plausible that 
eventually different subtargets might have other tunings (for subtargets that 
do want to use gp relaxation).




Comment at: clang/test/Driver/sanitizer-ld.c:746
 
 // RUN: %clang -target riscv64-linux-android -fsanitize=shadow-call-stack %s 
-### 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-SHADOWCALLSTACK-ANDROID-RISCV64

I'd also add tests here for android and fuchsia targets with explicit 
`-msmall-data-limit=` being rejected by default but being allowed with 
`-fno-sanitize=shadow-call-stack`.

There should also be tests independent of shadow-call-stack that just verify at 
least for fuchsia targets that `-msmall-data-limit=0` is passed by default for 
the default (PIE) case. Actually, it should also test that this remains so when 
`-fno-sanitize=shadow-call-stack` is passed, because it's still the standard 
target ABI that `gp` might be used for shadow-call-stack by interoperating code 
and so `gp` should never be used for something else without explicit 
`-msmall-data-limit=...` on fuchsia.





Comment at: llvm/docs/ReleaseNotes.rst:147
 * Added support for the vendor-defined XTHeadFMemIdx (indexed memory 
operations for floating point) extension.
+* Changed the ShadowCallStack register from x18(s2) to x3(gp). Note this breaks
+  the existing non-standard ABI for ShadowCallStack, but conforms with the new

Spaces before the parenthetical clauses.




Comment at: llvm/include/llvm/TargetParser/RISCVTargetParser.h:43
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector 
&Features);
-
-bool isX18ReservedByDefault(const Triple &TT);
+bool isSmallDataLimitZeroByDefault(const Triple &TT);
 

Why not return an integer value instead of "is zero"?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146463

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

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I don't see changes here to make the Fuchsia & Android targets stop doing 
implicit `-ffixed-x18` and to make them start doing implicit 
`-msmall-data-limit=0` for non-PIC modes.




Comment at: clang/lib/Driver/SanitizerArgs.cpp:546
 
-  if ((Kinds & SanitizerKind::ShadowCallStack) &&
-  ((TC.getTriple().isAArch64() &&
-!llvm::AArch64::isX18ReservedByDefault(TC.getTriple())) ||
-   (TC.getTriple().isRISCV() &&
-!llvm::RISCV::isX18ReservedByDefault(TC.getTriple( &&
+  if ((Kinds & SanitizerKind::ShadowCallStack) && TC.getTriple().isAArch64() &&
+  !llvm::AArch64::isX18ReservedByDefault(TC.getTriple()) &&

For RISC-V this needs a similar check that the equivalent of 
`-msmall-data-limit=0` is in force (that's the default under `-fPIC` but not 
other modes).




Comment at: llvm/test/CodeGen/RISCV/reserved-regs.ll:60
 
-; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s 
-check-prefix=X18
-; RUN: llc -mtriple=riscv64-linux-android -verify-machineinstrs < %s | 
FileCheck %s -check-prefix=X18
+;; Check that targets that reserve a register by default reserve the correct 
registers
+;; Android and Fuchsia reserve the ShadowCallStack register (gp/x3) by default.

gp is a fixed register for all RISC-V targets. What's important to check is 
that Fuchsia and Android disable the small-data-section behavior.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146463

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

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In D146463#4232252 , @craig.topper 
wrote:

> In D146463#4232221 , @mcgrathr 
> wrote:
>
>> In D146463#4232214 , @craig.topper 
>> wrote:
>>
>>> Does the kernel populate the initial value of the register for the shadow 
>>> stack or is that done by a runtime inside the application?
>>
>> This is a change to code generation and is orthogonal to how the runtime 
>> environment supports the ABI requirement. The details depend on the 
>> particular runtime environment, which is not the province of the compiler 
>> backend.
>
> I ask because GP is set in crtbegin.o on Linux today. So if its the kernel 
> that populates it, then GP isn't valid choice for Linux without changing 
> crtbegin.

To use ShadowCallStack, executables need to be built with 
`-msmall-data-limit=0` and the runtime environment they use needs to meet the 
ABI.  Neither of those things is provided out of the box for Linux, just as on 
AArch64 neither building with `-ffixed-x18` nor a runtime environment meeting 
the ABI exists out of the box on Linux. If a runtime environment for Linux were 
to support the ShadowCallStack ABI, that would be done in startup code that 
runs after the code injected by crtbegin.o and before any code built for the 
ShadowCallStack is expected to be usable, such as in the libc startup code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146463

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

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In D146463#4232214 , @craig.topper 
wrote:

> Does the kernel populate the initial value of the register for the shadow 
> stack or is that done by a runtime inside the application?

This is a change to code generation and is orthogonal to how the runtime 
environment supports the ABI requirement. The details depend on the particular 
runtime environment, which is not the province of the compiler backend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146463

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

[PATCH] D143357: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf08d86fc7f44: [RISCV] Default to 
-fsanitize=shadow-call-stack for Fuchsia (authored by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143357

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -31,6 +31,7 @@
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-RISCV64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-AARCH64: "-target-feature" "+outline-atomics"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -466,13 +466,13 @@
   SanitizerMask Res;
   switch (getTriple().getArch()) {
   case llvm::Triple::aarch64:
+  case llvm::Triple::riscv64:
 Res |= SanitizerKind::ShadowCallStack;
 break;
   case llvm::Triple::x86_64:
 Res |= SanitizerKind::SafeStack;
 break;
   default:
-// TODO: Enable SafeStack on RISC-V once tested.
 break;
   }
   return Res;


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -31,6 +31,7 @@
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-RISCV64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-AARCH64: "-target-feature" "+outline-atomics"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -466,13 +466,13 @@
   SanitizerMask Res;
   switch (getTriple().getArch()) {
   case llvm::Triple::aarch64:
+  case llvm::Triple::riscv64:
 Res |= SanitizerKind::ShadowCallStack;
 break;
   case llvm::Triple::x86_64:
 Res |= SanitizerKind::SafeStack;
 break;
   default:
-// TODO: Enable SafeStack on RISC-V once tested.
 break;
   }
   return Res;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG03ff435da540: [RISCV] Default to -ffixed-x18 for Fuchsia 
(authored by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143355

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/sanitizer-ld.c
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/test/CodeGen/RISCV/reserved-regs.ll

Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- llvm/test/CodeGen/RISCV/reserved-regs.ll
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -57,6 +57,8 @@
 ; RUN: llc -mtriple=riscv32 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 
+; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
 
Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -14,6 +14,7 @@
 #include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/TargetParser/Triple.h"
 
 namespace llvm {
 namespace RISCV {
@@ -100,5 +101,10 @@
   return true;
 }
 
+bool isX18ReservedByDefault(const Triple &TT) {
+  // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
+  return TT.isOSFuchsia();
+}
+
 } // namespace RISCV
 } // namespace llvm
Index: llvm/lib/Target/RISCV/RISCVSubtarget.cpp
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.cpp
+++ llvm/lib/Target/RISCV/RISCVSubtarget.cpp
@@ -83,6 +83,9 @@
   FrameLowering(
   initializeSubtargetDependencies(TT, CPU, TuneCPU, FS, ABIName)),
   InstrInfo(*this), RegInfo(getHwMode()), TLInfo(TM, *this) {
+  if (RISCV::isX18ReservedByDefault(TT))
+UserReservedRegister.set(RISCV::X18);
+
   CallLoweringInfo.reset(new RISCVCallLowering(*getTargetLowering()));
   Legalizer.reset(new RISCVLegalizerInfo(*this));
 
Index: llvm/include/llvm/TargetParser/RISCVTargetParser.h
===
--- llvm/include/llvm/TargetParser/RISCVTargetParser.h
+++ llvm/include/llvm/TargetParser/RISCVTargetParser.h
@@ -18,6 +18,9 @@
 #include 
 
 namespace llvm {
+
+class Triple;
+
 namespace RISCV {
 
 // We use 64 bits as the known part in the scalable vector types.
@@ -38,6 +41,8 @@
 void fillValidTuneCPUArchList(SmallVectorImpl &Values, bool IsRV64);
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector &Features);
 
+bool isX18ReservedByDefault(const Triple &TT);
+
 } // namespace RISCV
 } // namespace llvm
 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -731,6 +731,11 @@
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-RISCV64 %s
 // CHECK-SHADOWCALLSTACK-LINUX-RISCV64: '-fsanitize=shadow-call-stack' only allowed with '-ffixed-x18'
 
+// RUN: %clang -fsanitize=shadow-call-stack -### %s 2>&1 \
+// RUN: --target=riscv64-unknown-fuchsia -fuse-ld=ld \
+// RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-FUCHSIA-RISCV64 %s
+// CHECK-SHADOWCALLSTACK-FUCHSIA-RISCV64-NOT: error:
+
 // RUN: %clang -fsanitize=shadow-call-stack -### %s 2>&1 \
 // RUN: --target=aarch64-unknown-linux -fuse-ld=ld -ffixed-x18 \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
+#include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include 
 
@@ -545,7 +546,8 @@
   if ((Kinds & SanitizerKind::ShadowCallStack) &&
   ((TC.getTriple().isAArch64() &&
 !llvm::AArch64::isX18ReservedByDefault(TC.getTriple())) ||
-   TC.getTriple().isRISCV()) &&
+   (TC.getTriple().isRISCV() &&
+!llvm::RISCV::isX18ReservedByDefault(TC.getTriple( &&
   !Args.hasArg(options::OPT_ffixed_x18) && DiagnoseErrors) {
 D.Diag(diag::err_drv_argument_only_

[PATCH] D143357: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 494973.
mcgrathr marked an inline comment as done.
mcgrathr added a comment.

remove TODO


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143357

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -31,6 +31,7 @@
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-RISCV64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-AARCH64: "-target-feature" "+outline-atomics"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -466,13 +466,13 @@
   SanitizerMask Res;
   switch (getTriple().getArch()) {
   case llvm::Triple::aarch64:
+  case llvm::Triple::riscv64:
 Res |= SanitizerKind::ShadowCallStack;
 break;
   case llvm::Triple::x86_64:
 Res |= SanitizerKind::SafeStack;
 break;
   default:
-// TODO: Enable SafeStack on RISC-V once tested.
 break;
   }
   return Res;


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -31,6 +31,7 @@
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-RISCV64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-AARCH64: "-target-feature" "+outline-atomics"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -466,13 +466,13 @@
   SanitizerMask Res;
   switch (getTriple().getArch()) {
   case llvm::Triple::aarch64:
+  case llvm::Triple::riscv64:
 Res |= SanitizerKind::ShadowCallStack;
 break;
   case llvm::Triple::x86_64:
 Res |= SanitizerKind::SafeStack;
 break;
   default:
-// TODO: Enable SafeStack on RISC-V once tested.
 break;
   }
   return Res;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 494972.
mcgrathr added a comment.

rebased, added clang driver test vs -fsanitize=shadow-call-stack


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143355

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/sanitizer-ld.c
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/test/CodeGen/RISCV/reserved-regs.ll

Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- llvm/test/CodeGen/RISCV/reserved-regs.ll
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -57,6 +57,8 @@
 ; RUN: llc -mtriple=riscv32 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 
+; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
 
Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -14,6 +14,7 @@
 #include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/TargetParser/Triple.h"
 
 namespace llvm {
 namespace RISCV {
@@ -100,5 +101,10 @@
   return true;
 }
 
+bool isX18ReservedByDefault(const Triple &TT) {
+  // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
+  return TT.isOSFuchsia();
+}
+
 } // namespace RISCV
 } // namespace llvm
Index: llvm/lib/Target/RISCV/RISCVSubtarget.cpp
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.cpp
+++ llvm/lib/Target/RISCV/RISCVSubtarget.cpp
@@ -83,6 +83,9 @@
   FrameLowering(
   initializeSubtargetDependencies(TT, CPU, TuneCPU, FS, ABIName)),
   InstrInfo(*this), RegInfo(getHwMode()), TLInfo(TM, *this) {
+  if (RISCV::isX18ReservedByDefault(TT))
+UserReservedRegister.set(RISCV::X18);
+
   CallLoweringInfo.reset(new RISCVCallLowering(*getTargetLowering()));
   Legalizer.reset(new RISCVLegalizerInfo(*this));
 
Index: llvm/include/llvm/TargetParser/RISCVTargetParser.h
===
--- llvm/include/llvm/TargetParser/RISCVTargetParser.h
+++ llvm/include/llvm/TargetParser/RISCVTargetParser.h
@@ -18,6 +18,9 @@
 #include 
 
 namespace llvm {
+
+class Triple;
+
 namespace RISCV {
 
 // We use 64 bits as the known part in the scalable vector types.
@@ -38,6 +41,8 @@
 void fillValidTuneCPUArchList(SmallVectorImpl &Values, bool IsRV64);
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector &Features);
 
+bool isX18ReservedByDefault(const Triple &TT);
+
 } // namespace RISCV
 } // namespace llvm
 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -731,6 +731,11 @@
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-RISCV64 %s
 // CHECK-SHADOWCALLSTACK-LINUX-RISCV64: '-fsanitize=shadow-call-stack' only allowed with '-ffixed-x18'
 
+// RUN: %clang -fsanitize=shadow-call-stack -### %s 2>&1 \
+// RUN: --target=riscv64-unknown-fuchsia -fuse-ld=ld \
+// RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-FUCHSIA-RISCV64 %s
+// CHECK-SHADOWCALLSTACK-FUCHSIA-RISCV64-NOT: error:
+
 // RUN: %clang -fsanitize=shadow-call-stack -### %s 2>&1 \
 // RUN: --target=aarch64-unknown-linux -fuse-ld=ld -ffixed-x18 \
 // RUN:   | FileCheck --check-prefix=CHECK-SHADOWCALLSTACK-LINUX-AARCH64-X18 %s
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
+#include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include 
 
@@ -545,7 +546,8 @@
   if ((Kinds & SanitizerKind::ShadowCallStack) &&
   ((TC.getTriple().isAArch64() &&
 !llvm::AArch64::isX18ReservedByDefault(TC.getTriple())) ||
-   TC.getTriple().isRISCV()) &&
+   (TC.getTriple().isRISCV() &&
+!llvm::RISCV::isX18ReservedByDefault(TC.getTriple( &&
   !Args.hasArg(options::OPT_ffixed_x18) && DiagnoseErrors) {
 D.Diag(diag::err_drv_argument_only_allowed_with)
 << lastArgumentForMask(D, Args, Kinds & SanitizerKind::ShadowCallSta

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 494970.
mcgrathr added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143355

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/test/CodeGen/RISCV/reserved-regs.ll

Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- llvm/test/CodeGen/RISCV/reserved-regs.ll
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -57,6 +57,8 @@
 ; RUN: llc -mtriple=riscv32 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 
+; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
 
Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -14,6 +14,7 @@
 #include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/TargetParser/Triple.h"
 
 namespace llvm {
 namespace RISCV {
@@ -100,5 +101,10 @@
   return true;
 }
 
+bool isX18ReservedByDefault(const Triple &TT) {
+  // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
+  return TT.isOSFuchsia();
+}
+
 } // namespace RISCV
 } // namespace llvm
Index: llvm/lib/Target/RISCV/RISCVSubtarget.cpp
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.cpp
+++ llvm/lib/Target/RISCV/RISCVSubtarget.cpp
@@ -11,13 +11,13 @@
 //===--===//
 
 #include "RISCVSubtarget.h"
+#include "GISel/RISCVCallLowering.h"
+#include "GISel/RISCVLegalizerInfo.h"
+#include "GISel/RISCVRegisterBankInfo.h"
 #include "RISCV.h"
 #include "RISCVFrameLowering.h"
 #include "RISCVMacroFusion.h"
 #include "RISCVTargetMachine.h"
-#include "GISel/RISCVCallLowering.h"
-#include "GISel/RISCVLegalizerInfo.h"
-#include "GISel/RISCVRegisterBankInfo.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/ErrorHandling.h"
 
@@ -83,6 +83,9 @@
   FrameLowering(
   initializeSubtargetDependencies(TT, CPU, TuneCPU, FS, ABIName)),
   InstrInfo(*this), RegInfo(getHwMode()), TLInfo(TM, *this) {
+  if (RISCV::isX18ReservedByDefault(TT))
+UserReservedRegister.set(RISCV::X18);
+
   CallLoweringInfo.reset(new RISCVCallLowering(*getTargetLowering()));
   Legalizer.reset(new RISCVLegalizerInfo(*this));
 
Index: llvm/include/llvm/TargetParser/RISCVTargetParser.h
===
--- llvm/include/llvm/TargetParser/RISCVTargetParser.h
+++ llvm/include/llvm/TargetParser/RISCVTargetParser.h
@@ -18,6 +18,9 @@
 #include 
 
 namespace llvm {
+
+class Triple;
+
 namespace RISCV {
 
 // We use 64 bits as the known part in the scalable vector types.
@@ -38,6 +41,8 @@
 void fillValidTuneCPUArchList(SmallVectorImpl &Values, bool IsRV64);
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector &Features);
 
+bool isX18ReservedByDefault(const Triple &TT);
+
 } // namespace RISCV
 } // namespace llvm
 
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
+#include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include 
 
@@ -545,7 +546,8 @@
   if ((Kinds & SanitizerKind::ShadowCallStack) &&
   ((TC.getTriple().isAArch64() &&
 !llvm::AArch64::isX18ReservedByDefault(TC.getTriple())) ||
-   TC.getTriple().isRISCV()) &&
+   (TC.getTriple().isRISCV() &&
+!llvm::RISCV::isX18ReservedByDefault(TC.getTriple( &&
   !Args.hasArg(options::OPT_ffixed_x18) && DiagnoseErrors) {
 D.Diag(diag::err_drv_argument_only_allowed_with)
 << lastArgumentForMask(D, Args, Kinds & SanitizerKind::ShadowCallStack)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:16
+#include "GISel/RISCVLegalizerInfo.h"
+#include "GISel/RISCVRegisterBankInfo.h"
 #include "RISCV.h"

jrtc27 wrote:
> Unrelated change
arcanist requested it via clang-format



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143355

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

[PATCH] D143357: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, paulkirth, leonardchan.
Herald added subscribers: VincentWu, abrachet, vkmr, evandro, luismarques, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, rogfer01, shiva0217, kito-cheng, 
simoncook, arichardson.
Herald added a project: All.
mcgrathr requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: clang.

The ShadowCallStack is the preferred and default ABI for Fuchsia.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143357

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -31,6 +31,7 @@
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-RISCV64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-AARCH64: "-target-feature" "+outline-atomics"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -466,6 +466,7 @@
   SanitizerMask Res;
   switch (getTriple().getArch()) {
   case llvm::Triple::aarch64:
+  case llvm::Triple::riscv64:
 Res |= SanitizerKind::ShadowCallStack;
 break;
   case llvm::Triple::x86_64:


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -31,6 +31,7 @@
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-RISCV64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-AARCH64: "-target-feature" "+outline-atomics"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -466,6 +466,7 @@
   SanitizerMask Res;
   switch (getTriple().getArch()) {
   case llvm::Triple::aarch64:
+  case llvm::Triple::riscv64:
 Res |= SanitizerKind::ShadowCallStack;
 break;
   case llvm::Triple::x86_64:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, paulkirth, leonardchan.
Herald added subscribers: luke, VincentWu, abrachet, vkmr, frasercrmck, 
evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, 
jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, 
rbar, asb, hiraditya, arichardson.
Herald added a project: All.
mcgrathr requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead, eopXD, 
MaskRay.
Herald added projects: clang, LLVM.

Fuchsia's ABI always reserves the x18 (s2) register for the
ShadowCallStack ABI, even when -fsanitize=shadow-call-stack is
not enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143355

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/include/llvm/TargetParser/RISCVTargetParser.h
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/TargetParser/RISCVTargetParser.cpp
  llvm/test/CodeGen/RISCV/reserved-regs.ll

Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- llvm/test/CodeGen/RISCV/reserved-regs.ll
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -57,6 +57,8 @@
 ; RUN: llc -mtriple=riscv32 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 ; RUN: llc -mtriple=riscv64 -mattr=+reserve-x31 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X31
 
+; RUN: llc -mtriple=riscv64-fuchsia -verify-machineinstrs < %s | FileCheck %s -check-prefix=X18
+
 ; This program is free to use all registers, but needs a stack pointer for
 ; spill values, so do not test for reserving the stack pointer.
 
Index: llvm/lib/TargetParser/RISCVTargetParser.cpp
===
--- llvm/lib/TargetParser/RISCVTargetParser.cpp
+++ llvm/lib/TargetParser/RISCVTargetParser.cpp
@@ -14,6 +14,7 @@
 #include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/TargetParser/Triple.h"
 
 namespace llvm {
 namespace RISCV {
@@ -100,5 +101,10 @@
   return true;
 }
 
+bool isX18ReservedByDefault(const Triple &TT) {
+  // X18 is reserved for the ShadowCallStack ABI (even when not enabled).
+  return TT.isOSFuchsia();
+}
+
 } // namespace RISCV
 } // namespace llvm
Index: llvm/lib/Target/RISCV/RISCVSubtarget.cpp
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.cpp
+++ llvm/lib/Target/RISCV/RISCVSubtarget.cpp
@@ -11,13 +11,13 @@
 //===--===//
 
 #include "RISCVSubtarget.h"
+#include "GISel/RISCVCallLowering.h"
+#include "GISel/RISCVLegalizerInfo.h"
+#include "GISel/RISCVRegisterBankInfo.h"
 #include "RISCV.h"
 #include "RISCVFrameLowering.h"
 #include "RISCVMacroFusion.h"
 #include "RISCVTargetMachine.h"
-#include "GISel/RISCVCallLowering.h"
-#include "GISel/RISCVLegalizerInfo.h"
-#include "GISel/RISCVRegisterBankInfo.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/ErrorHandling.h"
 
@@ -84,6 +84,9 @@
   initializeSubtargetDependencies(TT, CPU, TuneCPU, FS, ABIName)),
   InstrInfo(*this), RegInfo(getHwMode()), TLInfo(TM, *this),
   TargetTriple(TT) {
+  if (RISCV::isX18ReservedByDefault(TT))
+UserReservedRegister.set(RISCV::X18);
+
   CallLoweringInfo.reset(new RISCVCallLowering(*getTargetLowering()));
   Legalizer.reset(new RISCVLegalizerInfo(*this));
 
Index: llvm/include/llvm/TargetParser/RISCVTargetParser.h
===
--- llvm/include/llvm/TargetParser/RISCVTargetParser.h
+++ llvm/include/llvm/TargetParser/RISCVTargetParser.h
@@ -18,6 +18,9 @@
 #include 
 
 namespace llvm {
+
+class Triple;
+
 namespace RISCV {
 
 // We use 64 bits as the known part in the scalable vector types.
@@ -38,6 +41,8 @@
 void fillValidTuneCPUArchList(SmallVectorImpl &Values, bool IsRV64);
 bool getCPUFeaturesExceptStdExt(CPUKind Kind, std::vector &Features);
 
+bool isX18ReservedByDefault(const Triple &TT);
+
 } // namespace RISCV
 } // namespace llvm
 
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
+#include "llvm/TargetParser/RISCVTargetParser.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include 
 
@@ -545,7 +546,8 @@
   if ((Kinds & SanitizerKind::ShadowCallStack) &&
   ((TC.getTriple().isAArch64() &&
 !llvm::AArch64::isX18ReservedByDefault(TC.getTriple())) ||
-   TC.getTriple().isRISCV()) &&
+   (TC.getTriple().isRISCV() &&
+!llvm::RISCV::isX

[PATCH] D140857: Fix Fuchsia dyld in asan-ubsan variant

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I don't think we want this in the generic compiler behavior. It would only make 
sense if the toolchain runtimes have extra versions, and I don't think we want 
that many extra versions.
It may be worthwhile to change the asan multilib builds to enable ubsan checks 
too, but I don't think we want to support two separate asan flavors in the 
driver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140857

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

[PATCH] D139589: [clang][Fuchsia] Re-enable hwasan global instrumentation on hwasan multilibs

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139589

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

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In D85802#3879950 , @dblaikie wrote:

> In D85802#3879888 , @mcgrathr wrote:
>
>> In D85802#3876106 , @dblaikie wrote:
>>
 The C++ ABI is not part of the Fuchsia system ABI, nor what we call the 
 "Fuchsia compiler ABI". Different users of C++ are free to use whatever 
 C++ ABI they like. Only the backend ABI independent of language-specific 
 issues is necessary to interoperate with other code on Fuchsia.
>>>
>>> Sure enough - but I'm still sort of confused by why the Fuschia Clang 
>>> target/compiler needs more than one C++ ABI. What is it interoperating 
>>> with? (GCC doesn't have a Fuschia target implemented, does it? So what's it 
>>> mean to target the GCC C++ ABI? what is compiling the code that Fuschia is 
>>> trying to interoperate with when Clang targets Fuschia with a non-default 
>>> C++ ABI?)
>>
>> When we use GCC we're using the generic ELF targets. I think it's sufficient 
>> for us to tell you that indeed we do want the option of multiple C++ ABIs to 
>> select from without justifying everything about our work to a Clang reviewer 
>> before we can proceed with meeting the requirements of our system.
>
> Would the generic ELF target support in Clang be adequate to meet that 
> requirement, then? (so Fuschia target could be the custom C++ ABI (& custom C 
> ABI if you likee) and a generic ELF target could be used for GCC ELF 
> compatibility) - then we wouldn't need any C++ ABI customizability?

No. All the aspects of the Fuchsia target not specific to C++ we still want 
done the same way when interoperating with this code.  We're quite confident 
that what we want is a Fuchsia target with multiple options for the C++ ABI.  
If you don't want that flexibility to be available to other targets, then fine 
(though I think that's a poor choice, personally).  Second-guessing everything 
about how we're organized our system and build is not a very helpful tactic in 
compiler reviews.  It would be much appreciated if this review were constrained 
to how the compiler should work rather than what we should want to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In D85802#3876106 , @dblaikie wrote:

>> The C++ ABI is not part of the Fuchsia system ABI, nor what we call the 
>> "Fuchsia compiler ABI". Different users of C++ are free to use whatever C++ 
>> ABI they like. Only the backend ABI independent of language-specific issues 
>> is necessary to interoperate with other code on Fuchsia.
>
> Sure enough - but I'm still sort of confused by why the Fuschia Clang 
> target/compiler needs more than one C++ ABI. What is it interoperating with? 
> (GCC doesn't have a Fuschia target implemented, does it? So what's it mean to 
> target the GCC C++ ABI? what is compiling the code that Fuschia is trying to 
> interoperate with when Clang targets Fuschia with a non-default C++ ABI?)

When we use GCC we're using the generic ELF targets. I think it's sufficient 
for us to tell you that indeed we do want the option of multiple C++ ABIs to 
select from without justifying everything about our work to a Clang reviewer 
before we can proceed with meeting the requirements of our system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I'm quite sure that we will always want a means to select the original Itanium 
ABI. It's also quite likely that there will be future innovations in the 
Fuchsia C++ ABI and we'll go through migration periods of supporting additional 
variants and changing the default for Fuchsia targets.

My earlier comment referred to `-fc++-abi` generally: that each target would be 
expected to enable a specific set of selections available, the Fuchsia targets 
being the first to have a set defined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

It's certainly correct that we envision each target having an explicit list of 
viable C++ ABIs to select from.

AIUI Clang has no generic multilib support but it's very patchily supported 
differently in different per-target drivers.  The Fuchsia target support in the 
driver has explicit logic about the supported multilibs, which we also use for 
sanitizer configurations, `-fno-exceptions`, and such.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

The C++ ABI is not part of the Fuchsia system ABI, nor what we call the 
"Fuchsia compiler ABI". Different users of C++ are free to use whatever C++ ABI 
they like. Only the backend ABI independent of language-specific issues is 
necessary to interoperate with other code on Fuchsia.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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

[PATCH] D135713: [cmake][Fuchsia] Add -ftrivial-auto-var-init=zero to runtimes build

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I had imagined making lsan always do this since there's a specific lsan-related 
rationale for it that applies to everybody.
I'm not clear on how cmake structures things and whether sanitizer_common would 
have to do it unconditionally since it's used by lsan code.

Doing this generally seems fine to me, but in the general case it's considered 
a bug-masking feature (with =zero) and we should contemplate how much we want 
to expect that compiler-rt might have bugs that we will be making it harder for 
us to notice and properly fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135713

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

[PATCH] D132425: [clang] Do not instrument relative vtables under hwasan

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm.
You might add some comments in CGVTables.cpp about why this is done and what 
the alternatives might be in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132425

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

[PATCH] D129512: [Driver] Don't use frame pointer on Fuchsia when optimizations are enabled

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm but be sure that Fuchsia target users get clear release-notes warning 
about the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129512

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

[PATCH] D127887: [CMake][Fuchsia] Use libunwind as the default unwinder

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127887

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

[PATCH] D122613: [Driver] Make -moutline-atomics default for aarch64-fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a963d3278c2: [Driver] Make -moutline-atomics default for 
aarch64-fuchsia targets (authored by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122613

Files:
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -34,6 +34,7 @@
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
+// CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -75,24 +75,27 @@
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+  CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const 
override;
+
+  bool IsAArch64OutlineAtomicsDefault(
+  const llvm::opt::ArgList &Args) const override {
+return true;
+  }
 
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- Action::OffloadKind DeviceOffloadKind) const 
override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-   llvm::opt::ArgStringList &CC1Args) const 
override;
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
-  const char *getDefaultLinker() const override {
-return "ld.lld";
-  }
+  const char *getDefaultLinker() const override { return "ld.lld"; }
 
 protected:
   Tool *buildLinker() const override;


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -34,6 +34,7 @@
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
+// CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "-z" "rel" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -75,24 +75,27 @@
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+  CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+
+  bool IsAArch64OutlineAtomicsDefault(
+  const llvm::opt::ArgList &Args) const override {
+return true;
+  }
 
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- Action::OffloadKind DeviceOffloadKind) const override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-   llvm::opt::ArgStringList &CC1Args) const override;
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt:

[PATCH] D122613: [Driver] Make -moutline-atomics default for aarch64-fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, abrachet.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
mcgrathr requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This makes Fuchsia consistent with Linux on AArch64.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122613

Files:
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -34,6 +34,7 @@
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
+// CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -75,24 +75,27 @@
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+  CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const 
override;
+
+  bool IsAArch64OutlineAtomicsDefault(
+  const llvm::opt::ArgList &Args) const override {
+return true;
+  }
 
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- Action::OffloadKind DeviceOffloadKind) const 
override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-   llvm::opt::ArgStringList &CC1Args) const 
override;
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
-  const char *getDefaultLinker() const override {
-return "ld.lld";
-  }
+  const char *getDefaultLinker() const override { return "ld.lld"; }
 
 protected:
   Tool *buildLinker() const override;


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -34,6 +34,7 @@
 // CHECK-AARCH64: "-fsanitize=shadow-call-stack"
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
+// CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "-z" "rel" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -75,24 +75,27 @@
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
-  CXXStdlibType
-  GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+  CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
+
+  bool IsAArch64OutlineAtomicsDefault(
+  const llvm::opt::ArgList &Args) const override {
+return true;
+  }
 
-  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args,
- Action::OffloadKind DeviceOffloadKind) const override;
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
-  void
-  AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
-   llvm::opt::ArgStringList &CC1Args) const override;
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::op

[PATCH] D119996: [safestack] Support safestack in stack size diagnostics

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1211
   const MachineFrameInfo &FrameInfo = MF.getFrameInfo();
-  uint64_t StackSize = FrameInfo.getStackSize();
+  uint64_t StackSize =
+  FrameInfo.getStackSize() + FrameInfo.getUnsafeStackSize();

Should we also update emitStackSizeSection to match?
I'm not sure whether that's meant to be used for diagnostic-like cases or for 
more concrete backend uses where the distinction between the two stacks still 
matters.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119996

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

[PATCH] D119201: [clang][Fuchsia] Ensure static sanitizer libs are only linked in after the -nostdlib check

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:157
 if (NeedsSanitizerDeps)
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 

This function is a no-op because it tests for fuchsia targets.  So it probably 
makes more sense not to call it and to remove its fuchsia special-case.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119201

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

[PATCH] D119201: [clang][Fuchsia] Ensure static sanitizer libs are only linked in after the -nostdlib check

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:130
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);

I think all these runtime additions need to be conditional.  There must be no 
kinds of implicit link inputs at all when using -nostdlib.
I'm not sure what matters or doesn't wrt their relative order vs 
AddLinkerInputs.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119201

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

[PATCH] D118306: [CMake][Fuchsia] Drop 32-bit ios runtimes

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118306

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

[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:91
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+  CmdArgs.push_back("--fix-cortex-a53-843419");

phosek wrote:
> mcgrathr wrote:
> > How does this relate to -march, -mtune, and/or -mcpu?
> > It's not at all clear to me how we discern automatically when a A53 might 
> > be included in the supported target CPU set.  I'm not entirely sure we 
> > should have automatic injection in some cases where A53 is a valid target 
> > but not all.  That is, if some compiler option combinations that produce 
> > code meant to be compatible with an A53 won't automatically get the --fix 
> > option, then perhaps it's better to require the explicit --fix option when 
> > supporting A53 is the explicit intent.
> This is the same logic as used by the [Gnu 
> driver](https://github.com/llvm/llvm-project/blob/7a7c059d867554e116244ad5639d05d75ed1a7cd/clang/lib/Driver/ToolChains/Gnu.cpp#L451).
>  `getCPUName` only considers `-mcpu`, it ignores `-march` and `-mtune` as far 
> as I can tell.
If the empty or "generic" case is what will apply when no `-mcpu` switch is 
given regardless of `-march` then I guess this is adequate since we're not 
really concerned with any architectures older than A53 where someone might use 
`-mcpu=...` as a proxy for "X and later" where Xhttps://reviews.llvm.org/D114023/new/

https://reviews.llvm.org/D114023

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

[PATCH] D114115: [Driver] Support for compressed debug info on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm, but we should be sure to advise users that their debugging tools may need 
build adjustments to ensure they support the compressed formats.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114115

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

[PATCH] D44605: [Driver] Default to DWARF 5 for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm, but we should be sure to advise users who may need to use `-gdwarf-4` 
explicitly if their debugging tools become unhappy.


Repository:
  rC Clang

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

https://reviews.llvm.org/D44605

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

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: libcxx/trunk/src/random.cpp:29
+#if defined(_LIBCPP_USING_GETENTROPY)
+#include 
+#elif defined(_LIBCPP_USING_DEV_RANDOM)

jwakely wrote:
> jwakely wrote:
> > musl only declares `getentropy` in `` not ``. Glibc 
> > declares it in both. Should `` also be included when 
> > `_LIBCPP_USING_GETENTROPY` is defined, so it can work more portably?
> I suppose it doesn't really matter, since `_LIBCPP_USING_GETENTROPY` is 
> currently only defined for fuchsia and wasi, which both declare `getentropy` 
> in ``.
FWIW, if there is an emerging norm to declare getentropy in  then 
Fuchsia's libc can add it to our  and there's no real problem relying 
on that in libc++ sources within about a week of when we land that change in 
Fuchsia's trunk.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D40319

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

[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:91
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+  CmdArgs.push_back("--fix-cortex-a53-843419");

How does this relate to -march, -mtune, and/or -mcpu?
It's not at all clear to me how we discern automatically when a A53 might be 
included in the supported target CPU set.  I'm not entirely sure we should have 
automatic injection in some cases where A53 is a valid target but not all.  
That is, if some compiler option combinations that produce code meant to be 
compatible with an A53 won't automatically get the --fix option, then perhaps 
it's better to require the explicit --fix option when supporting A53 is the 
explicit intent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114023

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

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGff11f0aa5de1: [Clang] Pass -z rel to linker for Fuchsia 
(authored by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113136

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -35,7 +35,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -60,6 +60,8 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("rel");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -35,7 +35,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "-z" "rel" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -60,6 +60,8 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("rel");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

Reordered the switches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113136

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

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 386293.
mcgrathr marked an inline comment as done.
mcgrathr added a comment.

reordered switches


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113136

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -35,7 +35,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -60,6 +60,8 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("rel");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -35,7 +35,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "-z" "rel" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -60,6 +60,8 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("rel");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 386262.
mcgrathr added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113136

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -35,7 +35,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr" "-z" 
"rel"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -61,6 +61,8 @@
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("rel");
   }
 
   if (!D.SysRoot.empty())


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -35,7 +35,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr" "-z" "rel"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -61,6 +61,8 @@
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("rel");
   }
 
   if (!D.SysRoot.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a subscriber: abrachet.
mcgrathr requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fuchsia already supports the more compact relocation format.
Make it the default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113136

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -35,7 +35,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr" "-z" 
"rel"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -61,6 +61,8 @@
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("rel");
   }
 
   if (!D.SysRoot.empty())


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -35,7 +35,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr" "-z" "rel"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -61,6 +61,8 @@
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("rel");
   }
 
   if (!D.SysRoot.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104085

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

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm with minor changes + clang-format.




Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:29
+  // now. Otherwise, ShadowBounds will be a zero-initialized global.
+  ShadowBounds = __sanitizer_shadow_bounds();
+  CHECK_NE(__sanitizer::ShadowBounds.shadow_limit, 0);

It feels sketchy to modify a variable defined in sanitizer_common directly like 
that.
Let's move this call into an `InitShadowBounds()` in 
sanitizer_common/sanitizer_fuchsia.h factored out of GetMaxUserVirtualAddress.




Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:37
+  // This initializes __sanitizer::ShadowBounds.
+  kHighMemEnd = GetMaxUserVirtualAddress();
+  kHighMemBeg = __sanitizer::ShadowBounds.shadow_limit;

leonardchan wrote:
> mcgrathr wrote:
> > Isn't this `__sanitizer::GetMaxUserVirtualAddress()` ?
> It is. It looks there's
> 
> ```
> namespace __hwasan {
> using namespace __sanitizer;
> }
> ```
> 
> in `sanitizer_internal_defs.h` included through `hwasan.h`. Will add the 
> `__sanitizer::` bits.
On further reflection, it's suboptimal that GetMaxUserVirtualAddress always 
resets the global.  It does that because in the asan implementation it's only 
called once at startup anyway so that was the minimal refactoring there.

Here since we're using ShadowBounds directly in this same code, I think using 
it directly here is more readable (as well as more optimal).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103936

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

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

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

[PATCH] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

Frankly I don't think it makes sense to have __sanitizer_mallinfo and those 
other allocator functions declared in hwasan_interface_internal.h at all.
Those are neither APIs that the compiler emits nor ones that anyone else should 
use.  They are just implementation details of the allocator interceptors.
I think a better cleanup is to move those declarations out of that file.  I 
don't see why they need to be in any header file rather than just in 
hwasan_allocation_functions.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99381

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

[PATCH] D104275: [compiler-rt][hwasan] Add GetShadowOffset function

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_mapping.h:54
+inline uptr GetShadowOffset() {
+  return SANITIZER_FUCHSIA ? 0 : __hwasan_shadow_memory_dynamic_address;
+}

vitalybuka wrote:
> mcgrathr wrote:
> > I think you'll eventually need to make this an `#if` or `if constexpr` for 
> > it to be kosher that the symbol isn't defined at all, which is what we'd 
> > ideally like to have on Fuchsia.
> static inline?
Plain inline has correct semantics here, and is consistent with the existing 
functions below.
static inside namespace scope in a header is a strange thing to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104275

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

[PATCH] D104275: [compiler-rt][hwasan] Add GetShadowOffset function

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_mapping.h:54
+inline uptr GetShadowOffset() {
+  return SANITIZER_FUCHSIA ? 0 : __hwasan_shadow_memory_dynamic_address;
+}

I think you'll eventually need to make this an `#if` or `if constexpr` for it 
to be kosher that the symbol isn't defined at all, which is what we'd ideally 
like to have on Fuchsia.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104275

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

[PATCH] D104248: [compiler-rt][hwasan] Refactor Thread::Init

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_linux.cpp:430
 
+void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) {
+  CHECK_EQ(0, unique_id_);  // try to catch bad stack reuse

vitalybuka wrote:
> leonardchan wrote:
> > leonardchan wrote:
> > > mcgrathr wrote:
> > > > Most of this code can actually be reused for Fuchsia (just not 
> > > > necessarily in Thread::Init).
> > > > It's probably better to split it up for reuse rather than just moving 
> > > > the whole thing to linux-specific.
> > > So maybe something like the current patch? Then on Fuchsia we could (1) 
> > > have a custom implementation of `InitStackAndTls`, (2) wrap the call to 
> > > `Thread::InitStackRingBuffer` in `Thread::Init` with a `#if 
> > > SANITIZER_FUCHSIA` so it's not called before thread creation, then (2) 
> > > call `Thread::InitStackRingBuffer` in the `__sanitizer_thread_start_hook` 
> > > hook.
> > Sorry. Meant wrapping with a `#if !SANITIZER_FUCHSIA`.
> in other sanitizers only GetThreadStackAndTls is platform specific
> 
> in other sanitizers only GetThreadStackAndTls is platform specific

That's not entirely accurate.  AsanThread::SetThreadStackAndTls might be a 
model here.  But asan on Fuchsia also splits up the initialization into 
pre-creation and on-created-thread portions (AsanThread::Init is called before 
thread creation, and AsanThreadRegistry::StartThread is called on the created 
thread).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104248

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

[PATCH] D104248: [compiler-rt][hwasan] Move Thread::Init into hwasan_linux.cpp

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_linux.cpp:430
 
+void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) {
+  CHECK_EQ(0, unique_id_);  // try to catch bad stack reuse

Most of this code can actually be reused for Fuchsia (just not necessarily in 
Thread::Init).
It's probably better to split it up for reuse rather than just moving the whole 
thing to linux-specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104248

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

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:29
+
+uptr kHighMemEnd;
+uptr kHighMemBeg;

leonardchan wrote:
> mcgrathr wrote:
> > These need comments about what they are and why they need to exist as 
> > runtime variables at all.
> `kHighMemEnd` and `kHighMemBeg` are used only by `MemIsApp` which is only 
> used in `hwasan_thread.cpp` for some debugging checks:
> 
> ```
>   if (stack_bottom_) {
> int local;
> CHECK(AddrIsInStack((uptr)&local));
> CHECK(MemIsApp(stack_bottom_));
> CHECK(MemIsApp(stack_top_ - 1));
>   }
> ```
> 
> Rather than having these, we could just use their values 
> (`__sanitizer::ShadowBounds.memory_limit - 1` and 
> `__sanitizer::ShadowBounds.shadow_limit`) directly in `MemIsApp` to avoid 
> these globals.
> 
> `kAliasRegionStart` is used in `HwasanAllocatorInit` for setting up the 
> allocator, but is only relevant for an experimental x86 implementation that 
> uses page aliasing for placing tags in heap allocations (see D98875). I 
> didn't look too much into   the machinery for this since I didn't think we 
> would be using hwasan for x86 anytime soon, but we can explore this now if we 
> want to plan ahead. We could also make it such that this is just defined as 0 
> on x86 platforms, similar to `SHADOW_OFFSET` in asan.
MemIsApp is defined in this file so don't define any globals on its account (if 
it needs anything, make it static in this file).

HWASAN_ALIASING_MODE is not supported for Fuchsia and we don't need to make 
sure it compiles.  Just leave out anything related to it entirely for now.





Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:34
+bool InitShadow() {
+  __hwasan_shadow_memory_dynamic_address = 0;
+

leonardchan wrote:
> mcgrathr wrote:
> > What actually refers to this?
> > The optimal implementation for Fuchsia would just know everywhere at 
> > compile time that it's a fized value.
> > If there's reason for the runtime variable to exist at all, it should have 
> > comments.
> It's only used for converting between application memory and shadow memory:
> 
> ```
> // hwasan_mapping.h
> inline uptr MemToShadow(uptr untagged_addr) {
>   return (untagged_addr >> kShadowScale) +
>  __hwasan_shadow_memory_dynamic_address;
> }
> inline uptr ShadowToMem(uptr shadow_addr) {
>   return (shadow_addr - __hwasan_shadow_memory_dynamic_address) << 
> kShadowScale;
> }
> ```
> 
> Perhaps we could have something similar to the `SHADOW_OFFSET` macro in asan 
> where it can be defined to either a constant or 
> `__hwasan_shadow_memory_dynamic_address` on different platforms and these 
> functions can just use the macro.
That sounds good.  But we can make that a separate refactoring cleanup of its 
own.  It's fine to just define it to 0 here and leave a TODO comment about 
removing the runtime variable altogether on Fuchsia.

That's a refactoring you could either land separately on its own first before 
landing the Fuchsia port, or do afterwards as a separate cleanup change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103936

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

[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

Usually we like to split changes up into separate small changes for the pure 
refactoring, and then changes that purely add new Fuchsia-specific code.
I'll do an initial review round of the Fuchsia code here since you've sent it.  
But then I think this review should be tightened to just the pure refactoring, 
and we should have a separate review thread for the Fuchsia parts.




Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:26
+// used to always find the hwasan thread object associated with the current
+// running thread.
+SANITIZER_INTERFACE_ATTRIBUTE

Specifically call out here that the compiler generates direct TLS references to 
this and that's why it has a public C ABI name.

For Fuchsia the hwasan will always be in a DSO that is a dependency of libc.so 
and so always loaded at startup.  Hence we can use 
`[[gnu::tls_model("initial-exec")]]` here.



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:32-33
+
+// These are known parameters passed to the hwasan runtime on thread creation
+// when creating a thread.
+struct Thread::InitOptions {

department of redundancy department



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:34
+// when creating a thread.
+struct Thread::InitOptions {
+  uptr stack_bottom, stack_top;

Nit: I'd call it something more like InitState.




Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:41
+void InitThreads() {
+  uptr alloc_size = UINT64_C(1) << kShadowBaseAlignment;
+  uptr thread_start = reinterpret_cast(

What depends on this alignment?  I'm not aware of anything that does in the 
compiler ABI we're using on Fuchsia.
If it's needed, it should have comments.




Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:84-88
+// are known before thread creation. However, we cannot setup the stack ring
+// buffer just yet because it is stored in the global __hwasan_tls, which we 
can
+// only correctly access once in the new thread. This will be set up in the
+// ThreadStartHook where we can safely reference __hwasan_tls while in the new
+// thread.

Actually we could do all of the allocation and setup except for storing the 
uptr in `__hwasan_tls`.
But that would take more refactoring of the common code and it's not clear 
there's a particular benefit.



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:137
+// __hwasan_tls can be referenced.
+static void FinishThreadInitialization(Thread *thread) {
+  CHECK_NE(thread, nullptr);

Most of what's happening in this function is common with the Linux code.
I think a shared function can be factored out and put into hwasan_thread.cpp.




Comment at: compiler-rt/lib/hwasan/hwasan_thread.cpp:42-45
-  static u64 unique_id;
-  unique_id_ = unique_id++;
-  if (auto sz = flags()->heap_history_size)
-heap_allocations_ = HeapAllocationsRingBuffer::New(sz);

This is stuff that could be done either in the before-create hook or in the 
on-start hook.  But it's probably not especially worthwhile to move it to 
before-create as long as we have on-start doing any allocation at all.  So to 
avoid a whole lot more refactoring to move the ringbuffer setup and such, might 
as well just leave this in on-start too.



Comment at: compiler-rt/lib/hwasan/hwasan_thread.cpp:58-63
-  uptr tls_size;
-  uptr stack_size;
-  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, 
&tls_begin_,
-   &tls_size);
-  stack_top_ = stack_bottom_ + stack_size;
-  tls_end_ = tls_begin_ + tls_size;

This is probably the only part that actually needs to be different between 
Linux and Fuchsia.
So this code could just move into an InitStackAndTls(options) that looks like 
this on Linux and on Fuchsia is just copying values out of options. 




Comment at: compiler-rt/lib/hwasan/hwasan_thread.h:85
   HeapAllocationsRingBuffer *heap_allocations_;
-  StackAllocationsRingBuffer *stack_allocations_;
+  StackAllocationsRingBuffer *stack_allocations_ = nullptr;
 

I think these need to be left uniformly uninitialized to guarantee they can be 
"linker-initialized".
At any rate, there's no reason one pointer member here should be initialized 
and the others not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104085

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

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:29
+
+uptr kHighMemEnd;
+uptr kHighMemBeg;

These need comments about what they are and why they need to exist as runtime 
variables at all.



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:34
+bool InitShadow() {
+  __hwasan_shadow_memory_dynamic_address = 0;
+

What actually refers to this?
The optimal implementation for Fuchsia would just know everywhere at compile 
time that it's a fized value.
If there's reason for the runtime variable to exist at all, it should have 
comments.



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:37
+  // This initializes __sanitizer::ShadowBounds.
+  kHighMemEnd = GetMaxUserVirtualAddress();
+  kHighMemBeg = __sanitizer::ShadowBounds.shadow_limit;

Isn't this `__sanitizer::GetMaxUserVirtualAddress()` ?



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:59
+
+// -- TSD  {{{
+

This comment isn't very meaningful, since it only really applies to the two 
functions after these three.
This is also a weird comment syntax not used elsewhere in this file.




Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:62
+extern "C" {
+void __sanitizer_thread_start_hook(void *hook, thrd_t self) {
+  hwasanThreadList().CreateCurrentThread()->InitRandomState();

As we discussed before, this is insufficient plumbing for the thread tracking.
It probably makes sense to either do all the necessary refactoring for the 
thread tracking plumbing first, or else start this file simpler without 
anything related to thread tracking, and then add more stuff later.

Also, as a matter of style it's best to define C++ functions in the __hwasan 
namespace or a local anonymous namespace, and then have an `extern "C"` block 
at the end where the libc hook API implementation functions are just simple 
wrapper calls with no more than a line or two in them.

See asan_fuchsia.cpp for a good example of how to arrange the hook functions.





Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:74
+
+void __sanitizer_exit() { __hwasan::HwasanAtExit(); }
+}  // extern "C"

This is not the name of the hook.  `__sanitizer_process_exit_hook` is the name 
of the hook.  This is a good example of a simple wrapper.

However, it's unclear whether this should use the exit hook or not.  We don't 
use that hook in asan, we just use its normal atexit method.  In hwasan, the 
atexit hook doesn't really do much.  ReportStats is a no-op in hwasan, and 
DumpProcessMap is always a no-op on Fuchsia anyway.  So all it's really doing 
in practice is changing the exit code, which is what's a perfect fit for the 
sanitizer hook.




Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:89
+// the rest of the mismatch handling code (C++).
+extern "C" void __hwasan_tag_mismatch4(uptr addr, uptr access_info,
+   uptr *registers_frame, size_t outsize) {

Why isn't this just in hwasan.cpp?



Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:95
+// These are defined because they are called from __hwasan_init, but empty
+// because they are not needed.
+void HwasanOnDeadlySignal(int signo, void *info, void *context) {}

I'd put blank lines between these.
Most or all of them merit an individual comment about how Fuchsia handles 
things differently and thus doesn't need that particular thing.




Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:101
+void HwasanTSDThreadInit() {}
+// TODO: On linux, this was used to check that TBI is enabled. Once we work 
this
+// out on Fuchsia, we should come back here and do an equivalent check.

I'd put this comment inside the function.  Seems like a good idea to rename 
this function to something not so Linuxy, e.g. InitializeOsSupport.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103936

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

[PATCH] D103564: [NFC][compiler-rt][hwasan] Move allocation functions into their own file

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_interceptors.cpp:171
 } // namespace __hwasan
+#else   // #if !SANITIZER_FUCHSIA
+namespace __hwasan {

blank lines around `#else` and `#endif` lines.  But here I think you just omit 
this.  We can define the empty function in a fuchsia-specific file.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103564

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

[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap fork interceptor with HWASAN_WITH_INTERCEPTORS

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In D103564#2797657 , @leonardchan 
wrote:

> I don't think we can move the `__sanitizer_*` allocation functions since 
> they're used for aliases for their libc equivalents which require definitions 
> in the same TU:

What I suggested is moving *everything* related to the allocation functions 
into a separate file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103564

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

[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap vfork interceptor with HWASAN_WITH_INTERCEPTORS

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I think probably the better refactoring here would be to split this file in 
two: one containing just the allocation functions; and a second with everything 
else.
On Fuchsia, we'll use the allocation functions but none of the rest of it.  So 
the second whole file could just be under `#if !SANITIZER_FUCHSIA`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103564

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

[PATCH] D103555: [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103555

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

[PATCH] D103543: [compiler-rt][Fuchsia] Support HWASan on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

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

I think this may need to be about the last thing to actually land.  It 
shouldn't land before building the runtime reliably works without error, and I 
think we should land any refactoring of the runtime we needed in preparation 
for Fuchsia-specific changes before landing anything Fuchsia-specific to make 
it compile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103543

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

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/CMakeLists.txt:45
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()

It might be better to force the value of COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
to OFF instead.

But I'll leave the cmake details to Petr.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

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

[PATCH] D101967: [Driver] Use x86-64 libc++ headers with -m32

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm. I'm not sure what better to do that's especially cleaner, except actually 
just installing i386-fuchsia header subdirs.  The easy way to do that would be 
to just build it all for the target, but we don't have fuchsia libc for that 
target so it would be a new configuration for bare metal libc++ and we don't 
actually have anybody who really needs libc++ compiled for that target, only 
the headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101967

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

[PATCH] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I don't think this is warranted.  There should not be any references to such 
things when building for Fuchsia.
This looks like it's intended to satisfy one reference in dead code in 
hwasan_interceptors.cpp.
I think the right solution is just to refactor the hwasan code so that the dead 
code is not compiled for Fuchsia at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99381

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

[PATCH] D98572: [Fuchsia] Add check-polly to CLANG_BOOTSTRAP_TARGETS

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98572

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

[PATCH] D93668: [clang] Override the Fuchsia platform ABI using the triple environment

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

I am also a C++ programmer working on C++ ABI support.  I'm glad to be working 
on a platform that made many intentional design decisions to enable keeping the 
C++ ABI out of the platform ABI.  It makes it vastly easier to do interesting 
work on C++ ABIs such as what Leo is doing.  It also makes it significantly 
easier to do interesting work on platform ABIs, which I also do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93668

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

[PATCH] D93668: [clang] Override the Fuchsia platform ABI using the triple environment

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

The Fuchsia platform ABI includes the SS & SCS ABI.  There is no compatibility 
issue with those.  PIC-friendly optimizations are purely an optimization and 
correctness issue for any PIC/PIE code, and is not an ABI issue.
The only thing where we have an incompatible difference in ABI between our 
compiler environments is the C++ ABI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93668

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

[PATCH] D93668: [clang] Add -ffuchsia-c++-abi flag to explicitly use the Fuchsia C++ ABI

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

You've said you want to avoid expressing the C++ ABI as a generic switch 
orthogonal to target because people can use it when it's not what they actually 
want or isn't actually useful to attempt.  This is true of all switches 
affecting ABI and they haven't all been rejected on this basis.  But fine, 
consistency is the hobgoblin of small minds.  If the concern there is either 
user confusion or support burden for configurations nobody wants to test, then 
both of those are easily addressed by having `-fc++abi=` constrain the settings 
it allows based on target.  For Fuchsia targets, we want to maintain and test 
both "fuchsia" and "compat" (or "itanium") modes.  If for other targets the 
only setting allowed is the one that's the default, or if using non-default 
settings generates a scare warning or suchlike, that's fine.  That seems to be 
the natural way to constrain the explicit control of the C++ ABI to the 
supported cases, while not conflating control of the C++ ABI with the 
language-independent platform ABI being targeted.

You've said you want to avoid a proliferation of explicit switches for specific 
per-target C++ ABI selections.  This is in fact the original rationale for 
`-fc++abi=`.  Instead you've proposed a proliferation of specific per-target 
pseudo-tuples.  This is exactly equivalent in material effect except that it's 
more hidden because possible tuples don't appear in `--help` output like 
separate switches do.  It's worse in the abstract sense of correctness and in 
the self-documenting sense of being misleading and confusing by conflating the 
language-specific ABI with the platform ABI.  You've cited the precedent of the 
mingw tuples.  I'm not sufficiently familiar with the mingw cases to know 
whether they in fact are about nothing but the language ABI or are in fact 
actually about properly different platform ABIs in some other sense.  If the 
latter, then your proposal exemplifies and worsens the kind of confusion 
between platform ABI (which tuples are explicitly about and always have been) 
with language ABI (which is outside the scope of what tuples were specified to 
describe) that I'm objecting to.  If the former, then this bad precedent is  a 
sign this problem has already caused the confusion I warned about and IMHO a 
precedent of doing the wrong thing is not a reason to compound the error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93668

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

[PATCH] D93668: [clang] Add -ffuchsia-c++-abi flag to explicitly use the Fuchsia C++ ABI

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

It's fine to have a target tuple translate to a default C++ ABI.
But the C++ ABI selection is fundamentally not a target flavor thing.  It's 
just a C++ ABI thing.
So using the target tuple as the sole mechanism to determine C++ ABI is 
fundamentally wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93668

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

[PATCH] D92444: [CMake][Fuchsia] Install llvm-elfabi

[Roland McGrath via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG70764c02e474: [CMake][Fuchsia] Install llvm-elfabi (authored 
by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92444

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -245,6 +245,7 @@
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
+  llvm-elfabi
   llvm-gsymutil
   llvm-lib
   llvm-mt


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -245,6 +245,7 @@
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
+  llvm-elfabi
   llvm-gsymutil
   llvm-lib
   llvm-mt
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92444: [CMake][Fuchsia] Install llvm-elfabi

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: leonardchan, haowei.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
mcgrathr requested review of this revision.

The canonical Fuchsia toolchain configuration installs llvm-elfabi.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92444

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -245,6 +245,7 @@
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
+  llvm-elfabi
   llvm-gsymutil
   llvm-lib
   llvm-mt


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -245,6 +245,7 @@
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
+  llvm-elfabi
   llvm-gsymutil
   llvm-lib
   llvm-mt
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91387: [Driver] Support UBSan multilib

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:214-215
 
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_BUILD_COMPILER_RT 
OFF CACHE BOOL "")
+set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_USE_SANITIZER 
"Undefined" CACHE STRING "")
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")

Don't you need plain +ubsan instances of these like for +asan above?




Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:216-217
+set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_USE_SANITIZER 
"Undefined" CACHE STRING "")
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")
+
set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS
 OFF CACHE BOOL "")

Standalone ubsan doesn't replace the allocator, so these shouldn't be 
overridden for it like they are for asan.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:205
   .flag("+fno-exceptions"));
+  // UBSan has lower priority than ASan so we pick the latter when both are 
set.
+  Multilibs.push_back(Multilib("ubsan", {}, {}, 2)

This being the case, should we be adding `-fsanitize=undefined` to the libc++ 
(et al) build for asan?
I guess that's an orthogonal change, really.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:217
   // Use the asan+noexcept variant with ASan and -fno-exceptions.
-  Multilibs.push_back(Multilib("asan+noexcept", {}, {}, 3)
+  Multilibs.push_back(Multilib("asan+noexcept", {}, {}, 5)
   .flag("+fsanitize=address")

This is getting to be a lot of individual ordered integer constants to maintain.
Can we just define a local enum to represent the ordering and use symbolic 
names in each call?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91387

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

[PATCH] D91226: [clang] Add missing header guard in

[Roland McGrath via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf36142d342a: [clang] Add missing header guard in 
 (authored by mcgrathr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91226

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple i386 -emit-llvm -o - | FileCheck 
%s --check-prefix=CHECK-32
 
 #include 
+#include  // Make sure multiple inclusion protection works.
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -7,6 +7,9 @@
  *===---===
  */
 
+#ifndef __CPUID_H
+#define __CPUID_H
+
 #if !(__x86_64__ || __i386__)
 #error this header is for x86 only
 #endif
@@ -312,3 +315,5 @@
 __cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
 return 1;
 }
+
+#endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple i386 -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-32
 
 #include 
+#include  // Make sure multiple inclusion protection works.
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -7,6 +7,9 @@
  *===---===
  */
 
+#ifndef __CPUID_H
+#define __CPUID_H
+
 #if !(__x86_64__ || __i386__)
 #error this header is for x86 only
 #endif
@@ -312,3 +315,5 @@
 __cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
 return 1;
 }
+
+#endif /* __CPUID_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91226: [clang] Add missing header guard in

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, leonardchan.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
mcgrathr requested review of this revision.

This header has long lacked a standard multiple inclusion guard
like other headers have, for no apparent reason.  The GCC header
of the same name likewise lacks one up through release 10.1, but
trunk GCC (release 11, and perhaps future 10.x) has fixed it
(see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96238).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91226

Files:
  clang/lib/Headers/cpuid.h
  clang/test/Headers/cpuid.c


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple i386 -emit-llvm -o - | FileCheck 
%s --check-prefix=CHECK-32
 
 #include 
+#include  // Make sure multiple inclusion protection works.
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A 
cpuid\0A xchgq %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  
cpuid\0A  xchgq  %rbx,${1:q}", 
"={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 
%{{[a-z0-9]+}})
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -7,6 +7,9 @@
  *===---===
  */
 
+#ifndef __CPUID_H
+#define __CPUID_H
+
 #if !(__x86_64__ || __i386__)
 #error this header is for x86 only
 #endif
@@ -312,3 +315,5 @@
 __cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
 return 1;
 }
+
+#endif /* __CPUID_H */


Index: clang/test/Headers/cpuid.c
===
--- clang/test/Headers/cpuid.c
+++ clang/test/Headers/cpuid.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -ffreestanding -triple i386 -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-32
 
 #include 
+#include  // Make sure multiple inclusion protection works.
 
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A cpuid\0A xchgq %rbx,${1:q}", "={ax},=r,={cx},={dx},0,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}})
 // CHECK-64: {{.*}} call { i32, i32, i32, i32 } asm "  xchgq  %rbx,${1:q}\0A  cpuid\0A  xchgq  %rbx,${1:q}", "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"(i32 %{{[a-z0-9]+}}, i32 %{{[a-z0-9]+}})
Index: clang/lib/Headers/cpuid.h
===
--- clang/lib/Headers/cpuid.h
+++ clang/lib/Headers/cpuid.h
@@ -7,6 +7,9 @@
  *===---===
  */
 
+#ifndef __CPUID_H
+#define __CPUID_H
+
 #if !(__x86_64__ || __i386__)
 #error this header is for x86 only
 #endif
@@ -312,3 +315,5 @@
 __cpuid_count(__leaf, __subleaf, *__eax, *__ebx, *__ecx, *__edx);
 return 1;
 }
+
+#endif /* __CPUID_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

The GCC documentation specifically gives the example of the standard C function 
`qsort` as one that does not qualify as `__attribute__((leaf))` because it uses 
a callback function (that presumably might be from the caller's own TU).  AIUI 
the claim the attribute makes is that no normal control flow (i.e. excluding 
only signal handlers) would call any function in the caller's TU by any means, 
nor `longjmp` to any state captured by a `setjmp` call in the caller's TU, and 
thus only normal return or normal C++ exception handling return/unwind would 
reach the caller's TU again after the jump (call) to this entry point.  The 
caller is thus presumed not to share potentially-called functions with the 
callee by any means, whether direct symbol references or function pointers 
passed in the call or function pointers previously stored somewhere (e.g. a C++ 
vtable).  The compiler can make whatever assumptions about potential 
dataflow/side-effect and the like are implied by this exclusion of TU-local 
code paths from the otherwise unknown call graph of the external call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

There is an RFC going out with this prototype as reference.  When there is 
consensus on the RFC, this will get in shape for landing with complete tests 
and all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D86822: [clang] Enable -fsanitize=thread on Fuchsia.

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.

I'm not 100% sure we don't need more fiddles in the driver, but we can iterate 
from here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86822

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

[PATCH] D85924: [clang][feature] Add cxx_abi_relative_vtable feature

[Roland McGrath via Phabricator via cfe-commits]

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

LGTM but I'm not sure who should sign off on new `__has_feature` symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85924

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

[PATCH] D85924: [clang] Add __RELATIVE_CXX_ABI_VTABLES predefine macro

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:1127
 
+  if (LangOpts.RelativeCXXABIVTables)
+Builder.defineMacro("__RELATIVE_CXX_ABI_VTABLES");

ldionne wrote:
> mcgrathr wrote:
> > This should also test `LangOpts.CplusPlus` so it's never defined in C 
> > compilation.
> > 
> > I don't know what precedents there are to follow for how to choose the 
> > exact name here.
> > Having something like `__CXX_ABI_` be a prefix for all such things seems 
> > wise.
> > 
> Should we be using some `__has_feature(...)` instead?
That does seem like a very good fit here, and really easy to use with just one 
line in Features.def AFAICT.
Maybe `__has_feature(cxx_abi_relative_vtable)` and in future add more 
`cxx_abi_foo` ones?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85924

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

[PATCH] D85924: [clang] Add __RELATIVE_CXX_ABI_VTABLES predefine macro

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:1127
 
+  if (LangOpts.RelativeCXXABIVTables)
+Builder.defineMacro("__RELATIVE_CXX_ABI_VTABLES");

This should also test `LangOpts.CplusPlus` so it's never defined in C 
compilation.

I don't know what precedents there are to follow for how to choose the exact 
name here.
Having something like `__CXX_ABI_` be a prefix for all such things seems wise.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85924

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

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/include/clang/Basic/TargetCXXABI.h:39
 
+  static const auto &getABIMap() {
+static llvm::StringMap ABIMap = {

The option UI should use lowercase values by default, or else just do 
case-insensitive matching.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:274
+
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");

This should match "fuchsia", either only lowercase or case-insensitive.  IMHO 
it seems wise to handle this in some way such that individual tests like this 
are not free-form string comparisons that could have typos, but test the 
TargetCXXABI enum value after decoding from string.

This should be a separate change.  We'll need to do staged roll-out, first of a 
compiler where `-fc++abi=compat` (or itanium whatever it's called) is available 
(complete with multilibs et al), and then later of a compiler where 
`-fc++abi=fuchsia` (and the default for *-fuchsia targets) has changed meaning.




Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:275
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");
 }

It's surprising to me that this is the way to do this.  Isn't there code in the 
actual front end that tests the TargetCXXABI value?  That seems like the place 
where it makes sense to have Fuchsia imply specific settings, rather than in 
the driver.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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

[PATCH] D85362: [CMake] Print the autodetected host linker version

[Roland McGrath via Phabricator via cfe-commits]

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

That's a great start.  IMHO we should give the driver a way to print out its 
embedded value too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85362

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

[PATCH] D84482: [Fuchsia] Use -z dead-reloc-in-nonalloc=*=0 for Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, leonardchan.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84482

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -53,6 +53,13 @@
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
+  // 0 is never a valid code address for Fuchsia (user or kernel), so using the
+  // traditional 0 value for references to discarded code is always fine on
+  // Fuchsia.  The new defaults of -1 and such in lld confuse some older tools
+  // for no benefit to Fuchsia.
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("dead-reloc-in-nonalloc=*=0");
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
   llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -53,6 +53,13 @@
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
+  // 0 is never a valid code address for Fuchsia (user or kernel), so using the
+  // traditional 0 value for references to discarded code is always fine on
+  // Fuchsia.  The new defaults of -1 and such in lld confuse some older tools
+  // for no benefit to Fuchsia.
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("dead-reloc-in-nonalloc=*=0");
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
   llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79835: [Fuchsia] Rely on linker switch rather than dead code ref for profile runtime

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 267336.
mcgrathr added a comment.

comment update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79835

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Fuchsia.h
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.profile.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.profile.a
  clang/test/Driver/fuchsia.c
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/linkage.ll


Index: llvm/test/Instrumentation/InstrProfiling/linkage.ll
===
--- llvm/test/Instrumentation/InstrProfiling/linkage.ll
+++ llvm/test/Instrumentation/InstrProfiling/linkage.ll
@@ -2,9 +2,11 @@
 
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -instrprof -S | FileCheck 
%s --check-prefixes=POSIX,MACHO
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -instrprof -S | FileCheck %s 
--check-prefixes=POSIX,LINUX
+; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -instrprof -S | FileCheck %s 
--check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -instrprof -S | FileCheck %s 
--check-prefixes=COFF
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -passes=instrprof -S | 
FileCheck %s --check-prefixes=POSIX,MACHO
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=instrprof -S | FileCheck 
%s --check-prefixes=POSIX,LINUX
+; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -passes=instrprof -S | 
FileCheck %s --check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -passes=instrprof -S | 
FileCheck %s --check-prefixes=COFF
 
 ; MACHO: @__llvm_profile_runtime = external global i32
Index: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1034,9 +1034,9 @@
 }
 
 bool InstrProfiling::emitRuntimeHook() {
-  // We expect the linker to be invoked with -u flag for linux,
-  // for which case there is no need to emit the user function.
-  if (TT.isOSLinux())
+  // We expect the linker to be invoked with -u flag for Linux or
+  // Fuchsia, in which case there is no need to emit the user function.
+  if (TT.isOSLinux() || TT.isOSFuchsia())
 return false;
 
   // If the module's provided its own runtime, we don't need to do anything.
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -242,3 +242,21 @@
 // RUN: -gsplit-dwarf -c %s 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SPLIT-DWARF
 // CHECK-SPLIT-DWARF: "-split-dwarf-output" "fuchsia.dwo"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fprofile-instr-generate -fcoverage-mapping \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-PROFRT-AARCH64
+// CHECK-PROFRT-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PROFRT-AARCH64: "-u__llvm_profile_runtime"
+// CHECK-PROFRT-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.profile.a"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fprofile-instr-generate -fcoverage-mapping \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-PROFRT-X86_64
+// CHECK-PROFRT-X86_64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PROFRT-X86_64: "-u__llvm_profile_runtime"
+// CHECK-PROFRT-X86_64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.profile.a"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -69,6 +69,9 @@
   SanitizerMask getSupportedSanitizers() const override;
   SanitizerMask getDefaultSanitizers() const override;
 
+  void addProfileRTLibs(const llvm::opt::ArgList &Args,
+llvm::opt::ArgStringList &CmdArgs) const override;
+
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
   CXXStdlibType
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -15,6 +15,7 @@
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Suppor

[PATCH] D79835: [Fuchsia] Rely on linker switch rather than dead code ref for profile runtime

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Follow the model used on Linux, where the clang driver passes the
linker a `-u` switch to force the profile runtime to be linked in,
rather than having every TU emit a dead function with a reference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79835

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Fuchsia.h
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.profile.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.profile.a
  clang/test/Driver/fuchsia.c
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/linkage.ll


Index: llvm/test/Instrumentation/InstrProfiling/linkage.ll
===
--- llvm/test/Instrumentation/InstrProfiling/linkage.ll
+++ llvm/test/Instrumentation/InstrProfiling/linkage.ll
@@ -2,9 +2,11 @@
 
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -instrprof -S | FileCheck 
%s --check-prefixes=POSIX,MACHO
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -instrprof -S | FileCheck %s 
--check-prefixes=POSIX,LINUX
+; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -instrprof -S | FileCheck %s 
--check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -instrprof -S | FileCheck %s 
--check-prefixes=COFF
 ; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -passes=instrprof -S | 
FileCheck %s --check-prefixes=POSIX,MACHO
 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=instrprof -S | FileCheck 
%s --check-prefixes=POSIX,LINUX
+; RUN: opt < %s -mtriple=x86_64-unknown-fuchsia -passes=instrprof -S | 
FileCheck %s --check-prefixes=POSIX,LINUX
 ; RUN: opt < %s  -mtriple=x86_64-pc-win32-coff -passes=instrprof -S | 
FileCheck %s --check-prefixes=COFF
 
 ; MACHO: @__llvm_profile_runtime = external global i32
Index: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -1036,7 +1036,7 @@
 bool InstrProfiling::emitRuntimeHook() {
   // We expect the linker to be invoked with -u flag for linux,
   // for which case there is no need to emit the user function.
-  if (TT.isOSLinux())
+  if (TT.isOSLinux() || TT.isOSFuchsia())
 return false;
 
   // If the module's provided its own runtime, we don't need to do anything.
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -242,3 +242,21 @@
 // RUN: -gsplit-dwarf -c %s 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SPLIT-DWARF
 // CHECK-SPLIT-DWARF: "-split-dwarf-output" "fuchsia.dwo"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fprofile-instr-generate -fcoverage-mapping \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-PROFRT-AARCH64
+// CHECK-PROFRT-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PROFRT-AARCH64: "-u__llvm_profile_runtime"
+// CHECK-PROFRT-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.profile.a"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fprofile-instr-generate -fcoverage-mapping \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-PROFRT-X86_64
+// CHECK-PROFRT-X86_64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PROFRT-X86_64: "-u__llvm_profile_runtime"
+// CHECK-PROFRT-X86_64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.profile.a"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -69,6 +69,9 @@
   SanitizerMask getSupportedSanitizers() const override;
   SanitizerMask getDefaultSanitizers() const override;
 
+  void addProfileRTLibs(const llvm::opt::ArgList &Args,
+llvm::opt::ArgStringList &CmdArgs) const override;
+
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
   CXXStdlibType
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -15,6 +15,7 @@
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/ProfileData/Instr

[PATCH] D79667: [Clang] Pass -z max-page-size to linker for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
mcgrathr added a parent revision: D79665: [Clang] Pass --pack-dyn-relocs=relr 
to lld for Fuchsia.

Currently all Fuchsia ABIs use a 4k page size, departing from
the recommended page sizes in the respective psABI documents.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79667

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -46,6 +46,9 @@
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -46,6 +46,9 @@
   // handled somewhere else.
   Args.ClaimAllArgs(options::OPT_w);
 
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("max-page-size=4096");
+
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79665: [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The compact format is fully supported on Fuchsia and is the
preferred default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79665

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" 
"separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -23,7 +23,7 @@
 // CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -56,6 +56,7 @@
 CmdArgs.push_back("rodynamic");
 CmdArgs.push_back("-z");
 CmdArgs.push_back("separate-loadable-segments");
+CmdArgs.push_back("--pack-dyn-relocs=relr");
   }
 
   if (!D.SysRoot.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78687: [Fuchsia] Build compiler-rt builtins for 32-bit x86

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78687



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

[PATCH] D72959: Relative VTables ABI on Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: libcxxabi/src/private_typeinfo.cpp:617
 // Get (dynamic_ptr, dynamic_type) from static_ptr
+#ifndef __Fuchsia__
 void **vtable = *static_cast(static_ptr);

ldionne wrote:
> Please introduce a macro that generically expresses that relative vtables are 
> enabled, and explain what it is. You can then enable that macro only on 
> `__Fuchsia__`. I'd like to avoid freely adding platform-specific `#ifdef`s in 
> the code when it's easy to avoid.
I think the libcxxabi change should be separated from the compiler change 
entirely.
It will need more configuration issues resolved before it usefully lands.
Let's keep this change just for the pure compiler feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72959



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

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

Thanks! Can you land it for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75002



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

[PATCH] D75003: [X86] Fix __code_model_*__ predefine names

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

Thanks! Can you land it for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75003



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

[PATCH] D75003: [X86] Fix __code_model_*__ predefine names

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: tamur.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC defines __code_model_*__ (two trailing underscores), not
__code_model_*_ (one trailing underscore).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75003

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -7528,7 +7528,7 @@
 // X86_64:#define __WINT_WIDTH__ 32
 // X86_64:#define __amd64 1
 // X86_64:#define __amd64__ 1
-// X86_64:#define __code_model_small_ 1
+// X86_64:#define __code_model_small__ 1
 // X86_64:#define __x86_64 1
 // X86_64:#define __x86_64__ 1
 //
@@ -7540,7 +7540,7 @@
 // X86_64H:#define __x86_64h__ 1
 //
 // RUN: %clang -xc - -E -dM -mcmodel=medium --target=i386-unknown-linux < 
/dev/null | FileCheck -match-full-lines -check-prefix X86_MEDIUM %s
-// X86_MEDIUM:#define __code_model_medium_ 1
+// X86_MEDIUM:#define __code_model_medium__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=x86_64-none-none-gnux32 < /dev/null | FileCheck -match-full-lines 
-check-prefix X32 %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=x86_64-none-none-gnux32 < /dev/null | FileCheck -match-full-lines 
-check-prefix X32 -check-prefix X32-CXX %s
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -911,7 +911,7 @@
   std::string CodeModel = getTargetOpts().CodeModel;
   if (CodeModel == "default")
 CodeModel = "small";
-  Builder.defineMacro("__code_model_" + CodeModel + "_");
+  Builder.defineMacro("__code_model_" + CodeModel + "__");
 
   // Target identification.
   if (getTriple().getArch() == llvm::Triple::x86_64) {


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -7528,7 +7528,7 @@
 // X86_64:#define __WINT_WIDTH__ 32
 // X86_64:#define __amd64 1
 // X86_64:#define __amd64__ 1
-// X86_64:#define __code_model_small_ 1
+// X86_64:#define __code_model_small__ 1
 // X86_64:#define __x86_64 1
 // X86_64:#define __x86_64__ 1
 //
@@ -7540,7 +7540,7 @@
 // X86_64H:#define __x86_64h__ 1
 //
 // RUN: %clang -xc - -E -dM -mcmodel=medium --target=i386-unknown-linux < /dev/null | FileCheck -match-full-lines -check-prefix X86_MEDIUM %s
-// X86_MEDIUM:#define __code_model_medium_ 1
+// X86_MEDIUM:#define __code_model_medium__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=x86_64-none-none-gnux32 < /dev/null | FileCheck -match-full-lines -check-prefix X32 %s
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=x86_64-none-none-gnux32 < /dev/null | FileCheck -match-full-lines -check-prefix X32 -check-prefix X32-CXX %s
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -911,7 +911,7 @@
   std::string CodeModel = getTargetOpts().CodeModel;
   if (CodeModel == "default")
 CodeModel = "small";
-  Builder.defineMacro("__code_model_" + CodeModel + "_");
+  Builder.defineMacro("__code_model_" + CodeModel + "__");
 
   // Target identification.
   if (getTriple().getArch() == llvm::Triple::x86_64) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.
mcgrathr updated this revision to Diff 246043.
mcgrathr added a comment.
mcgrathr edited the summary of this revision.

update log


Make Clang on aarch64 targets predefine `__AARCH64_CMODEL_SMALL__`
or `__AARCH64_CMODEL_TINY__`, etc.  These are the names that GCC
uses for its predefines.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75002

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -285,6 +285,7 @@
 // AARCH64-NOT:#define __AARCH64EB__ 1
 // AARCH64:#define __AARCH64EL__ 1
 // AARCH64-NOT:#define __AARCH_BIG_ENDIAN 1
+// AARCH64:#define __AARCH64_CMODEL_SMALL__ 1
 // AARCH64:#define __ARM_64BIT_STATE 1
 // AARCH64:#define __ARM_ARCH 8
 // AARCH64:#define __ARM_ARCH_ISA_A64 1
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -194,6 +194,14 @@
 Builder.defineMacro("__LP64__");
   }
 
+  std::string CodeModel = getTargetOpts().CodeModel;
+  if (CodeModel == "default")
+CodeModel = "small";
+  for (auto& c : CodeModel) {
+c = toupper(c);
+  }
+  Builder.defineMacro("__AARCH64_CMODEL_" + CodeModel + "__");
+
   // ACLE predefines. Many can only have one possible value on v8 AArch64.
   Builder.defineMacro("__ARM_ACLE", "200");
   Builder.defineMacro("__ARM_ARCH", "8");


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -285,6 +285,7 @@
 // AARCH64-NOT:#define __AARCH64EB__ 1
 // AARCH64:#define __AARCH64EL__ 1
 // AARCH64-NOT:#define __AARCH_BIG_ENDIAN 1
+// AARCH64:#define __AARCH64_CMODEL_SMALL__ 1
 // AARCH64:#define __ARM_64BIT_STATE 1
 // AARCH64:#define __ARM_ARCH 8
 // AARCH64:#define __ARM_ARCH_ISA_A64 1
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -194,6 +194,14 @@
 Builder.defineMacro("__LP64__");
   }
 
+  std::string CodeModel = getTargetOpts().CodeModel;
+  if (CodeModel == "default")
+CodeModel = "small";
+  for (auto& c : CodeModel) {
+c = toupper(c);
+  }
+  Builder.defineMacro("__AARCH64_CMODEL_" + CodeModel + "__");
+
   // ACLE predefines. Many can only have one possible value on v8 AArch64.
   Builder.defineMacro("__ARM_ACLE", "200");
   Builder.defineMacro("__ARM_ARCH", "8");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 246043.
mcgrathr added a comment.

update log


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75002

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -285,6 +285,7 @@
 // AARCH64-NOT:#define __AARCH64EB__ 1
 // AARCH64:#define __AARCH64EL__ 1
 // AARCH64-NOT:#define __AARCH_BIG_ENDIAN 1
+// AARCH64:#define __AARCH64_CMODEL_SMALL__ 1
 // AARCH64:#define __ARM_64BIT_STATE 1
 // AARCH64:#define __ARM_ARCH 8
 // AARCH64:#define __ARM_ARCH_ISA_A64 1
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -194,6 +194,14 @@
 Builder.defineMacro("__LP64__");
   }
 
+  std::string CodeModel = getTargetOpts().CodeModel;
+  if (CodeModel == "default")
+CodeModel = "small";
+  for (auto& c : CodeModel) {
+c = toupper(c);
+  }
+  Builder.defineMacro("__AARCH64_CMODEL_" + CodeModel + "__");
+
   // ACLE predefines. Many can only have one possible value on v8 AArch64.
   Builder.defineMacro("__ARM_ACLE", "200");
   Builder.defineMacro("__ARM_ARCH", "8");


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -285,6 +285,7 @@
 // AARCH64-NOT:#define __AARCH64EB__ 1
 // AARCH64:#define __AARCH64EL__ 1
 // AARCH64-NOT:#define __AARCH_BIG_ENDIAN 1
+// AARCH64:#define __AARCH64_CMODEL_SMALL__ 1
 // AARCH64:#define __ARM_64BIT_STATE 1
 // AARCH64:#define __ARM_ARCH 8
 // AARCH64:#define __ARM_ARCH_ISA_A64 1
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -194,6 +194,14 @@
 Builder.defineMacro("__LP64__");
   }
 
+  std::string CodeModel = getTargetOpts().CodeModel;
+  if (CodeModel == "default")
+CodeModel = "small";
+  for (auto& c : CodeModel) {
+c = toupper(c);
+  }
+  Builder.defineMacro("__AARCH64_CMODEL_" + CodeModel + "__");
+
   // ACLE predefines. Many can only have one possible value on v8 AArch64.
   Builder.defineMacro("__ARM_ACLE", "200");
   Builder.defineMacro("__ARM_ARCH", "8");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73734: [Fuchsia] Never link in implicit "system dependencies" of sanitizer runtimes

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 241561.
mcgrathr added a comment.

style nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73734

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -583,6 +583,11 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
+  // Fuchsia never needs these.  Any sanitizer runtimes with system
+  // dependencies use the `.deplibs` feature instead.
+  if (TC.getTriple().isOSFuchsia())
+return;
+
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -583,6 +583,11 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
+  // Fuchsia never needs these.  Any sanitizer runtimes with system
+  // dependencies use the `.deplibs` feature instead.
+  if (TC.getTriple().isOSFuchsia())
+return;
+
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73734: [Fuchsia] Never link in implicit "system dependencies" of sanitizer runtimes

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 241560.
mcgrathr added a comment.

builds now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73734

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -583,6 +583,12 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
+  if (TC.getTriple().isOSFuchsia()) {
+// Fuchsia never needs these.  Any sanitizer runtimes with system
+// dependencies use the `.deplibs` feature.
+return;
+  }
+
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -583,6 +583,12 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
+  if (TC.getTriple().isOSFuchsia()) {
+// Fuchsia never needs these.  Any sanitizer runtimes with system
+// dependencies use the `.deplibs` feature.
+return;
+  }
+
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73734: [Fuchsia] Never link in implicit "system dependencies" of sanitizer runtimes

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is never appropriate on Fuchsia and any future needs for
system library dependencies of compiler-supplied runtimes will
be addressed via `.deplibs` instead of driver hacks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73734

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -583,6 +583,12 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
+  if (Triple.isOSFuchsia()) {
+// Fuchsia never needs these.  Any sanitizer runtimes with system
+// dependencies use the `.deplibs` feature.
+return;
+  }
+
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -583,6 +583,12 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
+  if (Triple.isOSFuchsia()) {
+// Fuchsia never needs these.  Any sanitizer runtimes with system
+// dependencies use the `.deplibs` feature.
+return;
+  }
+
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
   CmdArgs.push_back("--no-as-needed");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73397: [Clang] Enable -fsanitize=leak on Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr updated this revision to Diff 240346.
mcgrathr added a comment.

Fixed Android SafeStack case, now passing check-clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73397

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-fuchsia/libclang_rt.lsan.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-fuchsia/libclang_rt.lsan.a
  clang/test/Driver/fuchsia.c

Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -174,6 +174,35 @@
 // CHECK-SCUDO-SHARED: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-SHARED: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.scudo.so"
 
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=leak 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-AARCH64
+// CHECK-LSAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-AARCH64: "-fsanitize=leak,shadow-call-stack"
+// CHECK-LSAN-AARCH64: "-pie"
+// CHECK-LSAN-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.lsan.a"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fsanitize=leak 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-X86
+// CHECK-LSAN-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-X86: "-fsanitize=leak,safe-stack"
+// CHECK-LSAN-X86: "-pie"
+// CHECK-LSAN-X86: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.lsan.a"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=leak -fPIC -shared 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-SHARED
+// CHECK-LSAN-SHARED: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-SHARED: "-fsanitize=leak,shadow-call-stack"
+// CHECK-LSAN-SHARED-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.lsan.a"
+
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fxray-instrument -fxray-modes=xray-basic \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -340,6 +340,7 @@
   Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
+  Res |= SanitizerKind::Leak;
   Res |= SanitizerKind::SafeStack;
   Res |= SanitizerKind::Scudo;
   return Res;
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -642,17 +642,21 @@
 StaticRuntimes.push_back("stats_client");
 
   // Collect static runtimes.
-  if (Args.hasArg(options::OPT_shared) || SanArgs.needsSharedRt()) {
-// Don't link static runtimes into DSOs or if -shared-libasan.
+  if (Args.hasArg(options::OPT_shared)) {
+// Don't link static runtimes into DSOs.
 return;
   }
-  if (SanArgs.needsAsanRt() && SanArgs.linkRuntimes()) {
+
+  // Each static runtime that has a DSO counterpart above is excluded below,
+  // but runtimes that exist only as static are not affected by needsSharedRt.
+
+  if (!SanArgs.needsSharedRt() && SanArgs.needsAsanRt() && SanArgs.linkRuntimes()) {
 StaticRuntimes.push_back("asan");
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("asan_cxx");
   }
 
-  if (SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
+  if (!SanArgs.needsSharedRt() && SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
 StaticRuntimes.push_back("hwasan");
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("hwasan_cxx");
@@ -671,7 +675,7 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("tsan_cxx");
   }
-  if (SanArgs.needsUbsanRt() && SanArgs.linkRuntimes()) {
+  if (!SanArgs.needsSharedRt() && SanArgs.needsUbsanRt() && SanArgs.linkRuntimes()) {
 if (SanArgs.requiresMinimalRuntime()) {
   StaticRuntimes.push_back("ubsan_minimal");
 } else {
@@ -684,18 +688,20 @@
 NonWholeStaticRuntimes.push_back("safestack");
 RequiredSymbols.push_back("__safestack_init");
   }
-  if (SanArgs.needsCfiRt() && SanArgs.linkRuntimes())
-StaticRuntimes.push_back("cfi");
-  if (SanArgs.need

[PATCH] D73397: [Clang] Enable -fsanitize=leak on Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, aarongreen, cryptoad, vitalybuka.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This required some fixes to the generic code for two issues:

1. -fsanitize=safe-stack is default on x86_64-fuchsia and is *not* incompatible 
with -fsanitize=leak on Fuchisa

2. -fsanitize=leak and other static-only runtimes must not be omitted under 
-shared-libsan (which is the default on Fuchsia)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73397

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c

Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -174,6 +174,35 @@
 // CHECK-SCUDO-SHARED: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-SHARED: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.scudo.so"
 
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=leak 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-AARCH64
+// CHECK-LSAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-AARCH64: "-fsanitize=leak,shadow-call-stack"
+// CHECK-LSAN-AARCH64: "-pie"
+// CHECK-LSAN-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.lsan-aarch64.a"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fsanitize=leak 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-X86
+// CHECK-LSAN-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-X86: "-fsanitize=leak,safe-stack"
+// CHECK-LSAN-X86: "-pie"
+// CHECK-LSAN-X86: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}x86_64-fuchsia{{/|}}libclang_rt.lsan-x86_64.a"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fsanitize=leak -fPIC -shared 2>&1 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld \
+// RUN: | FileCheck %s -check-prefix=CHECK-LSAN-SHARED
+// CHECK-LSAN-SHARED: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LSAN-SHARED: "-fsanitize=leak,shadow-call-stack"
+// CHECK-LSAN-SHARED-NOT: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.lsan-aarch64.a"
+
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fxray-instrument -fxray-modes=xray-basic \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -340,6 +340,7 @@
   Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
+  Res |= SanitizerKind::Leak;
   Res |= SanitizerKind::SafeStack;
   Res |= SanitizerKind::Scudo;
   return Res;
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -642,17 +642,21 @@
 StaticRuntimes.push_back("stats_client");
 
   // Collect static runtimes.
-  if (Args.hasArg(options::OPT_shared) || SanArgs.needsSharedRt()) {
-// Don't link static runtimes into DSOs or if -shared-libasan.
+  if (Args.hasArg(options::OPT_shared)) {
+// Don't link static runtimes into DSOs.
 return;
   }
-  if (SanArgs.needsAsanRt() && SanArgs.linkRuntimes()) {
+
+  // Each static runtime that has a DSO counterpart above is excluded below,
+  // but runtimes that exist only as static are not affected by needsSharedRt.
+
+  if (!SanArgs.needsSharedRt() && SanArgs.needsAsanRt() && SanArgs.linkRuntimes()) {
 StaticRuntimes.push_back("asan");
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("asan_cxx");
   }
 
-  if (SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
+  if (!SanArgs.needsSharedRt() && SanArgs.needsHwasanRt() && SanArgs.linkRuntimes()) {
 StaticRuntimes.push_back("hwasan");
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("hwasan_cxx");
@@ -671,7 +675,7 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("tsan_cxx");
   }
-  if (SanArgs.needsUbsanRt() && SanArgs.linkRuntimes()) {
+  if (!SanArgs.needsSharedRt() && SanArgs.needsUbsanRt() && SanArgs.linkRuntimes()) {
 if (SanArgs.requiresMinimalRuntime()) {
   StaticRuntimes.push_back("ubsan_minimal");
 } else {
@@ -684,18 +688,20 @@
 NonWholeStaticRuntimes.push_back("safestack");
 RequiredSymbols.push_back("__safestack_init");
   }
-  if (SanArgs.