[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

[PATCH] D19853: [safestack] Add -fruntime-init to support invoking functions during runtime init

[Michael LeMay via cfe-commits]

mlemay-intel abandoned this revision.
mlemay-intel added a comment.

I was able to setup a temporary thread control block early enough in musl libc 
initialization to obviate the need for an attribute like runtime_init.


https://reviews.llvm.org/D19853



___
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] D19854: Define Contiki OS toolchain

[Michael LeMay via cfe-commits]

mlemay-intel added a comment.

In https://reviews.llvm.org/D19854#551260, @Eugene.Zelenko wrote:

> Looks like patch was not committed.


This patch is only useful in combination with https://reviews.llvm.org/D19852, 
so I was waiting for that one to be approved so these can be committed at the 
same time.  I can go ahead and request that this one be committed now if you 
prefer, though.


https://reviews.llvm.org/D19854



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

Re: [PATCH] D19854: Define Contiki OS toolchain

[Michael LeMay via cfe-commits]

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

Fixed indentation of Contiki constructor in ToolChains.h.


https://reviews.llvm.org/D19854

Files:
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -391,6 +391,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=safe-stack 
-fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SP
 // RUN: %clang -target arm-linux-androideabi -fsanitize=safe-stack -### %s 
2>&1 | FileCheck %s -check-prefix=NO-SP
 // RUN: %clang -target aarch64-linux-android -fsanitize=safe-stack -### %s 
2>&1 | FileCheck %s -check-prefix=NO-SP
+// RUN: %clang -target i386-contiki-unknown -fsanitize=safe-stack -### %s 2>&1 
| FileCheck %s -check-prefix=NO-SP
 // NO-SP-NOT: stack-protector
 // NO-SP: "-fsanitize=safe-stack"
 // SP: "-fsanitize=safe-stack"
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1215,6 +1215,14 @@
   Tool *buildLinker() const override;
 };
 
+class LLVM_LIBRARY_VISIBILITY Contiki : public Generic_ELF {
+public:
+  Contiki(const Driver , const llvm::Triple ,
+  const llvm::opt::ArgList );
+
+  SanitizerMask getSupportedSanitizers() const override;
+};
+
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -5185,3 +5185,14 @@
   Res |= SanitizerKind::Vptr;
   return Res;
 }
+
+Contiki::Contiki(const Driver , const llvm::Triple , const ArgList 
)
+: Generic_ELF(D, Triple, Args) {}
+
+SanitizerMask Contiki::getSupportedSanitizers() const {
+  const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
+  SanitizerMask Res = ToolChain::getSupportedSanitizers();
+  if (IsX86)
+Res |= SanitizerKind::SafeStack;
+  return Res;
+}
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2715,6 +2715,9 @@
 case llvm::Triple::PS4:
   TC = new toolchains::PS4CPU(*this, Target, Args);
   break;
+case llvm::Triple::Contiki:
+  TC = new toolchains::Contiki(*this, Target, Args);
+  break;
 default:
   // Of these targets, Hexagon is the only one that might have
   // an OS of Linux, in which case it got handled above already.


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -391,6 +391,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=safe-stack -fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SP
 // RUN: %clang -target arm-linux-androideabi -fsanitize=safe-stack -### %s 2>&1 | FileCheck %s -check-prefix=NO-SP
 // RUN: %clang -target aarch64-linux-android -fsanitize=safe-stack -### %s 2>&1 | FileCheck %s -check-prefix=NO-SP
+// RUN: %clang -target i386-contiki-unknown -fsanitize=safe-stack -### %s 2>&1 | FileCheck %s -check-prefix=NO-SP
 // NO-SP-NOT: stack-protector
 // NO-SP: "-fsanitize=safe-stack"
 // SP: "-fsanitize=safe-stack"
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1215,6 +1215,14 @@
   Tool *buildLinker() const override;
 };
 
+class LLVM_LIBRARY_VISIBILITY Contiki : public Generic_ELF {
+public:
+  Contiki(const Driver , const llvm::Triple ,
+  const llvm::opt::ArgList );
+
+  SanitizerMask getSupportedSanitizers() const override;
+};
+
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -5185,3 +5185,14 @@
   Res |= SanitizerKind::Vptr;
   return Res;
 }
