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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D152405#4645719 , @dyung wrote:

> Hi @leonardchan your change was still causing a failure on the PS4 linux bot, 
> so I reverted it in 070493ddbd9473499d6f00ca62bc6aa92808ed79 
> . I 
> noticed earlier that the failing test omit-rtti-component-without-no-rtti.cpp 
> was failing on both the linux and Windows PS bots, and I don't think it is 
> because it was on Windows, but rather, these bots use PS4 and PS5 as the 
> default targets, on which rtti is not enabled by default. The run command on 
> line 4 in your test tests the "default" behavior which on most platforms 
> seems to have rtti enabled by default. If I remove that line, the test then 
> passes on both of the PS targets. To reland the change, you might want to 
> consider removing that line as it makes an assumption which does not seem to 
> be true for all targets.

Ahh. Thanks for the revert and the info. I'll reland with that removed.


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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2250
+def fexperimental_omit_rtti_component :
+  Flag<["-"], "fexperimental-omit-rtti-component">,
+  Group, Flags<[CC1Option]>,

phosek wrote:
> I think the name should signal that this is related to vtables (otherwise 
> users might be wondering what "component" is this referring to).
> 
> We could use `-fexperimental-omit-vtable-rtti-component` but that's quite a 
> mouthful, maybe `-fexperimental-omit-vtable-rtti` would suffice?
> I think the name should signal that this is related to vtables (otherwise 
> users might be wondering what "component" is this referring to).

Formally, this can be used independently of relative vtables. So under the 
regular itanium c++ abi this would reduce the vtable by a pointer size and 
under RV it would reduce the vtable by an offset size. "Component" I believe is 
the formal name for the actual elements in the vtable: 
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vtable-components.

> We could use -fexperimental-omit-vtable-rtti-component but that's quite a 
> mouthful, maybe -fexperimental-omit-vtable-rtti would suffice?

WFM


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

[Leonard Chan 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 rG6385c1df919f: [clang] Add experimental option to omit the 
RTTI component from the vtable when… (authored by leonardchan).

Changed prior to commit:
  https://reviews.llvm.org/D152405?vs=556724=556732#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152405

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/simple-vtable-definition.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
  clang/test/Driver/omit-rtti-component-flag.cpp
  clang/test/Driver/omit-rtti-component-without-no-rtti.cpp

Index: clang/test/Driver/omit-rtti-component-without-no-rtti.cpp
===
--- /dev/null
+++ clang/test/Driver/omit-rtti-component-without-no-rtti.cpp
@@ -0,0 +1,13 @@
+/// Ensure that -fexperimental-omit-vtable-rtti is only allowed if rtti is
+/// disabled.
+
+// RUN: not %clang -c -Xclang -fexperimental-omit-vtable-rtti %s 2>&1 | FileCheck -check-prefix=ERROR %s
+// RUN: not %clang -c -Xclang -fexperimental-omit-vtable-rtti -frtti %s 2>&1 | FileCheck -check-prefix=ERROR %s
+// RUN: not %clang -c -Xclang -fexperimental-omit-vtable-rtti -fno-rtti -frtti %s 2>&1 | FileCheck -check-prefix=ERROR %s
+
+// RUN: %clang -c -Xclang -fexperimental-omit-vtable-rtti -fno-rtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+// RUN: %clang -c -Xclang -fno-experimental-omit-vtable-rtti -frtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+// RUN: %clang -c -Xclang -fexperimental-omit-vtable-rtti -Xclang -fno-experimental-omit-vtable-rtti -frtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+
+// ERROR: -fexperimental-omit-vtable-rtti call only be used with -fno-rtti
+// NO-ERROR-NOT: -fexperimental-omit-vtable-rtti call only be used with -fno-rtti
Index: clang/test/Driver/omit-rtti-component-flag.cpp
===
--- /dev/null
+++ clang/test/Driver/omit-rtti-component-flag.cpp
@@ -0,0 +1,5 @@
+// RUN: %clangxx --target=aarch64-unknown-linux -fno-rtti -Xclang -fexperimental-omit-vtable-rtti -c %s -### 2>&1 | FileCheck %s --check-prefix=OMIT
+// RUN: %clangxx --target=aarch64-unknown-linux -fno-rtti -Xclang -fno-experimental-omit-vtable-rtti -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-OMIT
+
+// OMIT: "-fexperimental-omit-vtable-rtti"
+// NO-OMIT-NOT: "-fexperimental-omit-vtable-rtti"
Index: clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
@@ -0,0 +1,19 @@
+/// Ensure -fdump-vtable-layout omits the rtti component when passed -fexperimental-omit-vtable-rtti.
+
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fno-rtti -fexperimental-omit-vtable-rtti -emit-llvm-only -fdump-vtable-layouts | FileCheck %s
+
+// CHECK:  Vtable for 'A' (2 entries).
+// CHECK-NEXT:0 | offset_to_top (0)
+// CHECK-NEXT:-- (A, 0) vtable address --
+// CHECK-NEXT:1 | void A::foo()
+
+class A {
+public:
+  virtual void foo();
+};
+
+void A::foo() {}
+
+void A_foo(A *a) {
+  a->foo();
+}
Index: clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
@@ -0,0 +1,51 @@
+/// Check that the offset to top calculation is adjusted to account for the
+/// omitted RTTI entry.
+
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fexperimental-omit-vtable-rtti -fno-rtti -S -o - -emit-llvm | FileCheck -check-prefixes=POINTER %s
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fexperimental-relative-c++-abi-vtables -fexperimental-omit-vtable-rtti -fno-rtti -S -o - -emit-llvm | FileCheck -check-prefixes=RELATIVE %s
+
+/// Some important things to check:
+/// - The n16 here represents the virtual thunk size. Normally this would be 24
+///   to represent 3 components (offset to top, RTTI component, vcall offset),
+///   but since one 8-byte component is removed, this is now 16.
+// POINTER-LABEL: @_ZTv0_n16_N7Derived1fEi(
+// POINTER-NEXT:  entry:
+// POINTER:[[vtable:%.+]] = load ptr, ptr %this1, align 8
+
+/// Same here - When getting the vbase offset, we subtract 2 pointer sizes
+/// instead of 3.
+// POINTER-NEXT:   [[vbase_offset_ptr:%.+]] = getelementptr inbounds i8, ptr [[vtable]], i64 -16
+// 

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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 556724.
leonardchan marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152405

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/simple-vtable-definition.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
  clang/test/Driver/omit-rtti-component-flag.cpp
  clang/test/Driver/omit-rtti-component-without-no-rtti.cpp

Index: clang/test/Driver/omit-rtti-component-without-no-rtti.cpp
===
--- /dev/null
+++ clang/test/Driver/omit-rtti-component-without-no-rtti.cpp
@@ -0,0 +1,13 @@
+/// Ensure that -fexperimental-omit-vtable-rtti is only allowed if rtti is
+/// disabled.
+
+// RUN: not %clang -c -Xclang -fexperimental-omit-vtable-rtti %s 2>&1 | FileCheck -check-prefix=ERROR %s
+// RUN: not %clang -c -Xclang -fexperimental-omit-vtable-rtti -frtti %s 2>&1 | FileCheck -check-prefix=ERROR %s
+// RUN: not %clang -c -Xclang -fexperimental-omit-vtable-rtti -fno-rtti -frtti %s 2>&1 | FileCheck -check-prefix=ERROR %s
+
+// RUN: %clang -c -Xclang -fexperimental-omit-vtable-rtti -fno-rtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+// RUN: %clang -c -Xclang -fno-experimental-omit-vtable-rtti -frtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+// RUN: %clang -c -Xclang -fexperimental-omit-vtable-rtti -Xclang -fno-experimental-omit-vtable-rtti -frtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+
+// ERROR: -fexperimental-omit-vtable-rtti call only be used with -fno-rtti
+// NO-ERROR-NOT: -fexperimental-omit-vtable-rtti call only be used with -fno-rtti
Index: clang/test/Driver/omit-rtti-component-flag.cpp
===
--- /dev/null
+++ clang/test/Driver/omit-rtti-component-flag.cpp
@@ -0,0 +1,5 @@
+// RUN: %clangxx --target=aarch64-unknown-linux -fno-rtti -Xclang -fexperimental-omit-vtable-rtti -c %s -### 2>&1 | FileCheck %s --check-prefix=OMIT
+// RUN: %clangxx --target=aarch64-unknown-linux -fno-rtti -Xclang -fno-experimental-omit-vtable-rtti -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-OMIT
+
+// OMIT: "-fexperimental-omit-vtable-rtti"
+// NO-OMIT-NOT: "-fexperimental-omit-vtable-rtti"
Index: clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
@@ -0,0 +1,19 @@
+/// Ensure -fdump-vtable-layout omits the rtti component when passed -fexperimental-omit-vtable-rtti.
+
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fno-rtti -fexperimental-omit-vtable-rtti -emit-llvm-only -fdump-vtable-layouts | FileCheck %s
+
+// CHECK:  Vtable for 'A' (2 entries).
+// CHECK-NEXT:0 | offset_to_top (0)
+// CHECK-NEXT:-- (A, 0) vtable address --
+// CHECK-NEXT:1 | void A::foo()
+
+class A {
+public:
+  virtual void foo();
+};
+
+void A::foo() {}
+
+void A_foo(A *a) {
+  a->foo();
+}
Index: clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
@@ -0,0 +1,51 @@
+/// Check that the offset to top calculation is adjusted to account for the
+/// omitted RTTI entry.
+
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fexperimental-omit-vtable-rtti -fno-rtti -S -o - -emit-llvm | FileCheck -check-prefixes=POINTER %s
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fexperimental-relative-c++-abi-vtables -fexperimental-omit-vtable-rtti -fno-rtti -S -o - -emit-llvm | FileCheck -check-prefixes=RELATIVE %s
+
+/// Some important things to check:
+/// - The n16 here represents the virtual thunk size. Normally this would be 24
+///   to represent 3 components (offset to top, RTTI component, vcall offset),
+///   but since one 8-byte component is removed, this is now 16.
+// POINTER-LABEL: @_ZTv0_n16_N7Derived1fEi(
+// POINTER-NEXT:  entry:
+// POINTER:[[vtable:%.+]] = load ptr, ptr %this1, align 8
+
+/// Same here - When getting the vbase offset, we subtract 2 pointer sizes
+/// instead of 3.
+// POINTER-NEXT:   [[vbase_offset_ptr:%.+]] = getelementptr inbounds i8, ptr [[vtable]], i64 -16
+// POINTER-NEXT:   [[vbase_offset:%.+]] = load i64, ptr [[vbase_offset_ptr]], align 8
+// POINTER-NEXT:   [[adj_this:%.+]] = getelementptr inbounds i8, ptr %this1, i64 [[vbase_offset]]
+// POINTER:   [[call:%.+]] = tail call noundef i32 @_ZN7Derived1fEi(ptr 

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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D152405#4404742 , @mcgrathr wrote:

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

In the kernel it saves us about 664 bytes with RV. For some embedded devices I 
measured, savings is on the order of 1-2kB. For chromium with RV, it would save 
about 77076 bytes from rodata, but standalone without RV, it's twice that so 
154152 bytes saved from .data.rel.ro and this would scale based on the number 
of processes.


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] D72959: Relative VTables ABI on Fuchsia

[Leonard Chan via Phabricator via cfe-commits]

leonardchan abandoned this revision.
leonardchan marked an inline comment as done.
leonardchan added a comment.
Herald added a subscriber: bd1976llvm.

This was landed in various other smaller changes


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] D152472: [Clang][MS] Remove assertion on BaseOffset can't be smaller than Size.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hi. I think this caused the override-layout.cpp test to fail on our windows 
builder 
(https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8778279637538184401/+/u/clang/test/stdout?format=raw):

  FAIL: Clang :: CodeGenCXX/override-layout.cpp (7816 of 18732)
   TEST 'Clang :: CodeGenCXX/override-layout.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe -cc1 
-internal-isystem c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include 
-nostdsysteminc -std=c++14 -w -fdump-record-layouts-simple 
C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp > 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.layouts
  : 'RUN: at line 2';   c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe -cc1 
-internal-isystem c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include 
-nostdsysteminc -std=c++14 -w -fdump-record-layouts-simple 
C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp > 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.before
  : 'RUN: at line 3';   c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe -cc1 
-internal-isystem c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include 
-nostdsysteminc -std=c++14 -w -DPACKED= -DALIGNED16= 
-fdump-record-layouts-simple 
-foverride-record-layout=C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.layouts
 C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp > 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.after
  : 'RUN: at line 4';   diff -u 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.before
 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.after
  : 'RUN: at line 5';   c:\b\s\w\ir\x\w\staging\llvm_build\bin\filecheck.exe 
--check-prefixes=CHECK,PRE17 
C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp < 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.after
  : 'RUN: at line 7';   c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe -cc1 
-internal-isystem c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include 
-nostdsysteminc -std=c++17 -w -fdump-record-layouts-simple 
C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp > 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.layouts
  : 'RUN: at line 8';   c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe -cc1 
-internal-isystem c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include 
-nostdsysteminc -std=c++17 -w -fdump-record-layouts-simple 
C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp > 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.before
  : 'RUN: at line 9';   c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe -cc1 
-internal-isystem c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include 
-nostdsysteminc -std=c++17 -w -DPACKED= -DALIGNED16= 
-fdump-record-layouts-simple 
-foverride-record-layout=C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.layouts
 C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp > 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.after
  : 'RUN: at line 10';   diff -u 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.before
 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.after
  : 'RUN: at line 11';   c:\b\s\w\ir\x\w\staging\llvm_build\bin\filecheck.exe 
--check-prefixes=CHECK,CXX17 
C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp < 
C:\b\s\w\ir\x\w\staging\llvm_build\tools\clang\test\CodeGenCXX\Output\override-layout.cpp.tmp.after
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ "c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe" "-cc1" 
"-internal-isystem" "c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include" 
"-nostdsysteminc" "-std=c++14" "-w" "-fdump-record-layouts-simple" 
"C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp"
  $ ":" "RUN: at line 2"
  $ "c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe" "-cc1" 
"-internal-isystem" "c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include" 
"-nostdsysteminc" "-std=c++14" "-w" "-fdump-record-layouts-simple" 
"C:\b\s\w\ir\x\w\llvm-llvm-project\clang\test\CodeGenCXX\override-layout.cpp"
  $ ":" "RUN: at line 3"
  $ "c:\b\s\w\ir\x\w\staging\llvm_build\bin\clang.exe" "-cc1" 
"-internal-isystem" "c:\b\s\w\ir\x\w\staging\llvm_build\lib\clang\17\include" 
"-nostdsysteminc" 

[PATCH] D132275: [clang] Create alloca to pass into static lambda

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Ok here's some of my findings. So there's a step in the attributor where it 
replaces some instructions with `unreachable`. One step is:

  *** IR Dump Before AttributorPass on [module] ***
  ; Function Attrs: inlinehint minsize nounwind optsize
  define linkonce_odr dso_local i8 
@_ZZN2pw3rpc8internal12NanopbMethod16SynchronousUnaryIXadL_ZN2ax3bud7bt_core20global_state_manager18GlobalStateManager20PropagateGlobalStateERK26_global_state_StateR18_pw_protobuf_EmptyS2_jRKNS1_17NanopbMethodSerdeEENUlRNS0_7ServiceEPKvPvE_8__invokeESI_SK_SL_(ptr
 noundef nonnull align 4 dereferenceable(16) %0, ptr noundef %1, ptr noundef 
%2) #3 comdat align 2 !dbg !12909 {
call void @llvm.dbg.value(metadata ptr %0, metadata !12913, metadata 
!DIExpression()), !dbg !12916
call void @llvm.dbg.value(metadata ptr %1, metadata !12914, metadata 
!DIExpression()), !dbg !12916
call void @llvm.dbg.value(metadata ptr %2, metadata !12915, metadata 
!DIExpression()), !dbg !12916
%4 = call i8 
@_ZZN2pw3rpc8internal12NanopbMethod16SynchronousUnaryIXadL_ZN2ax3bud7bt_core20global_state_manager18GlobalStateManager20PropagateGlobalStateERK26_global_state_StateR18_pw_protobuf_EmptyS2_jRKNS1_17NanopbMethodSerdeEENKUlRNS0_7ServiceEPKvPvE_clESI_SK_SL_(ptr
 noundef nonnull align 1 dereferenceable(1) undef, ptr noundef nonnull align 4 
dereferenceable(16) %0, ptr noundef %1, ptr noundef %2) #12, !dbg !12917
ret i8 %4, !dbg !12917
  }
  *** IR Dump After AttributorPass on [module] ***
  ; Function Attrs: inlinehint minsize nounwind optsize
  define linkonce_odr dso_local i8 
@_ZZN2pw3rpc8internal12NanopbMethod16SynchronousUnaryIXadL_ZN2ax3bud7bt_core20global_state_manager18GlobalStateManager20PropagateGlobalStateERK26_global_state_StateR18_pw_protobuf_EmptyS2_jRKNS1_17NanopbMethodSerdeEENUlRNS0_7ServiceEPKvPvE_8__invokeESI_SK_SL_(ptr
 noundef nonnull align 4 dereferenceable(16) %0, ptr noundef %1, ptr noundef 
%2) #3 comdat align 2 !dbg !12906 {
call void @llvm.dbg.value(metadata ptr %0, metadata !12910, metadata 
!DIExpression()), !dbg !12913
call void @llvm.dbg.value(metadata ptr %1, metadata !12911, metadata 
!DIExpression()), !dbg !12913
call void @llvm.dbg.value(metadata ptr %2, metadata !12912, metadata 
!DIExpression()), !dbg !12913
unreachable, !dbg !12914
  }

The first argument in the call is an `undef` but the argument type is also 
marked as `noundef`, so this is unreachable. With this patch, in the final IR, 
the function will instead accept some non-undef argument:

  define linkonce_odr dso_local i8 
@_ZZN2pw3rpc8internal12NanopbMethod16SynchronousUnaryIXadL_ZN2ax3bud7bt_core20global_state_manager18GlobalStateManager20PropagateGlobalStateERK26_global_state_StateR18_pw_protobuf_EmptyS2_jRKNS1_17NanopbMethodSerdeEENUlRNS0_7ServiceEPKvPvE_8__invokeESI_SK_SL_(ptr
 noundef nonnull align 4 dereferenceable(16) %0, ptr noundef nonnull %1, ptr 
noundef %2) #3 comdat align 2 !dbg !14764 {
  ;; .. a bunch of llvm.dbg.values
tail call void 
@_ZN2ax3bud7bt_core20global_state_manager18GlobalStateManager12ReceiveStateERK26_global_state_State(ptr
 noundef nonnull align 8 dereferenceable(3864) %0, ptr noundef nonnull align 8 
dereferenceable(528) %1) #17, !dbg !14808
ret i8 0, !dbg !14809
  }

Without this patch, before any passes run, this call originally took an `undef`:

  define linkonce_odr dso_local i8 
