[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI
kubamracek added a comment. @kcc Any takes from you on this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D112965: [GlobalDCE][IR] Allow and support multiple !vcall_visibility ranges
kubamracek created this revision.
kubamracek added reviewers: pcc, rjmccall, fhahn.
kubamracek added a project: LLVM.
Herald added subscribers: ormris, dexonsmith, hiraditya.
kubamracek 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/D112965
Files:
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
llvm/include/llvm/IR/GlobalObject.h
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
llvm/lib/IR/Metadata.cpp
llvm/lib/IR/Verifier.cpp
llvm/lib/Transforms/IPO/GlobalDCE.cpp
llvm/lib/Transforms/IPO/GlobalSplit.cpp
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
llvm/test/Transforms/GlobalDCE/virtual-functions-non-vfunc-entries.ll
llvm/test/Transforms/GlobalDCE/virtual-functions-ranges.ll
llvm/test/Transforms/GlobalSplit/basic.ll
Index: llvm/test/Transforms/GlobalSplit/basic.ll
===
--- llvm/test/Transforms/GlobalSplit/basic.ll
+++ llvm/test/Transforms/GlobalSplit/basic.ll
@@ -54,7 +54,7 @@
; CHECK: [[T1]] = !{i32 0, !"foo"}
; CHECK: [[T2]] = !{i32 15, !"bar"}
; CHECK: [[T3]] = !{i32 16, !"a"}
-; CHECK: [[VIS]] = !{i64 2}
+; CHECK: [[VIS]] = !{i64 2, i64 0, i64 -1}
; CHECK: [[T4]] = !{i32 1, !"b"}
; CHECK: [[T5]] = !{i32 8, !"c"}
!0 = !{i32 0, !"foo"}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions-ranges.ll
===
--- llvm/test/Transforms/GlobalDCE/virtual-functions-ranges.ll
+++ llvm/test/Transforms/GlobalDCE/virtual-functions-ranges.ll
@@ -64,6 +64,26 @@
; CHECK-SAME: i8* bitcast (void ()* @regular_non_virtual_funcD to i8*)
; CHECK-SAME: }, align 8
+; A vtable that contains multiple ranges.
+@vtableE = internal unnamed_addr constant { [6 x i8*] } { [6 x i8*] [
+ i8* bitcast (void ()* @regular_non_virtual_funcE1 to i8*),
+ i8* bitcast (void ()* @vfunc1_live to i8*),
+ i8* bitcast (void ()* @vfunc2_dead to i8*),
+ i8* bitcast (void ()* @regular_non_virtual_funcE2 to i8*),
+ i8* bitcast (void ()* @vfunc3_live to i8*),
+ i8* bitcast (void ()* @vfunc4_dead to i8*)
+]}, align 8, !type !{i64 8, !"vfunc1.type"}, !type !{i64 16, !"vfunc2.type"}, !type !{i64 32, !"vfunc3.type"}, !type !{i64 40, !"vfunc4.type"},
+ !vcall_visibility !{i64 2, i64 8, i64 24}, !vcall_visibility !{i64 2, i64 32, i64 48}
+
+; CHECK: @vtableE = internal unnamed_addr constant { [6 x i8*] } { [6 x i8*] [
+; CHECK-SAME: i8* bitcast (void ()* @regular_non_virtual_funcE1 to i8*),
+; CHECK-SAME: i8* bitcast (void ()* @vfunc1_live to i8*),
+; CHECK-SAME: i8* null,
+; CHECK-SAME: i8* bitcast (void ()* @regular_non_virtual_funcE2 to i8*),
+; CHECK-SAME: i8* bitcast (void ()* @vfunc3_live to i8*),
+; CHECK-SAME: i8* null
+; CHECK-SAME: ] }, align 8
+
; (1) vfunc1_live is referenced from @main, stays alive
define internal void @vfunc1_live() {
; CHECK: define internal void @vfunc1_live(
@@ -76,6 +96,16 @@
ret void
}
+define internal void @vfunc3_live() {
+ ; CHECK: define internal void @vfunc3_live(
+ ret void
+}
+
+define internal void @vfunc4_dead() {
+ ; CHECK-NOT: define internal void @vfunc4_dead(
+ ret void
+}
+
; (3) not using a range in !vcall_visibility, global gets removed
define internal void @regular_non_virtual_funcA() {
; CHECK-NOT: define internal void @regular_non_virtual_funcA(
@@ -103,12 +133,24 @@
ret void
}
+define internal void @regular_non_virtual_funcE1() {
+ ; CHECK: define internal void @regular_non_virtual_funcE1(
+ ret void
+}
+
+define internal void @regular_non_virtual_funcE2() {
+ ; CHECK: define internal void @regular_non_virtual_funcE2(
+ ret void
+}
+
define void @main() {
%1 = ptrtoint { [3 x i8*] }* @vtableA to i64 ; to keep @vtableA alive
%2 = ptrtoint { [3 x i8*] }* @vtableB to i64 ; to keep @vtableB alive
%3 = ptrtoint { [3 x i8*] }* @vtableC to i64 ; to keep @vtableC alive
%4 = ptrtoint { i32, i32, i8* }* @vtableD to i64 ; to keep @vtableD alive
- %5 = tail call { i8*, i1 } @llvm.type.checked.load(i8* null, i32 0, metadata !"vfunc1.type")
+ %5 = ptrtoint { [6 x i8*] }* @vtableE to i64 ; to keep @vtableE alive
+ %6 = tail call { i8*, i1 } @llvm.type.checked.load(i8* null, i32 0, metadata !"vfunc1.type")
+ %7 = tail call { i8*, i1 } @llvm.type.checked.load(i8* null, i32 0, metadata !"vfunc3.type")
ret void
}
Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -796,16 +796,21 @@
const DenseSet ) {
if (!hasWholeProgramVisibility(WholeProgramVisibilityEnabledInLTO))
return;
- for (GlobalVariable : M.globals())
+ for (GlobalVariable : M.globals()) {
// Add linkage unit visibility to any variable with type metadata, which are
// the vtable
[PATCH] D112929: [Clang] For VFE, emit vtable ranges in !vcall_visibility metadata
kubamracek created this revision.
kubamracek added reviewers: rjmccall, pcc, fhahn.
kubamracek added a project: clang.
kubamracek requested review of this revision.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D112929
Files:
clang/lib/CodeGen/CGVTables.cpp
clang/test/CodeGenCXX/vcall-visibility-metadata-ranges.cpp
clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
Index: clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
===
--- clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
+++ clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
@@ -8,7 +8,7 @@
// Anonymous namespace.
namespace {
-// CHECK: @_ZTVN12_GLOBAL__N_11AE = {{.*}} !vcall_visibility [[VIS_TU:![0-9]+]]
+// CHECK: @_ZTVN12_GLOBAL__N_11AE = {{.*}} !vcall_visibility [[VIS_TU_A:![0-9]+]]
// CHECK-MS: @anon.{{.*}} = private unnamed_addr constant {{.*}}struct.(anonymous namespace)::A{{.*}} !vcall_visibility [[VIS_TU:![0-9]+]]
struct A {
A() {}
@@ -21,7 +21,7 @@
// Hidden visibility.
-// CHECK: @_ZTV1B = {{.*}} !vcall_visibility [[VIS_DSO:![0-9]+]]
+// CHECK: @_ZTV1B = {{.*}} !vcall_visibility [[VIS_DSO_B:![0-9]+]]
// CHECK-MS: @anon.{{.*}} = private unnamed_addr constant {{.*}}struct.B{{.*}} !vcall_visibility [[VIS_DSO:![0-9]+]]
struct __attribute__((visibility("hidden"))) B {
B() {}
@@ -100,8 +100,8 @@
// CHECK-MS-DAG: [[VIS_DSO]] = !{i64 1}
// CHECK-MS-DAG: [[VIS_TU]] = !{i64 2}
-// CHECK-DAG: [[VIS_DSO]] = !{i64 1}
-// CHECK-DAG: [[VIS_TU]] = !{i64 2}
+// CHECK-DAG: [[VIS_DSO_B]] = !{i64 1, i64 16, i64 24}
+// CHECK-DAG: [[VIS_TU_A]] = !{i64 2, i64 16, i64 24}
// CHECK-VFE-DAG: !{i32 1, !"Virtual Function Elim", i32 1}
// CHECK-NOVFE-DAG: !{i32 1, !"Virtual Function Elim", i32 0}
Index: clang/test/CodeGenCXX/vcall-visibility-metadata-ranges.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/vcall-visibility-metadata-ranges.cpp
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm -fvirtual-function-elimination -fwhole-program-vtables -fno-rtti -o - %s | FileCheck %s
+
+//
+// (1) Base class with DSO visibility
+//
+
+class __attribute__((visibility("hidden"))) Base1 {
+ virtual void baseFunc() { }
+};
+void *new_Base1() { return new Base1(); }
+
+// CHECK: @_ZTV5Base1 = {{.*}} constant { [3 x i8*] } { [3 x i8*] [
+// CHECK-SAME: i8* null
+// CHECK-SAME: i8* null
+// CHECK-SAME: i8* bitcast (void (%class.Base1*)* @_ZN5Base18baseFuncEv to i8*)
+// CHECK-SAME: ] }, {{.*}} !vcall_visibility [[VIS_BASE1:![0-9]+]]
+
+
+
+//
+// (2) A subclass with TU visibility, so the vtable should have one part with DSO visibility (from the superclass) and one part with TU visibility
+//
+
+namespace {
+class Sub : public Base1 {
+ virtual void baseFunc() { }
+ virtual void subOnlyFunc() { }
+};
+}
+void *new_Sub() { return new Sub(); }
+
+// CHECK: @_ZTVN12_GLOBAL__N_13SubE = {{.*}} constant { [4 x i8*] } { [4 x i8*] [
+// CHECK-SAME: i8* null
+// CHECK-SAME: i8* null
+// CHECK-SAME: i8* bitcast (void (%"class.(anonymous namespace)::Sub"*)* @_ZN12_GLOBAL__N_13Sub8baseFuncEv to i8*)
+// CHECK-SAME: i8* bitcast (void (%"class.(anonymous namespace)::Sub"*)* @_ZN12_GLOBAL__N_13Sub11subOnlyFuncEv to i8*)
+// CHECK-SAME: ] }, {{.*}}, !vcall_visibility [[VIS_SUB:![0-9]+]]
+
+
+
+//
+// (3) A subclass with multiple inheritance
+//
+
+namespace {
+struct __attribute__((visibility("hidden"))) Base2 {
+ virtual void secondBaseFunc() { }
+};
+}
+void *new_Base2() { return new Base2(); }
+
+namespace {
+class MultipleInheritanceSub : public Base1, public Base2 {
+ virtual void baseFunc() { }
+ virtual void secondBaseFunc() { }
+ virtual void anotherSubOnlyFunc() { }
+};
+}
+void *new_MultipleInheritanceSub() { return new MultipleInheritanceSub(); }
+
+// CHECK: @_ZTVN12_GLOBAL__N_122MultipleInheritanceSubE = {{.*}} constant { [5 x i8*], [3 x i8*] } {
+// CHECK-SAME: [5 x i8*] [
+// CHECK-SAME: i8* null,
+// CHECK-SAME i8* null,
+// CHECK-SAME: i8* bitcast (void (%"class.(anonymous namespace)::MultipleInheritanceSub"*)* @_ZN12_GLOBAL__N_122MultipleInheritanceSub8baseFuncEv to i8*),
+// CHECK-SAME: i8* bitcast (void (%"class.(anonymous namespace)::MultipleInheritanceSub"*)* @_ZN12_GLOBAL__N_122MultipleInheritanceSub14secondBaseFuncEv to i8*),
+// CHECK-SAME: i8* bitcast (void (%"class.(anonymous namespace)::MultipleInheritanceSub"*)* @_ZN12_GLOBAL__N_122MultipleInheritanceSub18anotherSubOnlyFuncEv to i8*)
+// CHECK-SAME: ],
+// CHECK-SAME: [3 x i8*] [
+// CHECK-SAME: i8* inttoptr (i64 -8 to i8*),
+// CHECK-SAME: i8* null,
+// CHECK-SAME: i8* bitcast (void (%"class.(anonymous namespace)::MultipleInheritanceSub"*)* @_ZThn8_N12_GLOBAL__N_122MultipleInheritanceSub14secondBaseFuncEv to i8*)
+// CHECK-SAME: ]
+// CHECK-SAME: }, {{.*}}, !vcall_visibility [[VIS_MUL_BASE1:![0-9]+]],
[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.
kubamracek accepted this revision. kubamracek added a comment. Looks good! By the way, Apple Clang releases also build compiler-rt this way, so it would be worth checking with @arphaman that he's okay with the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98291/new/ https://reviews.llvm.org/D98291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.
kubamracek added a comment. When using Ninja, does this mean running a null build is no longer a null build, but it at least always invokes this sub-build? Not saying that's bad, just want to understand the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98291/new/ https://reviews.llvm.org/D98291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84082: [tsan] Allow TSan in the Clang driver for Apple Silicon Macs
This revision was automatically updated to reflect the committed changes.
Closed by commit rG33d9c4109ac2: [tsan] Allow TSan in the Clang driver for
Apple Silicon Macs (authored by kubamracek).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84082/new/
https://reviews.llvm.org/D84082
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/fsanitize.c
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -458,6 +458,10 @@
// RUN: %clang -target x86_64-apple-darwin -fsanitize=thread %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-TSAN-X86-64-DARWIN
// CHECK-TSAN-X86-64-DARWIN-NOT: unsupported option
+// RUN: %clang -target x86_64-apple-macos -fsanitize=thread %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-TSAN-X86-64-MACOS
+// CHECK-TSAN-X86-64-MACOS-NOT: unsupported option
+// RUN: %clang -target arm64-apple-macos -fsanitize=thread %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-TSAN-ARM64-MACOS
+// CHECK-TSAN-ARM64-MACOS-NOT: unsupported option
// RUN: %clang -target x86_64-apple-iossimulator -fsanitize=thread %s -###
2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-IOSSIMULATOR
// CHECK-TSAN-X86-64-IOSSIMULATOR-NOT: unsupported option
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2714,6 +2714,7 @@
SanitizerMask Darwin::getSupportedSanitizers() const {
const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64;
+ const bool IsAArch64 = getTriple().getArch() == llvm::Triple::aarch64;
SanitizerMask Res = ToolChain::getSupportedSanitizers();
Res |= SanitizerKind::Address;
Res |= SanitizerKind::PointerCompare;
@@ -2731,9 +2732,8 @@
&& !(isTargetIPhoneOS() && isIPhoneOSVersionLT(5, 0)))
Res |= SanitizerKind::Vptr;
- if (isTargetMacOS()) {
-if (IsX86_64)
- Res |= SanitizerKind::Thread;
+ if ((IsX86_64 || IsAArch64) && isTargetMacOS()) {
+Res |= SanitizerKind::Thread;
} else if (isTargetIOSSimulator() || isTargetTvOSSimulator()) {
if (IsX86_64)
Res |= SanitizerKind::Thread;
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -458,6 +458,10 @@
// RUN: %clang -target x86_64-apple-darwin -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-DARWIN
// CHECK-TSAN-X86-64-DARWIN-NOT: unsupported option
+// RUN: %clang -target x86_64-apple-macos -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-MACOS
+// CHECK-TSAN-X86-64-MACOS-NOT: unsupported option
+// RUN: %clang -target arm64-apple-macos -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-ARM64-MACOS
+// CHECK-TSAN-ARM64-MACOS-NOT: unsupported option
// RUN: %clang -target x86_64-apple-iossimulator -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-IOSSIMULATOR
// CHECK-TSAN-X86-64-IOSSIMULATOR-NOT: unsupported option
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2714,6 +2714,7 @@
SanitizerMask Darwin::getSupportedSanitizers() const {
const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64;
+ const bool IsAArch64 = getTriple().getArch() == llvm::Triple::aarch64;
SanitizerMask Res = ToolChain::getSupportedSanitizers();
Res |= SanitizerKind::Address;
Res |= SanitizerKind::PointerCompare;
@@ -2731,9 +2732,8 @@
&& !(isTargetIPhoneOS() && isIPhoneOSVersionLT(5, 0)))
Res |= SanitizerKind::Vptr;
- if (isTargetMacOS()) {
-if (IsX86_64)
- Res |= SanitizerKind::Thread;
+ if ((IsX86_64 || IsAArch64) && isTargetMacOS()) {
+Res |= SanitizerKind::Thread;
} else if (isTargetIOSSimulator() || isTargetTvOSSimulator()) {
if (IsX86_64)
Res |= SanitizerKind::Thread;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84082: [tsan] Allow TSan in the Clang driver for Apple Silicon Macs
kubamracek added a comment. Let's handle simulators separately. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84082/new/ https://reviews.llvm.org/D84082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84082: [tsan] Allow TSan in the Clang driver for Apple Silicon Macs
kubamracek created this revision.
kubamracek added reviewers: dcoughlin, delcypher, yln, arphaman, steven_wu.
kubamracek added a project: Sanitizers.
Herald added subscribers: cfe-commits, Charusso, dexonsmith.
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84082
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/fsanitize.c
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -458,6 +458,10 @@
// RUN: %clang -target x86_64-apple-darwin -fsanitize=thread %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-TSAN-X86-64-DARWIN
// CHECK-TSAN-X86-64-DARWIN-NOT: unsupported option
+// RUN: %clang -target x86_64-apple-macos -fsanitize=thread %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-TSAN-X86-64-MACOS
+// CHECK-TSAN-X86-64-MACOS-NOT: unsupported option
+// RUN: %clang -target arm64-apple-macos -fsanitize=thread %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-TSAN-ARM64-MACOS
+// CHECK-TSAN-ARM64-MACOS-NOT: unsupported option
// RUN: %clang -target x86_64-apple-iossimulator -fsanitize=thread %s -###
2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-IOSSIMULATOR
// CHECK-TSAN-X86-64-IOSSIMULATOR-NOT: unsupported option
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2713,6 +2713,7 @@
SanitizerMask Darwin::getSupportedSanitizers() const {
const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64;
+ const bool IsAArch64 = getTriple().getArch() == llvm::Triple::aarch64;
SanitizerMask Res = ToolChain::getSupportedSanitizers();
Res |= SanitizerKind::Address;
Res |= SanitizerKind::PointerCompare;
@@ -2730,9 +2731,8 @@
&& !(isTargetIPhoneOS() && isIPhoneOSVersionLT(5, 0)))
Res |= SanitizerKind::Vptr;
- if (isTargetMacOS()) {
-if (IsX86_64)
- Res |= SanitizerKind::Thread;
+ if ((IsX86_64 || IsAArch64) && isTargetMacOS()) {
+Res |= SanitizerKind::Thread;
} else if (isTargetIOSSimulator() || isTargetTvOSSimulator()) {
if (IsX86_64)
Res |= SanitizerKind::Thread;
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -458,6 +458,10 @@
// RUN: %clang -target x86_64-apple-darwin -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-DARWIN
// CHECK-TSAN-X86-64-DARWIN-NOT: unsupported option
+// RUN: %clang -target x86_64-apple-macos -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-MACOS
+// CHECK-TSAN-X86-64-MACOS-NOT: unsupported option
+// RUN: %clang -target arm64-apple-macos -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-ARM64-MACOS
+// CHECK-TSAN-ARM64-MACOS-NOT: unsupported option
// RUN: %clang -target x86_64-apple-iossimulator -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-X86-64-IOSSIMULATOR
// CHECK-TSAN-X86-64-IOSSIMULATOR-NOT: unsupported option
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2713,6 +2713,7 @@
SanitizerMask Darwin::getSupportedSanitizers() const {
const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64;
+ const bool IsAArch64 = getTriple().getArch() == llvm::Triple::aarch64;
SanitizerMask Res = ToolChain::getSupportedSanitizers();
Res |= SanitizerKind::Address;
Res |= SanitizerKind::PointerCompare;
@@ -2730,9 +2731,8 @@
&& !(isTargetIPhoneOS() && isIPhoneOSVersionLT(5, 0)))
Res |= SanitizerKind::Vptr;
- if (isTargetMacOS()) {
-if (IsX86_64)
- Res |= SanitizerKind::Thread;
+ if ((IsX86_64 || IsAArch64) && isTargetMacOS()) {
+Res |= SanitizerKind::Thread;
} else if (isTargetIOSSimulator() || isTargetTvOSSimulator()) {
if (IsX86_64)
Res |= SanitizerKind::Thread;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50724: SafeStack: Disable Darwin support
kubamracek added a comment. Doesn't this need any tests to be updated? Otherwise, LGTM. Repository: rC Clang https://reviews.llvm.org/D50724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence
kubamracek added a comment. This looks great to me, but someone else should review this as well. https://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46363: Follow-up to r331378. Update tests to allow to use C atomics in C++.
kubamracek accepted this revision. kubamracek added a comment. This revision is now accepted and ready to land. https://reviews.llvm.org/D46363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43787: Fix which Darwin versions have ObjC runtime with full subscripting support.
kubamracek added a comment. Great! So the versions in `ObjCRuntime.h` didn't really match what the iOS and macOS supported? https://reviews.llvm.org/D43787 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
kubamracek added a comment. > This is our first patch so we're unfamiliar with the LLVM testing > infrastructure. Could you please tell us what kind of test you'd like? An > example would also be great. Thank you for your first contribution! I'm going to comment on the testing infrastructure only, as I don't know the answer to the portability/mangling/ABI question. Most user-visible features of ASan can usually be demonstrated in a single-file test that is completely standalone test program. E.g. ASan's ability to detect heap buffer overflows is demonstrated in `test/asan/TestCases/heap-overflow.cc` file. Notice that these tests don't include or reference any sanitizer internal functions. They are meant to be written in a way that user's code is written. Can you try adding such a test, which would be a standalone .cc file that would demonstrate the problematic scenario (C++ exception rethrow) without including ASan headers? The test's expectation should be that ASan doesn't report any issues, whereas without your patch, the test fails (or might fail). For an example of a negative test (that checks that ASan *doesn't* report any issues), see `test/asan/TestCases/Darwin/nil-return-struct.mm`. If the test should work on all ASan supported platforms, put it directly into `test/asan/TestCases`. If it's POSIX-only, place it under `Posix/` subdirectory. Repository: rCRT Compiler Runtime https://reviews.llvm.org/D42644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42644: [asan] Intercept std::rethrow_exception indirectly.
kubamracek added a comment. Cool. Can we get a lit test as well? Repository: rCRT Compiler Runtime https://reviews.llvm.org/D42644 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay][darwin] Initial XRay in Darwin Support
kubamracek added a comment. And we should probably introduce files named `xray_linux.cc` and `xray_mac.cc` to contain platform-specific functions. https://reviews.llvm.org/D39114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay][darwin] Initial XRay in Darwin Support
kubamracek added a comment. Hi, can you split the patch and send individual changes as separate patches? It's getting hard to follow. And I think we should land the parts that are "safe to land", e.g. the ASM changes or some of the NFC whitespace changes. The Darwin-specific changes in tests can also be landed (but don't enable the tests on Darwin, yet). https://reviews.llvm.org/D39114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay][darwin] Initial XRay in Darwin Support
kubamracek added a comment. Can we just not use clock_gettime on Darwin and instead use mach_absolute_time? Repository: rL LLVM https://reviews.llvm.org/D39114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay][darwin] Initial XRay in Darwin Support
kubamracek added inline comments. Comment at: compiler-rt/trunk/lib/xray/xray_trampoline_x86_64.S:75 - .globl __xray_FunctionEntry + .globl ASM_TSAN_SYMBOL(__xray_FunctionEntry) .align 16, 0x90 Since we want to use `ASM_TSAN_SYMBOL` outside of TSan, can we rename it to just `ASM_SYMBOL` (or something like that)? Repository: rL LLVM https://reviews.llvm.org/D39114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay] Initial XRay in Darwin Support
kubamracek added a comment.
> When linking the final binary, the XRay runtime can't seem to find the
> `__{start,stop}_xray_{fn_idx,instr_map}` symbols. I've marked them weak, but
> they seem to either not be resolved when statically linking the binary. The
> dynamic lib version of the XRay runtime isn't quite ready yet, but I'm
> willing to try some things there.
> There are also missing symbols from the sanitizer_common library that's
> linked to from the `libclang_rt.xray_osx.a` produced by the compiler-rt XRay
> build configuration.
> Any further hints/thoughts on this would be great, @kubamracek (cc @pelikan
> and @echristo)
This is one of the reasons why a dynamic shared library works better. Using a
dynamic library should solve this. What are the problems with that?
https://reviews.llvm.org/D39114
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay] Initial XRay in Darwin Support
kubamracek added a comment. Thanks for working on this! Can we split the patch and land parts that are LGTM? It's getting a bit crowded here. I think the ASM changes should be a separate patch, and the test changes as well, probably. https://reviews.llvm.org/D39114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay] Initial XRay in Darwin Support
kubamracek added a comment. Also, please make sure this builds and tests pass before actually landing this patch. FYI, there are bots that run on older macOS versions (10.11 for sure, probably even older). https://reviews.llvm.org/D39114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39114: [XRay] Initial XRay in Darwin Support
kubamracek accepted this revision.
kubamracek added a comment.
This revision is now accepted and ready to land.
Hi and sorry for replying so late! All of the changes LGTM with a few nits.
> Assembly for Darwin x86_64 and how we avoid the ELF'isms (do we need to
> implement darwin-specific assembly for MachO?)
This should be actually easily solvable by `#include`'ing
sanitizer_common/sanitizer_asm.h and then using the macros from there (perhaps
introducing some new macros or generalizing the existing ones). Take a look at
what was done in lib/tsan/rtl/tsan_rtl_amd64.S. Names of symbols should use a
macro like `ASM_SYMBOL` which will add an underscore on macOS. `.type`
directives should use `ASM_TYPE_FUNCTION`. And so on. It should be possible to
reuse almost all of the assembly instructions unchanged. I think I have an ugly
old patch for this somewhere, I can send it to you if you want.
> Syscalls, testing, etc. -- whether we need to do anything special here for
> making it work on macOS.
We should try to move most of the tests in the Linux directory to a common
tests directory. I don't see anything particularly Linux-y there.
Regarding syscalls, I'm not sure I understand the problem. Is XRay doing any
syscall interception or something like that?
Comment at: compiler-rt/cmake/config-ix.cmake:419-425
+ # For XRay, only support x86_64 in APPLE.
+ # list_intersect(XRAY_SUPPORTED_ARCH
+ # ALL_XRAY_SUPPORTED_ARCH
+ # SANITIZER_COMMON_SUPPORTED_ARCH)
+ set(XRAY_SUPPORTED_ARCH ${X86_64})
+
+
Instead of this change, please do something similar like we do for
`ALL_LSAN_SUPPORTED_ARCH` above (line 200).
Comment at: compiler-rt/lib/xray/CMakeLists.txt:82
+ add_compiler_rt_runtime(clang_rt.xray
+STATIC
+OS osx
`STATIC` is okay for now, but we have much better experience with dynamic
libraries for runtimes. Is there some reason to use a static library?
Comment at: compiler-rt/test/xray/TestCases/Darwin/always-never-instrument.cc:12
+// RUN:FileCheck %s --check-prefix ALWAYSINSTR
+// REQUIRES: x86_64-linux
+// REQUIRES: built-in-llvm-tree
Should this `REQUIRES` be here?
Similarly, in all the tests in the `Linux/` directory, we should change
`REQUIRES: x86_64-linux` to just `REQUIRES: x86_64`.
https://reviews.llvm.org/D39114
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36437: [clang] Get rid of "%T" expansions
kubamracek created this revision.
kubamracek added a project: clang.
Herald added subscribers: mehdi_amini, klimek.
The `%T` lit expansion expands to a common directory shared between all the
tests in the same directory, which is unexpected and unintuitive, and more
importantly, it's been a source of subtle race conditions and flaky tests. In
https://reviews.llvm.org/D35396, it was agreed that it would be best to simply
ban `%T` and only keep `%t`, which is unique to each test. When a test needs a
temporary directory, it can just create one using `mkdir %t`.
This patch removes `%T` in clang.
Repository:
rL LLVM
https://reviews.llvm.org/D36437
Files:
test/Analysis/html-diags.c
test/CoverageMapping/abspath.cpp
test/Driver/compilation_database.c
test/Driver/cpath.c
test/Driver/darwin-ld-lto.c
test/Driver/linker-opts.c
test/Driver/output-file-cleanup.c
test/Driver/parse-progname.c
test/Driver/ps4-linker-non-win.c
test/Driver/ps4-linker-win.c
test/Driver/warning-options.cpp
test/FixIt/fixit-include.c
test/Format/style-on-command-line.cpp
test/Lexer/case-insensitive-include-ms.c
test/Lexer/case-insensitive-include-pr31836.sh
test/Lexer/case-insensitive-include.c
test/Lexer/case-insensitive-system-include.c
test/Modules/crash-typo-correction-visibility.cpp
test/Modules/modules-cache-path-canonicalization.m
test/PCH/case-insensitive-include.c
test/PCH/include-timestamp.cpp
test/Preprocessor/cuda-types.cu
test/Tooling/clang-diff-basic.cpp
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -E %s > %T/src.cpp
-// RUN: %clang_cc1 -E %s > %T/dst.cpp -DDEST
-// RUN: clang-diff -no-compilation-database %T/src.cpp %T/dst.cpp | FileCheck %s
+// RUN: mkdir -p %t
+// RUN: %clang_cc1 -E %s > %t/src.cpp
+// RUN: %clang_cc1 -E %s > %t/dst.cpp -DDEST
+// RUN: clang-diff -no-compilation-database %t/src.cpp %t/dst.cpp | FileCheck %s
#ifndef DEST
namespace src {
Index: test/Preprocessor/cuda-types.cu
===
--- test/Preprocessor/cuda-types.cu
+++ test/Preprocessor/cuda-types.cu
@@ -5,42 +5,44 @@
// FIXME: We really should make __GCC_HAVE_SYNC_COMPARE_AND_SWAP identical on
// host and device, but architecturally this is difficult at the moment.
+// RUN: mkdir -p %t
+
// RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
// RUN: | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define __GCC_ATOMIC' \
-// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %T/i386-host-defines-filtered
+// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
// RUN: %clang --cuda-device-only -nocudainc -nocudalib -target i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
// RUN: | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define __GCC_ATOMIC' \
-// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %T/i386-device-defines-filtered
-// RUN: diff %T/i386-host-defines-filtered %T/i386-device-defines-filtered
+// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
// RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
// RUN: | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define __GCC_ATOMIC' \
-// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %T/x86_64-host-defines-filtered
+// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
// RUN: %clang --cuda-device-only -nocudainc -nocudalib -target x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
// RUN: | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define __GCC_ATOMIC' \
-// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %T/x86_64-device-defines-filtered
-// RUN: diff %T/x86_64-host-defines-filtered %T/x86_64-device-defines-filtered
+// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
+// RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
// RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
// RUN: | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define __GCC_ATOMIC' \
-// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %T/powerpc64-host-defines-filtered
+// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
// RUN: %clang --cuda-device-only -nocudainc -nocudalib -target powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
// RUN: | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define __GCC_ATOMIC' \
-// RUN: | grep -v '__LDBL\|_LONG_DOUBLE' > %T/powerpc64-device-defines-filtered
-// RUN: diff %T/powerpc64-host-defines-filtered %T/powerpc64-device-defines-filtered
+// RUN: | grep -v
[PATCH] D34175: [driver][macOS] Pick the system version for the deployment target if the SDK is newer than the system
kubamracek added a comment. Nice! Repository: rL LLVM https://reviews.llvm.org/D34175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32029: [ObjC] Fix lifetime markers of loop variable in EmitObjCForCollectionStmt
kubamracek updated this revision to Diff 95271.
kubamracek added a comment.
Trying a different approach: Keeping the loop variable alive for the whole
loop by extending ForScope and registering the cleanup function inside
EmitAutoVarAlloca.
https://reviews.llvm.org/D32029
Files:
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGObjC.cpp
test/CodeGenObjC/arc-blocks.m
test/CodeGenObjCXX/arc-references.mm
Index: test/CodeGenObjCXX/arc-references.mm
===
--- test/CodeGenObjCXX/arc-references.mm
+++ test/CodeGenObjCXX/arc-references.mm
@@ -21,7 +21,7 @@
// CHECK: call void @_Z6calleev
callee();
// CHECK: call void @objc_release
- // CHECK-NEXT: ret
+ // CHECK: ret
}
// No lifetime extension when we're binding a reference to an lvalue.
@@ -44,9 +44,9 @@
const __weak id = strong_id();
// CHECK-NEXT: call void @_Z6calleev()
callee();
+ // CHECK-NEXT: call void @objc_destroyWeak
// CHECK-NEXT: [[PTR:%.*]] = bitcast i8*** [[REF]] to i8*
// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[PTR]])
- // CHECK-NEXT: call void @objc_destroyWeak
// CHECK-NEXT: ret void
}
Index: test/CodeGenObjC/arc-blocks.m
===
--- test/CodeGenObjC/arc-blocks.m
+++ test/CodeGenObjC/arc-blocks.m
@@ -532,16 +532,16 @@
// CHECK-NEXT: [[T0:%.*]] = load void ()*, void ()** [[B]]
// CHECK-NEXT: [[T1:%.*]] = bitcast void ()* [[T0]] to i8*
// CHECK-NEXT: call void @objc_release(i8* [[T1]])
- // CHECK-NEXT: [[BPTR2:%.*]] = bitcast void ()** [[B]] to i8*
- // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[BPTR2]])
// CHECK-NEXT: [[T0:%.*]] = load i1, i1* [[CLEANUP_ACTIVE]]
// CHECK-NEXT: br i1 [[T0]]
// CHECK: [[T0:%.*]] = load i8*, i8** [[CLEANUP_ADDR]]
// CHECK-NEXT: call void @objc_release(i8* [[T0]])
// CHECK-NEXT: br label
- // CHECK: [[T0:%.*]] = load i8*, i8** [[X]]
+ // CHECK: [[BPTR2:%.*]] = bitcast void ()** [[B]] to i8*
+ // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[BPTR2]])
+ // CHECK-NEXT: [[T0:%.*]] = load i8*, i8** [[X]]
// CHECK-NEXT: call void @objc_release(i8* [[T0]])
// CHECK-NEXT: ret void
}
Index: lib/CodeGen/CGObjC.cpp
===
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -1469,6 +1469,8 @@
if (DI)
DI->EmitLexicalBlockStart(Builder, S.getSourceRange().getBegin());
+ RunCleanupsScope ForScope(*this);
+
// The local variable comes into scope immediately.
AutoVarEmission variable = AutoVarEmission::invalid();
if (const DeclStmt *SD = dyn_cast(S.getElement()))
@@ -1499,8 +1501,6 @@
ArrayType::Normal, 0);
Address ItemsPtr = CreateMemTemp(ItemsTy, "items.ptr");
- RunCleanupsScope ForScope(*this);
-
// Emit the collection pointer. In ARC, we do a retain.
llvm::Value *Collection;
if (getLangOpts().ObjCAutoRefCount) {
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1118,6 +1118,12 @@
if (D.hasAttr())
EmitVarAnnotations(, address.getPointer());
+ // Make sure we call @llvm.lifetime.end.
+ if (emission.useLifetimeMarkers())
+EHStack.pushCleanup(NormalEHLifetimeMarker,
+ emission.getAllocatedAddress(),
+ emission.getSizeForLifetimeMarkers());
+
return emission;
}
@@ -1408,13 +1414,6 @@
const VarDecl = *emission.Variable;
- // Make sure we call @llvm.lifetime.end. This needs to happen
- // *last*, so the cleanup needs to be pushed *first*.
- if (emission.useLifetimeMarkers())
-EHStack.pushCleanup(NormalEHLifetimeMarker,
- emission.getAllocatedAddress(),
- emission.getSizeForLifetimeMarkers());
-
// Check the type for a cleanup.
if (QualType::DestructionKind dtorKind = D.getType().isDestructedType())
emitAutoVarTypeCleanup(emission, dtorKind);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32029: [ObjC] Fix lifetime markers of loop variable in EmitObjCForCollectionStmt
kubamracek reopened this revision.
kubamracek added a comment.
This revision is now accepted and ready to land.
Reverted because this fails for-in.m by crashing the compiler when compiling:
void t2(NSArray *array) {
for (NSArray *array in array) { // expected-warning {{collection expression
type 'NSArray *' may not respond}}
}
}
Repository:
rL LLVM
https://reviews.llvm.org/D32029
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32029: [ObjC] Fix lifetime markers of loop variable in EmitObjCForCollectionStmt
kubamracek added a comment. Note that C++ foreach loops also generate lifetime.start and lifetime.end inside of the loop body. Repository: rL LLVM https://reviews.llvm.org/D32029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32029: [ObjC] Fix lifetime markers of loop variable in EmitObjCForCollectionStmt
kubamracek created this revision.
kubamracek added a project: Sanitizers.
CodeGenFunction::EmitObjCForCollectionStmt currently emits lifetime markers for
the loop variable in an inconsistent way: `lifetime.start` is emitted before
the loop is entered, but `lifetime.end` is emitted inside the loop:
; entry block
%u = alloca %1*, align 8
%1 = bitcast %1** %u to i8*
call void @llvm.lifetime.start(i64 8, i8* %1) #5
...
; loop body
...
%14 = bitcast %1** %u to i8*
call void @llvm.lifetime.end(i64 8, i8* %14) #5
...
br i1 %21, ... ; loop
; loop ended
ret void
AddressSanitizer uses these markers to track out-of-scope accesses to local
variables, and we get false positives in Obj-C foreach loops (in the 2nd
iteration of the loop). The markers of the loop variable need to be either
both inside the loop (so that we poison and unpoison the variable in each
iteration), or both outside. This patch implements the "both inside" approach
and makes EmitObjCForCollectionStmt emit:
; entry block
%u = alloca %1*, align 8
...
; loop body
%12 = bitcast %1** %u to i8*
call void @llvm.lifetime.start(i64 8, i8* %12) #5
...
%14 = bitcast %1** %u to i8*
call void @llvm.lifetime.end(i64 8, i8* %14) #5
br label %15
; loop ended
ret void
The test fixups are only changing the order of allocas. There's some related
discussion at https://reviews.llvm.org/D18618.
Repository:
rL LLVM
https://reviews.llvm.org/D32029
Files:
lib/CodeGen/CGObjC.cpp
test/CodeGenObjC/arc-foreach.m
test/CodeGenObjC/arc-ternary-op.m
Index: test/CodeGenObjC/arc-ternary-op.m
===
--- test/CodeGenObjC/arc-ternary-op.m
+++ test/CodeGenObjC/arc-ternary-op.m
@@ -120,9 +120,9 @@
// CHECK-LABEL:define void @test2(
// CHECK: [[COND:%.*]] = alloca i32,
- // CHECK: alloca i8*
// CHECK: [[CLEANUP_SAVE:%.*]] = alloca i8*
// CHECK: [[RUN_CLEANUP:%.*]] = alloca i1
+ // CHECK: alloca i8*
// Evaluate condition; cleanup disabled by default.
// CHECK: [[T0:%.*]] = load i32, i32* [[COND]],
// CHECK-NEXT: icmp ne i32 [[T0]], 0
Index: test/CodeGenObjC/arc-foreach.m
===
--- test/CodeGenObjC/arc-foreach.m
+++ test/CodeGenObjC/arc-foreach.m
@@ -24,9 +24,9 @@
// CHECK-LP64-LABEL:define void @test0(
// CHECK-LP64: [[ARRAY:%.*]] = alloca [[ARRAY_T:%.*]]*,
-// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
// CHECK-LP64-NEXT: [[STATE:%.*]] = alloca [[STATE_T:%.*]],
// CHECK-LP64-NEXT: [[BUFFER:%.*]] = alloca [16 x i8*], align 8
+// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
// CHECK-LP64-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
// Initialize 'array'.
@@ -97,9 +97,9 @@
// CHECK-LP64-LABEL:define void @test1(
// CHECK-LP64: alloca [[ARRAY_T:%.*]]*,
-// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
// CHECK-LP64-NEXT: [[STATE:%.*]] = alloca [[STATE_T:%.*]],
// CHECK-LP64-NEXT: alloca [16 x i8*], align 8
+// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
// CHECK-LP64-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
// CHECK-LP64: [[T0:%.*]] = getelementptr inbounds [[STATE_T]],
[[STATE_T]]* [[STATE]], i32 0, i32 1
@@ -160,7 +160,7 @@
// CHECK-LP64-LABEL:define void @test3(
// CHECK-LP64: [[ARRAY:%.*]] = alloca [[ARRAY_T]]*, align 8
- // CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*, align 8
+ // CHECK-LP64: [[X:%.*]] = alloca i8*, align 8
// CHECK-LP64: [[T0:%.*]] = load i8*, i8** [[X]], align 8
// CHECK-LP64-NEXT: [[T1:%.*]] = icmp ne i8* [[T0]], null
// CHECK-LP64-NEXT: br i1 [[T1]],
Index: lib/CodeGen/CGObjC.cpp
===
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -1469,11 +1469,6 @@
if (DI)
DI->EmitLexicalBlockStart(Builder, S.getSourceRange().getBegin());
- // The local variable comes into scope immediately.
- AutoVarEmission variable = AutoVarEmission::invalid();
- if (const DeclStmt *SD = dyn_cast(S.getElement()))
-variable = EmitAutoVarAlloca(*cast(SD->getSingleDecl()));
-
JumpDest LoopEnd = getJumpDestInCurrentScope("forcoll.end");
// Fast enumeration state.
@@ -1625,8 +1620,10 @@
bool elementIsVariable;
LValue elementLValue;
QualType elementType;
+ AutoVarEmission variable = AutoVarEmission::invalid();
if (const DeclStmt *SD = dyn_cast(S.getElement())) {
// Initialize the variable, in case it's a __block variable or something.
+variable = EmitAutoVarAlloca(*cast(SD->getSingleDecl()));
EmitAutoVarInit(variable);
const VarDecl* D = cast(SD->getSingleDecl());
Index: test/CodeGenObjC/arc-ternary-op.m
===
--- test/CodeGenObjC/arc-ternary-op.m
+++ test/CodeGenObjC/arc-ternary-op.m
@@ -120,9 +120,9 @@
// CHECK-LABEL:
[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.
kubabrecka added inline comments.
Comment at: libcxx/include/memory:3702
+inline T
+__libcpp_atomic_increment(T& t) _NOEXCEPT
+{
I don't think this should be named `__libcpp_atomic_increment`, because it uses
relaxed ordering and thus it's not a generic increment (same goes for
decrement). Could we rename this to `__libcpp_atomic_refcount_increment` or
something similar? That would suggest why we're using acq+rel on one side and
relaxed on other side. Using these functions for non-refcount purposes will be
wrong and the current names (`__libcpp_atomic_increment`) suggest that they're
doing generic atomic operations.
Repository:
rL LLVM
https://reviews.llvm.org/D24991
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