+
+Contiki::Contiki(const Driver , const llvm::Triple , const ArgList )
+: Generic_ELF(D, Triple, Args) {}
+
+SanitizerMask Contiki::getSupportedSanitizers() const {
+  const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
+  SanitizerMask Res = ToolChain::getSupportedSanitizers();
+  if (IsX86)
+Res |= SanitizerKind::SafeStack;
+  return Res;
+}
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2715,6 +2715,9 @@
 case llvm::Triple::PS4:
   TC = new toolchains::PS4CPU(*this, Target, Args);
   break;
+case llvm::Triple::Contiki:
+  TC = new toolchains::Contiki(*this, Target, Args);
+  break;
 default:
   // Of these targets, 

Re: [PATCH] D19854: Define Contiki OS toolchain

[Michael LeMay via cfe-commits]

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

Added driver test.


http://reviews.llvm.org/D19854

Files:
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -386,6 +386,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=safe-stack 
-fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SP
 // RUN: %clang -target arm-linux-androideabi -fsanitize=safe-stack -### %s 
2>&1 | FileCheck %s -check-prefix=NO-SP
 // RUN: %clang -target aarch64-linux-android -fsanitize=safe-stack -### %s 
2>&1 | FileCheck %s -check-prefix=NO-SP
+// RUN: %clang -target i386-contiki-unknown -fsanitize=safe-stack -### %s 2>&1 
| FileCheck %s -check-prefix=NO-SP
 // NO-SP-NOT: stack-protector
 // NO-SP: "-fsanitize=safe-stack"
 // SP: "-fsanitize=safe-stack"
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1191,6 +1191,14 @@
   Tool *buildLinker() const override;
 };
 
+class LLVM_LIBRARY_VISIBILITY Contiki : public Generic_ELF {
+public:
+  Contiki(const Driver , const llvm::Triple ,
+ const llvm::opt::ArgList );
+
+  SanitizerMask getSupportedSanitizers() const override;
+};
+
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -4984,3 +4984,14 @@
   Res |= SanitizerKind::Vptr;
   return Res;
 }
+
+Contiki::Contiki(const Driver , const llvm::Triple , const ArgList 
)
+: Generic_ELF(D, Triple, Args) {}
+
+SanitizerMask Contiki::getSupportedSanitizers() const {
+  const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
+  SanitizerMask Res = ToolChain::getSupportedSanitizers();
+  if (IsX86)
+Res |= SanitizerKind::SafeStack;
+  return Res;
+}
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2512,6 +2512,9 @@
 case llvm::Triple::PS4:
   TC = new toolchains::PS4CPU(*this, Target, Args);
   break;
+case llvm::Triple::Contiki:
+  TC = new toolchains::Contiki(*this, Target, Args);
+  break;
 default:
   // Of these targets, Hexagon is the only one that might have
   // an OS of Linux, in which case it got handled above already.


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -386,6 +386,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=safe-stack -fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SP
 // RUN: %clang -target arm-linux-androideabi -fsanitize=safe-stack -### %s 2>&1 | FileCheck %s -check-prefix=NO-SP
 // RUN: %clang -target aarch64-linux-android -fsanitize=safe-stack -### %s 2>&1 | FileCheck %s -check-prefix=NO-SP
+// RUN: %clang -target i386-contiki-unknown -fsanitize=safe-stack -### %s 2>&1 | FileCheck %s -check-prefix=NO-SP
 // NO-SP-NOT: stack-protector
 // NO-SP: "-fsanitize=safe-stack"
 // SP: "-fsanitize=safe-stack"
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1191,6 +1191,14 @@
   Tool *buildLinker() const override;
 };
 
+class LLVM_LIBRARY_VISIBILITY Contiki : public Generic_ELF {
+public:
+  Contiki(const Driver , const llvm::Triple ,
+ const llvm::opt::ArgList );
+
+  SanitizerMask getSupportedSanitizers() const override;
+};
+
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -4984,3 +4984,14 @@
   Res |= SanitizerKind::Vptr;
   return Res;
 }