@_ZZN2pw3rpc8internal12NanopbMethod16SynchronousUnaryIXadL_ZN2ax3bud7bt_core20global_state_manager18GlobalStateManager20PropagateGlobalStateERK26_global_state_StateR18_pw_protobuf_EmptyS2_jRKNS1_17NanopbMethodSerdeEENUlRNS0_7ServiceEPKvPv
  E_8__invokeESI_SK_SL_(ptr noundef nonnull align 4 dereferenceable(16) %0, ptr 
noundef %1, ptr noundef %2) #3 comdat align 2 !dbg !14493 {
%4 = alloca %"class.pw::Status", align 1
%5 = alloca ptr, align 4
%6 = alloca ptr, align 4
%7 = alloca ptr, align 4
%8 = alloca %"class.pw::Status", align 1
store ptr %0, ptr %5, align 4, !tbaa !8211
call void @llvm.dbg.declare(metadata ptr %5, metadata !14497, metadata 
!DIExpression()), !dbg !14500
store ptr %1, ptr %6, align 4, !tbaa !8211
call void @llvm.dbg.declare(metadata ptr %6, metadata !14498, metadata 
!DIExpression()), !dbg !14500
store ptr %2, ptr %7, align 4, !tbaa !8211
call void @llvm.dbg.declare(metadata ptr %7, metadata !14499, metadata 
!DIExpression()), !dbg !14500
%9 = load ptr, ptr %5, align 4, !dbg !14501
%10 = load ptr, ptr %6, align 4, !dbg !14501, !tbaa !8211
%11 = load ptr, ptr %7, align 4, !dbg !14501, !tbaa !8211
%12 = call i8 
@_ZZN2pw3rpc8internal12NanopbMethod16SynchronousUnaryIXadL_ZN2ax3bud7bt_core20global_state_manager18GlobalStateManager20PropagateGlobalStateERK26_global_state_StateR18_pw_protobuf_EmptyS2_jRKNS1_17NanopbMethodSerdeEENKUlRNS0_7ServiceEPKvPvE_clESI_SK_SL_(p
  tr noundef nonnull align 1 dereferenceable(1) undef, ptr noundef nonnull 
align 4 dereferenceable(16) %9, ptr 

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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.



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

Yup, the cases here already test with the regular itanium c++ abi and relative 
vtables abi.


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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D152405#4404596 , @mcgrathr wrote:

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

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

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


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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr.
leonardchan added a project: clang.
Herald added a project: All.
leonardchan requested review of this revision.
Herald added a subscriber: MaskRay.

For programs that don't use RTTI, the rtti component is just replaced with a 
zero. This way, vtables that don't use RTTI can still cooperate with vtables 
that use RTTI since offset calculations on the ABI level would still work. 
However, if throughout your whole program you don't use RTTI at all (such as 
the embedded case), then this is just an unused pointer-sized component that's 
wasting space. This adds an experimental option for removing the RTTI component 
from the vtable.

Some notes:

- This is only allowed when RTTI is disabled, so we don't have to worry about 
things like `typeid` or `dynamic_cast`.
- This is a "use at your own risk" since, similar to relative vtables, 
everything must be compiled with this since it's an ABI breakage. That is, a 
program compiled with this is not guaranteed to work with a program compiled 
without this, even if RTTI is disabled for both programs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152405

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/simple-vtable-definition.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
  clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
  clang/test/Driver/omit-rtti-component-flag.cpp
  clang/test/Driver/omit-rtti-component-without-no-rtti.cpp

Index: clang/test/Driver/omit-rtti-component-without-no-rtti.cpp
===
--- /dev/null
+++ clang/test/Driver/omit-rtti-component-without-no-rtti.cpp
@@ -0,0 +1,13 @@
+/// Ensure that -fexperimental-omit-rtti-component is only allowed if rtti is
+/// disabled.
+
+// RUN: not %clang -c -fexperimental-omit-rtti-component %s 2>&1 | FileCheck -check-prefix=ERROR %s
+// RUN: not %clang -c -fexperimental-omit-rtti-component -frtti %s 2>&1 | FileCheck -check-prefix=ERROR %s
+// RUN: not %clang -c -fexperimental-omit-rtti-component -fno-rtti -frtti %s 2>&1 | FileCheck -check-prefix=ERROR %s
+
+// RUN: %clang -c -fexperimental-omit-rtti-component -fno-rtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+// RUN: %clang -c -fno-experimental-omit-rtti-component -frtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+// RUN: %clang -c -fexperimental-omit-rtti-component -fno-experimental-omit-rtti-component -frtti %s 2>&1 | FileCheck -check-prefix=NO-ERROR %s --allow-empty
+
+// ERROR: -fexperimental-omit-rtti-component call only be used with -fno-rtti
+// NO-ERROR-NOT: -fexperimental-omit-rtti-component call only be used with -fno-rtti
Index: clang/test/Driver/omit-rtti-component-flag.cpp
===
--- /dev/null
+++ clang/test/Driver/omit-rtti-component-flag.cpp
@@ -0,0 +1,5 @@
+// RUN: %clangxx --target=aarch64-unknown-linux -fno-rtti -fexperimental-omit-rtti-component -c %s -### 2>&1 | FileCheck %s --check-prefix=OMIT
+// RUN: %clangxx --target=aarch64-unknown-linux -fno-rtti -fno-experimental-omit-rtti-component -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-OMIT
+
+// OMIT: "-fexperimental-omit-rtti-component"
+// NO-OMIT-NOT: "-fexperimental-omit-rtti-component"
Index: clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/OmitRTTIComponentABI/vtable-layout.cpp
@@ -0,0 +1,19 @@
+/// Ensure -fdump-vtable-layout omits the rtti component when passed -fexperimental-omit-rtti-component.
+
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fno-rtti -fexperimental-omit-rtti-component -emit-llvm-only -fdump-vtable-layouts | FileCheck %s
+
+// CHECK:  Vtable for 'A' (2 entries).
+// CHECK-NEXT:0 | offset_to_top (0)
+// CHECK-NEXT:-- (A, 0) vtable address --
+// CHECK-NEXT:1 | void A::foo()
+
+class A {
+public:
+  virtual void foo();
+};
+
+void A::foo() {}
+
+void A_foo(A *a) {
+  a->foo();
+}
Index: clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/OmitRTTIComponentABI/vbase-offset.cpp
@@ -0,0 +1,51 @@
+/// Check that the offset to top calculation is adjusted to account for the
+/// omitted RTTI entry.
+
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fexperimental-omit-rtti-component -fno-rtti -S -o - -emit-llvm | FileCheck -check-prefixes=POINTER %s
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-linux-gnu -fexperimental-relative-c++-abi-vtables 

[PATCH] D132275: [clang] Create alloca to pass into static lambda

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added subscribers: jdoerfert, leonardchan.
leonardchan added a comment.

Hi. For an embedded device, prior to this patch, we used to be able to see a 
significant savings of 28kB when we enabled the attributor. But with this 
patch, we don't see these savings anymore when enabling the attributor. I don't 
suppose folks might have any ideas on how this might affect the attributor? I'm 
going to attempt to make a minimal reproducer. Just chiming in early in case 
anyone might happen to know off the top of their head.

cc @jdoerfert who owns the attributor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132275

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

[PATCH] D147209: [clang][lit] Make LIT aware of env CLANG_CRASH_DIAGNOSTICS_DIR.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D147209#4245229 , @fpetrogalli 
wrote:

> In D147209#4244615 , @leonardchan 
> wrote:
>
>> Hi. I think this led to some test failures on our builder at 
>> https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8784708268307261009/overview:
>
> Hi @leonardchan - thank you for the heads up!
>
> I do not have access to this bot (the link you sent just shows me a blank 
> page), and I have not seen this (clang?) test failing in my local build. I 
> can revert the patch, but I'll need some more info to be able to fix those 
> failures.
>
> Francesco

Hmm, that's interesting since the link should be public and normally doesn't 
just appear as blank. Does it still show up as blank if you click it again? 
Otherwise I can share the full repro. I think you should be able to reproduce 
this if you define `CLANG_CRASH_DIAGNOSTICS_DIR` as an environment variable 
before running `ninja check-clang`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147209

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

[PATCH] D147209: [clang][lit] Make LIT aware of env CLANG_CRASH_DIAGNOSTICS_DIR.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hi. I think this led to some test failures on our builder at 
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8784708268307261009/overview:

  Script:
  --
  : 'RUN: at line 4';   rm -rf 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir
  : 'RUN: at line 5';   mkdir -p 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/i 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/m
  : 'RUN: at line 7';   env FORCE_CLANG_DIAGNOSTICS_CRASH= 
TMPDIR=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir 
TEMP=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir 
TMP=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir  not 
/b/s/w/ir/x/w/staging/llvm_build/bin/clang -fsyntax-only 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/crash-report-modules.m -I 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/Inputs/module -isysroot 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crash-report-modules.m.tmp/i/
  -fmodules 
-fmodules-cache-path=/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/m/
 -DFOO=BAR 2>&1 | /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/crash-report-modules.m
  : 'RUN: at line 11';   /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
--check-prefix=CHECKSRC 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/crash-report-modules.m 
-input-file 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/crash-report-*.m
  : 'RUN: at line 12';   /b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
--check-prefix=CHECKSH 
/b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/crash-report-modules.m 
-input-file 
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/crash-report-*.sh
  --
  Exit Code: 2
  
  Command Output (stderr):
  --
  Could not open input file 
'/b/s/w/ir/x/w/staging/llvm_build/tools/clang/test/Driver/Output/crmdir/crash-report-*.m':
 No such file or directory
  --

I think perhaps since we define `CLANG_CRASH_DIAGNOSTICS_DIR` as an env, tests 
that produce crash reports place them there, but those tests also expect the 
crash report to be in a temp dir `%T`, so perhaps some tests need to be updated 
along side this? Would you be able to take a look and fix or revert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147209

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

[PATCH] D144291: [clang-format-diff] Correctly parse start-of-file diffs

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG50563944ab96: [clang-format-diff] Correctly parse 
start-of-file diffs (authored by tamird, committed by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144291

Files:
  clang/tools/clang-format/clang-format-diff.py


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -84,12 +84,19 @@
   if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
 continue
 
-match = re.search(r'^@@.*\+(\d+)(,(\d+))?', line)
+match = re.search(r'^@@.*\+(\d+)(?:,(\d+))?', line)
 if match:
   start_line = int(match.group(1))
   line_count = 1
-  if match.group(3):
-line_count = int(match.group(3))
+  if match.group(2):
+line_count = int(match.group(2))
+# The input is something like
+#
+# @@ -1, +0,0 @@
+#
+# which means no lines were added.
+if line_count == 0:
+  continue
   # Also format lines range if line_count is 0 in case of deleting
   # surrounding statements.
   end_line = start_line


Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -84,12 +84,19 @@
   if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
 continue
 
-match = re.search(r'^@@.*\+(\d+)(,(\d+))?', line)
+match = re.search(r'^@@.*\+(\d+)(?:,(\d+))?', line)
 if match:
   start_line = int(match.group(1))
   line_count = 1
-  if match.group(3):
-line_count = int(match.group(3))
+  if match.group(2):
+line_count = int(match.group(2))
+# The input is something like
+#
+# @@ -1, +0,0 @@
+#
+# which means no lines were added.
+if line_count == 0:
+  continue
   # Also format lines range if line_count is 0 in case of deleting
   # surrounding statements.
   end_line = start_line
___
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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan abandoned this revision.
leonardchan added a comment.

This was landed in 
https://reviews.llvm.org/rG4308166403b4c28b6db7094a4e202e42da6e28a8


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] D144035: [SCEV] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a subscriber: reames.
leonardchan added a comment.

@reames let's continue the discussion here. What specific bits with the 
optimizer does this break? I'm not too familiar with SVEC. Prior to this, one 
thing I tried was instead just doing `Ops.push_back(GA)` and `getSCEV(GA)` but 
those seemed to result in infinite recursions last I checked probably because 
there's another pass attempting to resolve aliases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144035: [SCEV] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG608ee703e530: [SCEV] Ensure SCEV does not replace aliases 
with their aliasees (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

Files:
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll


Index: llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
===
--- /dev/null
+++ llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by 
utils/update_analyze_test_checks.py
+; RUN: opt -passes='print' -disable-output %s 2>&1 | 
FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@glob.private = private constant [32 x i32] zeroinitializer
+@glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint 
(ptr @glob.private to i64), i64 1234) to ptr)
+
+define hidden void @func(i64 %idx) local_unnamed_addr {
+; CHECK-LABEL: 'func'
+; CHECK-NEXT:  Classifying expressions for: @func
+; CHECK-NEXT:%arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 
0, i64 %idx
+; CHECK-NEXT:--> ((4 * %idx) + @glob) U: [0,-1) S: 
[-9223372036854775808,9223372036854775807)
+; CHECK-NEXT:  Determining loop execution counts for: @func
+;
+  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+  ret void
+}
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -7429,10 +7429,6 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V)) {
-if (!GA->isInterposable()) {
-  Ops.push_back(GA->getAliasee());
-  return nullptr;
-}
 return getUnknown(V);
   } else if (!isa(V))
 return getUnknown(V);
@@ -7619,7 +7615,7 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V))
-return GA->isInterposable() ? getUnknown(V) : getSCEV(GA->getAliasee());
+return getUnknown(V);
   else if (!isa(V))
 return getUnknown(V);
 


Index: llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
===
--- /dev/null
+++ llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
+; RUN: opt -passes='print' -disable-output %s 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@glob.private = private constant [32 x i32] zeroinitializer
+@glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint (ptr @glob.private to i64), i64 1234) to ptr)
+
+define hidden void @func(i64 %idx) local_unnamed_addr {
+; CHECK-LABEL: 'func'
+; CHECK-NEXT:  Classifying expressions for: @func
+; CHECK-NEXT:%arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+; CHECK-NEXT:--> ((4 * %idx) + @glob) U: [0,-1) S: [-9223372036854775808,9223372036854775807)
+; CHECK-NEXT:  Determining loop execution counts for: @func
+;
+  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+  ret void
+}
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -7429,10 +7429,6 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V)) {
-if (!GA->isInterposable()) {
-  Ops.push_back(GA->getAliasee());
-  return nullptr;
-}
 return getUnknown(V);
   } else if (!isa(V))
 return getUnknown(V);
@@ -7619,7 +7615,7 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V))
-return GA->isInterposable() ? getUnknown(V) : getSCEV(GA->getAliasee());
+return getUnknown(V);
   else if (!isa(V))
 return getUnknown(V);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144035: [SCEV] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

@pcc does this lgty?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D144035#4145662 , @MaskRay wrote:

>> Full context at https://github.com/llvm/llvm-project/issues/60668.
>
> Best to leave a simplified description in the summary so that a reader 
> doesn't have to go to the external link and summarize the discussions 
> themselves.
> And give a link to https://reviews.llvm.org/D66606
>
> This SCEV function is invoked by `FunctionPass Manager -> Loop Pass Manager 
> -> Induction Variable Users` in the codegen pipeline.

Updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 499668.
leonardchan marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

Files:
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll


Index: llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
===
--- /dev/null
+++ llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by 
utils/update_analyze_test_checks.py
+; RUN: opt -passes='print' -disable-output %s 2>&1 | 
FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@glob.private = private constant [32 x i32] zeroinitializer
+@glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint 
(ptr @glob.private to i64), i64 1234) to ptr)
+
+define hidden void @func(i64 %idx) local_unnamed_addr {
+; CHECK-LABEL: 'func'
+; CHECK-NEXT:  Classifying expressions for: @func
+; CHECK-NEXT:%arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 
0, i64 %idx
+; CHECK-NEXT:--> ((4 * %idx) + @glob) U: [0,-1) S: 
[-9223372036854775808,9223372036854775807)
+; CHECK-NEXT:  Determining loop execution counts for: @func
+;
+  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+  ret void
+}
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -7449,10 +7449,6 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V)) {
-if (!GA->isInterposable()) {
-  Ops.push_back(GA->getAliasee());
-  return nullptr;
-}
 return getUnknown(V);
   } else if (!isa(V))
 return getUnknown(V);
@@ -7639,7 +7635,7 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V))
-return GA->isInterposable() ? getUnknown(V) : getSCEV(GA->getAliasee());
+return getUnknown(V);
   else if (!isa(V))
 return getUnknown(V);
 


Index: llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
===
--- /dev/null
+++ llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
+; RUN: opt -passes='print' -disable-output %s 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@glob.private = private constant [32 x i32] zeroinitializer
+@glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint (ptr @glob.private to i64), i64 1234) to ptr)
+
+define hidden void @func(i64 %idx) local_unnamed_addr {
+; CHECK-LABEL: 'func'
+; CHECK-NEXT:  Classifying expressions for: @func
+; CHECK-NEXT:%arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+; CHECK-NEXT:--> ((4 * %idx) + @glob) U: [0,-1) S: [-9223372036854775808,9223372036854775807)
+; CHECK-NEXT:  Determining loop execution counts for: @func
+;
+  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+  ret void
+}
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -7449,10 +7449,6 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V)) {
-if (!GA->isInterposable()) {
-  Ops.push_back(GA->getAliasee());
-  return nullptr;
-}
 return getUnknown(V);
   } else if (!isa(V))
 return getUnknown(V);
@@ -7639,7 +7635,7 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V))
-return GA->isInterposable() ? getUnknown(V) : getSCEV(GA->getAliasee());
+return getUnknown(V);
   else if (!isa(V))
 return getUnknown(V);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144035: [llvm] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D144035#4143030 , @pcc wrote:

> Passes shouldn't be replacing aliases with aliasees in general, see D66606 
> . The right fix is to fix SCEV to not do 
> that.

Updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144035: [hwasan] Ensure SCEV does not replace aliases with their aliasees

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 499349.
leonardchan retitled this revision from "[hwasan] Ensure hwasan aliases do not 
have ODR linkage" to "[hwasan] Ensure SCEV does not replace aliases with their 
aliasees".
leonardchan edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

Files:
  clang/test/CodeGenCXX/pr60668.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll


Index: llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
===
--- /dev/null
+++ llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by 
utils/update_analyze_test_checks.py
+; RUN: opt -passes='print' -disable-output %s 2>&1 | 
FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@glob.private = private constant [32 x i32] zeroinitializer
+@glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint 
(ptr @glob.private to i64), i64 1234) to ptr)
+
+define hidden void @func(i64 %idx) local_unnamed_addr {
+; CHECK-LABEL: 'func'
+; CHECK-NEXT:  Classifying expressions for: @func
+; CHECK-NEXT:%arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 
0, i64 %idx
+; CHECK-NEXT:--> ((4 * %idx) + @glob) U: [0,-1) S: 
[-9223372036854775808,9223372036854775807)
+; CHECK-NEXT:  Determining loop execution counts for: @func
+;
+  %arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+  ret void
+}
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -7449,10 +7449,6 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V)) {
-if (!GA->isInterposable()) {
-  Ops.push_back(GA->getAliasee());
-  return nullptr;
-}
 return getUnknown(V);
   } else if (!isa(V))
 return getUnknown(V);
@@ -7639,7 +7635,7 @@
   } else if (ConstantInt *CI = dyn_cast(V))
 return getConstant(CI);
   else if (GlobalAlias *GA = dyn_cast(V))
-return GA->isInterposable() ? getUnknown(V) : getSCEV(GA->getAliasee());
+return getUnknown(V);
   else if (!isa(V))
 return getUnknown(V);
 
