[PATCH] D38908: Do not link clang_rt.cfi on Android.

2017-10-16 Thread Evgenii Stepanov via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315921: Do not link clang_rt.cfi on Android. (authored by 
eugenis).

Changed prior to commit:
  https://reviews.llvm.org/D38908?vs=118986=119180#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38908

Files:
  cfe/trunk/include/clang/Driver/SanitizerArgs.h
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/test/Driver/sanitizer-ld.c

Index: cfe/trunk/include/clang/Driver/SanitizerArgs.h
===
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h
@@ -44,6 +44,8 @@
   bool TsanFuncEntryExit = true;
   bool TsanAtomics = true;
   bool MinimalRuntime = false;
+  // True if cross-dso CFI support if provided by the system (i.e. Android).
+  bool ImplicitCfiRuntime = false;
 
  public:
   /// Parses the sanitizer arguments from an argument list.
Index: cfe/trunk/test/Driver/sanitizer-ld.c
===
--- cfe/trunk/test/Driver/sanitizer-ld.c
+++ cfe/trunk/test/Driver/sanitizer-ld.c
@@ -508,6 +508,24 @@
 // CHECK-CFI-CROSS-DSO-DIAG-LINUX: "-whole-archive" "{{[^"]*}}libclang_rt.cfi_diag-x86_64.a" "-no-whole-archive"
 // CHECK-CFI-CROSS-DSO-DIAG-LINUX: -export-dynamic
 
+// Cross-DSO CFI on Android does not link runtime libraries.
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso %s -### -o %t.o 2>&1 \
+// RUN: -target aarch64-linux-android -fuse-ld=ld \
+// RUN: --sysroot=%S/Inputs/basic_android_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CFI-CROSS-DSO-ANDROID %s
+// CHECK-CFI-CROSS-DSO-ANDROID: "{{.*}}ld{{(.exe)?}}"
+// CHECK-CFI-CROSS-DSO-ANDROID-NOT: libclang_rt.
+
+// Cross-DSO CFI with diagnostics on Android links just the UBSAN runtime.
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso %s -### -o %t.o 2>&1 \
+// RUN: -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
+// RUN: -target aarch64-linux-android -fuse-ld=ld \
+// RUN: --sysroot=%S/Inputs/basic_android_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CFI-CROSS-DSO-DIAG-ANDROID %s
+// CHECK-CFI-CROSS-DSO-DIAG-ANDROID: "{{.*}}ld{{(.exe)?}}"
+// CHECK-CFI-CROSS-DSO-DIAG-ANDROID: "{{[^"]*}}libclang_rt.ubsan_standalone-aarch64-android.so"
+// CHECK-CFI-CROSS-DSO-DIAG-ANDROID: "-export-dynamic-symbol=__cfi_check"
+
 // RUN: %clangxx -fsanitize=address %s -### -o %t.o 2>&1 \
 // RUN: -mmacosx-version-min=10.6 \
 // RUN: -target x86_64-apple-darwin13.4.0 -fuse-ld=ld -stdlib=platform \