+
+Contiki::Contiki(const Driver , const llvm::Triple , const ArgList )
+: Generic_ELF(D, Triple, Args) {}
+
+SanitizerMask Contiki::getSupportedSanitizers() const {
+  const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
+  SanitizerMask Res = ToolChain::getSupportedSanitizers();
+  if (IsX86)
+Res |= SanitizerKind::SafeStack;
+  return Res;
+}
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2512,6 +2512,9 @@
 case llvm::Triple::PS4:
   TC = new toolchains::PS4CPU(*this, Target, Args);
   break;
+case llvm::Triple::Contiki:
+  TC = new toolchains::Contiki(*this, Target, Args);
+  break;
 default:
   // Of these targets, Hexagon is the only one that might have

[PATCH] D19854: Define Contiki OS toolchain

[Michael LeMay via cfe-commits]

mlemay-intel created this revision.
mlemay-intel added reviewers: pcc, eugenis.
mlemay-intel added a subscriber: cfe-commits.

It is defined with support for the SafeStack sanitizer on x86.

http://reviews.llvm.org/D19854

Files:
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h

Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1175,6 +1175,14 @@
   Tool *buildLinker() const override;
 };
 
+class LLVM_LIBRARY_VISIBILITY Contiki : public Generic_ELF {
+public:
+  Contiki(const Driver , const llvm::Triple ,
+ const llvm::opt::ArgList );
+
+  SanitizerMask getSupportedSanitizers() const override;
+};
+
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -4624,3 +4624,14 @@
   Res |= SanitizerKind::Vptr;
   return Res;
 }
+
+Contiki::Contiki(const Driver , const llvm::Triple , const ArgList 
)
+: Generic_ELF(D, Triple, Args) {}
+
+SanitizerMask Contiki::getSupportedSanitizers() const {
+  const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
+  SanitizerMask Res = ToolChain::getSupportedSanitizers();
+  if (IsX86)
+Res |= SanitizerKind::SafeStack;
+  return Res;
+}
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2499,6 +2499,9 @@
 case llvm::Triple::PS4:
   TC = new toolchains::PS4CPU(*this, Target, Args);
   break;
+case llvm::Triple::Contiki:
+  TC = new toolchains::Contiki(*this, Target, Args);
+  break;
 default:
   // Of these targets, Hexagon is the only one that might have
   // an OS of Linux, in which case it got handled above already.


Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -1175,6 +1175,14 @@
   Tool *buildLinker() const override;
 };
 
+class LLVM_LIBRARY_VISIBILITY Contiki : public Generic_ELF {
+public:
+  Contiki(const Driver , const llvm::Triple ,
+ const llvm::opt::ArgList );
+
+  SanitizerMask getSupportedSanitizers() const override;
+};
+
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -4624,3 +4624,14 @@
   Res |= SanitizerKind::Vptr;
   return Res;
 }
+
+Contiki::Contiki(const Driver , const llvm::Triple , const ArgList )
+: Generic_ELF(D, Triple, Args) {}
+
+SanitizerMask Contiki::getSupportedSanitizers() const {
+  const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
+  SanitizerMask Res = ToolChain::getSupportedSanitizers();
+  if (IsX86)
+Res |= SanitizerKind::SafeStack;
+  return Res;
+}
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2499,6 +2499,9 @@
 case llvm::Triple::PS4:
   TC = new toolchains::PS4CPU(*this, Target, Args);
   break;
+case llvm::Triple::Contiki:
+  TC = new toolchains::Contiki(*this, Target, Args);
+  break;
 default:
   // Of these targets, Hexagon is the only one that might have
   // an OS of Linux, in which case it got handled above already.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D19853: [safestack] Add -fruntime-init to support invoking functions during runtime init

[Michael LeMay via cfe-commits]

mlemay-intel created this revision.
mlemay-intel added reviewers: pcc, eugenis.
mlemay-intel added a subscriber: cfe-commits.

SafeStack uses thread-local storage by default in most OSes for the unsafe stack
pointer. For SafeStack to be applied to functions that may be invoked while
initializing the dynamic linker and libc, the functions can be compiled to
support a single-threaded unsafe stack pointer. This flag signals that such code
should be emitted.