Index: clang/test/CodeGenCXX/pr60668.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pr60668.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang --target=aarch64-unknown-linux-gnu -O1 -fvisibility=hidden \
+// RUN:   -fsanitize=hwaddress -fsanitize=undefined -std=c++17 -fno-exceptions 
\
+// RUN:   -fno-rtti -c %s -S -o - | FileCheck %s
+
+struct AndroidSizeClassConfig {
+  static constexpr unsigned Classes[] = {
+  0x00020, 0x00030, 0x00040, 0x00050, 0x00060, 0x00070, 0x00090, 0x000b0,
+  0x000c0, 0x000e0, 0x00120, 0x00160, 0x001c0, 0x00250, 0x00320, 0x00450,
+  0x00670, 0x00830, 0x00a10, 0x00c30, 0x01010, 0x01210, 0x01bd0, 0x02210,
+  0x02d90, 0x03790, 0x04010, 0x04810, 0x05a10, 0x07310, 0x08210, 0x10010,
+  };
+};
+
+static const unsigned NumClasses =
+sizeof(AndroidSizeClassConfig::Classes) / 
sizeof(AndroidSizeClassConfig::Classes[0]);
+
+void func(unsigned);
+
+void printMap() {
+  for (unsigned I = 0; I < NumClasses; I++) {
+func(AndroidSizeClassConfig::Classes[I]);
+  }
+}
+
+// CHECK-LABEL: _Z8printMapv
+// CHECK:adrpx{{.*}}, 
:pg_hi21_nc:_ZN22AndroidSizeClassConfig7ClassesE
+// CHECK:movkx{{.*}}, 
#:prel_g3:_ZN22AndroidSizeClassConfig7ClassesE+4294967296
+// CHECK:add x{{.*}}, x19, 
:lo12:_ZN22AndroidSizeClassConfig7ClassesE


Index: llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
===
--- /dev/null
+++ llvm/test/Analysis/ScalarEvolution/no-follow-alias.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
+; RUN: opt -passes='print' -disable-output %s 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-unknown-linux-gnu"
+
+@glob.private = private constant [32 x i32] zeroinitializer
+@glob = linkonce_odr hidden alias [32 x i32], inttoptr (i64 add (i64 ptrtoint (ptr @glob.private to i64), i64 1234) to ptr)
+
+define hidden void @func(i64 %idx) local_unnamed_addr {
+; CHECK-LABEL: 'func'
+; CHECK-NEXT:  Classifying expressions for: @func
+; CHECK-NEXT:%arrayidx = getelementptr inbounds [32 x i32], ptr @glob, i64 0, i64 %idx
+; CHECK-NEXT:--> ((4 * %idx) + @glob) U: [0,-1) S: [-9223372036854775808,9223372036854775807)
+; CHECK-NEXT:  Determining loop execution counts for: @func
+;
+  %arrayidx = getelementptr inbounds 

[PATCH] D144035: [hwasan] Ensure hwasan aliases do not have ODR linkage

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144035: [hwasan] Ensure hwasan aliases do not have ODR linkage

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Lemme know if the clang regression test isn't necessary or should be in its own 
commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144035

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

[PATCH] D144035: [hwasan] Ensure hwasan aliases do not have ODR linkage

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: pcc, eugenis, vitalybuka.
leonardchan added a project: Sanitizers.
Herald added subscribers: Enna1, jeroen.dobbelaere, hiraditya.
Herald added a project: All.
leonardchan requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

If an existing global has linkonce_odr linkage, then the alias adopts the 
global's linkonce_odr linkage. The ODR linkages should only be used if its 
guaranteed by the frontend that all definitions of the symbol are "equivalent", 
hence they are effectively non-interposable. It's very unlikely however for two 
aliases to be the same across multiple TUs since they'll likely have different 
tags. This is a problem if a pass replaces a use of the alias with its local 
aliasee because if another definition with a separate tag wins at link time, 
then the local aliasee usage is incorrect.

Full context at https://github.com/llvm/llvm-project/issues/60668.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144035

Files:
  clang/test/CodeGenCXX/pr60668.cpp
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/test/Instrumentation/HWAddressSanitizer/odr.ll


Index: llvm/test/Instrumentation/HWAddressSanitizer/odr.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/odr.ll
@@ -0,0 +1,7 @@
+; RUN: opt < %s -passes=hwasan -S | FileCheck %s
+
+; CHECK: @linkonce_odr_obj = linkonce hidden alias
+@linkonce_odr_obj = linkonce_odr hidden constant i32 1
+
+; CHECK: weak_odr_obj = weak hidden alias
+@weak_odr_obj = weak_odr hidden constant i32 1
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1568,8 +1568,24 @@
   ConstantExpr::getPtrToInt(NewGV, Int64Ty),
   ConstantInt::get(Int64Ty, uint64_t(Tag) << PointerTagShift)),
   GV->getType());
+
+  // Ensure the global doesn't have *ODRLinkage. Those linkages should only be
+  // used if all other definitions of the global are "equivalent" to this one.
+  // Since this alias could be defined in other TU's, it's very unlikely 
they'll
+  // all have the same tag.
+  //
+  // This fixes PR60668.
+  GlobalValue::LinkageTypes Linkage;
+  if (GV->hasLinkOnceODRLinkage()) {
+Linkage = GlobalValue::LinkOnceAnyLinkage;
+  } else if (GV->hasWeakODRLinkage()) {
+Linkage = GlobalValue::WeakAnyLinkage;
+  } else {
+Linkage = GV->getLinkage();
+  }
+
   auto *Alias = GlobalAlias::create(GV->getValueType(), GV->getAddressSpace(),
-GV->getLinkage(), "", Aliasee, );
+Linkage, "", Aliasee, );
   Alias->setVisibility(GV->getVisibility());
   Alias->takeName(GV);
   GV->replaceAllUsesWith(Alias);
Index: clang/test/CodeGenCXX/pr60668.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pr60668.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang --target=aarch64-unknown-linux-gnu -O1 -fvisibility=hidden \
+// RUN:   -fsanitize=hwaddress -fsanitize=undefined -std=c++17 -fno-exceptions 
\
+// RUN:   -fno-rtti -c %s -S -o - | FileCheck %s
+
+struct AndroidSizeClassConfig {
+  static constexpr unsigned Classes[] = {
+  0x00020, 0x00030, 0x00040, 0x00050, 0x00060, 0x00070, 0x00090, 0x000b0,
+  0x000c0, 0x000e0, 0x00120, 0x00160, 0x001c0, 0x00250, 0x00320, 0x00450,
+  0x00670, 0x00830, 0x00a10, 0x00c30, 0x01010, 0x01210, 0x01bd0, 0x02210,
+  0x02d90, 0x03790, 0x04010, 0x04810, 0x05a10, 0x07310, 0x08210, 0x10010,
+  };
+};
+
+static const unsigned NumClasses =
+sizeof(AndroidSizeClassConfig::Classes) / 
sizeof(AndroidSizeClassConfig::Classes[0]);
+
+void func(unsigned);
+
+void printMap() {
+  for (unsigned I = 0; I < NumClasses; I++) {
+func(AndroidSizeClassConfig::Classes[I]);
+  }
+}
+
+// CHECK-LABEL: _Z8printMapv
+// CHECK:adrpx{{.*}}, 
:pg_hi21_nc:_ZN22AndroidSizeClassConfig7ClassesE
+// CHECK:movkx{{.*}}, 
#:prel_g3:_ZN22AndroidSizeClassConfig7ClassesE+4294967296
+// CHECK:add x{{.*}}, x19, 
:lo12:_ZN22AndroidSizeClassConfig7ClassesE


Index: llvm/test/Instrumentation/HWAddressSanitizer/odr.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/HWAddressSanitizer/odr.ll
@@ -0,0 +1,7 @@
+; RUN: opt < %s -passes=hwasan -S | FileCheck %s
+
+; CHECK: @linkonce_odr_obj = linkonce hidden alias
+@linkonce_odr_obj = linkonce_odr hidden constant i32 1
+
+; CHECK: weak_odr_obj = weak hidden alias
+@weak_odr_obj = weak_odr hidden constant i32 1
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- 

[PATCH] D143533: [clang] Allow gnu::aligned attribute to work with templated type alias declarations

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: rsmith, rnk, echristo.
leonardchan added a project: clang.
Herald added a subscriber: jeroen.dobbelaere.
Herald added a project: All.
leonardchan requested review of this revision.

Prior to this, the following would fail:

  template 
  using NaturallyAligned [[gnu::aligned(sizeof(T))]] = T;
  
  struct S { char x[2]; };
  using AlignedS = NaturallyAligned;
  static_assert(alignof(AlignedS) == 2);  // alignof(AlignedS) == 1

This is because clang ignores attributes on type aliases when evaluating type 
info and instead resolves to alignment of the replacement type `T`.

This patch attempts to check if the associated type declaration is an alias 
that contains an attribute and return whatever the value is indicated by the 
alignment expression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143533

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaTemplate/type-alias-aligned.cpp

Index: clang/test/SemaTemplate/type-alias-aligned.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/type-alias-aligned.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17 -triple=x86_64-linux-gnu
+// expected-no-diagnostics
+
+template 
+using Aligned8 [[gnu::aligned(8)]] = T;
+
+template 
+using NaturallyAligned [[gnu::aligned(sizeof(T))]] = T;
+
+template 
+using UnderAligned [[gnu::aligned(1)]] = T;
+
+template 
+using NaturallyAligned2 [[gnu::aligned(S)]] = T;
+
+template 
+using UnaryAligned [[gnu::aligned(+sizeof(T))]] = T;
+
+template 
+using BinaryAligned [[gnu::aligned(sizeof(T) * 2)]] = T;
+
+struct S { unsigned x[2]; };
+static_assert(alignof(S) == 4);
+
+using Aligned8_S = Aligned8;
+static_assert(alignof(Aligned8_S) == 8);
+
+using NatAligned_S = NaturallyAligned;
+static_assert(alignof(NatAligned_S) == 8);
+
+using UnderAligned_S = UnderAligned;
+static_assert(alignof(UnderAligned_S) == 1);
+
+using NatAligned_S2 = NaturallyAligned2;
+static_assert(alignof(NatAligned_S2) == 8);
+
+using UnaryAligned_S = UnaryAligned;
+static_assert(alignof(UnaryAligned_S) == 8);
+
+using BinaryAligned_S = BinaryAligned;
+static_assert(alignof(BinaryAligned_S) == 16);
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -41,6 +41,7 @@
 #include "clang/AST/RawCommentList.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/Stmt.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -2026,6 +2027,138 @@
   return TI;
 }
 
+/// This class attempts to substitute the template parameter for a TypeAliasDecl
+/// with the replacement type for a SubstTemplateTypeParmType in order to create
+/// a new expression that is evaluatable (ie. not value-dependent). We need to
+/// create a new expression rather than replace componenets of an existing
+/// expression to recompute `dependence`s. Normally, this would be done via the
+/// TemplateDeclInstantiator, but that requires a reference to Sema which may
+/// not be available since the TypeInfo machinery only operates on the
+/// ASTContext.
+///
+/// NOTE: This class should be updated for various dependent expressions that
+/// can appear in an AlignedAttr's alignment expression. This can be done by
+/// adding more Visit methods.
+///
+class SubstTypeVisitor
+: public StmtVisitor {
+public:
+  SubstTypeVisitor(ASTContext , QualType ReplacementTy)
+  : Ctx(Ctx), ReplacementTy(ReplacementTy) {}
+
+  uint64_t TryEval(Expr *E) {
+Expr *NewE = Visit(E);
+if (!NewE || NewE->isValueDependent())
+  return 0;
+return NewE->EvaluateKnownConstInt(Ctx).getZExtValue() * Ctx.getCharWidth();
+  }
+
+  Expr *VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) {
+return new (Ctx) UnaryExprOrTypeTraitExpr(
+E->getKind(), Ctx.CreateTypeSourceInfo(ReplacementTy), E->getType(),
+E->getBeginLoc(), E->getEndLoc());
+  }
+
+  Expr *VisitDeclRefExpr(DeclRefExpr *E) {
+if (const auto *NTPD =
+dyn_cast(E->getFoundDecl())) {
+  if (Expr *DefaultArg = NTPD->getDefaultArgument())
+return Visit(DefaultArg);
+}
+return E;
+  }
+
+  Expr *VisitUnaryOperator(UnaryOperator *E) {
+Expr *SubExpr = Visit(E->getSubExpr());
+if (!SubExpr)
+  return nullptr;
+return UnaryOperator::Create(Ctx, SubExpr, E->getOpcode(), E->getType(),
+ E->getValueKind(), E->getObjectKind(),
+ E->getExprLoc(), E->canOverflow(),
+ E->getFPOptionsOverride());
+  }
+
+  Expr *VisitImplicitCastExpr(ImplicitCastExpr *E) {
+Expr *SubExpr = Visit(E->getSubExpr());
+if (!SubExpr)
+  return nullptr;
+return ImplicitCastExpr::Create(Ctx, E->getType(), 

[PATCH] D134687: [clang] Ensure correct metadata for relative vtables

[Leonard Chan 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 rG003b6033e1b2: [clang] Ensure correct metadata for relative 
vtables (authored by leonardchan).

Changed prior to commit:
  https://reviews.llvm.org/D134687?vs=468701=481109#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134687

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/type-metadata.cpp

Index: clang/test/CodeGenCXX/type-metadata.cpp
===
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -1,19 +1,33 @@
 // Tests for the cfi-vcall feature:
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s
 
 // Tests for the whole-program-vtables feature:
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM-DEFAULTVIS --check-prefix=TT-ITANIUM-DEFAULT %s
-// RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT %s
+// RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT --check-prefix=ITANIUM-OPT-LAYOUT %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=MS --check-prefix=TT-MS %s
 
 // Tests for cfi + whole-program-vtables:
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-VT 

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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr.
leonardchan added a project: clang.
Herald added a subscriber: abrachet.
Herald added a project: All.
leonardchan requested review of this revision.

We can land this once we assert global instrumentation is enabled for the rest 
of Fuchsia.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139589

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
@@ -224,7 +224,6 @@
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LLVM_USE_SANITIZER "HWAddress" 
CACHE STRING "")
   
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")
   
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")
-  set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_CMAKE_CXX_FLAGS 
"${FUCHSIA_aarch64-unknown-fuchsia_COMPILER_FLAGS} -mllvm --hwasan-globals=0" 
CACHE STRING "")
 
   # HWASan+noexcept
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LLVM_BUILD_COMPILER_RT 
OFF CACHE BOOL "")
@@ -233,7 +232,6 @@
   
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS
 OFF CACHE BOOL "")
   
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS
 OFF CACHE BOOL "")
   
set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXX_ENABLE_EXCEPTIONS 
OFF CACHE BOOL "")
-  set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_CMAKE_CXX_FLAGS 
"${FUCHSIA_aarch64-unknown-fuchsia_COMPILER_FLAGS} -mllvm --hwasan-globals=0" 
CACHE STRING "")
 
   set(LLVM_RUNTIME_MULTILIBS 
"asan;noexcept;compat;asan+noexcept;hwasan;hwasan+noexcept" CACHE STRING "")
 


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -224,7 +224,6 @@
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LLVM_USE_SANITIZER "HWAddress" CACHE STRING "")
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
-  set(RUNTIMES_aarch64-unknown-fuchsia+hwasan_CMAKE_CXX_FLAGS "${FUCHSIA_aarch64-unknown-fuchsia_COMPILER_FLAGS} -mllvm --hwasan-globals=0" CACHE STRING "")
 
   # HWASan+noexcept
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "")
@@ -233,7 +232,6 @@
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS OFF CACHE BOOL "")
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXXABI_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
   set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
-  set(RUNTIMES_aarch64-unknown-fuchsia+hwasan+noexcept_CMAKE_CXX_FLAGS "${FUCHSIA_aarch64-unknown-fuchsia_COMPILER_FLAGS} -mllvm --hwasan-globals=0" CACHE STRING "")
 
   set(LLVM_RUNTIME_MULTILIBS "asan;noexcept;compat;asan+noexcept;hwasan;hwasan+noexcept" CACHE STRING "")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138855: [clang][Fuchsia] Allow for setting the default build type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added a reviewer: phosek.
leonardchan added a project: clang.
Herald added a subscriber: abrachet.
Herald added a project: All.
leonardchan requested review of this revision.

Prior to this, `CMAKE_BUILD_TYPE` would unconditionally be `Release`. This 
allows me to add `-DCMAKE_BUILD_TYPE=Debug` to override this rather than 
manually changing the cache file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138855

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
@@ -40,7 +40,10 @@
 set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
 set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
 
-set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+if (NOT DEFINED CMAKE_BUILD_TYPE)
+  set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+endif()
+
 if (APPLE)
   set(CMAKE_OSX_DEPLOYMENT_TARGET "10.13" CACHE STRING "")
 elseif(WIN32)


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -40,7 +40,10 @@
 set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
 set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
 
-set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+if (NOT DEFINED CMAKE_BUILD_TYPE)
+  set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+endif()
+
 if (APPLE)
   set(CMAKE_OSX_DEPLOYMENT_TARGET "10.13" CACHE STRING "")
 elseif(WIN32)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134687: [clang] Ensure correct metadata for relative vtables

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134687

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

[PATCH] D134687: [clang] Ensure correct metadata for relative vtables

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 468701.
leonardchan marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134687

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/type-metadata.cpp

Index: clang/test/CodeGenCXX/type-metadata.cpp
===
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -1,19 +1,33 @@
 // Tests for the cfi-vcall feature:
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s
 
 // Tests for the whole-program-vtables feature:
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM-DEFAULTVIS --check-prefix=TT-ITANIUM-DEFAULT %s
-// RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT %s
+// RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT --check-prefix=ITANIUM-OPT-LAYOUT %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=MS --check-prefix=TT-MS %s
 
 // Tests for cfi + whole-program-vtables:
-// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-VT --check-prefix=ITANIUM --check-prefix=TC-ITANIUM %s
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck 

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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

> Naive question, but what does it mean to target Fuschia but be 
> gnu-compatible? (how would that be different than using whatever gnu OS 
> (linux, etc) the other code was built for)

(Probably poor wording on my part.) When I mention gnu-compatible here, I don't 
mean ABI compatible to the OS but ABI compatible to whatever another compiler 
might generate. For example, if you want to use gcc to compile some code 
targeting fuchsia, gcc won't emit relative vtables but if you want to link 
against stuff like libcxx, libunwind, etc. then you'd need versions of those 
libs that also target fuchsia but would use the "regular" vtable layout.


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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D85802#3858760 , @rjmccall wrote:

> Does Fuchsia still need this?  If those experiments have stabilized, maybe we 
> can just remove it.  Also, it should probably have been a -cc1 flag in the 
> first place.

We still use this for making gnu-compatible multilibs (that is, multilibs which 
use regular itanium vtables): 
https://github.com/llvm/llvm-project/blob/2c7b7eca85c2ccc9b0d5c65169213ce499652f92/clang/cmake/caches/Fuchsia-stage2.cmake#L200

AFAICT there doesn't seem to be a way where one can select the C++ ABI 
independently of the target platform.


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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D135713#3850730 , @mcgrathr wrote:

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

Oh I see. Yeah I had just been thinking of turning it on for us rather than for 
all of lsan for everyone. Is something like https://reviews.llvm.org/D135716 
what you were thinking?


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] D135713: [cmake][Fuchsia] Add -ftrivial-auto-var-init=zero to runtimes build

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr.
leonardchan added a project: clang.
Herald added a subscriber: abrachet.
Herald added a project: All.
leonardchan requested review of this revision.