@@ -596,26 +614,6 @@
 // CHECK-SAFESTACK-ANDROID-AARCH64: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-SAFESTACK-ANDROID-AARCH64-NOT: libclang_rt.safestack
 
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=cfi \
-// RUN: --sysroot=%S/Inputs/basic_android_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-CFI-ANDROID %s
-//
-// CHECK-CFI-ANDROID: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-CFI-ANDROID-NOT: libclang_rt.cfi
-// CHECK-CFI-ANDROID-NOT: __cfi_check
-
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=cfi \
-// RUN: -fsanitize-cfi-cross-dso \
-// RUN: --sysroot=%S/Inputs/basic_android_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-CROSSDSO-CFI-ANDROID %s
-//
-// CHECK-CROSSDSO-CFI-ANDROID: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-CROSSDSO-CFI-ANDROID-NOT: libclang_rt.cfi
-// CHECK-CROSSDSO-CFI-ANDROID: -export-dynamic-symbol=__cfi_check
-// CHECK-CROSSDSO-CFI-ANDROID-NOT: libclang_rt.cfi
-
 // RUN: %clang -fsanitize=undefined %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
 // RUN: -shared \
Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -171,19 +171,23 @@
 }
 
 bool SanitizerArgs::needsUbsanRt() const {
-  return ((Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||
-  CoverageFeatures) &&
- !Sanitizers.has(Address) && !Sanitizers.has(Memory) &&
- !Sanitizers.has(Thread) && !Sanitizers.has(DataFlow) &&
- !Sanitizers.has(Leak) && !CfiCrossDso;
+  // All of these include ubsan.
+  if (needsAsanRt() || needsMsanRt() || needsTsanRt() || needsDfsanRt() ||
+  needsLsanRt() || needsCfiDiagRt())
+return false;
+
+  return (Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||
+ CoverageFeatures;
 }
 
 bool SanitizerArgs::needsCfiRt() const {
-  return !(Sanitizers.Mask & CFI & ~TrapSanitizers.Mask) && CfiCrossDso;
+  return !(Sanitizers.Mask & CFI & ~TrapSanitizers.Mask) && CfiCrossDso &&
+ !ImplicitCfiRuntime;
 }
 
 bool SanitizerArgs::needsCfiDiagRt() const {
-  return (Sanitizers.Mask & CFI & ~TrapSanitizers.Mask) &&

[PATCH] D38908: Do not link clang_rt.cfi on Android.

2017-10-13 Thread Peter Collingbourne via Phabricator via cfe-commits 
pcc accepted this revision.
pcc added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D38908



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

[PATCH] D38908: Do not link clang_rt.cfi on Android.

2017-10-13 Thread Evgenii Stepanov via Phabricator via cfe-commits 
eugenis added inline comments.



Comment at: clang/test/Driver/sanitizer-ld.c:605
-// CHECK-CFI-ANDROID: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-CFI-ANDROID-NOT: libclang_rt.cfi
-// CHECK-CFI-ANDROID-NOT: __cfi_check

pcc wrote:
> Why was this passing before?
because android has NeedsSharedRt, and clang driver never adds clang_rt.cfi as 
a shared library.


https://reviews.llvm.org/D38908



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

[PATCH] D38908: Do not link clang_rt.cfi on Android.

2017-10-13 Thread Peter Collingbourne via Phabricator via cfe-commits 
pcc added inline comments.



Comment at: clang/test/Driver/sanitizer-ld.c:605
-// CHECK-CFI-ANDROID: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-CFI-ANDROID-NOT: libclang_rt.cfi
-// CHECK-CFI-ANDROID-NOT: __cfi_check

Why was this passing before?


https://reviews.llvm.org/D38908



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

[PATCH] D38908: Do not link clang_rt.cfi on Android.

2017-10-13 Thread Evgenii Stepanov via Phabricator via cfe-commits 
eugenis created this revision.
Herald added a subscriber: srhines.

The OS provides cross-dso CFI support starting with Android O.
Trapping mode does not require any runtime at all, and diagnostic mode
requires just ubsan-standalone.


https://reviews.llvm.org/D38908

Files:
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -508,6 +508,24 @@
 // CHECK-CFI-CROSS-DSO-DIAG-LINUX: "-whole-archive" "{{[^"]*}}libclang_rt.cfi_diag-x86_64.a" "-no-whole-archive"
 // CHECK-CFI-CROSS-DSO-DIAG-LINUX: -export-dynamic
 
+// Cross-DSO CFI on Android does not link runtime libraries.
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso %s -### -o %t.o 2>&1 \
+// RUN: -target aarch64-linux-android -fuse-ld=ld \
+// RUN: --sysroot=%S/Inputs/basic_android_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CFI-CROSS-DSO-ANDROID %s
+// CHECK-CFI-CROSS-DSO-ANDROID: "{{.*}}ld{{(.exe)?}}"
+// CHECK-CFI-CROSS-DSO-ANDROID-NOT: libclang_rt.
+
+// Cross-DSO CFI with diagnostics on Android links just the UBSAN runtime.
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso %s -### -o %t.o 2>&1 \
+// RUN: -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
+// RUN: -target aarch64-linux-android -fuse-ld=ld \
+// RUN: --sysroot=%S/Inputs/basic_android_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CFI-CROSS-DSO-DIAG-ANDROID %s
+// CHECK-CFI-CROSS-DSO-DIAG-ANDROID: "{{.*}}ld{{(.exe)?}}"
+// CHECK-CFI-CROSS-DSO-DIAG-ANDROID: "{{[^"]*}}libclang_rt.ubsan_standalone-aarch64-android.so"
+// CHECK-CFI-CROSS-DSO-DIAG-ANDROID: "-export-dynamic-symbol=__cfi_check"
+
 // RUN: %clangxx -fsanitize=address %s -### -o %t.o 2>&1 \
 // RUN: -mmacosx-version-min=10.6 \
 // RUN: -target x86_64-apple-darwin13.4.0 -fuse-ld=ld -stdlib=platform \
@@ -596,26 +614,6 @@
 // CHECK-SAFESTACK-ANDROID-AARCH64: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-SAFESTACK-ANDROID-AARCH64-NOT: libclang_rt.safestack
 
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=cfi \
-// RUN: --sysroot=%S/Inputs/basic_android_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-CFI-ANDROID %s
-//
-// CHECK-CFI-ANDROID: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-CFI-ANDROID-NOT: libclang_rt.cfi
-// CHECK-CFI-ANDROID-NOT: __cfi_check
-
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target arm-linux-androideabi -fuse-ld=ld -fsanitize=cfi \
-// RUN: -fsanitize-cfi-cross-dso \
-// RUN: --sysroot=%S/Inputs/basic_android_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-CROSSDSO-CFI-ANDROID %s
-//
-// CHECK-CROSSDSO-CFI-ANDROID: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-CROSSDSO-CFI-ANDROID-NOT: libclang_rt.cfi
-// CHECK-CROSSDSO-CFI-ANDROID: -export-dynamic-symbol=__cfi_check
-// CHECK-CROSSDSO-CFI-ANDROID-NOT: libclang_rt.cfi
-
 // RUN: %clang -fsanitize=undefined %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
 // RUN: -shared \
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -171,19 +171,23 @@
 }
 
 bool SanitizerArgs::needsUbsanRt() const {
-  return ((Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||
-  CoverageFeatures) &&
- !Sanitizers.has(Address) && !Sanitizers.has(Memory) &&
- !Sanitizers.has(Thread) && !Sanitizers.has(DataFlow) &&
- !Sanitizers.has(Leak) && !CfiCrossDso;
+  // All of these include ubsan.
+  if (needsAsanRt() || needsMsanRt() || needsTsanRt() || needsDfsanRt() ||
+  needsLsanRt() || needsCfiDiagRt())
+return false;
+
+  return (Sanitizers.Mask & NeedsUbsanRt & ~TrapSanitizers.Mask) ||
+ CoverageFeatures;
 }
 
 bool SanitizerArgs::needsCfiRt() const {
-  return !(Sanitizers.Mask & CFI & ~TrapSanitizers.Mask) && CfiCrossDso;
+  return !(Sanitizers.Mask & CFI & ~TrapSanitizers.Mask) && CfiCrossDso &&
+ !ImplicitCfiRuntime;
 }
 
 bool SanitizerArgs::needsCfiDiagRt() const {
-  return (Sanitizers.Mask & CFI & ~TrapSanitizers.Mask) && CfiCrossDso;
+  return (Sanitizers.Mask & CFI & ~TrapSanitizers.Mask) && CfiCrossDso &&
+ !ImplicitCfiRuntime;
 }
 
 bool SanitizerArgs::requiresPIE() const {
@@ -615,6 +619,8 @@
TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia() ||
TC.getTriple().isOSDarwin());
 
+  ImplicitCfiRuntime = TC.getTriple().isAndroid();
+
   if (AllAddedKinds & Address) {
 NeedPIE |= TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia();
 if (Arg *A =
Index: clang/include/clang/Driver/SanitizerArgs.h