http://reviews.llvm.org/D19853

Files:
  docs/CommandGuide/clang.rst
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/runtime-init-attr.c

Index: test/CodeGen/runtime-init-attr.c
===
--- /dev/null
+++ test/CodeGen/runtime-init-attr.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - %s -fruntime-init | FileCheck -check-prefix=DEF -check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - %s -fruntime-init -fsanitize=safe-stack | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK %s
+
+// DEF: define {{.*}}void @test1() #[[A:.*]] {
+void test1() {
+}
+
+// CHECK: attributes #[[A]] = {{.*}} runtime_init
+// SAFESTACK: attributes #[[A]] = {{.*}} runtime_init safestack
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1730,6 +1730,7 @@
   if (!Opts.NoBuiltin)
 getAllNoBuiltinFuncValues(Args, Opts.NoBuiltinFuncs);
   Opts.NoMathBuiltin = Args.hasArg(OPT_fno_math_builtin);
+  Opts.RuntimeInit = Args.hasArg(OPT_fruntime_init);
   Opts.SizedDeallocation = Args.hasArg(OPT_fsized_deallocation);
   Opts.ConceptsTS = Args.hasArg(OPT_fconcepts_ts);
   Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4777,6 +4777,9 @@
   KernelOrKext)
 CmdArgs.push_back("-ffreestanding");
 
+  if (Args.hasArg(options::OPT_fruntime_init))
+CmdArgs.push_back("-fruntime-init");
+
   // Forward -f (flag) options which we can pass directly.
   Args.AddLastArg(CmdArgs, options::OPT_femit_all_decls);
   Args.AddLastArg(CmdArgs, options::OPT_fheinous_gnu_extensions);
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -698,6 +698,9 @@
   if (SanOpts.has(SanitizerKind::SafeStack))
 Fn->addFnAttr(llvm::Attribute::SafeStack);
 
+  if (getLangOpts().RuntimeInit)
+Fn->addFnAttr(llvm::Attribute::RuntimeInit);
+
   // Pass inline keyword to optimizer if it appears explicitly on any
   // declaration. Also, in the case of -fno-inline attach NoInline
   // attribute to all function that are not marked AlwaysInline.
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -279,6 +279,9 @@
   Fn->addFnAttr(llvm::Attribute::SafeStack);
   }
 
+  if (getLangOpts().RuntimeInit)
+Fn->addFnAttr(llvm::Attribute::RuntimeInit);
+
   return Fn;
 }
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1031,6 +1031,7 @@
 def freg_struct_return : Flag<["-"], "freg-struct-return">, Group, Flags<[CC1Option]>,
   HelpText<"Override the default ABI to return small structs in registers">;
 def frtti : Flag<["-"], "frtti">, Group;
+def fruntime_init : Flag<["-"], "fruntime-init">, Group, Flags<[CC1Option]>;
 def : Flag<["-"], "fsched-interblock">, Group;
 def fshort_enums : Flag<["-"], "fshort-enums">, Group, Flags<[CC1Option]>,
   HelpText<"Allocate to an enum type only as many bytes as it needs for the declared range of possible values">;
Index: include/clang/Basic/LangOptions.def
===
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -132,6 +132,7 @@
 LANGOPT(Freestanding, 1, 0, "freestanding implementation")
 LANGOPT(NoBuiltin , 1, 0, "disable builtin functions")
 LANGOPT(NoMathBuiltin , 1, 0, "disable math builtin functions")
+LANGOPT(RuntimeInit   , 1, 0, "used during runtime initialization")
 LANGOPT(GNUAsm, 1, 1, "GNU-style inline assembly")
 LANGOPT(Coroutines, 1, 0, "C++ coroutines")
 
Index: docs/CommandGuide/clang.rst
===
--- docs/CommandGuide/clang.rst
+++ docs/CommandGuide/clang.rst
@@ -313,6 +313,13 @@
   model can be overridden with 

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

[Michael LeMay via cfe-commits]

mlemay-intel added a comment.

In http://reviews.llvm.org/D19483#417865, @pcc wrote:

> > This makes sense for the example I gave. However, there are also more 
> > complicated situations. Sometimes it is necessary to specify different 
> > options for different files that are compiled for the same OS. For example, 
> > early during the initialization of a dynamic linker or C library, a 
> > single-threaded mode of USP storage needs to be supported. TLS is not 
> > available at that time. How should requirements like that be conveyed to 
> > the compiler?
>
>
> In cases like this, we should introduce a new `-m` flag in the Clang driver 
> and plumb that through to the SafeStack pass using a target-specific function 
> attribute.


It appears that Clang already supports an `-mthread_model=single` option, so do 
you recommend that I remove the `-mllvm -safe-stack-usp-storage=single-thread` 
option and rely on `-mthread_model` instead? I added @eugenis to get his 
feedback as well, since he approved my patch that added the new flag.  `-mllvm 
-safe-stack-usp-storage=single-thread` would have been documented for the first 
time by this patch, and I have not heard of anyone else using it yet.

I don't see any existing option that seems suitable for indicating that a file 
may be used during dynamic linker or C library initialization, so perhaps we 
should add an `-mruntime-init` flag?  I welcome other naming suggestions.


http://reviews.llvm.org/D19483



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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

[Michael LeMay via cfe-commits]

mlemay-intel added a comment.

In http://reviews.llvm.org/D19483#417857, @pcc wrote:

> You should be using `-target x86-unknown-contiki` or similar. That should 
> tune the behaviour to what is required for that OS. See what we have in 
> `TargetLoweringBase::getSafeStackPointerLocation` to provide Android-specific 
> behaviour for example.


This makes sense for the example I gave.  However, there are also more 
complicated situations.  Sometimes it is necessary to specify different options 
for different files that are compiled for the same OS.  For example, early 
during the initialization of a dynamic linker or C library, a single-threaded 
mode of USP storage needs to be supported.  TLS is not available at that time.  
How should requirements like that be conveyed to the compiler?

> The existence of flags is not justification to add more. Besides, it appears 
> that the `-safe-stack-usp-storage` flag was added without proper review. It 
> was reviewed in http://reviews.llvm.org/D15673, but the mailing list was not 
> added as a subscriber. If I had been aware of that review I would have made 
> the same objections at that time.


Sorry, I only recently learned that the mailing list should be added as a 
subscriber.  Prior to that, I thought that patches were automatically sent to 
the appropriate mailing lists.


http://reviews.llvm.org/D19483



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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and various USP storage modes

[Michael LeMay via cfe-commits]

mlemay-intel added a comment.

In http://reviews.llvm.org/D19483#417844, @pcc wrote:

> We shouldn't be adding (much less documenting) `-mllvm` flags. Is there any 
> reason why this behavior can't be gated on the OS?


We actually already have added at least one `-mllvm` flag: `-mllvm 
-safe-stack-usp-storage=single-thread`.  You can see an example of how it can 
be used here: 
https://github.com/contiki-os/contiki/pull/1642/commits/f13d1e22669d5526f2a721a337acd06f23a8d49d
  Can you please provide an example of how you think that setting should be 
specified without the use of the `-mllvm` flag?


http://reviews.llvm.org/D19483



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

Re: [PATCH] D19483: docs: Update SafeStack docs with separate-stack-seg feature and single-thread storage

[Michael LeMay via cfe-commits]

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

Add documentation for -safe-stack-usp-storage=tcb, 
-safe-stack-usp-tcb-offset=, and -safe-stack-usp-init.


http://reviews.llvm.org/D19483

Files:
  docs/SafeStack.rst

Index: docs/SafeStack.rst
===
--- docs/SafeStack.rst
+++ docs/SafeStack.rst
@@ -78,6 +78,49 @@
 address and the instrumentation ensures that no pointers to the safe stack are
 ever stored outside of the safe stack itself (see limitations below).
 