This might allow lsan to find more leaks that would have gone undetected. When 
lsan searches for leaked pointers on the stack, if a leaked pointer that was 
pushed to the stack in a prior function call would not be scrubbed on a future 
function call, then the memory snapshot will see the pointer on the stack and 
not mark it as leaked. Such holes can exist in the lsan runtime where there may 
 be uninitialized data. Adding auto-var-init can scrub some of that data and 
might be able to catch more leaks that would've gone undetected this way.

See https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=111351 for more details.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135713

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
@@ -145,7 +145,7 @@
   set(FUCHSIA_x86_64-unknown-fuchsia_NAME x64)
   set(FUCHSIA_riscv64-unknown-fuchsia_NAME riscv64)
   foreach(target 
i386-unknown-fuchsia;x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia)
-set(FUCHSIA_${target}_COMPILER_FLAGS "--target=${target} 
-I${FUCHSIA_SDK}/pkg/sync/include -I${FUCHSIA_SDK}/pkg/fdio/include")
+set(FUCHSIA_${target}_COMPILER_FLAGS "--target=${target} 
-I${FUCHSIA_SDK}/pkg/sync/include -I${FUCHSIA_SDK}/pkg/fdio/include 
-ftrivial-auto-var-init=zero")
 set(FUCHSIA_${target}_LINKER_FLAGS 
"-L${FUCHSIA_SDK}/arch/${FUCHSIA_${target}_NAME}/lib")
 set(FUCHSIA_${target}_SYSROOT 
"${FUCHSIA_SDK}/arch/${FUCHSIA_${target}_NAME}/sysroot")
   endforeach()


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -145,7 +145,7 @@
   set(FUCHSIA_x86_64-unknown-fuchsia_NAME x64)
   set(FUCHSIA_riscv64-unknown-fuchsia_NAME riscv64)
   foreach(target i386-unknown-fuchsia;x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia)
-set(FUCHSIA_${target}_COMPILER_FLAGS "--target=${target} -I${FUCHSIA_SDK}/pkg/sync/include -I${FUCHSIA_SDK}/pkg/fdio/include")
+set(FUCHSIA_${target}_COMPILER_FLAGS "--target=${target} -I${FUCHSIA_SDK}/pkg/sync/include -I${FUCHSIA_SDK}/pkg/fdio/include -ftrivial-auto-var-init=zero")
 set(FUCHSIA_${target}_LINKER_FLAGS "-L${FUCHSIA_SDK}/arch/${FUCHSIA_${target}_NAME}/lib")
 set(FUCHSIA_${target}_SYSROOT "${FUCHSIA_SDK}/arch/${FUCHSIA_${target}_NAME}/sysroot")
   endforeach()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134687: [clang] Ensure correct metadata for relative vtables

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr, tejohnson, pcc.
leonardchan added a project: clang.
Herald added a subscriber: Prazek.
Herald added a project: All.
leonardchan requested review of this revision.

Prior to this, metadata pertaining to the size or address point offsets into a 
relative vtable were twice the value they should be (treating component widths 
as pointer width rather than 4 bytes). This prevented some vtables from being 
devirtualized with D134320 . This ensures the 
correct metadata is written so whole program devirtualization can catch these 
remaining devirt targets.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134687

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/type-metadata.cpp

Index: clang/test/CodeGenCXX/type-metadata.cpp
===
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -1,19 +1,33 @@
 // Tests for the cfi-vcall feature:
-// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s
-// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s
-// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s
-// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s
+// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s
+// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-DIAG --check-prefix=ITANIUM-MD-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s
+// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-DIAG --check-prefix=ITANIUM-MD-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s
+// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s
 
 // Tests for the whole-program-vtables feature:
-// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM-HIDDEN %s
+// RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM -check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN %s
 // RUN: %clang_cc1 -no-opaque-pointers -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM-DEFAULTVIS --check-prefix=TT-ITANIUM-DEFAULT %s
-// RUN: %clang_cc1 -no-opaque-pointers -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT %s
+// RUN: %clang_cc1 -no-opaque-pointers -O2 -flto 

[PATCH] D134300: [llvm] Handle dso_local_equivalent in FunctionComparator

[Leonard Chan 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 rG21b03bf97061: [llvm] Handle dso_local_equivalent in 
FunctionComparator (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134300

Files:
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
  llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll

Index: llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll
===
--- /dev/null
+++ llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll
@@ -0,0 +1,69 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;; Check the cases involving dso_local_equivalent where we do not expect functions to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+@x = constant { i32 ()*, i32 ()* } { i32 ()* @a, i32 ()* @b }
+; CHECK: { i32 ()* @a, i32 ()* @b }
+
+@x2 = constant { i32 ()*, i32 ()* } { i32 ()* @c, i32 ()* @d }
+; CHECK: { i32 ()* @c, i32 ()* @d }
+
+;; func1 and func2 are different functions.
+declare i32 @func1()
+define i32 @func2() {
+; CHECK-LABEL: @func2(
+; CHECK-NEXT:ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @a() unnamed_addr {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func1()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func1()
+  ret i32 %1
+}
+
+define internal i32 @b() unnamed_addr {
+; CHECK-LABEL: @b(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func2()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func2()
+  ret i32 %1
+}
+
+;; func3 and func4 have the same body and signature but do not have merged
+;; callers because they are different functions.
+define i32 @func3() {
+; CHECK-LABEL: @func3(
+; CHECK-NEXT:ret i32 0
+;
+  ret i32 0
+}
+define i32 @func4() {
+; CHECK-LABEL: @func4(
+; CHECK-NEXT:ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @c() unnamed_addr {
+; CHECK-LABEL: @c(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func3()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func3()
+  ret i32 %1
+}
+
+define internal i32 @d() unnamed_addr {
+; CHECK-LABEL: @d(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func4()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func4()
+  ret i32 %1
+}
Index: llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
===
--- /dev/null
+++ llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;; Check the cases involving dso_local_equivalent where we do expect functions to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+; CHECK-NOT: @b
+
+@x = constant { i32 ()*, i32 ()* } { i32 ()* @a, i32 ()* @b }
+; CHECK: { i32 ()* @a, i32 ()* @a }
+
+define i32 @func() {
+; CHECK-LABEL: @func(
+; CHECK-NEXT:ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @a() unnamed_addr {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func()
+  ret i32 %1
+}
+
+define internal i32 @b() unnamed_addr {
+  %1 = call i32 dso_local_equivalent @func()
+  ret i32 %1
+}
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -402,6 +402,15 @@
   return cmpValues(LBA->getBasicBlock(), RBA->getBasicBlock());
 }
   }
+  case Value::DSOLocalEquivalentVal: {
+// dso_local_equivalent is functionally equivalent to whatever it points to.
+// This means the behavior of the IR should be the exact same as if the
+// function was referenced directly rather than through a
+// dso_local_equivalent.
+const auto *LEquiv = cast(L);
+const auto *REquiv = cast(R);
+return cmpGlobalValues(LEquiv->getGlobalValue(), REquiv->getGlobalValue());
+  }
   default: // Unknown constant, abort.
 LLVM_DEBUG(dbgs() << "Looking at valueID " << L->getValueID() << "\n");
 llvm_unreachable("Constant ValueID not recognized.");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134300: [llvm] Handle dso_local_equivalent in FunctionComparator

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Will wait until EOD before committing this unless others have more comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134300

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

[PATCH] D134300: [llvm] Handle dso_local_equivalent in FunctionComparator

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll:1
+;; Check the cases involving dso_local_equivalent where we do expect functions 
to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s

nikic wrote:
> Based on your check lines, it doesn't look like the functions actually get 
> merged? You probably need to add some dummy instructions to pass the 
> instruction threshold, or use internal linkage.
Woops. Yeah you're right. Updating the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134300

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

[PATCH] D134300: [llvm] Handle dso_local_equivalent in FunctionComparator

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 461686.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134300

Files:
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
  llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll

Index: llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll
===
--- /dev/null
+++ llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll
@@ -0,0 +1,69 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;; Check the cases involving dso_local_equivalent where we do not expect functions to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+@x = constant { i32 ()*, i32 ()* } { i32 ()* @a, i32 ()* @b }
+; CHECK: { i32 ()* @a, i32 ()* @b }
+
+@x2 = constant { i32 ()*, i32 ()* } { i32 ()* @c, i32 ()* @d }
+; CHECK: { i32 ()* @c, i32 ()* @d }
+
+;; func1 and func2 are different functions.
+declare i32 @func1()
+define i32 @func2() {
+; CHECK-LABEL: @func2(
+; CHECK-NEXT:ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @a() unnamed_addr {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func1()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func1()
+  ret i32 %1
+}
+
+define internal i32 @b() unnamed_addr {
+; CHECK-LABEL: @b(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func2()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func2()
+  ret i32 %1
+}
+
+;; func3 and func4 have the same body and signature but do not have merged
+;; callers because they are different functions.
+define i32 @func3() {
+; CHECK-LABEL: @func3(
+; CHECK-NEXT:ret i32 0
+;
+  ret i32 0
+}
+define i32 @func4() {
+; CHECK-LABEL: @func4(
+; CHECK-NEXT:ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @c() unnamed_addr {
+; CHECK-LABEL: @c(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func3()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func3()
+  ret i32 %1
+}
+
+define internal i32 @d() unnamed_addr {
+; CHECK-LABEL: @d(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func4()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func4()
+  ret i32 %1
+}
Index: llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
===
--- /dev/null
+++ llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;; Check the cases involving dso_local_equivalent where we do expect functions to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+; CHECK-NOT: @b
+
+@x = constant { i32 ()*, i32 ()* } { i32 ()* @a, i32 ()* @b }
+; CHECK: { i32 ()* @a, i32 ()* @a }
+
+define i32 @func() {
+; CHECK-LABEL: @func(
+; CHECK-NEXT:ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @a() unnamed_addr {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:[[TMP1:%.*]] = call i32 dso_local_equivalent @func()
+; CHECK-NEXT:ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func()
+  ret i32 %1
+}
+
+define internal i32 @b() unnamed_addr {
+  %1 = call i32 dso_local_equivalent @func()
+  ret i32 %1
+}
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -402,6 +402,15 @@
   return cmpValues(LBA->getBasicBlock(), RBA->getBasicBlock());
 }
   }
+  case Value::DSOLocalEquivalentVal: {
+// dso_local_equivalent is functionally equivalent to whatever it points to.
+// This means the behavior of the IR should be the exact same as if the
+// function was referenced directly rather than through a
+// dso_local_equivalent.
+const auto *LEquiv = cast(L);
+const auto *REquiv = cast(R);
+return cmpGlobalValues(LEquiv->getGlobalValue(), REquiv->getGlobalValue());
+  }
   default: // Unknown constant, abort.
 LLVM_DEBUG(dbgs() << "Looking at valueID " << L->getValueID() << "\n");
 llvm_unreachable("Constant ValueID not recognized.");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134300: [llvm] Handle dso_local_equivalent in FunctionComparator

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

@phosek feel free to add who you think is appropriate for reviewing this.

This is a conservative comparison of dso_local_equivalents where two are the 
same only if they point to the same underlying function. It could be argued 
that a `dso_local_equivalent @func` could be "the same" as just `@func`, but I 
think that can be argued/implemented separately. This is mainly for addressing 
the crash in pr51066.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134300

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

[PATCH] D134300: [llvm] Handle dso_local_equivalent in FunctionComparator

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added a reviewer: phosek.
leonardchan added a project: LLVM.
Herald added subscribers: abrachet, hiraditya.
Herald added a project: All.
leonardchan requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead.
Herald added a project: clang.

This addresses https://github.com/llvm/llvm-project/issues/51066.

Prior to this, dso_local_equivalent would lead to an llvm_unreachable in a 
switch in the FunctionComparator. This adds a conservative case in that switch 
that just compares the underlying functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134300

Files:
  clang/test/CodeGenCXX/pr51066.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
  llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll

Index: llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll
===
--- /dev/null
+++ llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll
@@ -0,0 +1,45 @@
+;; Check the cases involving dso_local_equivalent where we do not expect functions to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+;; func1 and func2 are different functions.
+declare i32 @func1()
+define i32 @func2() {
+  ret i32 0
+}
+
+; CHECK-LABEL: @call1
+; CHECK: call i32 dso_local_equivalent @func1()
+define i32 @call1() {
+  %1 = call i32 dso_local_equivalent @func1()
+  ret i32 %1
+}
+
+; CHECK-LABEL: @call2
+; CHECK: call i32 dso_local_equivalent @func2()
+define i32 @call2() {
+  %1 = call i32 dso_local_equivalent @func2()
+  ret i32 %1
+}
+
+;; func3 and func4 have the same body and signature but do not have merged
+;; callers because they are different functions.
+define i32 @func3() {
+  ret i32 0
+}
+define i32 @func4() {
+  ret i32 0
+}
+
+; CHECK-LABEL: @call3
+; CHECK: call i32 dso_local_equivalent @func3()
+define i32 @call3() {
+  %1 = call i32 dso_local_equivalent @func3()
+  ret i32 %1
+}
+
+; CHECK-LABEL: @call4
+; CHECK: call i32 dso_local_equivalent @func4()
+define i32 @call4() {
+  %1 = call i32 dso_local_equivalent @func4()
+  ret i32 %1
+}
Index: llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
===
--- /dev/null
+++ llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll
@@ -0,0 +1,18 @@
+;; Check the cases involving dso_local_equivalent where we do expect functions to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+declare i32 @func()
+
+; CHECK-LABEL: @call1
+; CHECK: call i32 dso_local_equivalent @func()
+define i32 @call1() {
+  %1 = call i32 dso_local_equivalent @func()
+  ret i32 %1
+}
+
+; CHECK-LABEL: @call2
+; CHECK: call i32 dso_local_equivalent @func()
+define i32 @call2() {
+  %1 = call i32 dso_local_equivalent @func()
+  ret i32 %1
+}
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -402,6 +402,15 @@
   return cmpValues(LBA->getBasicBlock(), RBA->getBasicBlock());
 }
   }
+  case Value::DSOLocalEquivalentVal: {
+// dso_local_equivalent is functionally equivalent to whatever it points to.
+// This means the behavior of the IR should be the exact same as if the
+// function was referenced directly rather than through a
+// dso_local_equivalent.
+const auto *LEquiv = cast(L);
+const auto *REquiv = cast(R);
+return cmpGlobalValues(LEquiv->getGlobalValue(), REquiv->getGlobalValue());
+  }
   default: // Unknown constant, abort.
 LLVM_DEBUG(dbgs() << "Looking at valueID " << L->getValueID() << "\n");
 llvm_unreachable("Constant ValueID not recognized.");
Index: clang/test/CodeGenCXX/pr51066.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pr51066.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-fuchsia -emit-obj -massembler-fatal-warnings --mrelax-relocations -disable-free -disable-llvm-verifier -discard-value-names -main-file-name zircon_platform_buffer.cc -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=none -fno-rounding-math -mconstructor-aliases -target-cpu x86-64-v2 -mllvm -x86-branches-within-32B-boundaries -debug-info-kind=constructor -dwarf-version=5 -debugger-tuning=gdb -mllvm -crash-diagnostics-dir=clang-crashreports -ffunction-sections -fdata-sections -fcoverage-compilation-dir=. -sys-header-deps -D TOOLCHAIN_VERSION=dO8igrHyLDfgq8txd8Qx0mI9oJqFLswAMLcIBKLF6TYC -D _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D NDEBUG=1 -D _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS=1 -D MAGMA_DEBUG_INTERNAL_USE_ONLY=0 -D MAGMA_ENABLE_TRACING -O3 -Wall -Wextra -Wnewline-eof -Wconversion -Wimplicit-fallthrough -Wno-unused-parameter -Wno-sign-conversion 

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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D132425#3752468 , @hctim wrote:

> Glad to see that refactoring the sanitizer metadata made someone's life 
> easier ;) (now allowing for disabling hwasanificiation of globals)
>
> Patch looks reasonable to me. Can you please add the negative test (that 
> vtables under the vanilla ABI still have hwasan)?

Yup. Will add a followup patch.

> I wans't fully aware of the relative vtables ABI, and it may have some 
> implications about MTE globals tagging (draft abi 
> ).
>  Because logical tags are synthesized at runtime into a synthetic GOT entry - 
> dynamic relocations I believe would be forced (removing any benefit of the 
> relative vtables ABI), so for now it seems like MTE globals and relative 
> vtables are mutually exclusive. Another option would be to disable MTE 
> globals for relative vtables as well. No action needed on your part, just 
> putting some wordso n paper that this might need some consideration at a 
> later date if Fuchsia wants to support MTE globals.

Thanks for bringing this up. For relative vtables, the important bit is that 
the vtable is populated with statically known offsets between the vtable and 
its virtual functions. The particular linker error this patch resolves occurs 
because the hwasan pass happens to replace references to the vtable (within the 
vtable) with the alias, so the tag causes an overflow when resolving the 
relocation.

Now if I'm understanding the draft correctly, the way MTE will work with 
globals is that all references to globals *must* go through the GOT because the 
dynamic linker will place the tagged address there, so the potential conflict 
with relative vtables is that we wouldn't know the full tagged address of the 
vtable at link time, hence the dynamic relocation. If so, simply disabling MTE 
on relative vtables would also be a short term solution.

We have a generic long term solution for hwasan+RV which I think might also be 
applicable for MTE+RV. For hwasan, since it's mainly the IR pass that converts 
usages of the vtable (within the vtable itself) to use tagged aliases, the 
ideal solution is to just have hwasan ignore these specific references in the 
vtable such that offset calculation can continue to use the untagged address 
allowing the relocation arithmetic to not overflow. Now for llvm, I'm assuming 
it's an instrumentation pass like memtagsanitizer that will ensure all 
references to globals go through the GOT by replacing all global references 
with the appropriate IR that gets lowered to this GOT reference. If this is the 
case, then I *think* a similar solution can be done here where particular 
references to the vtable continue to use the original vtable address and avoid 
instrumentation.


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] D132691: [clang] Do not instrument the rtti_proxies under hwasan

[Leonard Chan 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 rGcdb30f7a2635: [clang] Do not instrument the rtti_proxies 
under hwasan (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132691

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp


Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -6,6 +6,7 @@
 /// hwasan-instrumented.
 // CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 
x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1A.rtti_proxy to 
i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr 
@_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 
ptrtoint (ptr dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr 
getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) 
to i64)) to i32)] }, no_sanitize_hwaddress, align 4
 // CHECK-DAG: @_ZTV1A = unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
+// CHECK-DAG: @_ZTI1A.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1A, 
no_sanitize_hwaddress, comdat
 
 class A {
 public:
@@ -21,6 +22,7 @@
 /// If the vtable happens to be hidden, then the alias is not needed. In this
 /// case, the original vtable struct itself should be unsanitized.
 // CHECK-DAG: @_ZTV1B = hidden unnamed_addr constant { [3 x i32] } { [3 x i32] 
[i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 
ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, 
i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr 
dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr 
inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)] }, 
no_sanitize_hwaddress, align 4
+// CHECK-DAG: @_ZTI1B.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1B, 
no_sanitize_hwaddress, comdat
 
 class __attribute__((visibility("hidden"))) B {
 public:
Index: clang/lib/CodeGen/CGVTables.h
===
--- clang/lib/CodeGen/CGVTables.h
+++ clang/lib/CodeGen/CGVTables.h
@@ -155,8 +155,8 @@
   void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
llvm::StringRef AliasNameRef);
 
-  /// Specify a vtable should not be instrumented with hwasan.
-  void RemoveHwasanMetadata(llvm::GlobalValue *VTable);
+  /// Specify a global should not be instrumented with hwasan.
+  void RemoveHwasanMetadata(llvm::GlobalValue *GV) const;
 };
 
 } // end namespace CodeGen
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -664,6 +664,12 @@
 proxy->setVisibility(llvm::GlobalValue::HiddenVisibility);
 proxy->setComdat(module.getOrInsertComdat(rttiProxyName));
   }
+  // Do not instrument the rtti proxies with hwasan to avoid a duplicate
+  // symbol error. Aliases generated by hwasan will retain the same namebut
+  // the addresses they are set to may have different tags from different
+  // compilation units. We don't run into this without hwasan because the
+  // proxies are in comdat groups, but those aren't propagated to the 
alias.
+  RemoveHwasanMetadata(proxy);
 }
 target = proxy;
   }
@@ -938,13 +944,13 @@
 // relocations. A future alternative for this would be finding which usages of
 // the vtable can continue to use the untagged hwasan value without any loss of
 // value in hwasan.
-void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *VTable) {
+void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *GV) const {
   if (CGM.getLangOpts().Sanitize.has(SanitizerKind::HWAddress)) {
 llvm::GlobalValue::SanitizerMetadata Meta;
-if (VTable->hasSanitizerMetadata())
-  Meta = VTable->getSanitizerMetadata();
+if (GV->hasSanitizerMetadata())
+  Meta = GV->getSanitizerMetadata();
 Meta.NoHWAddress = true;
-VTable->setSanitizerMetadata(Meta);
+GV->setSanitizerMetadata(Meta);
   }
 }
 


Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -6,6 +6,7 @@
 /// hwasan-instrumented.
 // CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 x 

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

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG93e5cf6b9c08: [clang] Do not instrument relative vtables 
under hwasan (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132425

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp

Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -S -o - -emit-llvm -fsanitize=hwaddress | FileCheck %s
+
+/// The usual vtable will have default visibility. In this case, the actual
+/// vtable is hidden and the alias is made public. With hwasan enabled, we want
+/// to ensure the local one self-referenced in the hidden vtable is not
+/// hwasan-instrumented.
+// CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1A.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32)] }, no_sanitize_hwaddress, align 4
+// CHECK-DAG: @_ZTV1A = unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
+
+class A {
+public:
+  virtual void foo();
+};
+
+void A::foo() {}
+
+void A_foo(A *a) {
+  a->foo();
+}
+
+/// If the vtable happens to be hidden, then the alias is not needed. In this
+/// case, the original vtable struct itself should be unsanitized.
+// CHECK-DAG: @_ZTV1B = hidden unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)] }, no_sanitize_hwaddress, align 4
+
+class __attribute__((visibility("hidden"))) B {
+public:
+  virtual void foo();
+};
+
+void B::foo() {}
+
+void B_foo(B *b) {
+  b->foo();
+}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1769,8 +1769,11 @@
 }
   }
 
-  if (VTContext.isRelativeLayout() && !VTable->isDSOLocal())
-CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());
+  if (VTContext.isRelativeLayout()) {
+CGVT.RemoveHwasanMetadata(VTable);
+if (!VTable->isDSOLocal())
+  CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());
+  }
 }
 
 bool ItaniumCXXABI::isVirtualOffsetNeededForVTableField(
Index: clang/lib/CodeGen/CGVTables.h
===
--- clang/lib/CodeGen/CGVTables.h
+++ clang/lib/CodeGen/CGVTables.h
@@ -154,6 +154,9 @@
   /// when a vtable may not be dso_local.
   void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
llvm::StringRef AliasNameRef);
+
+  /// Specify a vtable should not be instrumented with hwasan.
+  void RemoveHwasanMetadata(llvm::GlobalValue *VTable);
 };
 
 } // end namespace CodeGen
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -921,12 +921,33 @@
 
   CGM.EmitVTableTypeMetadata(RD, VTable, *VTLayout.get());
 
