[PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Michael LeMay via cfe-commits]

mlemay-intel updated this revision to Diff 76636.
mlemay-intel added a comment.

Disabled linking of the compiler-rt SafeStack runtime library for musl
environments rather than for targets that use the separate stack segment
feature.  This reflects changes in my proposed musl libc patches to add
architecture-independent support for storing USP in the TCB.


https://reviews.llvm.org/D19170

Files:
  lib/Driver/Tools.cpp
  test/Driver/sanitizer-ld.c


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -406,6 +406,22 @@
 // CHECK-SAFESTACK-LINUX: "-lpthread"
 // CHECK-SAFESTACK-LINUX: "-ldl"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-unknown-linux-musl -fsanitize=safe-stack \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SAFESTACK-MUSL32 %s
+//
+// CHECK-SAFESTACK-MUSL32: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-SAFESTACK-MUSL32-NOT: libclang_rt.safestack-i386.a"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl -fsanitize=safe-stack \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SAFESTACK-MUSL64 %s
+//
+// CHECK-SAFESTACK-MUSL64: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-SAFESTACK-MUSL64-NOT: libclang_rt.safestack-x86_64.a"
+
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3254,7 +3254,7 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("ubsan_standalone_cxx");
   }
-  if (SanArgs.needsSafeStackRt())
+  if (SanArgs.needsSafeStackRt() && !TC.getTriple().isMusl())
 StaticRuntimes.push_back("safestack");
   if (SanArgs.needsCfiRt())
 StaticRuntimes.push_back("cfi");


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -406,6 +406,22 @@
 // CHECK-SAFESTACK-LINUX: "-lpthread"
 // CHECK-SAFESTACK-LINUX: "-ldl"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-unknown-linux-musl -fsanitize=safe-stack \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SAFESTACK-MUSL32 %s
+//
+// CHECK-SAFESTACK-MUSL32: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-SAFESTACK-MUSL32-NOT: libclang_rt.safestack-i386.a"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-unknown-linux-musl -fsanitize=safe-stack \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SAFESTACK-MUSL64 %s
+//
+// CHECK-SAFESTACK-MUSL64: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-SAFESTACK-MUSL64-NOT: libclang_rt.safestack-x86_64.a"
+
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3254,7 +3254,7 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("ubsan_standalone_cxx");
   }
-  if (SanArgs.needsSafeStackRt())
+  if (SanArgs.needsSafeStackRt() && !TC.getTriple().isMusl())
 StaticRuntimes.push_back("safestack");
   if (SanArgs.needsCfiRt())
 StaticRuntimes.push_back("cfi");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a comment.

Please add dependencies to Differential revisions, so reasons for holding will 
be clear.


https://reviews.llvm.org/D19170



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

Re: [PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Michael LeMay via cfe-commits]

mlemay-intel added a comment.

In https://reviews.llvm.org/D19170#551426, @Eugene.Zelenko wrote:

> Looks like patch was not committed.


This patch, https://reviews.llvm.org/D17092, https://reviews.llvm.org/D17094, 
and https://reviews.llvm.org/D17095 are all interdependent.  I think it makes 
sense to wait until all four are approved so that they can all be committed 
together.


https://reviews.llvm.org/D19170



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

Re: [PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

Looks like patch was not committed.


https://reviews.llvm.org/D19170



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

Re: [PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Evgeniy Stepanov via cfe-commits]

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

LGTM


http://reviews.llvm.org/D19170



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

Re: [PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Michael LeMay via cfe-commits]

mlemay-intel updated this revision to Diff 53956.
mlemay-intel added a comment.

Add test.


http://reviews.llvm.org/D19170

Files:
  lib/Driver/Tools.cpp
  test/Driver/sanitizer-ld.c

Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -347,6 +347,15 @@
 // CHECK-SAFESTACK-LINUX: "-lpthread"
 // CHECK-SAFESTACK-LINUX: "-ldl"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-unknown-linux -fsanitize=safe-stack \
+// RUN: -mseparate-stack-seg \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SAFESTACK-LINUX-SEP-STK %s
+//
+// CHECK-SAFESTACK-LINUX-SEP-STK: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-SAFESTACK-LINUX-SEP-STK-NOT: libclang_rt.safestack-i386.a"
+
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2993,7 +2993,9 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("ubsan_standalone_cxx");
   }
-  if (SanArgs.needsSafeStackRt())
+  // Using a separate stack segment with SafeStack requires more extensive
+  // runtime support than this provides.
+  if (SanArgs.needsSafeStackRt() && 
!Args.hasArg(options::OPT_mseparate_stack_seg))
 StaticRuntimes.push_back("safestack");
   if (SanArgs.needsCfiRt())
 StaticRuntimes.push_back("cfi");


Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -347,6 +347,15 @@
 // CHECK-SAFESTACK-LINUX: "-lpthread"
 // CHECK-SAFESTACK-LINUX: "-ldl"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-unknown-linux -fsanitize=safe-stack \
+// RUN: -mseparate-stack-seg \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SAFESTACK-LINUX-SEP-STK %s
+//
+// CHECK-SAFESTACK-LINUX-SEP-STK: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-SAFESTACK-LINUX-SEP-STK-NOT: libclang_rt.safestack-i386.a"
+
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2993,7 +2993,9 @@
 if (SanArgs.linkCXXRuntimes())
   StaticRuntimes.push_back("ubsan_standalone_cxx");
   }
-  if (SanArgs.needsSafeStackRt())
+  // Using a separate stack segment with SafeStack requires more extensive
+  // runtime support than this provides.
+  if (SanArgs.needsSafeStackRt() && !Args.hasArg(options::OPT_mseparate_stack_seg))
 StaticRuntimes.push_back("safestack");
   if (SanArgs.needsCfiRt())
 StaticRuntimes.push_back("cfi");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Michael LeMay via cfe-commits]

mlemay-intel added a comment.

In http://reviews.llvm.org/D19170#402861, @eugenis wrote:

> Test, please.


Do you know of any examples of the sort of test that you would like to see for 
a feature like this?

> Where is this runtime support implemented? Some platform's libc, or an 
> external library?


In my experience, supporting a separate stack segment seems to require a 
modified libc.  The whole program, including libc routines, needs to be 
recompiled to direct memory accesses to the appropriate segment.  This requires 
setting up an unsafe stack early enough that those recompiled routines can run 
successfully.


http://reviews.llvm.org/D19170



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

Re: [PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

In http://reviews.llvm.org/D19170#402939, @mlemay-intel wrote:

> In http://reviews.llvm.org/D19170#402861, @eugenis wrote:
>
> > Test, please.
>
>
> Do you know of any examples of the sort of test that you would like to see 
> for a feature like this?


test/Driver/sanitizer-ld.c


http://reviews.llvm.org/D19170



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

Re: [PATCH] D19170: [safestack] Link SafeStack runtime only when not using separate stack segment

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

Test, please.
Where is this runtime support implemented? Some platform's libc, or an external 
library?


http://reviews.llvm.org/D19170



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