+SafeStack can be combined with the ``separate-stack-seg`` feature (using the
+``-mseparate-stack-seg`` command line option) for x86-32. Each x86 memory
+operand has an effective segment. The ``separate-stack-seg`` feature emits
+segment override prefixes as necessary to direct safe stack accesses to the SS
+segment and ordinary data accesses to the DS and ES segments. That eliminates
+one obstacle preventing the limits of the DS and ES segments from being set
+below the start of the safe stacks. However, additional runtime support is
+needed to actually perform that segment configuration. Furthermore, all code in
+the program, including library routines, that may be run with such a segment
+configuration must be compiled to support it. This is a more extensive level of
+runtime support than is supplied by the compiler-rt library.  Thus, the
+compiler-rt library is not automatically linked when the ``separate-stack-seg``
+feature is enabled.
+
+The ``separate-stack-seg`` feature only performs intra-procedural analysis.  It
+assumes that only the stack register points to the safe stack at function
+entry. It assumes that no pointers passed into the function or stored as global
+variables point to the safe stack. SafeStack helps to satisfy this assumption
+when the ``separate-stack-seg`` feature is enabled by moving additional types 
of
+allocations to the unsafe stack if pointers to them may be passed to
+subroutines. Note that variables can be annotated as pointing to address space
+258 independent of the ``separate-stack-seg`` feature, and the compiler will
+generate code to access the referenced data in the SS segment.
+
+Special handling is needed for variadic argument lists when the
+``separate-stack-seg`` feature is enabled. Certain variadic argument intrinsics
+(e.g. ``va_start`` and ``va_arg``) may store the addresses of variadic 
arguments
+to memory. Variadic arguments are passed on the safe stack. Ordinarily, the
+``separate-stack-seg`` feature attempts to block most pointers to the safe 
stack
+from being stored, unless it is able to track them as register spills. The
+``separate-stack-seg`` feature does not support tracking other types of stores
+of safe stack pointers and pairing them with subsequent loads (i.e. register
+fills) so that it can emit the necessary segment override prefixes for safe
+stack accesses. However, the ``separate-stack-seg`` feature attempts to 
identify
+and permit instructions that store safe stack pointers to support variadic
+arguments. Its heuristics are only capable of identifying simple usages of the
+variadic argument intrinsics. More complex usages may result in assertion
+failures. The relevant assertion can be bypassed just for functions with a
+``va_start`` by passing an argument to LLVM: ``-mllvm
+-sep-stk-seg-sp-store-verif=ignore-va``. That assertion can be bypassed for all
+functions by passing ``-mllvm -sep-stk-seg-sp-store-verif=none``. However,
+passing either of these arguments may lead to incorrect code being generated.
+
 Known security limitations
 ~~
 
@@ -109,7 +152,8 @@
 The `CPI paper `_ describes two alternative,
 stronger safe stack protection mechanisms, that rely on software fault
 isolation, or hardware segmentation (as available on x86-32 and some x86-64
-CPUs).
+CPUs). The ``separate-stack-seg`` feature can be used with appropriate runtime
+support to enable hardware segmentation on x86-32.
 
 At the moment, SafeStack assumes that the compiler's implementation is correct.
 This has not been verified except through manual code inspection, and could
@@ -123,6 +167,32 @@
 To enable SafeStack, just pass ``-fsanitize=safe-stack`` flag to both compile
 and link command lines.
 
+To store the unsafe stack pointer in an ordinary global variable instead of a
+thread-local variable, add the ``-mllvm -safe-stack-usp-storage=single-thread``
+flag. This is only suitable for single-threaded programs and is incompatible
+with the compiler-rt library. It was originally intended for certain embedded
+systems. A linked object or library must define and initialize
+``__safestack_unsafe_stack_ptr`` with type ``void*`` as well as associated
+storage for the unsafe stack.
+
+A single-threaded unsafe stack pointer can also be useful during program
+initialization in the dynamic linker and C library startup routines, before
+thread-local storage is 

Re: [PATCH] D19458: Add address space 258 to Clang documentation

[Michael LeMay via cfe-commits]

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

Update heading.


http://reviews.llvm.org/D19458

Files:
  docs/LanguageExtensions.rst

Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1912,12 +1912,13 @@
 
 The X86 backend has these language extensions:
 
-Memory references off the GS segment
-
+Memory references to specified segments
+^^^
 
 Annotating a pointer with address space #256 causes it to be code generated