-  if (UsingRelativeLayout && !VTable->isDSOLocal())
-GenerateRelativeVTableAlias(VTable, OutName);
+  if (UsingRelativeLayout) {
+RemoveHwasanMetadata(VTable);
+if (!VTable->isDSOLocal())
+  GenerateRelativeVTableAlias(VTable, OutName);
+  }
 
   return VTable;
 }
 
+// Ensure this vtable is not instrumented by hwasan. That is, a global alias is
+// not generated for it. This is mainly used by the relative-vtables ABI where
+// vtables instead contain 32-bit offsets between the vtable and function
+// pointers. Hwasan is disabled for these vtables for now because the tag in a
+// vtable pointer may fail the overflow check when resolving 32-bit PLT
+// relocations. A future alternative for this would be finding which usages of
+// the vtable can continue to use the untagged hwasan value without any loss of
+// value in hwasan.
+void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *VTable) {
+  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::HWAddress)) {
+

[PATCH] D132691: [clang] Do not instrument the rtti_proxies under hwasan

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: mcgrathr, phosek.
leonardchan added a project: clang.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
leonardchan requested review of this revision.

We run into a duplicate symbol error when instrumenting the rtti_proxies 
generated as part of the relative vtables ABI with hwasan:

  ld.lld: error: duplicate symbol: typeinfo for icu_71::UObject (.rtti_proxy)
  >>> defined at brkiter.cpp
  >>>
arm64-hwasan-shared/obj/third_party/icu/source/common/libicuuc.brkiter.cpp.o:(typeinfo
 for icu_71::UObject (.rtti_proxy))
  >>> defined at locavailable.cpp
  >>>
arm64-hwasan-shared/obj/third_party/icu/source/common/libicuuc.locavailable.cpp.o:(.data.rel.ro..L_ZTIN6icu_717UObjectE.rtti_proxy.hwasan+0xE00)

The issue here is that the hwasan alias carries over the visibility and linkage 
of the original proxy, so we have duplicate external symbols that participate 
in linking. Similar to D132425  we can just 
disable hwasan for the proxies for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132691

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp


Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -6,6 +6,7 @@
 /// hwasan-instrumented.
 // CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 
x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1A.rtti_proxy to 
i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr 
@_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 
ptrtoint (ptr dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr 
getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) 
to i64)) to i32)] }, no_sanitize_hwaddress, align 4
 // CHECK-DAG: @_ZTV1A = unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
+// CHECK-DAG: @_ZTI1A.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1A, 
no_sanitize_hwaddress, comdat
 
 class A {
 public:
@@ -21,6 +22,7 @@
 /// If the vtable happens to be hidden, then the alias is not needed. In this
 /// case, the original vtable struct itself should be unsanitized.
 // CHECK-DAG: @_ZTV1B = hidden unnamed_addr constant { [3 x i32] } { [3 x i32] 
[i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 
ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, 
i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr 
dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr 
inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)] }, 
no_sanitize_hwaddress, align 4
+// CHECK-DAG: @_ZTI1B.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1B, 
no_sanitize_hwaddress, comdat
 
 class __attribute__((visibility("hidden"))) B {
 public:
Index: clang/lib/CodeGen/CGVTables.h
===
--- clang/lib/CodeGen/CGVTables.h
+++ clang/lib/CodeGen/CGVTables.h
@@ -155,8 +155,8 @@
   void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
llvm::StringRef AliasNameRef);
 
-  /// Specify a vtable should not be instrumented with hwasan.
-  void RemoveHwasanMetadata(llvm::GlobalValue *VTable);
+  /// Specify a global should not be instrumented with hwasan.
+  void RemoveHwasanMetadata(llvm::GlobalValue *GV) const;
 };
 
 } // end namespace CodeGen
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -664,6 +664,12 @@
 proxy->setVisibility(llvm::GlobalValue::HiddenVisibility);
 proxy->setComdat(module.getOrInsertComdat(rttiProxyName));
   }
+  // Do not instrument the rtti proxies with hwasan to avoid a duplicate
+  // symbol error. Aliases generated by hwasan will retain the same namebut
+  // the addresses they are set to may have different tags from different
+  // compilation units. We don't run into this without hwasan because the
+  // proxies are in comdat groups, but those aren't propagated to the 
alias.
+  RemoveHwasanMetadata(proxy);
 }
 target = proxy;
   }
@@ -938,13 +944,13 @@
 // relocations. A future alternative for this would be finding which usages of
 // the vtable can continue to use the untagged hwasan value without any loss of
 // value in hwasan.
-void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *VTable) {
+void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *GV) const {
   if 

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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D132425#3743276 , @mcgrathr wrote:

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

Done.


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] D132425: [clang] Do not instrument relative vtables under hwasan

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 455423.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132425

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp

Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -S -o - -emit-llvm -fsanitize=hwaddress | FileCheck %s
+
+/// The usual vtable will have default visibility. In this case, the actual
+/// vtable is hidden and the alias is made public. With hwasan enabled, we want
+/// to ensure the local one self-referenced in the hidden vtable is not
+/// hwasan-instrumented.
+// CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1A.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32)] }, no_sanitize_hwaddress, align 4
+// CHECK-DAG: @_ZTV1A = unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
+
+class A {
+public:
+  virtual void foo();
+};
+
+void A::foo() {}
+
+void A_foo(A *a) {
+  a->foo();
+}
+
+/// If the vtable happens to be hidden, then the alias is not needed. In this
+/// case, the original vtable struct itself should be unsanitized.
+// CHECK-DAG: @_ZTV1B = hidden unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)] }, no_sanitize_hwaddress, align 4
+
+class __attribute__((visibility("hidden"))) B {
+public:
+  virtual void foo();
+};
+
+void B::foo() {}
+
+void B_foo(B *b) {
+  b->foo();
+}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1769,8 +1769,11 @@
 }
   }
 
-  if (VTContext.isRelativeLayout() && !VTable->isDSOLocal())
-CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());
+  if (VTContext.isRelativeLayout()) {
+CGVT.RemoveHwasanMetadata(VTable);
+if (!VTable->isDSOLocal())
+  CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());
+  }
 }
 
 bool ItaniumCXXABI::isVirtualOffsetNeededForVTableField(
Index: clang/lib/CodeGen/CGVTables.h
===
--- clang/lib/CodeGen/CGVTables.h
+++ clang/lib/CodeGen/CGVTables.h
@@ -154,6 +154,9 @@
   /// when a vtable may not be dso_local.
   void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
llvm::StringRef AliasNameRef);
+
+  /// Specify a vtable should not be instrumented with hwasan.
+  void RemoveHwasanMetadata(llvm::GlobalValue *VTable);
 };
 
 } // end namespace CodeGen
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -921,12 +921,33 @@
 
   CGM.EmitVTableTypeMetadata(RD, VTable, *VTLayout.get());
 
-  if (UsingRelativeLayout && !VTable->isDSOLocal())
-GenerateRelativeVTableAlias(VTable, OutName);
+  if (UsingRelativeLayout) {
+RemoveHwasanMetadata(VTable);
+if (!VTable->isDSOLocal())
+  GenerateRelativeVTableAlias(VTable, OutName);
+  }
 
   return VTable;
 }
 
+// Ensure this vtable is not instrumented by hwasan. That is, a global alias is
+// not generated for it. This is mainly used by the relative-vtables ABI where
+// vtables instead contain 32-bit offsets between the vtable and function
+// pointers. Hwasan is disabled for these vtables for now because the tag in a
+// vtable pointer may fail the overflow check when resolving 32-bit PLT
+// relocations. A future alternative for this would be finding which usages of
+// the vtable can continue to use the untagged hwasan value without any loss of
+// value in hwasan.
+void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *VTable) {
+  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::HWAddress)) {
+llvm::GlobalValue::SanitizerMetadata Meta;
+if (VTable->hasSanitizerMetadata())
+  Meta = VTable->getSanitizerMetadata();
+

[PATCH] D132620: [WIP][llvm] Extend aarch64 and x86_64 elf targets to use gotpcrel relocations

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
Herald added subscribers: pengfei, hiraditya, kristof.beyls.
Herald added a project: All.
leonardchan requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132620

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/AArch64/AArch64TargetObjectFile.h
  llvm/lib/Target/X86/X86TargetObjectFile.h

Index: llvm/lib/Target/X86/X86TargetObjectFile.h
===
--- llvm/lib/Target/X86/X86TargetObjectFile.h
+++ llvm/lib/Target/X86/X86TargetObjectFile.h
@@ -42,6 +42,7 @@
   public:
 X86ELFTargetObjectFile() {
   PLTRelativeVariantKind = MCSymbolRefExpr::VK_PLT;
+  SupportIndirectSymViaGOTPCRel = true;
 }
 /// Describe a TLS variable address within debug info.
 const MCExpr *getDebugThreadLocalSymbol(const MCSymbol *Sym) const override;
Index: llvm/lib/Target/AArch64/AArch64TargetObjectFile.h
===
--- llvm/lib/Target/AArch64/AArch64TargetObjectFile.h
+++ llvm/lib/Target/AArch64/AArch64TargetObjectFile.h
@@ -21,6 +21,7 @@
 public:
   AArch64_ELFTargetObjectFile() {
 PLTRelativeVariantKind = MCSymbolRefExpr::VK_PLT;
+SupportIndirectSymViaGOTPCRel = true;
   }
 };
 
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1425,6 +1425,15 @@
   return SSym;
 }
 
+const MCExpr *TargetLoweringObjectFileELF::getIndirectSymViaGOTPCRel(
+const GlobalValue *GV, const MCSymbol *Sym, const MCValue ,
+int64_t Offset, MachineModuleInfo *MMI, MCStreamer ) const {
+  const MCExpr *Res =
+MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_GOTPCREL, getContext());
+  const MCExpr *Off = MCConstantExpr::create(Offset, getContext());
+  return MCBinaryExpr::createAdd(Res, Off, getContext());
+}
+
 const MCExpr *TargetLoweringObjectFileMachO::getIndirectSymViaGOTPCRel(
 const GlobalValue *GV, const MCSymbol *Sym, const MCValue ,
 int64_t Offset, MachineModuleInfo *MMI, MCStreamer ) const {
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1749,8 +1749,10 @@
   // GlobalVariable or Function, i.e., as GlobalValue.
   if (!GV->hasGlobalUnnamedAddr() || !GV->hasInitializer() ||
   !GV->isConstant() || !GV->isDiscardableIfUnused() ||
-  !isa(GV->getOperand(0)))
+  !isa(GV->getOperand(0))) {
+//llvm::errs() << "Failed isGOTEquivalentCandidate for " << GV->getName() << "\n";
 return false;
+  }
 
   // To be a got equivalent, at least one of its users need to be a constant
   // expression used by another global variable.
@@ -3172,6 +3174,8 @@
 static void handleIndirectSymViaGOTPCRel(AsmPrinter , const MCExpr **ME,
  const Constant *BaseCst,
  uint64_t Offset) {
+  //llvm::errs() << "BaseCst:\n"; BaseCst->print(llvm::errs()); llvm::errs() << "\n";
+  //llvm::errs() << "ME: " << (**ME) << "\n";
   // The global @foo below illustrates a global that uses a got equivalent.
   //
   //  @bar = global i32 42
@@ -3193,7 +3197,14 @@
   //  cstexpr :=  -  + gotpcrelcst, where
   //gotpcrelcst :=  + 
   MCValue MV;
-  if (!(*ME)->evaluateAsRelocatable(MV, nullptr, nullptr) || MV.isAbsolute())
+  if (!(*ME)->evaluateAsRelocatable(MV, nullptr, nullptr)) {
+//llvm::errs() << "ME not relocatable\n";
+return;
+  }
+
+  //llvm::errs() << "MV: "; MV.print(llvm::errs()); llvm::errs() << "\n";
+
+  if (MV.isAbsolute())
 return;
   const MCSymbolRefExpr *SymA = MV.getSymA();
   if (!SymA)
@@ -3201,19 +3212,24 @@
 
   // Check that GOT equivalent symbol is cached.
   const MCSymbol *GOTEquivSym = >getSymbol();
-  if (!AP.GlobalGOTEquivs.count(GOTEquivSym))
+  if (!AP.GlobalGOTEquivs.count(GOTEquivSym)) {
+//llvm::errs() << "No GlobalGOTEquivs for " << *GOTEquivSym << "\n";
 return;
+  }
 
   const GlobalValue *BaseGV = dyn_cast_or_null(BaseCst);
   if (!BaseGV)
 return;
+  //llvm::errs() << "Good BaseGV\n";
 
   // Check for a valid base symbol
   const MCSymbol *BaseSym = AP.getSymbol(BaseGV);
   const MCSymbolRefExpr *SymB = MV.getSymB();
 
-  if (!SymB || BaseSym != >getSymbol())
+  if (!SymB || BaseSym != >getSymbol()) {
+//llvm::errs() << "Bad base symbol\n";
 return;
+  }
 
   // Make sure to match:
   //
@@ -3223,8 +3239,9 @@
   // displacement into the 

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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: mcgrathr, eugenis, phosek, vitalybuka.
leonardchan added a project: clang.
Herald added subscribers: abrachet, kristof.beyls.
Herald added a project: All.
leonardchan requested review of this revision.

Full context in https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=107017.

Instrumenting hwasan with globals results in a linker error under the relative 
vtables abi:

  

ld.lld: error: 
libunwind.cpp:(.rodata..L_ZTVN9libunwind12UnwindCursorINS_17LocalAddressSpaceENS_15Registers_arm64EEE.hwasan+0x8):
 relocation R_AARCH64_PLT32 out of range: 6845471433603167792 is not in 
[-2147483648, 2147483647]; references 
libunwind::AbstractUnwindCursor::~AbstractUnwindCursor()

>>> defined in libunwind/src/CMakeFiles/unwind_shared.dir/libunwind.cpp.obj

  This is because the tag is included in the vtable address when calculating 
the offset between the vtable and virtual function. A temporary solution until 
we can resolve this is to just disable hwasan instrumentation on relative 
vtables specifically, which can be done in the frontend.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132425

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp

Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -S -o - -emit-llvm -fsanitize=hwaddress | FileCheck %s
+
+/// The usual vtable will have default visibility. In this case, the actual
+/// vtable is hidden and the alias is made public. With hwasan enabled, we want
+/// to ensure the local one self-referenced in the hidden vtable is not
+/// hwasan-instrumented.
+// CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1A.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32)] }, no_sanitize_hwaddress, align 4
+// CHECK-DAG: @_ZTV1A = unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
+
+class A {
+public:
+  virtual void foo();
+};
+
+void A::foo() {}
+
+void A_foo(A *a) {
+  a->foo();
+}
+
+/// If the vtable happens to be hidden, then the alias is not needed. In this
+/// case, the original vtable struct itself should be unsanitized.
+// CHECK-DAG: @_ZTV1B = hidden unnamed_addr constant { [3 x i32] } { [3 x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)] }, no_sanitize_hwaddress, align 4
+
+class __attribute__((visibility("hidden"))) B {
+public:
+  virtual void foo();
+};
+
+void B::foo() {}
+
+void B_foo(B *b) {
+  b->foo();
+}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1769,8 +1769,11 @@
 }
   }
 