-relative to the X86 GS segment register, and address space #257 causes it to be
-relative to the X86 FS segment.  Note that this is a very very low-level
+relative to the X86 GS segment register, address space #257 causes it to be
+relative to the X86 FS segment, and address space #258 causes it to be
+relative to the X86 SS segment.  Note that this is a very very low-level
 feature that should only be used if you know what you're doing (for example in
 an OS kernel).
 


Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1912,12 +1912,13 @@
 
 The X86 backend has these language extensions:
 
-Memory references off the GS segment
-
+Memory references to specified segments
+^^^
 
 Annotating a pointer with address space #256 causes it to be code generated
-relative to the X86 GS segment register, and address space #257 causes it to be
-relative to the X86 FS segment.  Note that this is a very very low-level
+relative to the X86 GS segment register, address space #257 causes it to be
+relative to the X86 FS segment, and address space #258 causes it to be
+relative to the X86 SS segment.  Note that this is a very very low-level
 feature that should only be used if you know what you're doing (for example in
 an OS kernel).
 
___
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] D17092: [X86] Add -mseparate-stack-seg

[Michael LeMay via cfe-commits]

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

Revise patch to fix line alignment.


http://reviews.llvm.org/D17092

Files:
  include/clang/Driver/Options.td
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/varargs.c

Index: test/CodeGen/varargs.c
===
--- test/CodeGen/varargs.c
+++ test/CodeGen/varargs.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck 
%s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature 
+separate-stack-seg -emit-llvm -o - %s | FileCheck -check-prefix=SEPARATE-SS %s
 
 // PR6433 - Don't crash on va_arg(typedef).
 typedef double gdouble;