-  if (VTContext.isRelativeLayout() && !VTable->isDSOLocal())
-CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());
+  if (VTContext.isRelativeLayout()) {
+CGVT.RemoveHwasanMetadata(VTable);
+if (!VTable->isDSOLocal())
+  CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());
+  }
 }
 
 bool ItaniumCXXABI::isVirtualOffsetNeededForVTableField(
Index: clang/lib/CodeGen/CGVTables.h
===
--- clang/lib/CodeGen/CGVTables.h
+++ clang/lib/CodeGen/CGVTables.h
@@ -154,6 +154,9 @@
   /// when a vtable may not be dso_local.
   void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
llvm::StringRef AliasNameRef);
+
+  /// Specify a vtable should not be instrumented with hwasan.
+  void RemoveHwasanMetadata(llvm::GlobalValue *VTable);
 };
 
 } // end namespace CodeGen
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -921,12 +921,25 @@
 
   CGM.EmitVTableTypeMetadata(RD, VTable, *VTLayout.get());
 
-  if (UsingRelativeLayout && !VTable->isDSOLocal())
-GenerateRelativeVTableAlias(VTable, OutName);
+  if (UsingRelativeLayout) {
+

[PATCH] D131274: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

[Leonard Chan via Phabricator via cfe-commits]

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

LGTM. Asserted locally the test passes. Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131274

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

[PATCH] D130516: [llvm] compression classes

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:255-256
+} else
+  return error("Compressed string table, but " +
+   (CompressionScheme->Name + " is unavailable").str());
+  }

nit: I think `error` accepts format-like arguments, so you could have something 
similar to above with:

```
return error("Compressed string table, but {0} is unavailable", 
CompressionScheme->Name);
```



Comment at: clang/lib/Serialization/ASTReader.cpp:1468-1470
+  if (!OptionalCompressionScheme) {
+return llvm::MemoryBuffer::getMemBuffer(Blob, Name, true);
+  }

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements



Comment at: clang/lib/Serialization/ASTReader.cpp:1474-1475
+  if (!CompressionScheme) {
+Error("compression class " +
+  (CompressionScheme->Name + " is not available").str());
 return nullptr;

I think this `Error` takes a StringRef, so I think you could have:

```
return Error("compression class " + CompressionScheme->Name + " is not 
available");
```



Comment at: lld/ELF/InputSection.cpp:1230
+if (Error e = compression::CompressionKind::Zlib->decompress(
+rawData.slice(sizeof(typename ELFT::Chdr)), buf, size))
+  fatal(toString(this) +

`typename` might not be needed here



Comment at: llvm/include/llvm/Support/Compression.h:28
 
-constexpr int NoCompression = 0;
-constexpr int BestSpeedCompression = 1;
-constexpr int DefaultCompression = 6;
-constexpr int BestSizeCompression = 9;
-
-bool isAvailable();
-
-void compress(ArrayRef Input,
-  SmallVectorImpl ,
-  int Level = DefaultCompression);
-
-Error uncompress(ArrayRef Input, uint8_t *UncompressedBuffer,
- size_t );
-
-Error uncompress(ArrayRef Input,
- SmallVectorImpl ,
- size_t UncompressedSize);
-
-} // End of namespace zlib
-
-namespace zstd {
-
-constexpr int NoCompression = -5;
-constexpr int BestSpeedCompression = 1;
-constexpr int DefaultCompression = 5;
-constexpr int BestSizeCompression = 12;
-
-bool isAvailable();
-
-void compress(ArrayRef Input,
-  SmallVectorImpl ,
-  int Level = DefaultCompression);
-
-Error uncompress(ArrayRef Input, uint8_t *UncompressedBuffer,
- size_t );
-
-Error uncompress(ArrayRef Input,
- SmallVectorImpl ,
- size_t UncompressedSize);
-
-} // End of namespace zstd
+struct CompressionAlgorithm {
+  const StringRef Name;

https://llvm.org/docs/CodingStandards.html#use-of-class-and-struct-keywords 
since the ctor is non-public



Comment at: llvm/include/llvm/Support/Compression.h:40
+SmallVectorImpl ) {
+return compress(Input, CompressedBuffer, this->DefaultLevel);
+  }

`this->` might not be needed here



Comment at: llvm/include/llvm/Support/Compression.h:86
+constexpr CompressionKind::operator bool() const {
+  switch (uint8_t(x)) {
+  case uint8_t(CompressionKind::Zlib):

I think this cast might not be needed



Comment at: llvm/include/llvm/Support/Compression.h:106
+  switch (y) {
+  case uint8_t(0):
+return NoneType();

I think this cast might not be needed



Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:442-460
+  DebugCompressionType CompressionType =
+  reinterpret_cast *>(Sec.OriginalData.data())
+  ->ch_type == ELF::ELFCOMPRESS_ZLIB
+  ? DebugCompressionType::Z
+  : DebugCompressionType::None;
+
+  switch (CompressionType) {

What's the explanation for having the `llvm_unreachable` branch and getting the 
compression type? I would've thought this section would just be:

```
if (Error Err1 = compression::CompressionKind::Zlib->decompress(
Compressed, DecompressedContent, static_cast(Sec.Size))) {
  return createStringError(errc::invalid_argument,
   "'" + Sec.Name +
   "': " + toString(std::move(Err1)));
}
```

which looks like it would have identical behavior to the old code.



Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:530-536
+  switch (CompressionType) {
+  case DebugCompressionType::Z:
+compression::CompressionKind::Zlib->compress(OriginalData, CompressedData);
+break;
+  case DebugCompressionType::None:
+break;
+  }

Same here. Should this just be 
`compression::CompressionKind::Zlib->compress(OriginalData, CompressedData);`? 
If this is in preparation for the ELF+zstd changes, perhaps we should save 
those for another patch once that lands?



Comment 

[PATCH] D130516: [llvm] compression classes

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: llvm/lib/Support/Compression.cpp:30-32
+ZStdCompressionAlgorithm
+*llvm::compression::ZStdCompressionAlgorithm::Instance =
+new ZStdCompressionAlgorithm();

Perhaps for each of these, you could instead have something like:

```
ZStdCompressionAlgorithm *getZStdCompressionAlgorithm() {
  static ZStdCompressionAlgorithm* instance = new ZStdCompressionAlgorithm;
  return instance;
}
```

This way the instances are only new'd when they're actually used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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

[PATCH] D130516: [llvm] compression classes

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:193-194
 }
-if (llvm::compression::zlib::isAvailable()) {
+llvm::compression::CompressionAlgorithm *CompressionScheme =
+new llvm::compression::ZlibCompressionAlgorithm();
+CompressionScheme = CompressionScheme->whenSupported();

Will this leak?



Comment at: llvm/include/llvm/Support/Compression.h:56-58
+  virtual Error decompress(ArrayRef Input,
+   SmallVectorImpl ,
+   size_t UncompressedSize) = 0;

Does the `uncompress` version of this just end up calling into the other 
`uncompress` function? If so, we could probably just have one `decompress` 
virtual method here and the one that accepts a `SmallVectorImpl` just calls 
into the virtual `decompress` rather than have two virtual methods that would 
do the same thing. It looks like you've done that in 
`CompressionAlgorithmImpl`, but I think it could be moved here.



Comment at: llvm/include/llvm/Support/Compression.h:60-61
+
+  virtual CompressionAlgorithm *when(bool useCompression) = 0;
+  virtual CompressionAlgorithm *whenSupported() = 0;
+

Perhaps add some comments for these functions? At least for me, it's not 
entirely clear what these are for.



Comment at: llvm/include/llvm/Support/Compression.h:66-67
+
+template 
+class CompressionAlgorithmImpl : public CompressionAlgorithm {
+public:

Perhaps it would be simpler to just have the individual subclasses inherit from 
`CompressionAlgorithm` rather than have them all go through 
`CompressionAlgorithmImpl`? It looks like each child class with methods like 
`getAlgorithmId` can just return the static values themselves rather than 
passing them up to a parent to be returned. I think unless some static 
polymorphism is needed here, CRTP might not be needed here.



Comment at: llvm/lib/Support/Compression.cpp:68
+
+void NoneCompressionAlgorithm::Compress(
+ArrayRef Input, SmallVectorImpl ,

Does `NoneCompressionAlgorithm` imply there's no compression at all? If so, I 
would think these methods should be empty.



Comment at: llvm/lib/Support/Compression.cpp:237-238
+llvm::compression::CompressionAlgorithmFromId(uint8_t CompressionSchemeId) {
+  llvm::compression::CompressionAlgorithm *CompressionScheme =
+  new llvm::compression::UnknownCompressionAlgorithm();
+  switch (CompressionSchemeId) {

Is the purpose of `UnknownCompressionAlgorithm` to be the default instance 
here? If so, would it be better perhaps to just omit this and have an 
`llvm_unreachable` in the `default` case below? I would assume users of this 
function should just have the right compression scheme ID they need and any 
error checking on if something is a valid ID would be done before calling this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516

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

[PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Leonard Chan via Phabricator via cfe-commits]

leonardchan accepted this revision.
leonardchan added a comment.

LGTM tested on mac on my end


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[PATCH] D128465: [llvm] cmake config groundwork to have ZSTD in LLVM

[Leonard Chan via Phabricator via cfe-commits]

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

LGTM although I think I'd be more comfortable with Petr's +2 for cmake changes.

Would it be better instead to expose `LLVM_ENABLE_ZSTD` to subrepos like clangd 
or flang until they're needed there? Not sure in this case if this will need 
+2s from their respective owners.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[PATCH] D128754: Refactor LLVM compression namespaces (Part 1: remove crc32)

[Leonard Chan via Phabricator via cfe-commits]

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

LGTM. Should probably add `[llvm]` to the subject also. Generally it's helpful 
for immediately knowing which subrepo you're touching.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128754

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

[PATCH] D128953: refactor compression namespaces making way for a possible introduction of alternatives to zlib compression in the llvm toolchain.

[Leonard Chan via Phabricator via cfe-commits]

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

LGTM. Should also probably add `[NFC]` to the start of the subject




Comment at: lld/ELF/InputSection.cpp:77
+if (!compression::zlib::isAvailable())
+  error(toString(file) + ": contains a compressed section, " + "but " +
+compression::zlib::AlgorithmName + " is not available");

`": contains a compressed section, " + "but "` -> `": contains a compressed 
section, but "`



Comment at: llvm/lib/ProfileData/InstrProf.cpp:154
+OS << ("profile uses " + compression::profile::AlgorithmName +
+   " compression but the profile reader was built " + "without " +
+   compression::profile::AlgorithmName + " support");

`" compression but the profile reader was built " + "without "` -> `" 
compression but the profile reader was built without "`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128953

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

[PATCH] D128754: Refactor LLVM compression namespaces

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:183-191
+* Refactor compression namespaces across the project, making way for a possible
+  introduction of alternatives to zlib compression in the llvm toolchain.
+  Changes are as follows:
+  * Remove crc32 from zlib compression namespace, people should use the 
``llvm::crc32`` instead.
+  * Relocate the ``llvm::zlib`` namespace to ``llvm::compression::zlib``.
+  * Code that explictly needs ``zlib`` compression (IE zlib elf debug 
sections) should use ``llvm::compression::zlib``.
+  * Code interfacing with compressed profile data should use 
``llvm::compression::profile``.

I think maybe even more of these could be split into further patches. I would 
expect that this patch contain just the namespace refactoring but not 
necessarily any code deletion or cmake changes. I think this could be:

- One patch that's just the namespace changes; anything else like variable name 
changes or string changes would be in separate patches, then this patch would 
be pure RFC and can get a quick LGTM
  - One followup patch to this that adds `AlgorithmName` to the nested `zlib` 
namespace.
- One patch that removes crc32 (and its test)
- One patch for cmake changes
- One patch for the `zlib_unavailable -> compression_unavailable` change and 
logging string changes in profdata

And if necessary, each of them would have an appropriate ReleaseNodes.rst 
update.



Comment at: llvm/lib/Support/Compression.cpp:32
 
+#if LLVM_ENABLE_ZLIB
+

Should `createError` still be wrapped with `#if LLVM_ENABLE_ZLIB`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128754

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

[PATCH] D128754: Refactor LLVM compression namespaces

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

> Feedback needed on:
> this namespace approach will result in tooling compression methods being set 
> at compile time, however we may want to aim for a runtime configurable 
> approach in the future, likely a new revision of the compressed data formats 
> that llvm interfaces with with outside tools IE:
>
> - clang ast serialization code could be changed so compressed data could have 
> a flag indicating a compression type.
> - profile data code could be changed so compressed data could have a flag 
> indicating a compression type as well.

Using the compile time approach for now with the runtime approach in the future 
SGTM. I think some flag that could specify the decompression type is what we 
want in the long term, but the compile time approach I think is ok for now to 
not block you.




Comment at: clang-tools-extra/clangd/index/Serialization.cpp:242
+return error(
+"Compressed string table, but compression::serialize is unavailable");
 

nit: Could we add some function that returns a string of whatever compression 
is used? This way we have a more descriptive error message showing what 
specifically is unavailable. Same comment elsewhere there's logging/error 
reporting but the string is "compression::serialize".



Comment at: llvm/unittests/Support/CompressionTest.cpp:68
-  0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
-}

Should this be replaced with `llvm::crc32` rather than deleting this?


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

https://reviews.llvm.org/D128754

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

[PATCH] D108469: Improve handling of static assert messages.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

I think this might be the cause of the many libc++ static_assert failures we're 
seeing on our builders: 
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8810079977358640657/overview


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108469

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

[PATCH] D128567: [WIP][Fuchsia] Set LLVM_TOOL_LLD_BUILD to allow some extra runtimes tests to run

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D128567#3609635 , @leonardchan 
wrote:

> Won't be landing this until I'm sure any new tests that will be running from 
> this won't fail locally.

Or given https://reviews.llvm.org/D126936 and this could just pass magically on 
our bots, maybe we could just land this at the cost of being able to reproduce 
failures locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128567

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

[PATCH] D128567: [WIP][Fuchsia] Set LLVM_TOOL_LLD_BUILD to allow some extra runtimes tests to run

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

An alternative approach to this is update this bit of cmake in 
compiler-rt/CMakeLists.txt:

  set(COMPILER_RT_LLD_PATH ${LLVM_MAIN_SRC_DIR}/tools/lld)
  if(EXISTS ${COMPILER_RT_LLD_PATH}/ AND LLVM_TOOL_LLD_BUILD)
set(COMPILER_RT_HAS_LLD TRUE)
  else()
set(COMPILER_RT_LLD_PATH ${LLVM_MAIN_SRC_DIR}/../lld)
if(EXISTS ${COMPILER_RT_LLD_PATH}/ AND LLVM_TOOL_LLD_BUILD)
  set(COMPILER_RT_HAS_LLD TRUE)
endif()
  endif()
  
  if(ANDROID)
set(COMPILER_RT_HAS_LLD TRUE)
set(COMPILER_RT_TEST_USE_LLD TRUE)
append_list_if(COMPILER_RT_HAS_FUSE_LD_LLD_FLAG -fuse-ld=lld 
SANITIZER_COMMON_LINK_FLAGS)
append_list_if(COMPILER_RT_HAS_LLD -fuse-ld=lld 
COMPILER_RT_UNITTEST_LINK_FLAGS)
  endif()

to something like:

  set(COMPILER_RT_LLD_PATH ${LLVM_MAIN_SRC_DIR}/../lld)
  if(EXISTS ${COMPILER_RT_LLD_PATH})
set(COMPILER_RT_HAS_LLD TRUE)
  endif()
  
  if(ANDROID)
set(COMPILER_RT_HAS_LLD TRUE)
set(COMPILER_RT_TEST_USE_LLD TRUE)
  endif()
  append_list_if(COMPILER_RT_HAS_FUSE_LD_LLD_FLAG -fuse-ld=lld 
SANITIZER_COMMON_LINK_FLAGS)
  append_list_if(COMPILER_RT_HAS_LLD -fuse-ld=lld 
COMPILER_RT_UNITTEST_LINK_FLAGS)

I'm thinking `LLVM_TOOL_LLD_BUILD` might be a remnant of times before the 
monorepo since lld doesn't exist under `llvm/tools` anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128567

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

[PATCH] D128567: [WIP][Fuchsia] Set LLVM_TOOL_LLD_BUILD to allow some extra runtimes tests to run

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Won't be landing this until I'm sure any new tests that will be running from 
this won't fail locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128567

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

[PATCH] D128567: [WIP][Fuchsia] Set LLVM_TOOL_LLD_BUILD to allow some extra runtimes tests to run

[Leonard Chan via Phabricator via cfe-commits]

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

`LLVM_TOOL_LLD_BUILD` is required for `COMPILER_RT_HAS_LLD` to be set which 
impacts tests in the following ways:

- This sets `config.has_lld` in various lit files. This allows hwasan tests to 
run for arm runtimes and sets the feature `lld-available` which is required for 
a couple of tests.
- Adds an "lld variant" of test suites for various sanitizers like msan that 
essentially run a suite of msan tests, but explicitly linked with lld.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128567

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
@@ -134,6 +134,7 @@
 set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
 set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind 
-static-libgcc" CACHE STRING "")
 set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL 
"")
+set(RUNTIMES_${target}_LLVM_TOOL_LLD_BUILD ON CACHE BOOL "")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES 
"compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 # Use .build-id link.


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -134,6 +134,7 @@
 set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
 set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
 set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL "")
+set(RUNTIMES_${target}_LLVM_TOOL_LLD_BUILD ON CACHE BOOL "")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 # Use .build-id link.
___
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

[Leonard Chan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
leonardchan marked an inline comment as done.
Closed by commit rG4ac58b61022d: [clang][Fuchsia] Ensure static sanitizer libs 
are only linked in after the… (authored by leonardchan).

Changed prior to commit:
  https://reviews.llvm.org/D119201?vs=406871=406896#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119201

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  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
@@ -127,10 +127,7 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
-  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-  ToolChain.addProfileRTLibs(Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
options::OPT_r)) {
@@ -153,11 +150,14 @@
   }
 }
 
-if (NeedsSanitizerDeps)
-  linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
+// Note that Fuchsia never needs to link in sanitizer runtime deps.  Any
+// sanitizer runtimes with system dependencies use the `.deplibs` feature
+// instead.
+addSanitizerRuntimes(ToolChain, Args, CmdArgs);
 
-if (NeedsXRayDeps)
-  linkXRayRuntimeDeps(ToolChain, CmdArgs);
+addXRayRuntime(ToolChain, Args, CmdArgs);
+
+ToolChain.addProfileRTLibs(Args, CmdArgs);
 
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -771,11 +771,6 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain ,
  ArgStringList ) {
-  // 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(getAsNeededOption(TC, false));


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -127,10 +127,7 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
-  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-  ToolChain.addProfileRTLibs(Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
options::OPT_r)) {
@@ -153,11 +150,14 @@
   }
 }
 
-if (NeedsSanitizerDeps)
-  linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
+// Note that Fuchsia never needs to link in sanitizer runtime deps.  Any
+// sanitizer runtimes with system dependencies use the `.deplibs` feature
+// instead.
+addSanitizerRuntimes(ToolChain, Args, CmdArgs);
 
-if (NeedsXRayDeps)
-  linkXRayRuntimeDeps(ToolChain, CmdArgs);
+addXRayRuntime(ToolChain, Args, CmdArgs);
+
+ToolChain.addProfileRTLibs(Args, CmdArgs);
 
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -771,11 +771,6 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain ,
  ArgStringList ) {
-  // 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(getAsNeededOption(TC, false));
___
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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan marked an inline comment as done.
leonardchan added a comment.

In D119201#3303275 , @phosek wrote:

> Have you checked if all the tests are still passing?

All the clang tests seem to be passing, but I'll run the llvm and runtimes 
tests also.


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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 406871.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119201

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  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
@@ -127,10 +127,7 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
-  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-  ToolChain.addProfileRTLibs(Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
options::OPT_r)) {
@@ -153,12 +150,17 @@
   }
 }
 
-if (NeedsSanitizerDeps)
-  linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
+// Note that Fuchsia never needs to link in sanitizer runtime deps.  Any
+// sanitizer runtimes with system dependencies use the `.deplibs` feature
+// instead.
+addSanitizerRuntimes(ToolChain, Args, CmdArgs);
 
+bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
 if (NeedsXRayDeps)
   linkXRayRuntimeDeps(ToolChain, CmdArgs);
 
+ToolChain.addProfileRTLibs(Args, CmdArgs);
+
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
 if (Args.hasArg(options::OPT_pthread) ||
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -771,11 +771,6 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain ,
  ArgStringList ) {
-  // 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(getAsNeededOption(TC, false));


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -127,10 +127,7 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
-  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-  ToolChain.addProfileRTLibs(Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
options::OPT_r)) {
@@ -153,12 +150,17 @@
   }
 }
 
-if (NeedsSanitizerDeps)
-  linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
+// Note that Fuchsia never needs to link in sanitizer runtime deps.  Any
+// sanitizer runtimes with system dependencies use the `.deplibs` feature
+// instead.
+addSanitizerRuntimes(ToolChain, Args, CmdArgs);
 
+bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
 if (NeedsXRayDeps)
   linkXRayRuntimeDeps(ToolChain, CmdArgs);
 
+ToolChain.addProfileRTLibs(Args, CmdArgs);
+
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
 if (Args.hasArg(options::OPT_pthread) ||
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -771,11 +771,6 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain ,
  ArgStringList ) {
-  // 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(getAsNeededOption(TC, false));
___
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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 406664.
leonardchan marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119201

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  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
@@ -127,11 +127,6 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
-  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
-  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-  ToolChain.addProfileRTLibs(Args, CmdArgs);
-
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
options::OPT_r)) {
 if (Args.hasArg(options::OPT_static))
@@ -153,12 +148,18 @@
   }
 }
 
-if (NeedsSanitizerDeps)
-  linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
+// Note that Fuchsia never needs to link in sanitizer runtime deps.  Any
+// sanitizer runtimes with system dependencies use the `.deplibs` feature
+// instead.
+addSanitizerRuntimes(ToolChain, Args, CmdArgs);
 
+bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
 if (NeedsXRayDeps)
   linkXRayRuntimeDeps(ToolChain, CmdArgs);
 
+AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
+ToolChain.addProfileRTLibs(Args, CmdArgs);
+
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
 if (Args.hasArg(options::OPT_pthread) ||
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -771,11 +771,6 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain ,
  ArgStringList ) {
-  // 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(getAsNeededOption(TC, false));


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -127,11 +127,6 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
-  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
-  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-  ToolChain.addProfileRTLibs(Args, CmdArgs);
-
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
options::OPT_r)) {
 if (Args.hasArg(options::OPT_static))
@@ -153,12 +148,18 @@
   }
 }
 
-if (NeedsSanitizerDeps)
-  linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
+// Note that Fuchsia never needs to link in sanitizer runtime deps.  Any
+// sanitizer runtimes with system dependencies use the `.deplibs` feature
+// instead.
+addSanitizerRuntimes(ToolChain, Args, CmdArgs);
 
+bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
 if (NeedsXRayDeps)
   linkXRayRuntimeDeps(ToolChain, CmdArgs);
 
+AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
+ToolChain.addProfileRTLibs(Args, CmdArgs);
+
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
 if (Args.hasArg(options::OPT_pthread) ||
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -771,11 +771,6 @@
 
 void tools::linkSanitizerRuntimeDeps(const ToolChain ,
  ArgStringList ) {
-  // 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(getAsNeededOption(TC, false));
___
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

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr.
leonardchan added projects: clang, Sanitizers.
Herald added a subscriber: abrachet.
leonardchan requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119201

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
@@ -127,7 +127,6 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
   ToolChain.addProfileRTLibs(Args, CmdArgs);
@@ -153,6 +152,7 @@
   }
 }
 
+bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
 if (NeedsSanitizerDeps)
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -127,7 +127,6 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
   ToolChain.addProfileRTLibs(Args, CmdArgs);
@@ -153,6 +152,7 @@
   }
 }
 
+bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
 if (NeedsSanitizerDeps)
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113143: [clang][asan] Add test for ensuring PR52382 is fixed

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG456a7e52310d: [clang][asan] Add test for ensuring PR52382 is 
fixed (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113143

Files:
  clang/test/CodeGen/pr52382.c


Index: clang/test/CodeGen/pr52382.c
===
--- /dev/null
+++ clang/test/CodeGen/pr52382.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -S -emit-llvm -o - 
-fsanitize=address %s | FileCheck %s
+
+// Ensure that ASan properly instruments a load into a global where the index
+// happens to be within the padding after the global which is used for the
+// redzone.
+
+// This global is 400 bytes long, but gets padded with 112 bytes for redzones,
+// rounding the total size after instrumentation to 512.
+int global_array[100] = {-1};
+
+// This access is 412 bytes after the start of the global: past the end of the
+// uninstrumented array, but within the bounds of the extended instrumented
+// array. We should ensure this is still instrumented.
+int main(void) { return global_array[103]; }
+
+// CHECK: @main
+// CHECK-NEXT: entry:
+// CHECK: call void @__asan_report_load4
+// CHECK: }


Index: clang/test/CodeGen/pr52382.c
===
--- /dev/null
+++ clang/test/CodeGen/pr52382.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -S -emit-llvm -o - -fsanitize=address %s | FileCheck %s
+
+// Ensure that ASan properly instruments a load into a global where the index
+// happens to be within the padding after the global which is used for the
+// redzone.
+
+// This global is 400 bytes long, but gets padded with 112 bytes for redzones,
+// rounding the total size after instrumentation to 512.
+int global_array[100] = {-1};
+
+// This access is 412 bytes after the start of the global: past the end of the
+// uninstrumented array, but within the bounds of the extended instrumented
+// array. We should ensure this is still instrumented.
+int main(void) { return global_array[103]; }
+
+// CHECK: @main
+// CHECK-NEXT: entry:
+// CHECK: call void @__asan_report_load4
+// CHECK: }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113143: [clang][asan] Add test for ensuring PR52382 is fixed

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: aeubanks, vitalybuka.
leonardchan added a project: Sanitizers.
leonardchan requested review of this revision.
Herald added a project: clang.

The fix for PR52382 was already introduced in D112732 
 which ensures that module instrumentation 
always runs after function instrumentation. This adds a test that ensures the 
PR is addressed and prevent regression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113143

Files:
  clang/test/CodeGen/pr52382.c


Index: clang/test/CodeGen/pr52382.c
===
--- /dev/null
+++ clang/test/CodeGen/pr52382.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -S -emit-llvm -o - 
-fsanitize=address %s | FileCheck %s
+
+// Ensure that ASan properly instruments a load into a global where the index
+// happens to be within the padding after the global which is used for the
+// redzone.
+
+// This global is 400 bytes long, but gets padded with 112 bytes for redzones,
+// rounding the total size after instrumentation to 512.
+int global_array[100] = {-1};
+
+// This access is 412 bytes after the start of the global: past the end of the
+// uninstrumented array, but within the bounds of the extended instrumented
+// array. We should ensure this is still instrumented.
+int main(void) { return global_array[103]; }
+
+// CHECK: @main
+// CHECK-NEXT: entry:
+// CHECK: call void @__asan_report_load4
+// CHECK: }


Index: clang/test/CodeGen/pr52382.c
===
--- /dev/null
+++ clang/test/CodeGen/pr52382.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -S -emit-llvm -o - -fsanitize=address %s | FileCheck %s
+
+// Ensure that ASan properly instruments a load into a global where the index
+// happens to be within the padding after the global which is used for the
+// redzone.
+
+// This global is 400 bytes long, but gets padded with 112 bytes for redzones,
+// rounding the total size after instrumentation to 512.
+int global_array[100] = {-1};
+
+// This access is 412 bytes after the start of the global: past the end of the
+// uninstrumented array, but within the bounds of the extended instrumented
+// array. We should ensure this is still instrumented.
+int main(void) { return global_array[103]; }
+
+// CHECK: @main
+// CHECK-NEXT: entry:
+// CHECK: call void @__asan_report_load4
+// CHECK: }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110451: [IR] Increase max alignment to 4GB

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Finally got the stacktrace:

  leonardchan@cp-snakewater:~/llvm-monorepo/llvm-build-1-clang-only-debug$ 
bin/clang++ "-cc1" "-triple" "x86_64-unknown-linux-gnu" "-emit-obj" 
"--mrelax-relocations" "-disable-free" "-main-file-name" 
"AArch64SpeculationHardening.cpp" "-mrelocation-model" "pic" "-pic-level" "2" 
"-mframe-pointer=none" "-fmath-errno" "-fno-rounding-math" 
"-mconstructor-aliases" "-target-cpu" "x86-64" "-tune-cpu" "generic" 
"-debugger-tuning=gdb" "-ffunction-sections" "-fdata-sections" 
"-fcoverage-compilation-dir=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins"
 "-sys-header-deps" "-D" "GTEST_HAS_RTTI=0" "-D" "_GNU_SOURCE" "-D" 
"__STDC_CONSTANT_MACROS" "-D" "__STDC_FORMAT_MACROS" "-D" "__STDC_LIMIT_MACROS" 
"-D" "NDEBUG" 
"-fmacro-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins=../llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins"
 
"-fmacro-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/="
 
"-fcoverage-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins=../llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins"
 
"-fcoverage-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/="
 "-O3" "-Werror=date-time" "-Werror=unguarded-availability-new" "-Wall" 
"-Wextra" "-Wno-unused-parameter" "-Wwrite-strings" "-Wcast-qual" 
"-Wmissing-field-initializers" "-Wno-long-long" "-Wc++98-compat-extra-semi" 
"-Wimplicit-fallthrough" "-Wcovered-switch-default" "-Wno-noexcept-type" 
"-Wnon-virtual-dtor" "-Wdelete-non-virtual-dtor" "-Wsuggest-override" 
"-Wno-comment" "-Wstring-conversion" "-Wmisleading-indentation" "-pedantic" 
"-std=c++14" "-fdeprecated-macro" 
"-fdebug-compilation-dir=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins"
 
"-fdebug-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins=../llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins"
 
"-fdebug-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/="
 "-ferror-limit" "19" "-fvisibility" "hidden" "-fvisibility-inlines-hidden" 
"-fno-rtti" "-fgnuc-version=4.2.1" "-fcolor-diagnostics" "-vectorize-loops" 
"-vectorize-slp" "-faddrsig" "-x" "c++" 
~/misc/AArch64SpeculationHardening-33e9d2.cpp
  clang++: 
/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/llvm/include/llvm/Support/Alignment.h:77:
 llvm::Align::Align(uint64_t): Assertion `Value > 0 && "Value must not be 0"' 
failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: bin/clang++ -cc1 -triple x86_64-unknown-linux-gnu 
-emit-obj --mrelax-relocations -disable-free -main-file-name 
AArch64SpeculationHardening.cpp -mrelocation-model pic -pic-level 2 
-mframe-pointer=none -fmath-errno -fno-rounding-math -mconstructor-aliases 
-target-cpu x86-64 -tune-cpu generic -debugger-tuning=gdb -ffunction-sections 
-fdata-sections 
-fcoverage-compilation-dir=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins
 -sys-header-deps -D GTEST_HAS_RTTI=0 -D _GNU_SOURCE -D __STDC_CONSTANT_MACROS 
-D __STDC_FORMAT_MACROS -D __STDC_LIMIT_MACROS -D NDEBUG 
-fmacro-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins=../llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins
 
-fmacro-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/=
 
-fcoverage-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins=../llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins
 
-fcoverage-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/=
 -O3 -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra 
-Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion 
-Wmisleading-indentation -pedantic -std=c++14 -fdeprecated-macro 
-fdebug-compilation-dir=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins
 
-fdebug-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins=../llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins
 

[PATCH] D110451: [IR] Increase max alignment to 4GB

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hi. It seems the reland caused an assertion to fail on our clang builders 
(https://luci-milo.appspot.com/ui/p/fuchsia/builders/prod/clang-linux-x64/b8834065059600721665/overview):

  [194/194] Building CXX object 
lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64SpeculationHardening.cpp.o
  FAILED: 
lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64SpeculationHardening.cpp.o
 
  
/usr/local/google/home/leonardchan/fuchsia/prebuilt/third_party/goma/linux-x64/gomacc
 
/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/./bin/clang++
 --sysroot=/usr/local/google/home/leonardchan/sysroot/linux -DGTEST_HAS_RTTI=0 
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS -Ilib/Target/AArch64 
-I/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/llvm/lib/Target/AArch64
 -Iinclude 
-I/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/llvm/include 
-I/usr/local/google/home/leonardchan/misc/zlib-install/include -stdlib=libc++ 
-fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections 
-ffile-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins=../llvm-build-1-two-stage-fuchsia-toolchain/tools/clang/stage2-bins
 
-ffile-prefix-map=/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/=
 -no-canonical-prefixes -O3 -DNDEBUG -fvisibility=hidden  -fno-exceptions 
-fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -std=c++14 -MD -MT 
lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64SpeculationHardening.cpp.o
 -MF 
lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64SpeculationHardening.cpp.o.d
 -o 
lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64SpeculationHardening.cpp.o
 -c 
/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/llvm/lib/Target/AArch64/AArch64SpeculationHardening.cpp
  clang++: 
/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-1/llvm/include/llvm/Support/Alignment.h:77:
 llvm::Align::Align(uint64_t): Assertion `Value > 0 && "Value must not be 0"' 
failed.

Would you be able to send out a fix or revert? I'll see if I can also provide a 
stack trace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110451

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

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hi. I think our clang builders are failing from this after the reland 
(https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8833962834812688769/overview):

  Script:
  --
  
/b/s/w/ir/x/w/staging/llvm_build/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests
 --gtest_filter=InterpreterTest.CatchException
  --
  Note: Google Test filter = InterpreterTest.CatchException
  [==] Running 1 test from 1 test suite.
  [--] Global test environment set-up.
  [--] 1 test from InterpreterTest
  [ RUN  ] InterpreterTest.CatchException
  JIT session error: Symbols not found: [ _Unwind_Resume, _ZSt9terminatev, 
_ZTVN10__cxxabiv117__class_type_infoE, __cxa_allocate_exception, 
__cxa_begin_catch, __cxa_end_catch, __cxa_free_exception, __cxa_throw, 
__gxx_personality_v0 ]
  Failure value returned from cantFail wrapped call
  Failed to materialize symbols: { (main, { __clang_call_terminate, 
_ZTI16custom_exception, _ZN16custom_exceptionC2EPKc, _ZTS16custom_exception, 
throw_exception }) }
  UNREACHABLE executed at llvm/include/llvm/Support/Error.h:778!

Would you be able to send out a fix or revert?


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

https://reviews.llvm.org/D107049

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

[PATCH] D110778: [clang][Fuchsia] Re-enable compiler-rt tests in runtimes build

[Leonard Chan 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 rG8480063f25b8: [clang][Fuchsia] Re-enable compiler-rt tests 
in runtimes build (authored by leonardchan).
Herald added a subscriber: abrachet.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110778

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
@@ -120,6 +120,7 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -120,6 +120,7 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hmm. Just found out about this, but for future reference, I think this will 
only work for `ninja check-runtimes` and `ninja check-runtimes-{target}`, but 
not necessarily `ninja -C runtimes/runtimes-{target} 
check-runtimes/check-compilter-rt`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

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

[PATCH] D110889: [clang][Fuchsia] Add -DSCUDO_DISABLE_TBI to test flags

[Leonard Chan via Phabricator via cfe-commits]

leonardchan abandoned this revision.
leonardchan added a comment.
Herald added a subscriber: abrachet.

In D110889#3035597 , @phosek wrote:

> Is this needed even with D110888 ?

Nope, at least after the updated patch this isn't needed anymore. Abandoning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110889

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

[PATCH] D110889: [clang][Fuchsia] Add -DSCUDO_DISABLE_TBI to test flags

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added a reviewer: phosek.
Herald added subscribers: cryptoad, kristof.beyls, mgorny.
leonardchan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

After D110888 , this will ensure that MTE 
tests are skipped when running scudo tests on aarch64.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110889

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
@@ -134,7 +135,7 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
-set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS 
"--unwindlib=libunwind -static-libgcc -DSCUDO_DISABLE_TBI" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
 set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind" 
CACHE STRING "")
 set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL 
"")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -134,7 +135,7 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
-set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS "--unwindlib=libunwind -static-libgcc -DSCUDO_DISABLE_TBI" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
 set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind" CACHE STRING "")
 set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110778: [clang][Fuchsia] Re-enable compiler-rt tests in runtimes build

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added a reviewer: phosek.
Herald added subscribers: mgorny, dberris.
leonardchan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110778

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
@@ -120,6 +120,7 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -120,6 +120,7 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan 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 rG79b422080612: [runtimes] Ensure required deps for tests 
targets are actually built (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

Files:
  llvm/runtimes/CMakeLists.txt


Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -294,6 +294,7 @@
 set(runtimes-test-depends-${name} runtimes-test-depends)
 set(check-runtimes-${name} check-runtimes)
 list(APPEND ${name}_test_targets runtimes-test-depends-${name} 
check-runtimes-${name})
+list(APPEND test_targets ${${name}_test_targets})
 foreach(target_name IN LISTS SUB_CHECK_TARGETS)
   set(${target_name}-${name} ${target_name})
   list(APPEND ${name}_test_targets ${target_name}-${name})


Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -294,6 +294,7 @@
 set(runtimes-test-depends-${name} runtimes-test-depends)
 set(check-runtimes-${name} check-runtimes)
 list(APPEND ${name}_test_targets runtimes-test-depends-${name} check-runtimes-${name})
+list(APPEND test_targets ${${name}_test_targets})
 foreach(target_name IN LISTS SUB_CHECK_TARGETS)
   set(${target_name}-${name} ${target_name})
   list(APPEND ${name}_test_targets ${target_name}-${name})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 376042.
leonardchan edited the summary of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

Files:
  llvm/runtimes/CMakeLists.txt


Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -294,6 +294,7 @@
 set(runtimes-test-depends-${name} runtimes-test-depends)
 set(check-runtimes-${name} check-runtimes)
 list(APPEND ${name}_test_targets runtimes-test-depends-${name} 
check-runtimes-${name})
+list(APPEND test_targets ${${name}_test_targets})
 foreach(target_name IN LISTS SUB_CHECK_TARGETS)
   set(${target_name}-${name} ${target_name})
   list(APPEND ${name}_test_targets ${target_name}-${name})


Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -294,6 +294,7 @@
 set(runtimes-test-depends-${name} runtimes-test-depends)
 set(check-runtimes-${name} check-runtimes)
 list(APPEND ${name}_test_targets runtimes-test-depends-${name} check-runtimes-${name})
+list(APPEND test_targets ${${name}_test_targets})
 foreach(target_name IN LISTS SUB_CHECK_TARGETS)
   set(${target_name}-${name} ${target_name})
   list(APPEND ${name}_test_targets ${target_name}-${name})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D109625#3029581 , @phosek wrote:

> I'm still unsure if this is the right strategy because it creates a 
> dependency cycle. Specifically, you have the LLVM build trigger the build of 
> runtimes, and that build would invoke LLVM build again to ensure that the 
> necessary tools have been built. That shouldn't be necessary though. The LLVM 
> build already ensures that those tools are being built by making the runtimes 
> build depend on these tools, see 
> https://github.com/llvm/llvm-project/blob/ac2daacb310cbb1732de1c139be7a0e8e982169e/llvm/runtimes/CMakeLists.txt#L461
>
> What might be needed is a way to tell the compiler-rt build where to find 
> these tools. I looked elsewhere in LLVM and it seems like Polly is already 
> taking that approach, see 
> https://github.com/llvm/llvm-project/blob/main/polly/test/CMakeLists.txt#L6. 
> Do you think that we could use this strategy?

Ah, so it seems that both`test_targets` and `SUB_CHECK_TARGETS` in 
https://github.com/llvm/llvm-project/blob/ac2daacb310cbb1732de1c139be7a0e8e982169e/llvm/runtimes/CMakeLists.txt#L474
 are both empty, so these dependencies are never added. Lemme see if I can find 
why they're empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

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

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 375713.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

Files:
  compiler-rt/test/CMakeLists.txt


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -36,6 +37,21 @@
 if (WIN32)
   list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
 endif()
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# These are tools needed for testing in a standalone/runtimes build, but we
+# don't have a corresponding CMAKE_* flag for. These will need to be 
rebuilt.
+set(LIT_ONLY_TOOLS FileCheck count not)
+foreach(dep ${LIT_ONLY_TOOLS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
+
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_ONLY_TOOLS})
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -36,6 +37,21 @@
 if (WIN32)
   list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
 endif()
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# These are tools needed for testing in a standalone/runtimes build, but we
+# don't have a corresponding CMAKE_* flag for. These will need to be rebuilt.
+set(LIT_ONLY_TOOLS FileCheck count not)
+foreach(dep ${LIT_ONLY_TOOLS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
+
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_ONLY_TOOLS})
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Updated to exclude readelf. We also don't need any of the other tools added in 
the `NOT COMPILER_RT_STANDALONE_BUILD` case, so I just added them to 
`LIT_ONLY_TOOLS` in the `COMPILER_RT_STANDALONE_BUILD` case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

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

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 374984.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

Files:
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  compiler-rt/test/CMakeLists.txt


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -36,6 +37,21 @@
 if (WIN32)
   list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
 endif()
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# These are tools needed for testing in a standalone/runtimes build, but we
+# don't have a corresponding CMAKE_* flag for. These will need to be 
rebuilt.
+set(LIT_ONLY_TOOLS FileCheck count not clang-resource-headers)
+foreach(dep ${LIT_ONLY_TOOLS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
+
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_ONLY_TOOLS})
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -598,6 +598,7 @@
 CMAKE_OBJCOPY
 CMAKE_OBJDUMP
 CMAKE_STRIP
+CMAKE_READELF
 CMAKE_SYSROOT
 LIBCXX_HAS_MUSL_LIBC
 PYTHON_EXECUTABLE


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -36,6 +37,21 @@
 if (WIN32)
   list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
 endif()
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# These are tools needed for testing in a standalone/runtimes build, but we
+# don't have a corresponding CMAKE_* flag for. These will need to be rebuilt.
+set(LIT_ONLY_TOOLS FileCheck count not clang-resource-headers)
+foreach(dep ${LIT_ONLY_TOOLS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
+
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_ONLY_TOOLS})
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -598,6 +598,7 @@
 CMAKE_OBJCOPY
 CMAKE_OBJDUMP
 CMAKE_STRIP
+CMAKE_READELF
 CMAKE_SYSROOT
 LIBCXX_HAS_MUSL_LIBC
 PYTHON_EXECUTABLE
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: compiler-rt/test/CMakeLists.txt:48
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}