@@ -20,4 +21,5 @@
   __builtin_va_list ap;
   void *p;
   p = __builtin_va_arg(ap, typeof (int (*)[++n])); // CHECK: add nsw i32 
{{.*}}, 1
+  // SEPARATE-SS: load i32*, i32* addrspace(258)* {{.*}}
 }
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -1690,9 +1690,26 @@
   TypeInfo.second = CharUnits::fromQuantity(
 getTypeStackAlignInBytes(Ty, TypeInfo.second.getQuantity()));
 
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false,
-  TypeInfo, CharUnits::fromQuantity(4),
-  /*AllowHigherAlign*/ true);
+  const Address Addr = emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ 
false,
+TypeInfo, CharUnits::fromQuantity(4),
+/*AllowHigherAlign*/ true);
+
+  const std::vector  =
+CGF.getTarget().getTargetOpts().Features;
+  if (std::find(TargetFeatures.begin(), TargetFeatures.end(),
+"+separate-stack-seg") != TargetFeatures.end()) {
+// Cast the pointer into the address space for the stack segment.
+// This is to help support multi-segment memory models in which DS and SS
+// may differ from each other.
+llvm::Type *DirectTy = CGF.ConvertTypeForMem(Ty);
+llvm::Value *PtrAsInt =
+  CGF.Builder.CreatePtrToInt(Addr.getPointer(), CGF.IntPtrTy);
+llvm::Value *PtrInStackSeg =
+  CGF.Builder.CreateIntToPtr(PtrAsInt, DirectTy->getPointerTo(258));
+return Address(PtrInStackSeg, Addr.getAlignment());
+  }
+
+  return Addr;
 }
 
 bool X86_32TargetCodeGenInfo::isStructReturnInRegABI(
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1407,6 +1407,7 @@
 def mno_xsavec : Flag<["-"], "mno-xsavec">, Group;
 def mno_xsaves : Flag<["-"], "mno-xsaves">, Group;
 def mno_pku : Flag<["-"], "mno-pku">, Group;
+def mseparate_stack_seg : Flag<["-"], "mseparate-stack-seg">, 
Group;
 
 def munaligned_access : Flag<["-"], "munaligned-access">, 
Group,
   HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;


Index: test/CodeGen/varargs.c
===
--- test/CodeGen/varargs.c
+++ test/CodeGen/varargs.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +separate-stack-seg -emit-llvm -o - %s | FileCheck -check-prefix=SEPARATE-SS %s
 
 // PR6433 - Don't crash on va_arg(typedef).
 typedef double gdouble;
@@ -20,4 +21,5 @@
   __builtin_va_list ap;
   void *p;
   p = __builtin_va_arg(ap, typeof (int (*)[++n])); // CHECK: add nsw i32 {{.*}}, 1
+  // SEPARATE-SS: load i32*, i32* addrspace(258)* {{.*}}
 }
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -1690,9 +1690,26 @@
   TypeInfo.second = CharUnits::fromQuantity(
 getTypeStackAlignInBytes(Ty, TypeInfo.second.getQuantity()));
 
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false,
-  TypeInfo, CharUnits::fromQuantity(4),
-  /*AllowHigherAlign*/ true);
+  const Address Addr = emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false,
+TypeInfo, CharUnits::fromQuantity(4),
+/*AllowHigherAlign*/ true);
+
+  const std::vector  =
+CGF.getTarget().getTargetOpts().Features;
+  if (std::find(TargetFeatures.begin(), TargetFeatures.end(),
+"+separate-stack-seg") != TargetFeatures.end()) {
+// Cast the pointer into the address space for the stack segment.
+// This is to help support multi-segment memory models in which DS and SS
+// may differ from each other.
+llvm::Type *DirectTy = CGF.ConvertTypeForMem(Ty);
+llvm::Value *PtrAsInt =
+  CGF.Builder.CreatePtrToInt(Addr.getPointer(), CGF.IntPtrTy);
+llvm::Value *PtrInStackSeg =
+  

Re: [PATCH] D17092: [X86] Add -mseparate-stack-seg

[Michael LeMay via cfe-commits]

mlemay-intel added a comment.

Thank you for your feedback!



Comment at: lib/CodeGen/TargetInfo.cpp:1569
@@ +1568,3 @@
+CGF.getTarget().getTargetOpts().Features;
+  if (std::find(TargetFeatures.begin(), TargetFeatures.end(),
+"+separate-stack-seg") != TargetFeatures.end()) {

delena wrote:
> Hi,
> I'm not clang reviewer at all and you can ignore my comment.
> 
> Searching string looks strange for me. I suppose that this string should be 
> defined in another form. Something like
> options::OPT_separate_stack_seg.
> 
I haven't quite found an example of how an option like this should be handled.  
String comparisons of this form are performed in clang/lib/Basic/Targets.cpp, 
but there they are used to initialize variables.  Furthermore, all of those 
tests are for CPU features, whereas this one is for a particular segment 
configuration.


Comment at: lib/CodeGen/TargetInfo.cpp:1577
@@ +1576,3 @@
+  CGF.Builder.CreatePtrToInt(Addr.getPointer(), CGF.IntPtrTy);
+llvm::Value *PtrInStackSeg = CGF.Builder.CreateIntToPtr(PtrAsInt,
+   
DirectTy->getPointerTo(258));

delena wrote:
> You should not use 258 as a constant. It should be defined somewhere as 
> address space enum.
I agree with this principle.  However, the address space numbers for FS and GS 
are listed in the Clang documentation [1] and are used as literals in LLVM 
code.  Thus, I think this patch is consistent with existing usage of address 
space numbers.

[1] 
http://clang.llvm.org/docs/LanguageExtensions.html#memory-references-off-the-gs-segment


Comment at: lib/CodeGen/TargetInfo.cpp:1578
@@ +1577,3 @@
+llvm::Value *PtrInStackSeg = CGF.Builder.CreateIntToPtr(PtrAsInt,
+   
DirectTy->getPointerTo(258));
+return Address(PtrInStackSeg, Addr.getAlignment());

delena wrote:
> This line alignment does not match LLVM style.
I'll fix it.


Comment at: lib/CodeGen/TargetInfo.cpp:1580
@@ +1579,3 @@
+return Address(PtrInStackSeg, Addr.getAlignment());
+  }
+

delena wrote:
> Again, not sure that I'm right. You are trying to create addressspacecast. Is 
> it the right way to create ptrtoint + inttoptr?
I first attempted to create an address space cast, but LLVM disallows that.  I 
think it is necessary to perform the conversion through an int, but I am not 
entirely sure of that.


http://reviews.llvm.org/D17092



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