leonardchan wrote:
> phosek wrote:
> > This is going to run Ninja in the LLVM build once for each dependency 
> > listed above, correct? That seems quite expensive.
> > 
> > We already pass most of these to the child build via corresponding CMake 
> > variables, see 
> > https://github.com/llvm/llvm-project/blob/ed921282e551f2252ccfcbddd7a85ad8a006ed3f/llvm/cmake/modules/LLVMExternalProjectUtils.cmake#L160
> > 
> > For example, if just need some readelf implementation and not necessarily 
> > llvm-readelf, it may be better to use the value of `CMAKE_READELF` and 
> > propagate that down to tests through substitution (that is wherever the 
> > tests invoke `llvm-readelf`, we would replace it with `%readelf`).
> > 
> > We're still going to need this logic for tools where there's no 
> > corresponding CMake variable like `FileCheck` but it should be 
> > significantly fewer ones.
> So it looks like only FileCheck, count, and not are required but don't have 
> cmake variables. Would what I have now be ok where instead we just iterate 
> over a trimmed list of tools?
I lied, there's also clang-resource-headers and llvm-readelf.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

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

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 374396.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

Files:
  compiler-rt/test/CMakeLists.txt


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -28,14 +29,34 @@
 # When ANDROID, we build tests with the host compiler (i.e. CMAKE_C_COMPILER),
 # and run tests with tools from the host toolchain.
 if(NOT ANDROID)
-  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
-# Use LLVM utils and Clang from the same build tree.
-list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS
+  set(LIT_TEST_DEPS
   clang clang-resource-headers FileCheck count not llvm-config llvm-nm 
llvm-objdump
   llvm-readelf llvm-readobj llvm-size llvm-symbolizer compiler-rt-headers 
sancov)
-if (WIN32)
-  list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
-endif()
+  if (WIN32)
+list(APPEND LIT_TEST_DEPS KillTheDoctor)
+  endif()
+
+  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
+# Use LLVM utils and Clang from the same build tree.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# If we are making a standalone build, we need to ensure some targets used
+# for testing are known and built as deps.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+
+# These are tools needed for testing in a standalone/runtimes build, but we
+# don't have a corresponding CMAKE_* flag for. These will need to be 
rebuilt.
+set(LIT_ONLY_TOOLS
+FileCheck count not clang-resource-headers llvm-readelf)
+foreach(dep ${LIT_ONLY_TOOLS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -28,14 +29,34 @@
 # When ANDROID, we build tests with the host compiler (i.e. CMAKE_C_COMPILER),
 # and run tests with tools from the host toolchain.
 if(NOT ANDROID)
-  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
-# Use LLVM utils and Clang from the same build tree.
-list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS
+  set(LIT_TEST_DEPS
   clang clang-resource-headers FileCheck count not llvm-config llvm-nm llvm-objdump
   llvm-readelf llvm-readobj llvm-size llvm-symbolizer compiler-rt-headers sancov)
-if (WIN32)
-  list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
-endif()
+  if (WIN32)
+list(APPEND LIT_TEST_DEPS KillTheDoctor)
+  endif()
+
+  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
+# Use LLVM utils and Clang from the same build tree.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# If we are making a standalone build, we need to ensure some targets used
+# for testing are known and built as deps.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+
+# These are tools needed for testing in a standalone/runtimes build, but we
+# don't have a corresponding CMAKE_* flag for. These will need to be rebuilt.
+set(LIT_ONLY_TOOLS
+FileCheck count not clang-resource-headers llvm-readelf)
+foreach(dep ${LIT_ONLY_TOOLS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: compiler-rt/test/CMakeLists.txt:48
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}

phosek wrote:
> This is going to run Ninja in the LLVM build once for each dependency listed 
> above, correct? That seems quite expensive.
> 
> We already pass most of these to the child build via corresponding CMake 
> variables, see 
> https://github.com/llvm/llvm-project/blob/ed921282e551f2252ccfcbddd7a85ad8a006ed3f/llvm/cmake/modules/LLVMExternalProjectUtils.cmake#L160
> 
> For example, if just need some readelf implementation and not necessarily 
> llvm-readelf, it may be better to use the value of `CMAKE_READELF` and 
> propagate that down to tests through substitution (that is wherever the tests 
> invoke `llvm-readelf`, we would replace it with `%readelf`).
> 
> We're still going to need this logic for tools where there's no corresponding 
> CMake variable like `FileCheck` but it should be significantly fewer ones.
So it looks like only FileCheck, count, and not are required but don't have 
cmake variables. Would what I have now be ok where instead we just iterate over 
a trimmed list of tools?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

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

[PATCH] D109625: [compiler-rt] Ensure required deps for tests targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 374393.
leonardchan retitled this revision from "[compiler-rt] Ensure LIT_TEST_DEP 
targets are actually built" to "[compiler-rt] Ensure required deps for tests 
targets are actually built".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

Files:
  compiler-rt/test/CMakeLists.txt


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -28,14 +29,34 @@
 # When ANDROID, we build tests with the host compiler (i.e. CMAKE_C_COMPILER),
 # and run tests with tools from the host toolchain.
 if(NOT ANDROID)
-  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
-# Use LLVM utils and Clang from the same build tree.
-list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS
+  set(LIT_TEST_DEPS
   clang clang-resource-headers FileCheck count not llvm-config llvm-nm 
llvm-objdump
   llvm-readelf llvm-readobj llvm-size llvm-symbolizer compiler-rt-headers 
sancov)
-if (WIN32)
-  list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
-endif()
+  if (WIN32)
+list(APPEND LIT_TEST_DEPS KillTheDoctor)
+  endif()
+
+  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
+# Use LLVM utils and Clang from the same build tree.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# If we are making a standalone build, we need to ensure some targets used
+# for testing are known and built as deps.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+
+# These are tools needed for testing in a standalone/runtimes build, but we
+# don't have a corresponding CMAKE_* flag for. These will need to be 
rebuilt.
+set(LIT_ONLY_TOOLS
+FileCheck count not)
+foreach(dep ${LIT_ONLY_TOOLS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -28,14 +29,34 @@
 # When ANDROID, we build tests with the host compiler (i.e. CMAKE_C_COMPILER),
 # and run tests with tools from the host toolchain.
 if(NOT ANDROID)
-  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
-# Use LLVM utils and Clang from the same build tree.
-list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS
+  set(LIT_TEST_DEPS
   clang clang-resource-headers FileCheck count not llvm-config llvm-nm llvm-objdump
   llvm-readelf llvm-readobj llvm-size llvm-symbolizer compiler-rt-headers sancov)
-if (WIN32)
-  list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
-endif()
+  if (WIN32)
+list(APPEND LIT_TEST_DEPS KillTheDoctor)
+  endif()
+
+  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
+# Use LLVM utils and Clang from the same build tree.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# If we are making a standalone build, we need to ensure some targets used
+# for testing are known and built as deps.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+
+# These are tools needed for testing in a standalone/runtimes build, but we
+# don't have a corresponding CMAKE_* flag for. These will need to be rebuilt.
+set(LIT_ONLY_TOOLS
+FileCheck count not)
+foreach(dep ${LIT_ONLY_TOOLS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109625: [compiler-rt] Ensure LIT_TEST_DEP targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109625

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

[PATCH] D109625: [compiler-rt] Ensure LIT_TEST_DEP targets are actually built

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added a reviewer: phosek.
Herald added subscribers: mgorny, dberris.
leonardchan requested review of this revision.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

When building compiler-rt via runtimes, many tests fail because tools like 
FileCheck and count aren't built yet. If it's also through a standalone build, 
then compiler-rt doesn't know of these targets. This copies logic from other 
parts of the monorepo and adds custom commands for building these external 
projects if we are in the case of a runtimes build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109625

Files:
  compiler-rt/test/CMakeLists.txt


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -28,14 +29,29 @@
 # When ANDROID, we build tests with the host compiler (i.e. CMAKE_C_COMPILER),
 # and run tests with tools from the host toolchain.
 if(NOT ANDROID)
-  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
-# Use LLVM utils and Clang from the same build tree.
-list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS
+  set(LIT_TEST_DEPS
   clang clang-resource-headers FileCheck count not llvm-config llvm-nm 
llvm-objdump
   llvm-readelf llvm-readobj llvm-size llvm-symbolizer compiler-rt-headers 
sancov)
-if (WIN32)
-  list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
-endif()
+  if (WIN32)
+list(APPEND LIT_TEST_DEPS KillTheDoctor)
+  endif()
+
+  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
+# Use LLVM utils and Clang from the same build tree.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# If we are making a standalone build, we need to ensure some targets used
+# for testing are known and built as deps.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+foreach(dep ${LIT_TEST_DEPS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)


Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Needed for lit support in standalone builds.
 include(AddLLVM)
+include(LLVMExternalProjectUtils)
 
 option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
   "When set to ON and testing in a standalone build, test the runtime \
@@ -28,14 +29,29 @@
 # When ANDROID, we build tests with the host compiler (i.e. CMAKE_C_COMPILER),
 # and run tests with tools from the host toolchain.
 if(NOT ANDROID)
-  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
-# Use LLVM utils and Clang from the same build tree.
-list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS
+  set(LIT_TEST_DEPS
   clang clang-resource-headers FileCheck count not llvm-config llvm-nm llvm-objdump
   llvm-readelf llvm-readobj llvm-size llvm-symbolizer compiler-rt-headers sancov)
-if (WIN32)
-  list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS KillTheDoctor)
-endif()
+  if (WIN32)
+list(APPEND LIT_TEST_DEPS KillTheDoctor)
+  endif()
+
+  if(NOT COMPILER_RT_STANDALONE_BUILD AND NOT LLVM_RUNTIMES_BUILD)
+# Use LLVM utils and Clang from the same build tree.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+  elseif(COMPILER_RT_STANDALONE_BUILD)
+# If we are making a standalone build, we need to ensure some targets used
+# for testing are known and built as deps.
+list(APPEND SANITIZER_COMMON_LIT_TEST_DEPS ${LIT_TEST_DEPS})
+foreach(dep ${LIT_TEST_DEPS})
+  if (NOT TARGET ${dep})
+llvm_ExternalProject_BuildCmd(build_${dep} ${dep} ${BINARY_DIR})
+add_custom_target(${dep}
+  COMMAND ${build_${dep}}
+  WORKING_DIRECTORY ${BINARY_DIR}
+  VERBATIM USES_TERMINAL)
+  endif()
+endforeach()
   endif()
   # Tests use C++ standard library headers.
   if (TARGET cxx-headers OR HAVE_LIBCXX)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

[Leonard Chan 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 rGe5673564a06b: [compiler-rt][Fuchsia] Support building + 
running compiler-rt tests on… (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109199

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
@@ -60,7 +60,6 @@
   set(COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
   set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
@@ -121,9 +120,9 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
@@ -136,6 +135,10 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind" 
CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL 
"")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES 
"compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 # Use .build-id link.


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -60,7 +60,6 @@
   set(COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
   set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
@@ -121,9 +120,9 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
@@ -136,6 +135,10 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind" CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL "")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 # Use .build-id link.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:123
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_BUILD_BUILTINS ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")

phosek wrote:
> leonardchan wrote:
> > phosek wrote:
> > > This shouldn't be needed.
> > So while we do build builtins, this is actually needed to run builtins 
> > tests: 
> > https://github.com/llvm/llvm-project/blob/ad7e12226f6b1ebf91511899016cdfb29aa8919e/compiler-rt/test/CMakeLists.txt#L59
> We are already build builtins in a separate build so I think we should be 
> running the tests there rather than building the twice.
Ah, I didn't know that. Removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109199

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

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 371163.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109199

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
@@ -60,7 +60,6 @@
   set(COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
   set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
@@ -121,9 +120,9 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
@@ -136,6 +135,10 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind" 
CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL 
"")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES 
"compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 # Use .build-id link.


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -60,7 +60,6 @@
   set(COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
   set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
@@ -121,9 +120,9 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
@@ -136,6 +135,10 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind" CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL "")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 # Use .build-id link.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Split also into https://reviews.llvm.org/D109207 and 
https://reviews.llvm.org/D109208


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109199

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

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 370447.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109199

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
@@ -60,7 +60,6 @@
   set(COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
   set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
@@ -121,9 +120,10 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_BUILD_BUILTINS ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
@@ -136,6 +136,10 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind 
-static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL 
"")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES 
"compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 # Use .build-id link.


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -60,7 +60,6 @@
   set(COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 
   set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-  set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
@@ -121,9 +120,10 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_BUILD_BUILTINS ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
@@ -136,6 +136,10 @@
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
 set(RUNTIMES_${target}_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_TEST_COMPILER_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL "")
 set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
 
 # Use .build-id link.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:141
+set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind" 
CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL 
"")

leonardchan wrote:
> phosek wrote:
> > Don't we need the `-static-libgcc` flag here as well?
> When testing locally, there was actually a tsan test that wasn't working if 
> `-static-libgcc` was here. I can try to reproduce it. I believe it had to do 
> with static linking in an archive, but a particular object file in that 
> archive wasn't getting linked in which changed some runtime logic and caused 
> the test to fail.
Oh ok. Disregard my last explanation. So the test is 
`compiler-rt/test/tsan/Linux/check_memcpy.c`, and the actual reason it failed 
is because the test just checks to make sure calls to `memcpy/memset` are not 
used at all (since tsan should intercept them and replace them with its own 
functions/code).

The issue is that if we statically link in an uninstrumented libunwind, then 
the test will fail bc it will find memcpy/set calls in that libunwind code. 
Removing the `-static-libgcc` seems to instead opt for dynamically linking 
against libunwind.so, and not linking in any uninstrumented code. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109199

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

[PATCH] D109199: [compiler-rt][Fuchsia] Support building + running compiler-rt tests on fuchsia's host toolchain

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D109199#2981174 , @phosek wrote:

> @leonardchan would it be possible to break this up into 3 changes?

Can do.




Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:123
 set(RUNTIMES_${target}_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_BUILD_BUILTINS ON CACHE BOOL "")
+set(RUNTIMES_${target}_COMPILER_RT_CAN_EXECUTE_TESTS ON CACHE BOOL "")

phosek wrote:
> This shouldn't be needed.
So while we do build builtins, this is actually needed to run builtins tests: 
https://github.com/llvm/llvm-project/blob/ad7e12226f6b1ebf91511899016cdfb29aa8919e/compiler-rt/test/CMakeLists.txt#L59



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:141
+set(RUNTIMES_${target}_SANITIZER_COMMON_TEST_TARGET_CFLAGS 
"--unwindlib=libunwind -static-libgcc" CACHE STRING "")
+set(RUNTIMES_${target}_TSAN_TEST_TARGET_CFLAGS "--unwindlib=libunwind" 
CACHE STRING "")
+set(RUNTIMES_${target}_LLVM_TOOLS_DIR "${CMAKE_BINARY_DIR}/bin" CACHE BOOL 
"")

phosek wrote:
> Don't we need the `-static-libgcc` flag here as well?
When testing locally, there was actually a tsan test that wasn't working if 
`-static-libgcc` was here. I can try to reproduce it. I believe it had to do 
with static linking in an archive, but a particular object file in that archive 
wasn't getting linked in which changed some runtime logic and caused the test 
to fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109199

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