[PATCH] D46998: [XRay][clang+compiler-rt] Make XRay depend on a C++ standard lib

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: dblaikie, echristo.

This is related to http://llvm.org/PR32274.

When building with XRay, always depend on a C++ standard library.

We're doing this to automate the linking of the C++ ABI functionality
that the modes use by default. In particular, we depend on some
function-local static initialisation.

The alternative change here is to re-write the modes to only use
libc/pthreads functionality. We're choosing to do this instead as it's
minimally invasive. In the future we can revisit this when we have a
better idea as to why not depending on the C++ ABI functionality is a
better solution.


https://reviews.llvm.org/D46998

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  compiler-rt/test/xray/TestCases/Posix/c-test.cc


Index: compiler-rt/test/xray/TestCases/Posix/c-test.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/c-test.cc
@@ -0,0 +1,4 @@
+// RUN: %clang_xray -g -o %t %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+int main() {}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -713,7 +713,8 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
-bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, 
ArgStringList &CmdArgs) {
+bool tools::addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
   if (Args.hasArg(options::OPT_shared))
 return false;
 
@@ -723,6 +724,11 @@
 for (const auto &Mode : TC.getXRayArgs().modeList())
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, Mode, false));
 CmdArgs.push_back("-no-whole-archive");
+
+// If we're linking a non-C++ application, we'd need to link in the C++
+// runtime.
+if (!Args.hasArg(clang::driver::options::OPT_nostdlibxx))
+  TC.AddCXXStdlibLibArgs(Args, CmdArgs);
 return true;
   }
 


Index: compiler-rt/test/xray/TestCases/Posix/c-test.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/c-test.cc
@@ -0,0 +1,4 @@
+// RUN: %clang_xray -g -o %t %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+int main() {}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -713,7 +713,8 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
-bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, ArgStringList &CmdArgs) {
+bool tools::addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
   if (Args.hasArg(options::OPT_shared))
 return false;
 
@@ -723,6 +724,11 @@
 for (const auto &Mode : TC.getXRayArgs().modeList())
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, Mode, false));
 CmdArgs.push_back("-no-whole-archive");
+
+// If we're linking a non-C++ application, we'd need to link in the C++
+// runtime.
+if (!Args.hasArg(clang::driver::options::OPT_nostdlibxx))
+  TC.AddCXXStdlibLibArgs(Args, CmdArgs);
 return true;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46998: [XRay][clang+compiler-rt] Make XRay depend on a C++ standard lib

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In https://reviews.llvm.org/D46998#1103397, @dblaikie wrote:

> "In the future we can revisit this when we have a better idea as to why not 
> depending on the C++ ABI functionality is a better solution."  - this was 
> discussed previously in the thread linked from the bug.
>
> A big thing, so far as I understand it, is that Clang doesn't require some 
> specific C++ library, so wouldn't this break for users who didn't have the 
> specific C++ library installed that compiler-rt for XRay was built against?


We don't use anything from the C++ library directly, we depend on some features 
provided by the implementation. In some cases, this isn't a library:

  extern int g();
  
  int f() {
static int val = g();
return val;
  }

This will emit calls to guards for the thread-safe initialisation of val.

The alternatives here are that XRay just avoids these completely, and use 
pthreads instead to do a pthread_once_init(...).

The other alternative is to avoid pthreads and use the sanitizer_common mutexes 
too, to make it self-contained in compiler-rt.

The other alternative is to just link in the ABI library (libcxxabi or 
something else) instead of the standard library.

I can make one of those changes work, but that's also a lot of work, which I 
suppose is worth doing anyway (unless I'm missing something simpler).

Any preferences/stack-ranking of which solution might be more palatable?


https://reviews.llvm.org/D46998



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

[PATCH] D46998: [XRay][compiler-rt] Limit reliance on C++ ABI features

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 147451.
dberris retitled this revision from "[XRay][clang+compiler-rt] Make XRay depend 
on a C++ standard lib" to "[XRay][compiler-rt] Limit reliance on C++ ABI 
features".
dberris edited the summary of this revision.
dberris added a comment.

Retitle, and update to limit only to compiler-rt changes.

This is still a work in progress, but working towards a cleaner solution.


https://reviews.llvm.org/D46998

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  compiler-rt/lib/xray/xray_basic_logging.cc
  compiler-rt/test/xray/TestCases/Posix/c-test.cc

Index: compiler-rt/test/xray/TestCases/Posix/c-test.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/c-test.cc
@@ -0,0 +1,4 @@
+// RUN: %clang_xray -g -o %t %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+int main() {}
Index: compiler-rt/lib/xray/xray_basic_logging.cc
===
--- compiler-rt/lib/xray/xray_basic_logging.cc
+++ compiler-rt/lib/xray/xray_basic_logging.cc
@@ -16,7 +16,6 @@
 //===--===//
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -39,16 +38,16 @@
 
 namespace __xray {
 
-__sanitizer::SpinMutex LogMutex;
+SpinMutex LogMutex;
 
 // We use elements of this type to record the entry TSC of every function ID we
 // see as we're tracing a particular thread's execution.
 struct alignas(16) StackEntry {
   int32_t FuncId;
   uint16_t Type;
   uint8_t CPU;
   uint8_t Padding;
-  uint64_t TSC;
+  u64 TSC;
 };
 
 static_assert(sizeof(StackEntry) == 16, "Wrong size for StackEntry");
@@ -66,38 +65,35 @@
 
 static pthread_key_t PThreadKey;
 
-static __sanitizer::atomic_uint8_t BasicInitialized{0};
+static atomic_uint8_t BasicInitialized{0};
 
 BasicLoggingOptions GlobalOptions;
 
 thread_local volatile bool RecursionGuard = false;
 
-static uint64_t thresholdTicks() XRAY_NEVER_INSTRUMENT {
-  static uint64_t TicksPerSec = probeRequiredCPUFeatures()
-? getTSCFrequency()
-: __xray::NanosecondsPerSecond;
-  static const uint64_t ThresholdTicks =
-  TicksPerSec * GlobalOptions.DurationFilterMicros / 100;
-  return ThresholdTicks;
-}
+static atomic_uint8_t UseRealTSC{0};
+static atomic_uint64_t ThresholdTicks{0};
+static atomic_uint64_t TicksPerSec{0};
+static atomic_uint64_t CycleFrequency{__xray::NanosecondsPerSecond};
 
 static int openLogFile() XRAY_NEVER_INSTRUMENT {
   int F = getLogFD();
   if (F == -1)
 return -1;
 
-  // Test for required CPU features and cache the cycle frequency
-  static bool TSCSupported = probeRequiredCPUFeatures();
-  static uint64_t CycleFrequency =
-  TSCSupported ? getTSCFrequency() : __xray::NanosecondsPerSecond;
+  static pthread_once_t DetectOnce;
+  pthread_once(&DetectOnce, +[] {
+if (atomic_load(&UseRealTSC, memory_order_relaxed))
+  atomic_store(&CycleFrequency, getTSCFrequency(), memory_order_release);
+  });
 
   // Since we're here, we get to write the header. We set it up so that the
   // header will only be written once, at the start, and let the threads
   // logging do writes which just append.
   XRayFileHeader Header;
   Header.Version = 2; // Version 2 includes tail exit records.
   Header.Type = FileTypes::NAIVE_LOG;
-  Header.CycleFrequency = CycleFrequency;
+  Header.CycleFrequency = atomic_load(&CycleFrequency, memory_order_relaxed);
 
   // FIXME: Actually check whether we have 'constant_tsc' and 'nonstop_tsc'
   // before setting the values in the header.
@@ -108,29 +104,31 @@
   return F;
 }
 
-int getGlobalFd() XRAY_NEVER_INSTRUMENT {
-  static int Fd = openLogFile();
+static int getGlobalFd() XRAY_NEVER_INSTRUMENT {
+  static pthread_once_t OnceInit;
+  static int Fd = 0;
+  pthread_once(&OnceInit, +[] { Fd = openLogFile(); });
   return Fd;
 }
 
 ThreadLocalData &getThreadLocalData() XRAY_NEVER_INSTRUMENT {
   thread_local ThreadLocalData TLD;
   thread_local bool UNUSED TOnce = [] {
 if (GlobalOptions.ThreadBufferSize == 0) {
-  if (__sanitizer::Verbosity())
+  if (Verbosity())
 Report("Not initializing TLD since ThreadBufferSize == 0.\n");
   return false;
 }
-TLD.TID = __sanitizer::GetTid();
+TLD.TID = GetTid();
 pthread_setspecific(PThreadKey, &TLD);
 TLD.Fd = getGlobalFd();
 TLD.InMemoryBuffer = reinterpret_cast(
 InternalAlloc(sizeof(XRayRecord) * GlobalOptions.ThreadBufferSize,
   nullptr, alignof(XRayRecord)));
 TLD.BufferSize = GlobalOptions.ThreadBufferSize;
 TLD.BufferOffset = 0;
 if (GlobalOptions.MaxStackDepth == 0) {
-  if (__sanitizer::Verbosity())
+  if (Verbosity())
 Report("Not initializing the ShadowStack since MaxStackDepth == 0.\n");
   TLD.StackSize = 0;
   TLD.StackEntries = 0;
@@ -142,13 +140,6 @@

[PATCH] D46998: [XRay][compiler-rt] Limit reliance on C++ ABI features

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 147453.
dberris added a comment.

- fixup: revert unnecessary change to clang


https://reviews.llvm.org/D46998

Files:
  compiler-rt/lib/xray/xray_basic_logging.cc
  compiler-rt/test/xray/TestCases/Posix/c-test.cc

Index: compiler-rt/test/xray/TestCases/Posix/c-test.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/c-test.cc
@@ -0,0 +1,4 @@
+// RUN: %clang_xray -g -o %t %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+int main() {}
Index: compiler-rt/lib/xray/xray_basic_logging.cc
===
--- compiler-rt/lib/xray/xray_basic_logging.cc
+++ compiler-rt/lib/xray/xray_basic_logging.cc
@@ -16,7 +16,6 @@
 //===--===//
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -39,16 +38,16 @@
 
 namespace __xray {
 
-__sanitizer::SpinMutex LogMutex;
+SpinMutex LogMutex;
 
 // We use elements of this type to record the entry TSC of every function ID we
 // see as we're tracing a particular thread's execution.
 struct alignas(16) StackEntry {
   int32_t FuncId;
   uint16_t Type;
   uint8_t CPU;
   uint8_t Padding;
-  uint64_t TSC;
+  u64 TSC;
 };
 
 static_assert(sizeof(StackEntry) == 16, "Wrong size for StackEntry");
@@ -66,38 +65,35 @@
 
 static pthread_key_t PThreadKey;
 
-static __sanitizer::atomic_uint8_t BasicInitialized{0};
+static atomic_uint8_t BasicInitialized{0};
 
 BasicLoggingOptions GlobalOptions;
 
 thread_local volatile bool RecursionGuard = false;
 
-static uint64_t thresholdTicks() XRAY_NEVER_INSTRUMENT {
-  static uint64_t TicksPerSec = probeRequiredCPUFeatures()
-? getTSCFrequency()
-: __xray::NanosecondsPerSecond;
-  static const uint64_t ThresholdTicks =
-  TicksPerSec * GlobalOptions.DurationFilterMicros / 100;
-  return ThresholdTicks;
-}
+static atomic_uint8_t UseRealTSC{0};
+static atomic_uint64_t ThresholdTicks{0};
+static atomic_uint64_t TicksPerSec{0};
+static atomic_uint64_t CycleFrequency{__xray::NanosecondsPerSecond};
 
 static int openLogFile() XRAY_NEVER_INSTRUMENT {
   int F = getLogFD();
   if (F == -1)
 return -1;
 
-  // Test for required CPU features and cache the cycle frequency
-  static bool TSCSupported = probeRequiredCPUFeatures();
-  static uint64_t CycleFrequency =
-  TSCSupported ? getTSCFrequency() : __xray::NanosecondsPerSecond;
+  static pthread_once_t DetectOnce;
+  pthread_once(&DetectOnce, +[] {
+if (atomic_load(&UseRealTSC, memory_order_relaxed))
+  atomic_store(&CycleFrequency, getTSCFrequency(), memory_order_release);
+  });
 
   // Since we're here, we get to write the header. We set it up so that the
   // header will only be written once, at the start, and let the threads
   // logging do writes which just append.
   XRayFileHeader Header;
   Header.Version = 2; // Version 2 includes tail exit records.
   Header.Type = FileTypes::NAIVE_LOG;
-  Header.CycleFrequency = CycleFrequency;
+  Header.CycleFrequency = atomic_load(&CycleFrequency, memory_order_relaxed);
 
   // FIXME: Actually check whether we have 'constant_tsc' and 'nonstop_tsc'
   // before setting the values in the header.
@@ -108,29 +104,31 @@
   return F;
 }
 
-int getGlobalFd() XRAY_NEVER_INSTRUMENT {
-  static int Fd = openLogFile();
+static int getGlobalFd() XRAY_NEVER_INSTRUMENT {
+  static pthread_once_t OnceInit;
+  static int Fd = 0;
+  pthread_once(&OnceInit, +[] { Fd = openLogFile(); });
   return Fd;
 }
 
 ThreadLocalData &getThreadLocalData() XRAY_NEVER_INSTRUMENT {
   thread_local ThreadLocalData TLD;
   thread_local bool UNUSED TOnce = [] {
 if (GlobalOptions.ThreadBufferSize == 0) {
-  if (__sanitizer::Verbosity())
+  if (Verbosity())
 Report("Not initializing TLD since ThreadBufferSize == 0.\n");
   return false;
 }
-TLD.TID = __sanitizer::GetTid();
+TLD.TID = GetTid();
 pthread_setspecific(PThreadKey, &TLD);
 TLD.Fd = getGlobalFd();
 TLD.InMemoryBuffer = reinterpret_cast(
 InternalAlloc(sizeof(XRayRecord) * GlobalOptions.ThreadBufferSize,
   nullptr, alignof(XRayRecord)));
 TLD.BufferSize = GlobalOptions.ThreadBufferSize;
 TLD.BufferOffset = 0;
 if (GlobalOptions.MaxStackDepth == 0) {
-  if (__sanitizer::Verbosity())
+  if (Verbosity())
 Report("Not initializing the ShadowStack since MaxStackDepth == 0.\n");
   TLD.StackSize = 0;
   TLD.StackEntries = 0;
@@ -142,13 +140,6 @@
   alignof(StackEntry)));
 TLD.StackSize = GlobalOptions.MaxStackDepth;
 TLD.StackEntries = 0;
-if (__sanitizer::Verbosity() >= 2) {
-  static auto UNUSED Once = [] {
-auto ticks = thresholdTicks();
-Report("Ticks threshold: %d\n", ticks);
-return false;
-  }();
-}
 retur

[PATCH] D38226: [XRay][Driver] Do not link in XRay runtime in shared libs

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 116631.
dberris added a comment.

- fixup: limit tests to run on supported platforms and oses


https://reviews.llvm.org/D38226

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/XRay/lit.local.cfg
  test/Driver/XRay/xray-shared-noxray.cpp


Index: test/Driver/XRay/xray-shared-noxray.cpp
===
--- /dev/null
+++ test/Driver/XRay/xray-shared-noxray.cpp
@@ -0,0 +1,16 @@
+// RUN: %clangxx -shared -fPIC -o /dev/null -v -fxray-instrument %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=SHARED
+// RUN: %clangxx -static -o /dev/null -v -fxray-instrument %s 2>&1 -DMAIN | \
+// RUN: FileCheck %s --check-prefix=STATIC
+// RUN: %clangxx -static -fPIE -o /dev/null -v -fxray-instrument %s 2>&1 \
+// RUN: -DMAIN | FileCheck %s --check-prefix=STATIC
+//
+// SHARED-NOT: {{clang_rt\.xray-}}
+// STATIC: {{clang_rt\.xray-}}
+//
+// REQUIRES: linux
+int foo() { return 42; }
+
+#ifdef MAIN
+int main() { return foo(); }
+#endif
Index: test/Driver/XRay/lit.local.cfg
===
--- test/Driver/XRay/lit.local.cfg
+++ test/Driver/XRay/lit.local.cfg
@@ -1,2 +1,20 @@
 target_triple_components = config.target_triple.split('-')
 config.available_features.update(target_triple_components)
+
+# Only run the tests in platforms where XRay instrumentation is supported.
+supported_targets = [
+'amd64', 'x86_64', 'x86_64h', 'arm7', 'arm8', 'aarch64', 'arm64',
+'powerpc64le', 'mips', 'mipsel', 'mips64', 'mips64el'
+]
+
+# Only on platforms we support.
+supported_oses = [
+'linux'
+]
+
+triple_set = set(target_triple_components)
+if len(triple_set.intersection(supported_targets)) == 0:
+  config.unsupported = True
+
+if len(triple_set.intersection(supported_oses)) == 0:
+  config.unsupported = True
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -206,13 +206,18 @@
 
 static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
ArgStringList &CmdArgs) {
+  // Do not add the XRay runtime to shared libraries.
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 CmdArgs.push_back("-whole-archive");
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
 CmdArgs.push_back("-no-whole-archive");
 return true;
   }
+
   return false;
 }
 


Index: test/Driver/XRay/xray-shared-noxray.cpp
===
--- /dev/null
+++ test/Driver/XRay/xray-shared-noxray.cpp
@@ -0,0 +1,16 @@
+// RUN: %clangxx -shared -fPIC -o /dev/null -v -fxray-instrument %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=SHARED
+// RUN: %clangxx -static -o /dev/null -v -fxray-instrument %s 2>&1 -DMAIN | \
+// RUN: FileCheck %s --check-prefix=STATIC
+// RUN: %clangxx -static -fPIE -o /dev/null -v -fxray-instrument %s 2>&1 \
+// RUN: -DMAIN | FileCheck %s --check-prefix=STATIC
+//
+// SHARED-NOT: {{clang_rt\.xray-}}
+// STATIC: {{clang_rt\.xray-}}
+//
+// REQUIRES: linux
+int foo() { return 42; }
+
+#ifdef MAIN
+int main() { return foo(); }
+#endif
Index: test/Driver/XRay/lit.local.cfg
===
--- test/Driver/XRay/lit.local.cfg
+++ test/Driver/XRay/lit.local.cfg
@@ -1,2 +1,20 @@
 target_triple_components = config.target_triple.split('-')
 config.available_features.update(target_triple_components)
+
+# Only run the tests in platforms where XRay instrumentation is supported.
+supported_targets = [
+'amd64', 'x86_64', 'x86_64h', 'arm7', 'arm8', 'aarch64', 'arm64',
+'powerpc64le', 'mips', 'mipsel', 'mips64', 'mips64el'
+]
+
+# Only on platforms we support.
+supported_oses = [
+'linux'
+]
+
+triple_set = set(target_triple_components)
+if len(triple_set.intersection(supported_targets)) == 0:
+  config.unsupported = True
+
+if len(triple_set.intersection(supported_oses)) == 0:
+  config.unsupported = True
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -206,13 +206,18 @@
 
 static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
ArgStringList &CmdArgs) {
+  // Do not add the XRay runtime to shared libraries.
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 CmdArgs.push_back("-whole-archive");
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
 CmdArgs.push_back("-no-whole-archive");
 return true;
   }
+
   return false;
 }
 
__

[PATCH] D38226: [XRay][Driver] Do not link in XRay runtime in shared libs

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 116637.
dberris added a comment.
Herald added a subscriber: srhines.

- fixup: use supported target list in test from clang implementation


https://reviews.llvm.org/D38226

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/XRay/lit.local.cfg
  test/Driver/XRay/xray-shared-noxray.cpp


Index: test/Driver/XRay/xray-shared-noxray.cpp
===
--- /dev/null
+++ test/Driver/XRay/xray-shared-noxray.cpp
@@ -0,0 +1,16 @@
+// RUN: %clangxx -shared -fPIC -o /dev/null -v -fxray-instrument %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=SHARED
+// RUN: %clangxx -static -o /dev/null -v -fxray-instrument %s 2>&1 -DMAIN | \
+// RUN: FileCheck %s --check-prefix=STATIC
+// RUN: %clangxx -static -fPIE -o /dev/null -v -fxray-instrument %s 2>&1 \
+// RUN: -DMAIN | FileCheck %s --check-prefix=STATIC
+//
+// SHARED-NOT: {{clang_rt\.xray-}}
+// STATIC: {{clang_rt\.xray-}}
+//
+// REQUIRES: linux
+int foo() { return 42; }
+
+#ifdef MAIN
+int main() { return foo(); }
+#endif
Index: test/Driver/XRay/lit.local.cfg
===
--- test/Driver/XRay/lit.local.cfg
+++ test/Driver/XRay/lit.local.cfg
@@ -1,2 +1,21 @@
 target_triple_components = config.target_triple.split('-')
 config.available_features.update(target_triple_components)
+
+# Only run the tests in platforms where XRay instrumentation is supported.
+supported_targets = [
+'amd64', 'x86_64', 'x86_64h', 'arm', 'aarch64', 'arm64', 'powerpc64le',
+'mips', 'mipsel', 'mips64', 'mips64el'
+]
+
+# Only on platforms we support.
+supported_oses = [
+'linux'
+]
+
+triple_set = set(target_triple_components)
+if len(triple_set.intersection(supported_targets)) == 0:
+  config.unsupported = True
+
+# Do not run for 'android' despite being linux.
+if len(triple_set.intersection(supported_oses)) == 0 or 'android' in 
triple_set:
+  config.unsupported = True
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -206,13 +206,18 @@
 
 static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
ArgStringList &CmdArgs) {
+  // Do not add the XRay runtime to shared libraries.
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 CmdArgs.push_back("-whole-archive");
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
 CmdArgs.push_back("-no-whole-archive");
 return true;
   }
+
   return false;
 }
 


Index: test/Driver/XRay/xray-shared-noxray.cpp
===
--- /dev/null
+++ test/Driver/XRay/xray-shared-noxray.cpp
@@ -0,0 +1,16 @@
+// RUN: %clangxx -shared -fPIC -o /dev/null -v -fxray-instrument %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=SHARED
+// RUN: %clangxx -static -o /dev/null -v -fxray-instrument %s 2>&1 -DMAIN | \
+// RUN: FileCheck %s --check-prefix=STATIC
+// RUN: %clangxx -static -fPIE -o /dev/null -v -fxray-instrument %s 2>&1 \
+// RUN: -DMAIN | FileCheck %s --check-prefix=STATIC
+//
+// SHARED-NOT: {{clang_rt\.xray-}}
+// STATIC: {{clang_rt\.xray-}}
+//
+// REQUIRES: linux
+int foo() { return 42; }
+
+#ifdef MAIN
+int main() { return foo(); }
+#endif
Index: test/Driver/XRay/lit.local.cfg
===
--- test/Driver/XRay/lit.local.cfg
+++ test/Driver/XRay/lit.local.cfg
@@ -1,2 +1,21 @@
 target_triple_components = config.target_triple.split('-')
 config.available_features.update(target_triple_components)
+
+# Only run the tests in platforms where XRay instrumentation is supported.
+supported_targets = [
+'amd64', 'x86_64', 'x86_64h', 'arm', 'aarch64', 'arm64', 'powerpc64le',
+'mips', 'mipsel', 'mips64', 'mips64el'
+]
+
+# Only on platforms we support.
+supported_oses = [
+'linux'
+]
+
+triple_set = set(target_triple_components)
+if len(triple_set.intersection(supported_targets)) == 0:
+  config.unsupported = True
+
+# Do not run for 'android' despite being linux.
+if len(triple_set.intersection(supported_oses)) == 0 or 'android' in triple_set:
+  config.unsupported = True
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -206,13 +206,18 @@
 
 static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
ArgStringList &CmdArgs) {
+  // Do not add the XRay runtime to shared libraries.
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 CmdArgs.push_back("-whole-archive");
 C

[PATCH] D38226: [XRay][Driver] Do not link in XRay runtime in shared libs

[Dean Michael Berris via Phabricator via cfe-commits]

dberris closed this revision.
dberris added a comment.

Already landed; manually closing.


https://reviews.llvm.org/D38226



___
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

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
Herald added a subscriber: mgorny.

This is a work-in-progress change that attempts to allow us to build
and use the XRay runtime in Darwin. Current state:

- Assembler files are broken due to assumptions about ELFisms.
- Test infrastructure is laid out, but we aren't able to test yet.

We also:

- Use a better preprocessor check for preinint array support.
- Only build for osx.
- Enable the use of -fxray-instrument (and other flags) in clang.

This patch uses the monorepo layout for the chnage.


https://reviews.llvm.org/D39114

Files:
  clang/lib/Driver/XRayArgs.cpp
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/tests/CMakeLists.txt
  compiler-rt/lib/xray/xray_init.cc
  compiler-rt/test/xray/TestCases/Darwin/always-never-instrument.cc
  compiler-rt/test/xray/TestCases/Darwin/lit.local.cfg
  compiler-rt/test/xray/TestCases/Linux/lit.local.cfg
  compiler-rt/test/xray/lit.cfg

Index: compiler-rt/test/xray/lit.cfg
===
--- compiler-rt/test/xray/lit.cfg
+++ compiler-rt/test/xray/lit.cfg
@@ -40,7 +40,7 @@
 # Default test suffixes.
 config.suffixes = ['.c', '.cc', '.cpp']
 
-if config.host_os not in ['Linux']:
+if config.host_os not in ['Linux', 'Darwin']:
   config.unsupported = True
 elif '64' not in config.host_arch:
   if 'arm' in config.host_arch:
Index: compiler-rt/test/xray/TestCases/Linux/lit.local.cfg
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Linux/lit.local.cfg
@@ -0,0 +1,9 @@
+def getRoot(config):
+  if not config.parent:
+return config
+  return getRoot(config.parent)
+
+root = getRoot(config)
+
+if root.host_os not in ['Linux']:
+  config.unsupported = True
Index: compiler-rt/test/xray/TestCases/Darwin/lit.local.cfg
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Darwin/lit.local.cfg
@@ -0,0 +1,9 @@
+def getRoot(config):
+  if not config.parent:
+return config
+  return getRoot(config.parent)
+
+root = getRoot(config)
+
+if root.host_os not in ['Darwin']:
+  config.unsupported = True
Index: compiler-rt/test/xray/TestCases/Darwin/always-never-instrument.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Darwin/always-never-instrument.cc
@@ -0,0 +1,23 @@
+// Test that the always/never instrument lists apply.
+// RUN: echo "fun:main" > %tmp-always.txt
+// RUN: echo "fun:__xray*" > %tmp-never.txt
+// RUN: %clangxx_xray \
+// RUN: -fxray-never-instrument=%tmp-never.txt \
+// RUN: -fxray-always-instrument=%tmp-always.txt \
+// RUN: %s -o %t
+// RUN: %llvm_xray extract -symbolize %t | \
+// RUN:FileCheck %s --check-prefix NOINSTR
+// RUN: %llvm_xray extract -symbolize %t | \
+// RUN:FileCheck %s --check-prefix ALWAYSINSTR
+// REQUIRES: x86_64-linux
+// REQUIRES: built-in-llvm-tree
+
+// NOINSTR-NOT: {{.*__xray_NeverInstrumented.*}}
+int __xray_NeverInstrumented() {
+  return 0;
+}
+
+// ALWAYSINSTR: {{.*function-name:.*main.*}}
+int main(int argc, char *argv[]) {
+  return __xray_NeverInstrumented();
+}
Index: compiler-rt/lib/xray/xray_init.cc
===
--- compiler-rt/lib/xray/xray_init.cc
+++ compiler-rt/lib/xray/xray_init.cc
@@ -88,7 +88,8 @@
 #endif
 }
 
-#ifndef XRAY_NO_PREINIT
+// Only add the preinit array initialization if the sanitizers can.
+#if !defined(XRAY_NO_PREINIT) && SANITIZER_CAN_USE_PREINIT_ARRAY
 __attribute__((section(".preinit_array"),
used)) void (*__local_xray_preinit)(void) = __xray_init;
 #endif
Index: compiler-rt/lib/xray/tests/CMakeLists.txt
===
--- compiler-rt/lib/xray/tests/CMakeLists.txt
+++ compiler-rt/lib/xray/tests/CMakeLists.txt
@@ -12,9 +12,19 @@
   -I${COMPILER_RT_SOURCE_DIR}/lib)
 
 set(XRAY_TEST_ARCH ${XRAY_SUPPORTED_ARCH})
+set(XRAY_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LIBRT -lrt XRAY_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LIBM -lm XRAY_LINK_FLAGS)
+append_list_if(COMPILER_RT_HAS_LIBPTHREAD -lpthread XRAY_LINK_FLAGS)
+if (APPLE)
+  list(APPEND XRAY_LINK_FLAGS -lc++)
+else()
+  append_list_if(COMPILER_RT_HAS_LIBSTDCXX lstdc++ XRAY_LINK_FLAGS)
+endif()
+
 macro(add_xray_unittest testname)
   cmake_parse_arguments(TEST "" "" "SOURCES;HEADERS" ${ARGN})
-  if(UNIX AND NOT APPLE)
+  if(UNIX)
 foreach(arch ${XRAY_TEST_ARCH})
   set(TEST_OBJECTS)
   generate_compiler_rt_tests(TEST_OBJECTS
@@ -24,9 +34,8 @@
 CFLAGS ${XRAY_UNITTEST_CFLAGS}
 LINK_FLAGS -fxray-instrument
   ${TARGET_LINK_FLAGS}
-  -lstdc++ -lm ${CMAKE_THREAD_LIBS_INIT}
-  -lpthread
-  -ldl -lrt)
+  ${CMAKE_THREAD_LIBS_INIT}
+  ${XRAY_LINK_FLAGS})
   set_target_properties(XRayUnitTests PROPERTIES RUNTIME_O

[PATCH] D51269: [Xray] Darwin - Enable in the driver side

[Dean Michael Berris via Phabricator via cfe-commits]

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

Thanks for this @devnexen -- but do any of the tests actually run on Darwin yet?


Repository:
  rC Clang

https://reviews.llvm.org/D51269



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

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: mboerger, eizan.
Herald added subscribers: dexonsmith, mehdi_amini.

Before this change, we only emit the XRay attributes in LLVM IR when the
-fxray-instrument flag is provided. This may cause issues with thinlto
when the final binary is being built/linked with -fxray-instrument, and
the constitutent LLVM IR gets re-lowered with xray instrumentation.

With this change, we can honour the attributes provided in the source
code and preserve those in the IR. This way, even in thinlto builds, we
retain the attributes which say whether functions should never be XRay
instrumented, or whether they must always be XRay instrumented.

This change addresses llvm.org/PR38922.


https://reviews.llvm.org/D52015

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/xray-attributes-supported.cpp


Index: clang/test/CodeGen/xray-attributes-supported.cpp
===
--- clang/test/CodeGen/xray-attributes-supported.cpp
+++ clang/test/CodeGen/xray-attributes-supported.cpp
@@ -6,6 +6,18 @@
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mips64el-unknown-linux-gnu | FileCheck %s
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple powerpc64le-unknown-linux-gnu | FileCheck %s
 
+// We also want to ensure that the attributes show up even if we explicitly 
turn
+// off XRay instrumentation. We let the back-end decide whether to honour the
+// attributes instead.
+//
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple powerpc64le-unknown-linux-gnu | FileCheck %s
+
 // Make sure that the LLVM attribute for XRay-annotated functions do show up.
 [[clang::xray_always_instrument]] void foo() {
 // CHECK: define void @_Z3foov() #0
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1967,9 +1967,6 @@
 
 bool CodeGenModule::imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc,
StringRef Category) const {
-  if (!LangOpts.XRayInstrument)
-return false;
-
   const auto &XRayFilter = getContext().getXRayFilter();
   using ImbueAttr = XRayFunctionFilter::ImbueAttribute;
   auto Attr = ImbueAttr::NONE;
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -901,21 +901,20 @@
   }
 
   // Apply xray attributes to the function (as a string, for now)
-  bool InstrumentXray = ShouldXRayInstrumentFunction() &&
-CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::Function);
-  if (D && InstrumentXray) {
+  if (D) {
 if (const auto *XRayAttr = D->getAttr()) {
-  if (XRayAttr->alwaysXRayInstrument())
-Fn->addFnAttr("function-instrument", "xray-always");
-  if (XRayAttr->neverXRayInstrument())
-Fn->addFnAttr("function-instrument", "xray-never");
-  if (const auto *LogArgs = D->getAttr()) {
-Fn->addFnAttr("xray-log-args",
-  llvm::utostr(LogArgs->getArgumentCount()));
+  if (CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::Function)) {
+if (XRayAttr->alwaysXRayInstrument())
+  Fn->addFnAttr("function-instrument", "xray-always");
+if (XRayAttr->neverXRayInstrument())
+  Fn->addFnAttr("function-instrument", "xray-never");
+if (const auto *LogArgs = D->getAttr())
+  Fn->addFnAttr("xray-log-args",
+llvm::utostr(LogArgs->getArgumentCount()));
   }
 } else {
-  if (!CGM.imbueXRayAttrs(Fn, Loc))
+  if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc))
 Fn->addFnAttr(
 "xray-instruction-threshold",
 llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold));


Index: clang/test/CodeGen/xray-attributes-supported.cpp
===

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Dean Michael Berris via Phabricator via cfe-commits]

dberris planned changes to this revision.
dberris added a comment.

This change is still incomplete -- we should really only convey that the 
'never' attribute gets preserved. Otherwise we're going to have to invent a way 
to communicate to the XRay pass in LLVM that we should ignore the XRay 
attributes.


https://reviews.llvm.org/D52015



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

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 165219.
dberris added a subscriber: llvm-commits.
dberris added a comment.
This revision is now accepted and ready to land.

Adding an end-to-end test in compiler-rt to ensure that we are not suddenly 
instrumenting functions that must not be instrumented. Making this a 
version-locked commit between compiler-rt and clang.


https://reviews.llvm.org/D52015

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/xray-attributes-supported.cpp
  compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc

Index: compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
@@ -0,0 +1,11 @@
+// Test that we cannot actually find XRay instrumentation when we build with
+// -fno-xray-instrument but have code that's marked as 'xray_always_instrument'.
+//
+// RUN: %clangxx -fno-xray-instrument -c %s -o %t.o
+// RUN: not %llvm_xray extract -symbolize %t.o 2>&1 | FileCheck %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+
+// CHECK: llvm-xray: Cannot extract instrumentation map
+// CHECK-NOT: {{.*always_instrumented.*}}
+[[clang::xray_always_instrument]] int always_instrumented() { return 42; }
Index: clang/test/CodeGen/xray-attributes-supported.cpp
===
--- clang/test/CodeGen/xray-attributes-supported.cpp
+++ clang/test/CodeGen/xray-attributes-supported.cpp
@@ -1,19 +1,57 @@
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+
+// We also want to ensure that the attributes show up even if we explicitly turn
+// off XRay instrumentation. We let the back-end decide whether to honour the
+// attributes instead.
+//
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | \
+// RUN; FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
 
 // Make sure that the LLVM attribute for 

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 165220.
dberris edited the summary of this revision.
dberris added a comment.

Revise description.


https://reviews.llvm.org/D52015

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/xray-attributes-supported.cpp
  compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc

Index: compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
@@ -0,0 +1,11 @@
+// Test that we cannot actually find XRay instrumentation when we build with
+// -fno-xray-instrument but have code that's marked as 'xray_always_instrument'.
+//
+// RUN: %clangxx -fno-xray-instrument -c %s -o %t.o
+// RUN: not %llvm_xray extract -symbolize %t.o 2>&1 | FileCheck %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+
+// CHECK: llvm-xray: Cannot extract instrumentation map
+// CHECK-NOT: {{.*always_instrumented.*}}
+[[clang::xray_always_instrument]] int always_instrumented() { return 42; }
Index: clang/test/CodeGen/xray-attributes-supported.cpp
===
--- clang/test/CodeGen/xray-attributes-supported.cpp
+++ clang/test/CodeGen/xray-attributes-supported.cpp
@@ -1,19 +1,57 @@
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+
+// We also want to ensure that the attributes show up even if we explicitly turn
+// off XRay instrumentation. We let the back-end decide whether to honour the
+// attributes instead.
+//
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | \
+// RUN; FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
 
 // Make sure that the LLVM attribute for XRay-annotated functions do show up.
 [[clang::xray_always_instrument]] void foo() {
 // CHECK: define void @_Z3foov() #0
+// NOXRAY: define void @_Z3foov() #0
 };
 
 [[clang::xray_never_instrument]] void bar() {
 // CHECK: de

[PATCH] D52015: [XRay][clang] Emit "never-instrument" attribute

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 165407.
dberris retitled this revision from "[XRay][clang] Always emit XRay attributes 
for LLVM IR" to "[XRay][clang] Emit "never-instrument" attribute".
dberris added a comment.

Retitle, add different test case for -fno-xray-instrument.


https://reviews.llvm.org/D52015

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/xray-attributes-noxray-supported.cpp
  clang/test/CodeGen/xray-attributes-supported.cpp
  compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc

Index: compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
@@ -0,0 +1,11 @@
+// Test that we cannot actually find XRay instrumentation when we build with
+// -fno-xray-instrument but have code that's marked as 'xray_always_instrument'.
+//
+// RUN: %clangxx -fno-xray-instrument -c %s -o %t.o
+// RUN: not %llvm_xray extract -symbolize %t.o 2>&1 | FileCheck %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+
+// CHECK: llvm-xray: Cannot extract instrumentation map
+// CHECK-NOT: {{.*always_instrumented.*}}
+[[clang::xray_always_instrument]] int always_instrumented() { return 42; }
Index: clang/test/CodeGen/xray-attributes-supported.cpp
===
--- clang/test/CodeGen/xray-attributes-supported.cpp
+++ clang/test/CodeGen/xray-attributes-supported.cpp
@@ -1,10 +1,17 @@
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s
 
 // Make sure that the LLVM attribute for XRay-annotated functions do show up.
 [[clang::xray_always_instrument]] void foo() {
Index: clang/test/CodeGen/xray-attributes-noxray-supported.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-attributes-noxray-supported.cpp
@@ -0,0 +1,29 @@
+// We want to ensure that the "never instrument" attributes show up even if we
+// explicitly turn off XRay instrumentation.
+//
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+
+[[clang::xray_always_instrument]] voi

[PATCH] D52015: [XRay][clang] Emit "never-instrument" attribute

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342200: [XRay][clang] Emit "never-instrument" 
attribute (authored by dberris, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52015?vs=165407&id=165408#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52015

Files:
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGen/xray-attributes-noxray-supported.cpp
  test/CodeGen/xray-attributes-supported.cpp

Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1967,9 +1967,6 @@
 
 bool CodeGenModule::imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc,
StringRef Category) const {
-  if (!LangOpts.XRayInstrument)
-return false;
-
   const auto &XRayFilter = getContext().getXRayFilter();
   using ImbueAttr = XRayFunctionFilter::ImbueAttribute;
   auto Attr = ImbueAttr::NONE;
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -901,21 +901,21 @@
   }
 
   // Apply xray attributes to the function (as a string, for now)
-  bool InstrumentXray = ShouldXRayInstrumentFunction() &&
-CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::Function);
-  if (D && InstrumentXray) {
+  if (D) {
 if (const auto *XRayAttr = D->getAttr()) {
-  if (XRayAttr->alwaysXRayInstrument())
-Fn->addFnAttr("function-instrument", "xray-always");
-  if (XRayAttr->neverXRayInstrument())
-Fn->addFnAttr("function-instrument", "xray-never");
-  if (const auto *LogArgs = D->getAttr()) {
-Fn->addFnAttr("xray-log-args",
-  llvm::utostr(LogArgs->getArgumentCount()));
+  if (CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::Function)) {
+if (XRayAttr->alwaysXRayInstrument() && ShouldXRayInstrumentFunction())
+  Fn->addFnAttr("function-instrument", "xray-always");
+if (XRayAttr->neverXRayInstrument())
+  Fn->addFnAttr("function-instrument", "xray-never");
+if (const auto *LogArgs = D->getAttr())
+  if (ShouldXRayInstrumentFunction())
+Fn->addFnAttr("xray-log-args",
+  llvm::utostr(LogArgs->getArgumentCount()));
   }
 } else {
-  if (!CGM.imbueXRayAttrs(Fn, Loc))
+  if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc))
 Fn->addFnAttr(
 "xray-instruction-threshold",
 llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold));
Index: test/CodeGen/xray-attributes-supported.cpp
===
--- test/CodeGen/xray-attributes-supported.cpp
+++ test/CodeGen/xray-attributes-supported.cpp
@@ -1,10 +1,17 @@
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s
 
 // Make sure that the LLVM attribute for XRay-annotated functions do show up.

[PATCH] D43110: [Sema] Don't mark plain MS enums as fixed

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D43110



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

[PATCH] D43279: Add Xray instrumentation compile-time/link-time support to FreeBSD

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D43279



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

[PATCH] D43378: FreeBSD driver / Xray flags moving pthread to compile flags.

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

Looks OK, but can we have a test for this somehow?


https://reviews.llvm.org/D43378



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

[PATCH] D33392: [XRay][clang] Allow imbuing arg1 logging attribute via -fxray-always-instrument=

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL303719: [XRay][clang] Allow imbuing arg1 logging attribute 
via -fxray-always-instrument= (authored by dberris).

Changed prior to commit:
  https://reviews.llvm.org/D33392?vs=99715&id=100042#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33392

Files:
  cfe/trunk/include/clang/Basic/XRayLists.h
  cfe/trunk/lib/Basic/XRayLists.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp


Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -1508,6 +1508,10 @@
   case ImbueAttr::ALWAYS:
 Fn->addFnAttr("function-instrument", "xray-always");
 break;
+  case ImbueAttr::ALWAYS_ARG1:
+Fn->addFnAttr("function-instrument", "xray-always");
+Fn->addFnAttr("xray-log-args", "1");
+break;
   case ImbueAttr::NEVER:
 Fn->addFnAttr("function-instrument", "xray-never");
 break;
Index: cfe/trunk/lib/Basic/XRayLists.cpp
===
--- cfe/trunk/lib/Basic/XRayLists.cpp
+++ cfe/trunk/lib/Basic/XRayLists.cpp
@@ -26,6 +26,8 @@
 XRayFunctionFilter::shouldImbueFunction(StringRef FunctionName) const {
   // First apply the always instrument list, than if it isn't an "always" see
   // whether it's treated as a "never" instrument function.
+  if (AlwaysInstrument->inSection("fun", FunctionName, "arg1"))
+return ImbueAttribute::ALWAYS_ARG1;
   if (AlwaysInstrument->inSection("fun", FunctionName))
 return ImbueAttribute::ALWAYS;
   if (NeverInstrument->inSection("fun", FunctionName))
Index: cfe/trunk/include/clang/Basic/XRayLists.h
===
--- cfe/trunk/include/clang/Basic/XRayLists.h
+++ cfe/trunk/include/clang/Basic/XRayLists.h
@@ -37,6 +37,7 @@
 NONE,
 ALWAYS,
 NEVER,
+ALWAYS_ARG1,
   };
 
   ImbueAttribute shouldImbueFunction(StringRef FunctionName) const;
Index: cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp
===
--- cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp
+++ cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp
@@ -0,0 +1,12 @@
+// RUN: echo "fun:*arg1*=arg1" >> %t.always-instrument
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 
-fxray-always-instrument=%t.always-instrument -emit-llvm -o - %s -triple 
x86_64-unknown-linux-gnu | FileCheck %s
+
+void foo() {}
+
+void arg1(void*) {}
+
+// CHECK: define void @_Z3foov() #[[FOO:[0-9]+]] {
+// CHECK: define void {{.*}}arg1{{.*}} #[[ALWAYSARG1:[0-9]+]] {
+
+// CHECK: attributes #[[FOO]] = {{.*}}
+// CHECK: attributes #[[ALWAYSARG1]] = {{.*}} 
"function-instrument"="xray-always" {{.*}} "xray-log-args"="1"


Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -1508,6 +1508,10 @@
   case ImbueAttr::ALWAYS:
 Fn->addFnAttr("function-instrument", "xray-always");
 break;
+  case ImbueAttr::ALWAYS_ARG1:
+Fn->addFnAttr("function-instrument", "xray-always");
+Fn->addFnAttr("xray-log-args", "1");
+break;
   case ImbueAttr::NEVER:
 Fn->addFnAttr("function-instrument", "xray-never");
 break;
Index: cfe/trunk/lib/Basic/XRayLists.cpp
===
--- cfe/trunk/lib/Basic/XRayLists.cpp
+++ cfe/trunk/lib/Basic/XRayLists.cpp
@@ -26,6 +26,8 @@
 XRayFunctionFilter::shouldImbueFunction(StringRef FunctionName) const {
   // First apply the always instrument list, than if it isn't an "always" see
   // whether it's treated as a "never" instrument function.
+  if (AlwaysInstrument->inSection("fun", FunctionName, "arg1"))
+return ImbueAttribute::ALWAYS_ARG1;
   if (AlwaysInstrument->inSection("fun", FunctionName))
 return ImbueAttribute::ALWAYS;
   if (NeverInstrument->inSection("fun", FunctionName))
Index: cfe/trunk/include/clang/Basic/XRayLists.h
===
--- cfe/trunk/include/clang/Basic/XRayLists.h
+++ cfe/trunk/include/clang/Basic/XRayLists.h
@@ -37,6 +37,7 @@
 NONE,
 ALWAYS,
 NEVER,
+ALWAYS_ARG1,
   };
 
   ImbueAttribute shouldImbueFunction(StringRef FunctionName) const;
Index: cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp
===
--- cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp
+++ cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp
@@ -0,0 +1,12 @@
+// RUN: echo "fun:*arg1*=arg1" >> %t.always-instrument
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -fxray-always-instrument=%t.always-instrument -emit-llvm -o - %s -triple x86_64-unknown-linux-gnu | FileCheck %s
+
+void foo() {}
+
+void arg1(void*) {}
+
+// CHECK: define void @_Z3fo

[PATCH] D34050: Support capturing the implicit `this` argument to C++ class member functions

[Dean Michael Berris via Phabricator via cfe-commits]

dberris abandoned this revision.
dberris added a comment.

Fail... it's been a while since I've done this. Will try again.


https://reviews.llvm.org/D34050



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

[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.

Before this change, we couldn't capture the `this` pointer that's
implicitly the first argument of class member functions. There are some
interesting things we can do with capturing even just this single
argument for zero-argument member functions.


https://reviews.llvm.org/D34052

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/xray-log-args-class.cpp


Index: test/Sema/xray-log-args-class.cpp
===
--- /dev/null
+++ test/Sema/xray-log-args-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+
+class Class {
+  [[clang::xray_always_instrument, clang::xray_log_args(1)]] void Method();
+  [[clang::xray_log_args(-1)]] void Invalid(); // expected-error 
{{'xray_log_args' attribute parameter 1 is out of bounds}}
+  [[clang::xray_log_args("invalid")]] void InvalidStringArg(); // 
expected-error {{'xray_log_args'}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4604,14 +4604,40 @@
 static void handleXRayLogArgsAttr(Sema &S, Decl *D,
   const AttributeList &Attr) {
   uint64_t ArgCount;
+
+  // Treat member functions especially, since we do want to support saving the
+  // implicit this argument for member functions.
+  if (isInstanceMethod(D)) {
+auto *IdxExpr = Attr.getArgAsExpr(0);
+llvm::APSInt IdxInt;
+if (!IdxExpr->isIntegerConstantExpr(IdxInt, S.Context)) {
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_n_type)
+  << getAttrName(Attr) << 1u << AANT_ArgumentIntegerConstant
+  << IdxExpr->getSourceRange();
+  return;
+}
+ArgCount = IdxInt.getLimitedValue();
+bool HP = hasFunctionProto(D);
+unsigned NumParams = HP ? getFunctionOrMethodNumParams(D) + 1 : 1;
+if (ArgCount > NumParams) {
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_out_of_bounds)
+  << getAttrName(Attr) << 1 << IdxExpr->getSourceRange();
+  return;
+}
+D->addAttr(::new (S.Context)
+   XRayLogArgsAttr(Attr.getRange(), S.Context, ArgCount,
+   Attr.getAttributeSpellingListIndex()));
+return;
+  }
+
   if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, Attr.getArgAsExpr(0),
ArgCount))
 return;
 
   // ArgCount isn't a parameter index [0;n), it's a count [1;n] - hence + 1.
   D->addAttr(::new (S.Context)
- XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
- Attr.getAttributeSpellingListIndex()));
+ XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
+ Attr.getAttributeSpellingListIndex()));
 }
 
 
//===--===//


Index: test/Sema/xray-log-args-class.cpp
===
--- /dev/null
+++ test/Sema/xray-log-args-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+
+class Class {
+  [[clang::xray_always_instrument, clang::xray_log_args(1)]] void Method();
+  [[clang::xray_log_args(-1)]] void Invalid(); // expected-error {{'xray_log_args' attribute parameter 1 is out of bounds}}
+  [[clang::xray_log_args("invalid")]] void InvalidStringArg(); // expected-error {{'xray_log_args'}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4604,14 +4604,40 @@
 static void handleXRayLogArgsAttr(Sema &S, Decl *D,
   const AttributeList &Attr) {
   uint64_t ArgCount;
+
+  // Treat member functions especially, since we do want to support saving the
+  // implicit this argument for member functions.
+  if (isInstanceMethod(D)) {
+auto *IdxExpr = Attr.getArgAsExpr(0);
+llvm::APSInt IdxInt;
+if (!IdxExpr->isIntegerConstantExpr(IdxInt, S.Context)) {
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_n_type)
+  << getAttrName(Attr) << 1u << AANT_ArgumentIntegerConstant
+  << IdxExpr->getSourceRange();
+  return;
+}
+ArgCount = IdxInt.getLimitedValue();
+bool HP = hasFunctionProto(D);
+unsigned NumParams = HP ? getFunctionOrMethodNumParams(D) + 1 : 1;
+if (ArgCount > NumParams) {
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_out_of_bounds)
+  << getAttrName(Attr) << 1 << IdxExpr->getSourceRange();
+  return;
+}
+D->addAttr(::new (S.Context)
+   XRayLogArgsAttr(Attr.getRange(), S.Context, ArgCount,
+   Attr.getAttributeSpellingListIndex()));
+return;
+  }
+
   if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, Attr.getArgAsExpr(0),

[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 102018.
dberris edited the summary of this revision.
dberris added a comment.

Adding related bug.


https://reviews.llvm.org/D34052

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/xray-log-args-class.cpp


Index: test/Sema/xray-log-args-class.cpp
===
--- /dev/null
+++ test/Sema/xray-log-args-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+
+class Class {
+  [[clang::xray_always_instrument, clang::xray_log_args(1)]] void Method();
+  [[clang::xray_log_args(-1)]] void Invalid(); // expected-error 
{{'xray_log_args' attribute parameter 1 is out of bounds}}
+  [[clang::xray_log_args("invalid")]] void InvalidStringArg(); // 
expected-error {{'xray_log_args'}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4604,14 +4604,40 @@
 static void handleXRayLogArgsAttr(Sema &S, Decl *D,
   const AttributeList &Attr) {
   uint64_t ArgCount;
+
+  // Treat member functions especially, since we do want to support saving the
+  // implicit this argument for member functions.
+  if (isInstanceMethod(D)) {
+auto *IdxExpr = Attr.getArgAsExpr(0);
+llvm::APSInt IdxInt;
+if (!IdxExpr->isIntegerConstantExpr(IdxInt, S.Context)) {
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_n_type)
+  << getAttrName(Attr) << 1u << AANT_ArgumentIntegerConstant
+  << IdxExpr->getSourceRange();
+  return;
+}
+ArgCount = IdxInt.getLimitedValue();
+bool HP = hasFunctionProto(D);
+unsigned NumParams = HP ? getFunctionOrMethodNumParams(D) + 1 : 1;
+if (ArgCount > NumParams) {
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_out_of_bounds)
+  << getAttrName(Attr) << 1 << IdxExpr->getSourceRange();
+  return;
+}
+D->addAttr(::new (S.Context)
+   XRayLogArgsAttr(Attr.getRange(), S.Context, ArgCount,
+   Attr.getAttributeSpellingListIndex()));
+return;
+  }
+
   if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, Attr.getArgAsExpr(0),
ArgCount))
 return;
 
   // ArgCount isn't a parameter index [0;n), it's a count [1;n] - hence + 1.
   D->addAttr(::new (S.Context)
- XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
- Attr.getAttributeSpellingListIndex()));
+ XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
+ Attr.getAttributeSpellingListIndex()));
 }
 
 
//===--===//


Index: test/Sema/xray-log-args-class.cpp
===
--- /dev/null
+++ test/Sema/xray-log-args-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+
+class Class {
+  [[clang::xray_always_instrument, clang::xray_log_args(1)]] void Method();
+  [[clang::xray_log_args(-1)]] void Invalid(); // expected-error {{'xray_log_args' attribute parameter 1 is out of bounds}}
+  [[clang::xray_log_args("invalid")]] void InvalidStringArg(); // expected-error {{'xray_log_args'}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4604,14 +4604,40 @@
 static void handleXRayLogArgsAttr(Sema &S, Decl *D,
   const AttributeList &Attr) {
   uint64_t ArgCount;
+
+  // Treat member functions especially, since we do want to support saving the
+  // implicit this argument for member functions.
+  if (isInstanceMethod(D)) {
+auto *IdxExpr = Attr.getArgAsExpr(0);
+llvm::APSInt IdxInt;
+if (!IdxExpr->isIntegerConstantExpr(IdxInt, S.Context)) {
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_n_type)
+  << getAttrName(Attr) << 1u << AANT_ArgumentIntegerConstant
+  << IdxExpr->getSourceRange();
+  return;
+}
+ArgCount = IdxInt.getLimitedValue();
+bool HP = hasFunctionProto(D);
+unsigned NumParams = HP ? getFunctionOrMethodNumParams(D) + 1 : 1;
+if (ArgCount > NumParams) {
+  S.Diag(getAttrLoc(Attr), diag::err_attribute_argument_out_of_bounds)
+  << getAttrName(Attr) << 1 << IdxExpr->getSourceRange();
+  return;
+}
+D->addAttr(::new (S.Context)
+   XRayLogArgsAttr(Attr.getRange(), S.Context, ArgCount,
+   Attr.getAttributeSpellingListIndex()));
+return;
+  }
+
   if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, Attr.getArgAsExpr(0),
ArgCount))
 return;
 
   // ArgCount isn't a parameter index [0;n), it's a count [1;n] - hence + 1.
   D->addAttr(::new (S.C

[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a reviewer: dblaikie.
dberris added a subscriber: dblaikie.
dberris added a comment.

@dblaikie -- do you have time to have a look?


https://reviews.llvm.org/D34052



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

[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In https://reviews.llvm.org/D34052#778670, @dblaikie wrote:

> I take it there's already handling for these attributes on non-member 
> functions?


Yes, we're just extending it to also apply to the case where it doesn't support 
this one case where we actually do need the implicit this argument

> There's probably a reason this code can't be wherever that code is & subtract 
> one from the index? (reducing code duplication by having all the error 
> handling, etc, in one place/once)

I tried doing it for the `checkFunctionOrMethodNumParams` function, but it 
ended up being much more complicated. :(

The choice is between adding a bool argument (e.g. AllowImplicitThis) in 
`checkFunctionOrMethodParameterIndex(...)` that's default to always true (to 
preserve existing behaviour) but the checks and implementation of that template 
has a number of assumptions as to whether the index is valid for member 
functions.

I can change this so that the logic is contained in 
`checkFunctionOrMethodParameterIndex(...)` if that's more readable?


https://reviews.llvm.org/D34052



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

[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 102491.
dberris added a comment.

- fixup: use bool arg in checkFunctionOrMethodParameterIndex


https://reviews.llvm.org/D34052

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/xray-log-args-class.cpp


Index: test/Sema/xray-log-args-class.cpp
===
--- /dev/null
+++ test/Sema/xray-log-args-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+
+class Class {
+  [[clang::xray_always_instrument, clang::xray_log_args(1)]] void Method();
+  [[clang::xray_log_args(-1)]] void Invalid(); // expected-error 
{{'xray_log_args' attribute parameter 1 is out of bounds}}
+  [[clang::xray_log_args("invalid")]] void InvalidStringArg(); // 
expected-error {{'xray_log_args'}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -313,8 +313,8 @@
 /// \returns true if IdxExpr is a valid index.
 template 
 static bool checkFunctionOrMethodParameterIndex(
-Sema &S, const Decl *D, const AttrInfo& Attr,
-unsigned AttrArgNum, const Expr *IdxExpr, uint64_t &Idx) {
+Sema &S, const Decl *D, const AttrInfo &Attr, unsigned AttrArgNum,
+const Expr *IdxExpr, uint64_t &Idx, bool AllowImplicitThis = false) {
   assert(isFunctionOrMethodOrBlock(D));
 
   // In C++ the implicit 'this' function parameter also counts.
@@ -341,7 +341,7 @@
 return false;
   }
   Idx--; // Convert to zero-based.
-  if (HasImplicitThisParam) {
+  if (HasImplicitThisParam && !AllowImplicitThis) {
 if (Idx == 0) {
   S.Diag(getAttrLoc(Attr),
  diag::err_attribute_invalid_implicit_this_argument)
@@ -4604,14 +4604,16 @@
 static void handleXRayLogArgsAttr(Sema &S, Decl *D,
   const AttributeList &Attr) {
   uint64_t ArgCount;
+
   if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, Attr.getArgAsExpr(0),
-   ArgCount))
+   ArgCount,
+   true /* AllowImplicitThis*/))
 return;
 
   // ArgCount isn't a parameter index [0;n), it's a count [1;n] - hence + 1.
   D->addAttr(::new (S.Context)
- XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
- Attr.getAttributeSpellingListIndex()));
+ XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
+ Attr.getAttributeSpellingListIndex()));
 }
 
 
//===--===//


Index: test/Sema/xray-log-args-class.cpp
===
--- /dev/null
+++ test/Sema/xray-log-args-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+
+class Class {
+  [[clang::xray_always_instrument, clang::xray_log_args(1)]] void Method();
+  [[clang::xray_log_args(-1)]] void Invalid(); // expected-error {{'xray_log_args' attribute parameter 1 is out of bounds}}
+  [[clang::xray_log_args("invalid")]] void InvalidStringArg(); // expected-error {{'xray_log_args'}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -313,8 +313,8 @@
 /// \returns true if IdxExpr is a valid index.
 template 
 static bool checkFunctionOrMethodParameterIndex(
-Sema &S, const Decl *D, const AttrInfo& Attr,
-unsigned AttrArgNum, const Expr *IdxExpr, uint64_t &Idx) {
+Sema &S, const Decl *D, const AttrInfo &Attr, unsigned AttrArgNum,
+const Expr *IdxExpr, uint64_t &Idx, bool AllowImplicitThis = false) {
   assert(isFunctionOrMethodOrBlock(D));
 
   // In C++ the implicit 'this' function parameter also counts.
@@ -341,7 +341,7 @@
 return false;
   }
   Idx--; // Convert to zero-based.
-  if (HasImplicitThisParam) {
+  if (HasImplicitThisParam && !AllowImplicitThis) {
 if (Idx == 0) {
   S.Diag(getAttrLoc(Attr),
  diag::err_attribute_invalid_implicit_this_argument)
@@ -4604,14 +4604,16 @@
 static void handleXRayLogArgsAttr(Sema &S, Decl *D,
   const AttributeList &Attr) {
   uint64_t ArgCount;
+
   if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, Attr.getArgAsExpr(0),
-   ArgCount))
+   ArgCount,
+   true /* AllowImplicitThis*/))
 return;
 
   // ArgCount isn't a parameter index [0;n), it's a count [1;n] - hence + 1.
   D->addAttr(::new (S.Context)
- XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
- Attr.getAttributeSpellingListIndex()));
+ XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
+ Attr.getAttributeSpellingListIndex()))

[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

@dblaikie it turns out that this is a much simpler change now. PTAL?


https://reviews.llvm.org/D34052



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

[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305544: [XRay][clang] Support capturing the implicit `this` 
argument to C++ class… (authored by dberris).

Changed prior to commit:
  https://reviews.llvm.org/D34052?vs=102491&id=102775#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34052

Files:
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/Sema/xray-log-args-class.cpp


Index: cfe/trunk/test/Sema/xray-log-args-class.cpp
===
--- cfe/trunk/test/Sema/xray-log-args-class.cpp
+++ cfe/trunk/test/Sema/xray-log-args-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+
+class Class {
+  [[clang::xray_always_instrument, clang::xray_log_args(1)]] void Method();
+  [[clang::xray_log_args(-1)]] void Invalid(); // expected-error 
{{'xray_log_args' attribute parameter 1 is out of bounds}}
+  [[clang::xray_log_args("invalid")]] void InvalidStringArg(); // 
expected-error {{'xray_log_args'}}
+};
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -313,8 +313,8 @@
 /// \returns true if IdxExpr is a valid index.
 template 
 static bool checkFunctionOrMethodParameterIndex(
-Sema &S, const Decl *D, const AttrInfo& Attr,
-unsigned AttrArgNum, const Expr *IdxExpr, uint64_t &Idx) {
+Sema &S, const Decl *D, const AttrInfo &Attr, unsigned AttrArgNum,
+const Expr *IdxExpr, uint64_t &Idx, bool AllowImplicitThis = false) {
   assert(isFunctionOrMethodOrBlock(D));
 
   // In C++ the implicit 'this' function parameter also counts.
@@ -341,7 +341,7 @@
 return false;
   }
   Idx--; // Convert to zero-based.
-  if (HasImplicitThisParam) {
+  if (HasImplicitThisParam && !AllowImplicitThis) {
 if (Idx == 0) {
   S.Diag(getAttrLoc(Attr),
  diag::err_attribute_invalid_implicit_this_argument)
@@ -4604,14 +4604,16 @@
 static void handleXRayLogArgsAttr(Sema &S, Decl *D,
   const AttributeList &Attr) {
   uint64_t ArgCount;
+
   if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, Attr.getArgAsExpr(0),
-   ArgCount))
+   ArgCount,
+   true /* AllowImplicitThis*/))
 return;
 
   // ArgCount isn't a parameter index [0;n), it's a count [1;n] - hence + 1.
   D->addAttr(::new (S.Context)
- XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
- Attr.getAttributeSpellingListIndex()));
+ XRayLogArgsAttr(Attr.getRange(), S.Context, ++ArgCount,
+ Attr.getAttributeSpellingListIndex()));
 }
 
 
//===--===//


Index: cfe/trunk/test/Sema/xray-log-args-class.cpp
===
--- cfe/trunk/test/Sema/xray-log-args-class.cpp
+++ cfe/trunk/test/Sema/xray-log-args-class.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -std=c++11 -x c++
+
+class Class {
+  [[clang::xray_always_instrument, clang::xray_log_args(1)]] void Method();
+  [[clang::xray_log_args(-1)]] void Invalid(); // expected-error {{'xray_log_args' attribute parameter 1 is out of bounds}}
+  [[clang::xray_log_args("invalid")]] void InvalidStringArg(); // expected-error {{'xray_log_args'}}
+};
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -313,8 +313,8 @@
 /// \returns true if IdxExpr is a valid index.
 template 
 static bool checkFunctionOrMethodParameterIndex(
-Sema &S, const Decl *D, const AttrInfo& Attr,
-unsigned AttrArgNum, const Expr *IdxExpr, uint64_t &Idx) {
+Sema &S, const Decl *D, const AttrInfo &Attr, unsigned AttrArgNum,
+const Expr *IdxExpr, uint64_t &Idx, bool AllowImplicitThis = false) {
   assert(isFunctionOrMethodOrBlock(D));
 
   // In C++ the implicit 'this' function parameter also counts.
@@ -341,7 +341,7 @@
 return false;
   }
   Idx--; // Convert to zero-based.
-  if (HasImplicitThisParam) {
+  if (HasImplicitThisParam && !AllowImplicitThis) {
 if (Idx == 0) {
   S.Diag(getAttrLoc(Attr),
  diag::err_attribute_invalid_implicit_this_argument)
@@ -4604,14 +4604,16 @@
 static void handleXRayLogArgsAttr(Sema &S, Decl *D,
   const AttributeList &Attr) {
   uint64_t ArgCount;
+
   if (!checkFunctionOrMethodParameterIndex(S, D, Attr, 1, Attr.getArgAsExpr(0),
-   ArgCount))
+   ArgCount,
+   true /* AllowImplicitThis*/))
 return;
 
   // ArgCount isn'

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

I'm really excited by this clang-tidy check and I think it's worth having. I do 
think there's more work needed to bring it up to a level of quality that would 
be helpful to more users.




Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:6
+
+Finds functions which may throw an excpetion directly or indirectly, but they
+should not. The functions which should not throw exceptions are the following:

excpetion -> exception



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
+  implicit_int_thrower();

Can we make the warning more accurate here? Something like:

```
warning: call to 'implicit_int_thrower' may throw an exception and propagate 
through noexcept function 'indirect_implicit'
```

It would be helpful to diagnose the point at which the exception may be thrown 
from within the function (if it's an operator, a function call, etc.) that 
doesn't have exceptions handled. If you can highlight not just the line number 
but the actual expression in which the uncaught exception may propagate, it 
would make this warning much better.

If you think it's worth it (or if it's feasible), having a FixIt hint to wrap a 
block of statements where exceptions may propagate in a `try { ... } catch 
(...) { ... }` block would turn this warning from a good warning, to a great 
warning -- potentially something that could be automatically applied by a tool 
as well.


https://reviews.llvm.org/D33537



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

[PATCH] D54784: Use --push/pop-state with XRay link deps

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D54784



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

[PATCH] D52160: [Driver] Support XRay on Fuchsia

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D52160



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

[PATCH] D52342: [XRay][clang] Propagate -fxray-instrumentation-bundle to -cc1

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: mboerger, tejohnson.

Add a test and ensure that we propagate the
-fxray-instrumentation-bundle flag from the driver invocation to the
-cc1 options.


https://reviews.llvm.org/D52342

Files:
  clang/include/clang/Basic/XRayInstr.h
  clang/lib/Driver/XRayArgs.cpp
  clang/test/CodeGen/xray-instrumentation-bundles-flags.cpp


Index: clang/test/CodeGen/xray-instrumentation-bundles-flags.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-instrumentation-bundles-flags.cpp
@@ -0,0 +1,8 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-instrumentation-bundle= flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-instrumentation-bundle=function -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-instrumentation-bundle=function
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -215,4 +215,19 @@
 ModeOpt += Mode;
 CmdArgs.push_back(Args.MakeArgString(ModeOpt));
   }
+
+  SmallString<64> Bundle("-fxray-instrumentation-bundle=");
+  if (InstrumentationBundle.full()) {
+Bundle += "all";
+  } else if (InstrumentationBundle.empty()) {
+Bundle += "none";
+  } else {
+if (InstrumentationBundle.has(XRayInstrKind::Function))
+  Bundle += "function";
+if (InstrumentationBundle.has(XRayInstrKind::Custom))
+  Bundle += "custom";
+if (InstrumentationBundle.has(XRayInstrKind::Typed))
+  Bundle += "typed";
+  }
+  CmdArgs.push_back(Args.MakeArgString(Bundle));
 }
Index: clang/include/clang/Basic/XRayInstr.h
===
--- clang/include/clang/Basic/XRayInstr.h
+++ clang/include/clang/Basic/XRayInstr.h
@@ -60,6 +60,8 @@
 
   bool empty() const { return Mask == 0; }
 
+  bool full() const { return Mask == XRayInstrKind::All; }
+
   XRayInstrMask Mask = 0;
 };
 


Index: clang/test/CodeGen/xray-instrumentation-bundles-flags.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-instrumentation-bundles-flags.cpp
@@ -0,0 +1,8 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-instrumentation-bundle= flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-instrumentation-bundle=function -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-instrumentation-bundle=function
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -215,4 +215,19 @@
 ModeOpt += Mode;
 CmdArgs.push_back(Args.MakeArgString(ModeOpt));
   }
+
+  SmallString<64> Bundle("-fxray-instrumentation-bundle=");
+  if (InstrumentationBundle.full()) {
+Bundle += "all";
+  } else if (InstrumentationBundle.empty()) {
+Bundle += "none";
+  } else {
+if (InstrumentationBundle.has(XRayInstrKind::Function))
+  Bundle += "function";
+if (InstrumentationBundle.has(XRayInstrKind::Custom))
+  Bundle += "custom";
+if (InstrumentationBundle.has(XRayInstrKind::Typed))
+  Bundle += "typed";
+  }
+  CmdArgs.push_back(Args.MakeArgString(Bundle));
 }
Index: clang/include/clang/Basic/XRayInstr.h
===
--- clang/include/clang/Basic/XRayInstr.h
+++ clang/include/clang/Basic/XRayInstr.h
@@ -60,6 +60,8 @@
 
   bool empty() const { return Mask == 0; }
 
+  bool full() const { return Mask == XRayInstrKind::All; }
+
   XRayInstrMask Mask = 0;
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52342: [XRay][clang] Propagate -fxray-instrumentation-bundle to -cc1

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 166410.
dberris added a comment.

Move the test to the driver, more appropriate location.


https://reviews.llvm.org/D52342

Files:
  clang/include/clang/Basic/XRayInstr.h
  clang/lib/Driver/XRayArgs.cpp
  clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp


Index: clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
@@ -0,0 +1,11 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-instrumentation-bundle= flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-instrumentation-bundle=function -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-instrumentation-bundle=function
+//
+// REQUIRES-ANY: linux, freebsd
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -215,4 +215,19 @@
 ModeOpt += Mode;
 CmdArgs.push_back(Args.MakeArgString(ModeOpt));
   }
+
+  SmallString<64> Bundle("-fxray-instrumentation-bundle=");
+  if (InstrumentationBundle.full()) {
+Bundle += "all";
+  } else if (InstrumentationBundle.empty()) {
+Bundle += "none";
+  } else {
+if (InstrumentationBundle.has(XRayInstrKind::Function))
+  Bundle += "function";
+if (InstrumentationBundle.has(XRayInstrKind::Custom))
+  Bundle += "custom";
+if (InstrumentationBundle.has(XRayInstrKind::Typed))
+  Bundle += "typed";
+  }
+  CmdArgs.push_back(Args.MakeArgString(Bundle));
 }
Index: clang/include/clang/Basic/XRayInstr.h
===
--- clang/include/clang/Basic/XRayInstr.h
+++ clang/include/clang/Basic/XRayInstr.h
@@ -60,6 +60,8 @@
 
   bool empty() const { return Mask == 0; }
 
+  bool full() const { return Mask == XRayInstrKind::All; }
+
   XRayInstrMask Mask = 0;
 };
 


Index: clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
@@ -0,0 +1,11 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-instrumentation-bundle= flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-instrumentation-bundle=function -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-instrumentation-bundle=function
+//
+// REQUIRES-ANY: linux, freebsd
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -215,4 +215,19 @@
 ModeOpt += Mode;
 CmdArgs.push_back(Args.MakeArgString(ModeOpt));
   }
+
+  SmallString<64> Bundle("-fxray-instrumentation-bundle=");
+  if (InstrumentationBundle.full()) {
+Bundle += "all";
+  } else if (InstrumentationBundle.empty()) {
+Bundle += "none";
+  } else {
+if (InstrumentationBundle.has(XRayInstrKind::Function))
+  Bundle += "function";
+if (InstrumentationBundle.has(XRayInstrKind::Custom))
+  Bundle += "custom";
+if (InstrumentationBundle.has(XRayInstrKind::Typed))
+  Bundle += "typed";
+  }
+  CmdArgs.push_back(Args.MakeArgString(Bundle));
 }
Index: clang/include/clang/Basic/XRayInstr.h
===
--- clang/include/clang/Basic/XRayInstr.h
+++ clang/include/clang/Basic/XRayInstr.h
@@ -60,6 +60,8 @@
 
   bool empty() const { return Mask == 0; }
 
+  bool full() const { return Mask == XRayInstrKind::All; }
+
   XRayInstrMask Mask = 0;
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52342: [XRay][clang] Propagate -fxray-instrumentation-bundle to -cc1

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342715: [XRay][clang] Propagate 
-fxray-instrumentation-bundle to -cc1 (authored by dberris, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52342?vs=166410&id=166412#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52342

Files:
  include/clang/Basic/XRayInstr.h
  lib/Driver/XRayArgs.cpp
  test/Driver/XRay/xray-instrumentation-bundles-flags.cpp


Index: lib/Driver/XRayArgs.cpp
===
--- lib/Driver/XRayArgs.cpp
+++ lib/Driver/XRayArgs.cpp
@@ -215,4 +215,19 @@
 ModeOpt += Mode;
 CmdArgs.push_back(Args.MakeArgString(ModeOpt));
   }
+
+  SmallString<64> Bundle("-fxray-instrumentation-bundle=");
+  if (InstrumentationBundle.full()) {
+Bundle += "all";
+  } else if (InstrumentationBundle.empty()) {
+Bundle += "none";
+  } else {
+if (InstrumentationBundle.has(XRayInstrKind::Function))
+  Bundle += "function";
+if (InstrumentationBundle.has(XRayInstrKind::Custom))
+  Bundle += "custom";
+if (InstrumentationBundle.has(XRayInstrKind::Typed))
+  Bundle += "typed";
+  }
+  CmdArgs.push_back(Args.MakeArgString(Bundle));
 }
Index: include/clang/Basic/XRayInstr.h
===
--- include/clang/Basic/XRayInstr.h
+++ include/clang/Basic/XRayInstr.h
@@ -60,6 +60,8 @@
 
   bool empty() const { return Mask == 0; }
 
+  bool full() const { return Mask == XRayInstrKind::All; }
+
   XRayInstrMask Mask = 0;
 };
 
Index: test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
===
--- test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
+++ test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
@@ -0,0 +1,11 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-instrumentation-bundle= flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-instrumentation-bundle=function -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-instrumentation-bundle=function
+//
+// REQUIRES-ANY: linux, freebsd
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64


Index: lib/Driver/XRayArgs.cpp
===
--- lib/Driver/XRayArgs.cpp
+++ lib/Driver/XRayArgs.cpp
@@ -215,4 +215,19 @@
 ModeOpt += Mode;
 CmdArgs.push_back(Args.MakeArgString(ModeOpt));
   }
+
+  SmallString<64> Bundle("-fxray-instrumentation-bundle=");
+  if (InstrumentationBundle.full()) {
+Bundle += "all";
+  } else if (InstrumentationBundle.empty()) {
+Bundle += "none";
+  } else {
+if (InstrumentationBundle.has(XRayInstrKind::Function))
+  Bundle += "function";
+if (InstrumentationBundle.has(XRayInstrKind::Custom))
+  Bundle += "custom";
+if (InstrumentationBundle.has(XRayInstrKind::Typed))
+  Bundle += "typed";
+  }
+  CmdArgs.push_back(Args.MakeArgString(Bundle));
 }
Index: include/clang/Basic/XRayInstr.h
===
--- include/clang/Basic/XRayInstr.h
+++ include/clang/Basic/XRayInstr.h
@@ -60,6 +60,8 @@
 
   bool empty() const { return Mask == 0; }
 
+  bool full() const { return Mask == XRayInstrKind::All; }
+
   XRayInstrMask Mask = 0;
 };
 
Index: test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
===
--- test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
+++ test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
@@ -0,0 +1,11 @@
+// This test ensures that when we invoke the clang compiler, that the -cc1
+// options include the -fxray-instrumentation-bundle= flag we provide in the
+// invocation.
+//
+// RUN: %clang -fxray-instrument -fxray-instrumentation-bundle=function -### \
+// RUN: -x c++ -std=c++11 -emit-llvm -c -o - %s 2>&1 \
+// RUN: | FileCheck %s
+// CHECK:  -fxray-instrumentation-bundle=function
+//
+// REQUIRES-ANY: linux, freebsd
+// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM

Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57521



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

[PATCH] D37925: Allow specifying sanitizers in blacklists

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: lib/Basic/XRayLists.cpp:29
   // whether it's treated as a "never" instrument function.
-  if (AlwaysInstrument->inSection("fun", FunctionName, "arg1"))
+  if (AlwaysInstrument->inSection("xray_always_instrument", "fun", 
FunctionName,
+  "arg1"))

vlad.tsyrklevich wrote:
> eugenis wrote:
> > It feels redundant to have AlwaysInstrument and NeverInstrument lists, and 
> > then the same distinction in section names. Maybe sections could be named 
> > "xray" in  both cases? Or, better, the lists could be merged into a single 
> > list with always and never sections? There is also an issue of backward 
> > compatibility. Anyway, that's for xray devs to decide. @dberris 
> I chose this approach for backwards compatibility, but I'd defer to what 
> @dberris thinks is best.
Sorry for being late here.

I'm fine with keeping this as-is, then merging the lists into a single one. At 
the time this was designed/implemented, I hadn't thought about whether we could 
have used a single list. At the time it made sense to separate the always/never 
lists and applied in a set order (always wins over never).

If this is all-new functionality anyway, I'd think using "xray" as the section 
header and then using per-entry "always" and "never" identifiers/sections make 
sense.

If you leave a TODO here (and/or file a bug on XRay) I can do the migration to 
a single list later. I'm fine with how this is set as-is.


https://reviews.llvm.org/D37925



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

[PATCH] D38226: [XRay][Driver] Do not link in XRay runtime in shared libs

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.

This change ensures that we don't link in the XRay runtime when building
shared libraries with clang. This doesn't prevent us from building
shared libraris tht have XRay instrumentation sleds, but it does prevent
us from linking in the static XRay runtime into a shared library.

The XRay runtime currently doesn't support dynamic registration of
instrumentation sleds in shared objects, which we'll start enabling in
the future. That work has to happen in the back-end and in the runtime.


https://reviews.llvm.org/D38226

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/XRay/xray-shared-noxray.cpp


Index: test/Driver/XRay/xray-shared-noxray.cpp
===
--- /dev/null
+++ test/Driver/XRay/xray-shared-noxray.cpp
@@ -0,0 +1,14 @@
+// RUN: %clangxx -shared -fPIC -o /dev/null -v -fxray-instrument %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=SHARED
+// RUN: %clangxx -static -o /dev/null -v -fxray-instrument %s 2>&1 -DMAIN | \
+// RUN: FileCheck %s --check-prefix=STATIC
+// RUN: %clangxx -static -fPIE -o /dev/null -v -fxray-instrument %s 2>&1 \
+// RUN: -DMAIN | FileCheck %s --check-prefix=STATIC
+//
+// SHARED-NOT: {{clang_rt\.xray-}}
+// STATIC: {{clang_rt\.xray-}}
+int foo() { return 42; }
+
+#ifdef MAIN
+int main() { return foo(); }
+#endif
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -206,13 +206,18 @@
 
 static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
ArgStringList &CmdArgs) {
+  // Do not add the XRay runtime to shared libraries.
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 CmdArgs.push_back("-whole-archive");
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
 CmdArgs.push_back("-no-whole-archive");
 return true;
   }
+
   return false;
 }
 


Index: test/Driver/XRay/xray-shared-noxray.cpp
===
--- /dev/null
+++ test/Driver/XRay/xray-shared-noxray.cpp
@@ -0,0 +1,14 @@
+// RUN: %clangxx -shared -fPIC -o /dev/null -v -fxray-instrument %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=SHARED
+// RUN: %clangxx -static -o /dev/null -v -fxray-instrument %s 2>&1 -DMAIN | \
+// RUN: FileCheck %s --check-prefix=STATIC
+// RUN: %clangxx -static -fPIE -o /dev/null -v -fxray-instrument %s 2>&1 \
+// RUN: -DMAIN | FileCheck %s --check-prefix=STATIC
+//
+// SHARED-NOT: {{clang_rt\.xray-}}
+// STATIC: {{clang_rt\.xray-}}
+int foo() { return 42; }
+
+#ifdef MAIN
+int main() { return foo(); }
+#endif
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -206,13 +206,18 @@
 
 static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
ArgStringList &CmdArgs) {
+  // Do not add the XRay runtime to shared libraries.
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
   if (Args.hasFlag(options::OPT_fxray_instrument,
options::OPT_fnoxray_instrument, false)) {
 CmdArgs.push_back("-whole-archive");
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
 CmdArgs.push_back("-no-whole-archive");
 return true;
   }
+
   return false;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55891: [compiler-rt] [xray] [tests] Detect and handle missing LLVMTestingSupport gracefully

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM

Thanks, @mgorny!


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D55891



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

[PATCH] D56000: [compiler-rt] [xray] Disable alignas() for thread_local objects on NetBSD

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: lib/xray/xray_defs.h:22
 
+#if !SANITIZER_NETBSD
+#define XRAY_TLS_ALIGNAS(x) alignas(x)

krytarowski wrote:
> I would switch the order, in order to remove unneeded negation.
> 
> ```
> #if SANITIZER_NETBSD
> ...
> #else
> ...
> #endif
> ```
> 
> `#define XRAY_HAS_TLS_ALIGNAS 0` is not needed as `#if XRAY_HAS_TLS_ALIGNAS` 
> will evaluate to false anyway and `#define XRAY_HAS_TLS_ALIGNAS 1` is 
> equivalent to `#define XRAY_HAS_TLS_ALIGNAS`. But it's just a matter of style.
FWIW, I agree to this suggestion. I also prefer:

```
#if SANITIZER_NETBSD
...
#else
...
```

But I do like the explicit definition of `XRAY_HAS_TLS_ALIGNAS`.


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

https://reviews.llvm.org/D56000



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

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 142315.
dberris marked 5 inline comments as done.
dberris added a comment.

- fixup: Address comments


https://reviews.llvm.org/D44970

Files:
  clang/include/clang/Basic/XRayInstr.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/include/clang/Frontend/CodeGenOptions.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/XRayInstr.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-instrumentation-bundles.cpp

Index: clang/test/CodeGen/xray-instrumentation-bundles.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-instrumentation-bundles.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-instrumentation-bundle=none -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,NOCUSTOM %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,NOCUSTOM %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=custom -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,CUSTOM %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function,custom -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,CUSTOM %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function \
+// RUN: -fxray-instrumentation-bundle=custom -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,CUSTOM %s
+
+// CHECK: define void @_Z16alwaysInstrumentv() #[[ALWAYSATTR:[0-9]+]] {
+[[clang::xray_always_instrument]] void alwaysInstrument() {
+  static constexpr char kPhase[] = "always";
+  __xray_customevent(kPhase, 6);
+  // CUSTOM: call void @llvm.xray.customevent(i8*{{.*}}, i32 6)
+  // NOCUSTOM-NOT: call void @llvm.xray.customevent(i8*{{.*}}, i32 6)
+}
+
+// FUNCTION: attributes #[[ALWAYSATTR]] = {{.*}} "function-instrument"="xray-always" {{.*}}
+// NOFUNCTION-NOT: attributes #[[ALWAYSATTR]] = {{.*}} "function-instrument"="xray-always" {{.*}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -26,6 +26,7 @@
 #include "clang/Basic/VersionTuple.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Basic/Visibility.h"
+#include "clang/Basic/XRayInstr.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
@@ -75,9 +76,9 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetOptions.h"
-#include "llvm/Support/ScopedPrinter.h"
 #include 
 #include 
 #include 
@@ -446,6 +447,25 @@
   }
 }
 
+static void parseXRayInstrumentationBundle(StringRef FlagName, StringRef Bundle,
+   ArgList &Args, DiagnosticsEngine &D,
+   XRayInstrSet &S) {
+  llvm::SmallVector BundleParts;
+  llvm::SplitString(Bundle, BundleParts, ",");
+  for (const auto B : BundleParts) {
+auto Mask = parseXRayInstrValue(B);
+if (Mask == XRayInstrKind::None)
+  if (B != "none")
+D.Report(diag::err_drv_invalid_value) << FlagName << Bundle;
+  else
+S.Mask = Mask;
+else if (Mask == XRayInstrKind::All)
+  S.Mask = Mask;
+else
+  S.set(Mask, true);
+  }
+}
+
 // Set the profile kind for fprofile-instrument.
 static void setPGOInstrumentor(CodeGenOptions &Opts, ArgList &Args,
DiagnosticsEngine &Diags) {
@@ -820,11 +840,23 @@
   Args.hasArg(OPT_finstrument_functions_after_inlining);
   Opts.InstrumentFunctionEntryBare =
   Args.hasArg(OPT_finstrument_function_entry_bare);
-  Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
+
+  Opts.XRayInstrumentFunctions =
+  Args.hasArg(OPT_fxray_instrument);
   Opts.XRayAlwaysEmitCustomEvents =
   Args.hasArg(OPT_fxray_always_emit_customevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
+
+  auto XRayInstrBundles =
+  Args.getAllArgValues(OPT_fxray_instrumentation_bundle);
+ 

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

Thanks, Martin! Landing now, after suggested changes.




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:471
 bool CodeGenFunction::AlwaysEmitXRayCustomEvents() const {
-  return CGM.getCodeGenOpts().XRayAlwaysEmitCustomEvents;
+  return CGM.getCodeGenOpts().XRayInstrumentFunctions &&
+ (CGM.getCodeGenOpts().XRayAlwaysEmitCustomEvents ||

pelikan wrote:
> I kind of don't like how the "-fxray-instrument" variable is called 
> "XRayInstrumentFunctions" because that's not what it means any more.  I think 
> in a later diff, we should clean this up.  Or maybe even clean up some of the 
> old flags whose functionality has been superseded by this.  But the logic 
> here is fine.
> 
> Same with the misleading "ShouldXRayInstrumentFunction()" which controls 
> custom events too, and not just functions.
Good point. Yes, we could make this cleaner.


https://reviews.llvm.org/D44970



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

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC329985: [XRay][clang] Add flag to choose instrumentation 
bundles (authored by dberris, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44970?vs=142315&id=142317#toc

Repository:
  rC Clang

https://reviews.llvm.org/D44970

Files:
  include/clang/Basic/XRayInstr.h
  include/clang/Driver/Options.td
  include/clang/Driver/XRayArgs.h
  include/clang/Frontend/CodeGenOptions.h
  lib/Basic/CMakeLists.txt
  lib/Basic/XRayInstr.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Driver/XRayArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/xray-instrumentation-bundles.cpp

Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1846,9 +1846,10 @@
StringRef Category) const {
   if (!LangOpts.XRayInstrument)
 return false;
+
   const auto &XRayFilter = getContext().getXRayFilter();
   using ImbueAttr = XRayFunctionFilter::ImbueAttribute;
-  auto Attr = XRayFunctionFilter::ImbueAttribute::NONE;
+  auto Attr = ImbueAttr::NONE;
   if (Loc.isValid())
 Attr = XRayFilter.shouldImbueLocation(Loc, Category);
   if (Attr == ImbueAttr::NONE)
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -3341,6 +3341,11 @@
   case Builtin::BI__xray_customevent: {
 if (!ShouldXRayInstrumentFunction())
   return RValue::getIgnored();
+
+if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+XRayInstrKind::Custom))
+  return RValue::getIgnored();
+
 if (const auto *XRayAttr = CurFuncDecl->getAttr())
   if (XRayAttr->neverXRayInstrument() && !AlwaysEmitXRayCustomEvents())
 return RValue::getIgnored();
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -468,7 +468,10 @@
 /// AlwaysEmitXRayCustomEvents - Return true if we should emit IR for calls to
 /// the __xray_customevent(...) builin calls, when doing XRay instrumentation.
 bool CodeGenFunction::AlwaysEmitXRayCustomEvents() const {
-  return CGM.getCodeGenOpts().XRayAlwaysEmitCustomEvents;
+  return CGM.getCodeGenOpts().XRayInstrumentFunctions &&
+ (CGM.getCodeGenOpts().XRayAlwaysEmitCustomEvents ||
+  CGM.getCodeGenOpts().XRayInstrumentationBundle.Mask ==
+  XRayInstrKind::Custom);
 }
 
 llvm::Constant *
@@ -900,7 +903,9 @@
   }
 
   // Apply xray attributes to the function (as a string, for now)
-  bool InstrumentXray = ShouldXRayInstrumentFunction();
+  bool InstrumentXray = ShouldXRayInstrumentFunction() &&
+CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+XRayInstrKind::Function);
   if (D && InstrumentXray) {
 if (const auto *XRayAttr = D->getAttr()) {
   if (XRayAttr->alwaysXRayInstrument())
Index: lib/Driver/XRayArgs.cpp
===
--- lib/Driver/XRayArgs.cpp
+++ lib/Driver/XRayArgs.cpp
@@ -58,8 +58,7 @@
   }
 } else {
   D.Diag(diag::err_drv_clang_unsupported)
-  << (std::string(XRayInstrumentOption) +
-  " on non-supported target OS");
+  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
 }
 XRayInstrument = true;
 if (const Arg *A =
@@ -82,6 +81,36 @@
   options::OPT_fnoxray_link_deps, true))
   XRayRT = false;
 
+auto Bundles =
+Args.getAllArgValues(options::OPT_fxray_instrumentation_bundle);
+if (Bundles.empty())
+  InstrumentationBundle.Mask = XRayInstrKind::All;
+else
+  for (const auto &B : Bundles) {
+llvm::SmallVector BundleParts;
+llvm::SplitString(B, BundleParts, ",");
+for (const auto &P : BundleParts) {
+  // TODO: Automate the generation of the string case table.
+  auto Valid = llvm::StringSwitch(P)
+   .Cases("none", "all", "function", "custom", true)
+   .Default(false);
+
+  if (!Valid) {
+D.Diag(clang::diag::err_drv_invalid_value)
+<< "-fxray-instrumentation-bundle=" << P;
+continue;
+  }
+
+  auto Mask = parseXRayInstrValue(P);
+  if (Mask == XRayInstrKind::None) {
+InstrumentationBundle.clear();
+break;
+  }
+
+  InstrumentationBundle.Mask |= Mask;
+}
+  }
+
 // Validate the always/never attribute files. We also make sure that they
 // are treated as actual dependencies.
 for (const auto &Filename :
@@ -165,7 +194,7 @@
 CmdArgs.push_back(Args.MakeArgSt

[PATCH] D45610: [XRay][clang] Make -fxray-modes= additive

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: eizan, kpw, pelikan.

This allows us to do the following:

  clang -fxray-modes=none ... -fxray-modes=xray-basic

It's important to be able to do this in cases where we'd like to
specialise the configuration for the invocation of the compiler, in
various scripting environments.

This is related to llvm.org/PR37066, a follow-up to 
https://reviews.llvm.org/D45474.


https://reviews.llvm.org/D45610

Files:
  clang/lib/Driver/XRayArgs.cpp
  clang/test/Driver/XRay/xray-mode-flags.cpp


Index: clang/test/Driver/XRay/xray-mode-flags.cpp
===
--- clang/test/Driver/XRay/xray-mode-flags.cpp
+++ clang/test/Driver/XRay/xray-mode-flags.cpp
@@ -14,6 +14,18 @@
 // RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
 // RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none -### %s \
 // RUN: 2>&1 | FileCheck --check-prefixes NONE %s
+//
+// We also should support overriding the modes in an additive manner.
+//
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,xray-fdr \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=xray-fdr,none \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes NONE %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,all \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
 
 // BASIC: libclang_rt.xray-basic
 // FDR: libclang_rt.xray-fdr
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -146,21 +146,16 @@
   llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
 else
   for (const auto &Arg : SpecifiedModes) {
-if (Arg == "none") {
-  Modes.clear();
-  break;
-}
-if (Arg == "all") {
-  Modes.clear();
-  llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
-  break;
-}
-
 // Parse CSV values for -fxray-modes=...
 llvm::SmallVector ModeParts;
 llvm::SplitString(Arg, ModeParts, ",");
 for (const auto &M : ModeParts)
-  Modes.push_back(M);
+  if (M == "none")
+Modes.clear();
+  else if (M == "all")
+llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
+  else
+Modes.push_back(M);
   }
 
 // Then we want to sort and unique the modes we've collected.
@@ -205,4 +200,10 @@
 ExtraDepOpt += Dep;
 CmdArgs.push_back(Args.MakeArgString(ExtraDepOpt));
   }
+
+  for (const auto &Mode : Modes) {
+SmallString<64> ModeOpt("-fxray-modes=");
+ModeOpt += Mode;
+CmdArgs.push_back(Args.MakeArgString(ModeOpt));
+  }
 }


Index: clang/test/Driver/XRay/xray-mode-flags.cpp
===
--- clang/test/Driver/XRay/xray-mode-flags.cpp
+++ clang/test/Driver/XRay/xray-mode-flags.cpp
@@ -14,6 +14,18 @@
 // RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
 // RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none -### %s \
 // RUN: 2>&1 | FileCheck --check-prefixes NONE %s
+//
+// We also should support overriding the modes in an additive manner.
+//
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,xray-fdr \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=xray-fdr,none \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes NONE %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,all \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
 
 // BASIC: libclang_rt.xray-basic
 // FDR: libclang_rt.xray-fdr
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -146,21 +146,16 @@
   llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
 else
   for (const auto &Arg : SpecifiedModes) {
-if (Arg == "none") {
-  Modes.clear();
-  break;
-}
-if (Arg == "all") {
-  Modes.clear();
-  llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
-  break;
-}
-
 // Parse CSV values for -fxray-modes=...
 llvm::SmallVector ModeParts;
 llvm::SplitString(Arg, ModeParts, ",");
 for (const auto &M : ModeParts)
-  Modes.push_back(M);
+  if (M == "none")
+Modes.clear();
+  else if (M == "all")
+llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
+  else
+Modes.push_back(M);
   }
 
 // Then we want to sort and unique the modes we've

[PATCH] D45610: [XRay][clang] Make -fxray-modes= additive

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329989: [XRay][clang] Make -fxray-modes= additive (authored 
by dberris, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45610?vs=142333&id=142335#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45610

Files:
  cfe/trunk/lib/Driver/XRayArgs.cpp
  cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp


Index: cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp
===
--- cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp
+++ cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp
@@ -14,6 +14,18 @@
 // RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
 // RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none -### %s \
 // RUN: 2>&1 | FileCheck --check-prefixes NONE %s
+//
+// We also should support overriding the modes in an additive manner.
+//
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,xray-fdr \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=xray-fdr,none \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes NONE %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,all \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
 
 // BASIC: libclang_rt.xray-basic
 // FDR: libclang_rt.xray-fdr
Index: cfe/trunk/lib/Driver/XRayArgs.cpp
===
--- cfe/trunk/lib/Driver/XRayArgs.cpp
+++ cfe/trunk/lib/Driver/XRayArgs.cpp
@@ -146,21 +146,16 @@
   llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
 else
   for (const auto &Arg : SpecifiedModes) {
-if (Arg == "none") {
-  Modes.clear();
-  break;
-}
-if (Arg == "all") {
-  Modes.clear();
-  llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
-  break;
-}
-
 // Parse CSV values for -fxray-modes=...
 llvm::SmallVector ModeParts;
 llvm::SplitString(Arg, ModeParts, ",");
 for (const auto &M : ModeParts)
-  Modes.push_back(M);
+  if (M == "none")
+Modes.clear();
+  else if (M == "all")
+llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
+  else
+Modes.push_back(M);
   }
 
 // Then we want to sort and unique the modes we've collected.
@@ -205,4 +200,10 @@
 ExtraDepOpt += Dep;
 CmdArgs.push_back(Args.MakeArgString(ExtraDepOpt));
   }
+
+  for (const auto &Mode : Modes) {
+SmallString<64> ModeOpt("-fxray-modes=");
+ModeOpt += Mode;
+CmdArgs.push_back(Args.MakeArgString(ModeOpt));
+  }
 }


Index: cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp
===
--- cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp
+++ cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp
@@ -14,6 +14,18 @@
 // RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
 // RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none -### %s \
 // RUN: 2>&1 | FileCheck --check-prefixes NONE %s
+//
+// We also should support overriding the modes in an additive manner.
+//
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,xray-fdr \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=xray-fdr,none \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes NONE %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,all \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
 
 // BASIC: libclang_rt.xray-basic
 // FDR: libclang_rt.xray-fdr
Index: cfe/trunk/lib/Driver/XRayArgs.cpp
===
--- cfe/trunk/lib/Driver/XRayArgs.cpp
+++ cfe/trunk/lib/Driver/XRayArgs.cpp
@@ -146,21 +146,16 @@
   llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
 else
   for (const auto &Arg : SpecifiedModes) {
-if (Arg == "none") {
-  Modes.clear();
-  break;
-}
-if (Arg == "all") {
-  Modes.clear();
-  llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
-  break;
-}
-
 // Parse CSV values for -fxray-modes=...
 llvm::SmallVector ModeParts;
 llvm::SplitString(Arg, ModeParts, ",");
 for (const auto &M : ModeParts)
-  Modes.push_back(M);
+  if (M == "none")
+Modes.clear();
+  else if (M == "all")
+llvm::copy(XRaySupportedModes, std::back_inserter(Modes));
+  else
+Modes.push_back(M);
   }
 
 // Then we want to sort and unique the modes we've collected.
@@ -205,4 +200,10 @@

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:678-679
   TC.AddCXXStdlibLibArgs(Args, CmdArgs);
+  if (TC.getTriple().getOS() == llvm::Triple::OpenBSD)
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");
+}

Maybe this should be in the `TC.AddCXXStdlibLibArgs(...)` function instead?


Repository:
  rC Clang

https://reviews.llvm.org/D45662



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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:678-679
   TC.AddCXXStdlibLibArgs(Args, CmdArgs);
+  if (TC.getTriple().getOS() == llvm::Triple::OpenBSD)
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");
+}

devnexen wrote:
> dberris wrote:
> > Maybe this should be in the `TC.AddCXXStdlibLibArgs(...)` function instead?
> I did not dare since it s the only case where it is needed (e.g. no need for 
> X-ray for example)
Okay, let me ask this another way.

Is there ever a case for OpenBSD when we're doing `TC.AddCXXStdlibLibArgs(...)` 
that we don't need to also add these flags when `OPT_pg` is specified?


Repository:
  rC Clang

https://reviews.llvm.org/D45662



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

[PATCH] D45716: [XRay] Add clang builtin for xray typed events.

[Dean Michael Berris via Phabricator via cfe-commits]

dberris requested changes to this revision.
dberris added a comment.
This revision now requires changes to proceed.

Thanks, Keith -- we're going to need to add this to the list of instrumentation 
points we can enable/disable through flags.




Comment at: lib/CodeGen/CGBuiltin.cpp:3376-3377
+
+if (!CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+XRayInstrKind::Custom))
+  return RValue::getIgnored();

Please define a new `XRayInstrKind::` for `Typed` instead of re-using `Custom`. 
This allows us to provide finer-grained control on the enabling/disabling of 
specific flags.

See `clang/include/clang/Basic/XRayInstr.h` on adding new values to 
`XRayInstrKind` and `clang/lib/Basic/XRayInstr.cpp` in supporting the new value 
when parsing from `-fxray-instrumentation-bundle=`. Also, you may need to 
update XRayArgs.{h,cpp} to teach it to understand how to parse the 'typed' flag 
value.


Repository:
  rC Clang

https://reviews.llvm.org/D45716



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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

Okay, can you help me understand why this isn't required for builds in OpenBSD 
which don't use sanitizers, but want C++ builds that also add the `-pg` flag? I 
would have thought that those situations would break, no?


https://reviews.llvm.org/D45662



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

[PATCH] D45716: [XRay] Add clang builtin for xray typed events.

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In https://reviews.llvm.org/D45716#1069557, @kpw wrote:

> My editor got a bit carried away with automatically clang-formatting 
> lib/CodeGen/CodeGenFunction.cpp. I'll fix that so that I'm not messing up the 
> revision history.


Yes please. :)

Also, if you can add to the tests for instrumentation bundles in 
`test/Driver/XRay/...` to ensure that we're properly not seeing the typed 
events there either, that would be great.


Repository:
  rC Clang

https://reviews.llvm.org/D45716



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

[PATCH] D45716: [XRay] Add clang builtin for xray typed events.

[Dean Michael Berris via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D45716#1069567, @dberris wrote:

> In https://reviews.llvm.org/D45716#1069557, @kpw wrote:
>
> > My editor got a bit carried away with automatically clang-formatting 
> > lib/CodeGen/CodeGenFunction.cpp. I'll fix that so that I'm not messing up 
> > the revision history.
>
>
> Yes please. :)
>
> Also, if you can add to the tests for instrumentation bundles in 
> `test/Driver/XRay/...` to ensure that we're properly not seeing the typed 
> events there either, that would be great.


Actually, I see that change too. Ignore that. :)

LGTM (pending the revert of formatting changes in 
`lib/CodeGen/CodeGenFunction.cpp`).


Repository:
  rC Clang

https://reviews.llvm.org/D45716



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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

Do you actually need this change? Why isn't 
`getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:197-199
+  if (getToolChain().getSanitizerArgs().needsFuzzer() &&
+!Args.hasArg(options::OPT_shared))
+CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++");

Doesn't the common args implementation already handle this case? What am I 
missing?



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:265-269
+void OpenBSD::AddCXXStdlibLibArgs(const ArgList &Args,
+  ArgStringList &CmdArgs) const {
+  bool Profiling = Args.hasArg(options::OPT_pg);
+  CmdArgs.push_back(Profiling ? "-lstdc++_p" : "-lstdc++");
+}

Almost there -- but I think you want to look at the implementation of something 
like the FreeBSD driver which does a suitably complete definition, which 
supports either libstdc++ or libc++.

That way you don't need the other change specific to when you need the 
sanitizer deps, and rely on the default implementation for LibFuzzer in 
CommonArgs. No?

Am looking at https://clang.llvm.org/doxygen/FreeBSD_8cpp_source.html#l00345 
for reference.


https://reviews.llvm.org/D45662



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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

devnexen wrote:
> dberris wrote:
> > Do you actually need this change? Why isn't 
> > `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
> That s the thing, I wish it was simple as FreeBSD, but seemingly in OpenBSD 
> needs both c++98 gcc runtime and libc++ for fuzzer (I tried libc++ alone 
> already)
Right, but this comment is on this specific line change. I don't think you need 
to reach into `Toolchain.` direcly, since you can already use `getToolChain()` 
just from the above line (188).


https://reviews.llvm.org/D45662



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

[PATCH] D45662: Fuzzer, add libcxx for OpenBSD

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: lib/Driver/ToolChains/OpenBSD.cpp:189
   if (getToolChain().ShouldLinkCXXStdlib(Args))
-getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
   if (Args.hasArg(options::OPT_pg))

devnexen wrote:
> dberris wrote:
> > devnexen wrote:
> > > dberris wrote:
> > > > Do you actually need this change? Why isn't 
> > > > `getToolChain().AddCXXStdlibLibArgs(...)` not sufficient here?
> > > That s the thing, I wish it was simple as FreeBSD, but seemingly in 
> > > OpenBSD needs both c++98 gcc runtime and libc++ for fuzzer (I tried 
> > > libc++ alone already)
> > Right, but this comment is on this specific line change. I don't think you 
> > need to reach into `Toolchain.` direcly, since you can already use 
> > `getToolChain()` just from the above line (188).
> Right, so I guess this diff https://reviews.llvm.org/D45662?id=142686 is 
> sufficient then ?
No. Let me try and explain again.

You were on the right path, with overriding the `AddCXXStdlibLibArgs` function 
in the OpenBSD Toolchain type. It's just that you weren't handling the case for 
when the binary was being built with libc++ or libstdc++ properly. I was 
referring you to what FreeBSD was doing for their implementation of 
`AddCXXStdlibLibArgs`. This means, checking first whether the invocation of the 
compiler was using libc++ or libstdc++, and then adding the appropriate link 
spelling. That all happens in the `AddCXXStdlibLibArgs` implementation, because 
there's no need to special-case just for the sanitizers.

This means, if you're building a normal binary with `-pg` in OpenBSD against 
either libc++ or libstdc++, it wouldn't work correctly regardless of whether 
you were using libFuzzer.

Does that make more sense?


https://reviews.llvm.org/D45662



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

[PATCH] D45662: OpenBSD add C++ runtime in a driver's standpoint

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM

Thanks for working through this @devnexen!

I can land this a little later on my day (Sydney, Australia).


https://reviews.llvm.org/D45662



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

[PATCH] D45662: OpenBSD add C++ runtime in a driver's standpoint

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL330310: OpenBSD add C++ runtime in a driver's 
standpoint (authored by dberris, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45662?vs=142811&id=143049#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45662

Files:
  cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
  cfe/trunk/lib/Driver/ToolChains/OpenBSD.h


Index: cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
+++ cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
@@ -259,6 +259,14 @@
   getFilePaths().push_back("/usr/lib");
 }
 
+void OpenBSD::AddCXXStdlibLibArgs(const ArgList &Args,
+  ArgStringList &CmdArgs) const {
+  bool Profiling = Args.hasArg(options::OPT_pg);
+
+  CmdArgs.push_back(Profiling ? "-lc++_p" : "-lc++");
+  CmdArgs.push_back(Profiling ? "-lc++abi_p" : "-lc++abi");
+}
+
 Tool *OpenBSD::buildAssembler() const {
   return new tools::openbsd::Assembler(*this);
 }
Index: cfe/trunk/lib/Driver/ToolChains/OpenBSD.h
===
--- cfe/trunk/lib/Driver/ToolChains/OpenBSD.h
+++ cfe/trunk/lib/Driver/ToolChains/OpenBSD.h
@@ -58,6 +58,8 @@
   bool IsMathErrnoDefault() const override { return false; }
   bool IsObjCNonFragileABIDefault() const override { return true; }
   bool isPIEDefault() const override { return true; }
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs) const override;
 
   unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
 return 2;


Index: cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
+++ cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
@@ -259,6 +259,14 @@
   getFilePaths().push_back("/usr/lib");
 }
 
+void OpenBSD::AddCXXStdlibLibArgs(const ArgList &Args,
+  ArgStringList &CmdArgs) const {
+  bool Profiling = Args.hasArg(options::OPT_pg);
+
+  CmdArgs.push_back(Profiling ? "-lc++_p" : "-lc++");
+  CmdArgs.push_back(Profiling ? "-lc++abi_p" : "-lc++abi");
+}
+
 Tool *OpenBSD::buildAssembler() const {
   return new tools::openbsd::Assembler(*this);
 }
Index: cfe/trunk/lib/Driver/ToolChains/OpenBSD.h
===
--- cfe/trunk/lib/Driver/ToolChains/OpenBSD.h
+++ cfe/trunk/lib/Driver/ToolChains/OpenBSD.h
@@ -58,6 +58,8 @@
   bool IsMathErrnoDefault() const override { return false; }
   bool IsObjCNonFragileABIDefault() const override { return true; }
   bool isPIEDefault() const override { return true; }
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs) const override;
 
   unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
 return 2;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

How about tests for functions with conditional noexcept?




Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:8-9
+* Destructors
+* Copy constructors
+* Copy assignment operators
+* The ``main()`` functions

Are copy constructors and assignment operators not supposed to throw? Or did 
you mean *move* constructors?



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

`EnabledFunctions` but they *should not* throw?


https://reviews.llvm.org/D33537



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:154-155
+  if (const auto *TD = ThrownType->getAsTagDecl()) {
+if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc")
+  return Results;
+  }

Does this actually catch `std::bad_alloc` or just any exception called 
`bad_alloc`? Should this be a fully qualified type, those defined by the 
implementation? I suspect this isn't as simple to figure out, because an 
implementation may be using nested namespaces (inline namespace?) to bring in 
the `bad_alloc` type in the `std` namespace.



Comment at: docs/ReleaseNotes.rst:230-233
+<<< HEAD
+===
 - The 'google-runtime-member-string-references' check was removed.
+>>> master

You probably want to fix this...



Comment at: docs/clang-tidy/checks/bugprone-exception-escape.rst:28
+
+   Comma separated list containing function names which should not throw. An
+   example for using this parameter is the function ``WinMain()`` in the

baloghadamsoftware wrote:
> dberris wrote:
> > `EnabledFunctions` but they *should not* throw?
> Maybe we should come up with a better name for this option. I just took this 
> from our company internal check.
My suggestion here would be something like 'FunctionBlacklist' or 
'IgnoreFunctions'.

Are these exact names, or regular expressions? Should they be regular 
expressions? How about those that are in namespaces?

You might want to explore being able to configure this similar to the Sanitizer 
blacklist, which supports function name globs. I can imagine this immediately 
getting really brittle too.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 
throws
+  implicit_int_thrower();

How deep does this go? Say we have a call to a function that's extern which 
doesn't have 'noexcept' nor a dynamic exception specifier -- do we assume that 
the call to an extern function may throw? Does that warn? What does the warning 
look like? Should it warn? How about when you call a function through a 
function pointer?

The documentation should cover these cases and/or more explicitly say in the 
warning that an exception may throw in a noexcept function (rather than just 
"function <...> throws").


https://reviews.llvm.org/D33537



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

[PATCH] D48574: OpenBSD driver needs ld.lld in sanitiser context

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM

PS. I really wish at some point this will lead to an OpenBSD build bot!


Repository:
  rC Clang

https://reviews.llvm.org/D48574



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

[PATCH] D81995: [xray] Option to omit the function index

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81995



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

[PATCH] D85773: [Driver][XRay][test] Update the macOS support check

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM from me if XRay actually does work on non-x86_64 macOS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85773

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

[PATCH] D39114: [XRay][darwin] Initial XRay in Darwin Support

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In https://reviews.llvm.org/D39114#930461, @kubamracek wrote:

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


Yes, I'm going to be breaking this up into smaller parts -- just trying to see 
where it's going at this point.

I've just finally gotten to the point where I can get back to this again. :)

In https://reviews.llvm.org/D39114#930477, @kubamracek wrote:

> And we should probably introduce files named `xray_linux.cc` and 
> `xray_mac.cc` to contain platform-specific functions.


I fully agree. That should happen in smaller steps.

Expect smaller reviews coming soon, from breaking this patch up. :D




Comment at: compiler-rt/lib/xray/xray_x86_64.cc:25
+  size_t Len = 0;
+  if (sysctlbyname("hw.cpufrequency_max", &CPUFreq, &Len, NULL, 0) == -1) {
+Report("Unable to determine CPU frequency for TSC accounting; errno = 
%d\n",

krytarowski wrote:
> Is it OK for you to call here internally malloc(3)? At least the NetBSD 
> version calls it.
Good point. We should be caching this information earlier on in the process 
(i.e. before we start tracing) to avoid deadlocks in case we have an 
instrumented malloc implementation. Let me put a `TODO` or at least make sure 
we're calling this at a point that is safe (probably at initialisation time).


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][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 124538.
dberris retitled this revision from "[XRay][darwin] Initial XRay in Darwin 
Support" to "[XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin".
dberris edited the summary of this revision.
dberris added a comment.
This revision is now accepted and ready to land.

Retitle and reduce patch to smaller bits.


https://reviews.llvm.org/D39114

Files:
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/weak_symbols.txt
  compiler-rt/lib/xray/xray_fdr_logging.cc
  compiler-rt/lib/xray/xray_flags.inc
  compiler-rt/lib/xray/xray_init.cc
  compiler-rt/lib/xray/xray_trampoline_x86_64.S

Index: compiler-rt/lib/xray/xray_trampoline_x86_64.S
===
--- compiler-rt/lib/xray/xray_trampoline_x86_64.S
+++ compiler-rt/lib/xray/xray_trampoline_x86_64.S
@@ -14,10 +14,13 @@
 //===--===//
 
 #include "../builtins/assembly.h"
+#include "../sanitizer_common/sanitizer_asm.h"
+
+
 
 .macro SAVE_REGISTERS
 	subq $192, %rsp
-	.cfi_def_cfa_offset 200
+	CFI_DEF_CFA_OFFSET(200)
 	// At this point, the stack pointer should be aligned to an 8-byte boundary,
 	// because any call instructions that come after this will add another 8
 	// bytes and therefore align it to 16-bytes.
@@ -57,7 +60,7 @@
 	movq	8(%rsp), %r8
 	movq	0(%rsp), %r9
 	addq	$192, %rsp
-	.cfi_def_cfa_offset 8
+	CFI_DEF_CFA_OFFSET(8)
 .endm
 
 .macro ALIGNED_CALL_RAX
@@ -75,21 +78,25 @@
 .endm
 
 	.text
+#if !defined(__APPLE__)
+	.section .text
+#else
+	.section __TEXT,__text
+#endif
 	.file "xray_trampoline_x86.S"
 
 //===--===//
 
-	.globl __xray_FunctionEntry
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionEntry)
 	.align 16, 0x90
-	.type __xray_FunctionEntry,@function
-
-__xray_FunctionEntry:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionEntry)
+ASM_TSAN_SYMBOL(__xray_FunctionEntry):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
 	// This load has to be atomic, it's concurrent with __xray_patch().
 	// On x86/amd64, a simple (type-aligned) MOV instruction is enough.
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq	%rax, %rax
 	je	.Ltmp0
 
@@ -101,28 +108,27 @@
 .Ltmp0:
 	RESTORE_REGISTERS
 	retq
-.Ltmp1:
-	.size __xray_FunctionEntry, .Ltmp1-__xray_FunctionEntry
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionEntry)
+	CFI_ENDPROC
 
 //===--===//
 
-	.globl __xray_FunctionExit
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionExit)
 	.align 16, 0x90
-	.type __xray_FunctionExit,@function
-__xray_FunctionExit:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionExit)
+ASM_TSAN_SYMBOL(__xray_FunctionExit):
+	CFI_STARTPROC
 	// Save the important registers first. Since we're assuming that this
 	// function is only jumped into, we only preserve the registers for
 	// returning.
 	subq	$56, %rsp
-	.cfi_def_cfa_offset 64
+	CFI_DEF_CFA_OFFSET(64)
 	movq  %rbp, 48(%rsp)
 	movupd	%xmm0, 32(%rsp)
 	movupd	%xmm1, 16(%rsp)
 	movq	%rax, 8(%rsp)
 	movq	%rdx, 0(%rsp)
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq %rax,%rax
 	je	.Ltmp2
 
@@ -138,22 +144,21 @@
 	movq	8(%rsp), %rax
 	movq	0(%rsp), %rdx
 	addq	$56, %rsp
-	.cfi_def_cfa_offset 8
+	CFI_DEF_CFA_OFFSET(8)
 	retq
-.Ltmp3:
-	.size __xray_FunctionExit, .Ltmp3-__xray_FunctionExit
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionExit)
+	CFI_ENDPROC
 
 //===--===//
 
-	.global __xray_FunctionTailExit
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionTailExit)
 	.align 16, 0x90
-	.type __xray_FunctionTailExit,@function
-__xray_FunctionTailExit:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionTailExit)
+ASM_TSAN_SYMBOL(__xray_FunctionTailExit):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq %rax,%rax
 	je	.Ltmp4
 
@@ -165,26 +170,25 @@
 .Ltmp4:
 	RESTORE_REGISTERS
 	retq
-.Ltmp5:
-	.size __xray_FunctionTailExit, .Ltmp5-__xray_FunctionTailExit
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionTailExit)
+	CFI_ENDPROC
 
 //===--===//
 
-	.globl __xray_ArgLoggerEntry
+	.globl ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry)
 	.align 16, 0x90
-	.type __xray_ArgLoggerEntry,@function
-__xray_ArgLoggerEntry:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_ArgLoggerEntry)
+ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
 	// Again, these function pointer loads must be atomic; MOV is fine.
-	movq	_ZN6__xray13XRayArgLoggerE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray13XRayArgLoggerE)(%rip), %rax
 	testq	%rax, %rax
 	j

[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 124539.
dberris edited the summary of this revision.
dberris added a comment.

Update description to not use tabs.


https://reviews.llvm.org/D39114

Files:
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/weak_symbols.txt
  compiler-rt/lib/xray/xray_fdr_logging.cc
  compiler-rt/lib/xray/xray_flags.inc
  compiler-rt/lib/xray/xray_init.cc
  compiler-rt/lib/xray/xray_trampoline_x86_64.S

Index: compiler-rt/lib/xray/xray_trampoline_x86_64.S
===
--- compiler-rt/lib/xray/xray_trampoline_x86_64.S
+++ compiler-rt/lib/xray/xray_trampoline_x86_64.S
@@ -14,10 +14,13 @@
 //===--===//
 
 #include "../builtins/assembly.h"
+#include "../sanitizer_common/sanitizer_asm.h"
+
+
 
 .macro SAVE_REGISTERS
 	subq $192, %rsp
-	.cfi_def_cfa_offset 200
+	CFI_DEF_CFA_OFFSET(200)
 	// At this point, the stack pointer should be aligned to an 8-byte boundary,
 	// because any call instructions that come after this will add another 8
 	// bytes and therefore align it to 16-bytes.
@@ -57,7 +60,7 @@
 	movq	8(%rsp), %r8
 	movq	0(%rsp), %r9
 	addq	$192, %rsp
-	.cfi_def_cfa_offset 8
+	CFI_DEF_CFA_OFFSET(8)
 .endm
 
 .macro ALIGNED_CALL_RAX
@@ -75,21 +78,25 @@
 .endm
 
 	.text
+#if !defined(__APPLE__)
+	.section .text
+#else
+	.section __TEXT,__text
+#endif
 	.file "xray_trampoline_x86.S"
 
 //===--===//
 
-	.globl __xray_FunctionEntry
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionEntry)
 	.align 16, 0x90
-	.type __xray_FunctionEntry,@function
-
-__xray_FunctionEntry:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionEntry)
+ASM_TSAN_SYMBOL(__xray_FunctionEntry):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
 	// This load has to be atomic, it's concurrent with __xray_patch().
 	// On x86/amd64, a simple (type-aligned) MOV instruction is enough.
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq	%rax, %rax
 	je	.Ltmp0
 
@@ -101,28 +108,27 @@
 .Ltmp0:
 	RESTORE_REGISTERS
 	retq
-.Ltmp1:
-	.size __xray_FunctionEntry, .Ltmp1-__xray_FunctionEntry
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionEntry)
+	CFI_ENDPROC
 
 //===--===//
 
-	.globl __xray_FunctionExit
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionExit)
 	.align 16, 0x90
-	.type __xray_FunctionExit,@function
-__xray_FunctionExit:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionExit)
+ASM_TSAN_SYMBOL(__xray_FunctionExit):
+	CFI_STARTPROC
 	// Save the important registers first. Since we're assuming that this
 	// function is only jumped into, we only preserve the registers for
 	// returning.
 	subq	$56, %rsp
-	.cfi_def_cfa_offset 64
+	CFI_DEF_CFA_OFFSET(64)
 	movq  %rbp, 48(%rsp)
 	movupd	%xmm0, 32(%rsp)
 	movupd	%xmm1, 16(%rsp)
 	movq	%rax, 8(%rsp)
 	movq	%rdx, 0(%rsp)
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq %rax,%rax
 	je	.Ltmp2
 
@@ -138,22 +144,21 @@
 	movq	8(%rsp), %rax
 	movq	0(%rsp), %rdx
 	addq	$56, %rsp
-	.cfi_def_cfa_offset 8
+	CFI_DEF_CFA_OFFSET(8)
 	retq
-.Ltmp3:
-	.size __xray_FunctionExit, .Ltmp3-__xray_FunctionExit
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionExit)
+	CFI_ENDPROC
 
 //===--===//
 
-	.global __xray_FunctionTailExit
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionTailExit)
 	.align 16, 0x90
-	.type __xray_FunctionTailExit,@function
-__xray_FunctionTailExit:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionTailExit)
+ASM_TSAN_SYMBOL(__xray_FunctionTailExit):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq %rax,%rax
 	je	.Ltmp4
 
@@ -165,26 +170,25 @@
 .Ltmp4:
 	RESTORE_REGISTERS
 	retq
-.Ltmp5:
-	.size __xray_FunctionTailExit, .Ltmp5-__xray_FunctionTailExit
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionTailExit)
+	CFI_ENDPROC
 
 //===--===//
 
-	.globl __xray_ArgLoggerEntry
+	.globl ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry)
 	.align 16, 0x90
-	.type __xray_ArgLoggerEntry,@function
-__xray_ArgLoggerEntry:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_ArgLoggerEntry)
+ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
 	// Again, these function pointer loads must be atomic; MOV is fine.
-	movq	_ZN6__xray13XRayArgLoggerE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray13XRayArgLoggerE)(%rip), %rax
 	testq	%rax, %rax
 	jne	.Larg1entryLog
 
 	// If [arg1 logging handler] not set, defer to no-arg logging.
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	tes

[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

Landing this now and seeing whether there's fallout from the build bots.


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][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In https://reviews.llvm.org/D39114#938134, @Hahnfeld wrote:

> I'm getting "multiple definition" errors on powerpc64le when linking the unit 
> tests, for example `XRayBufferQueueTest`:
>
>   src/projects/compiler-rt/lib/xray/xray_utils.cc:34: multiple definition of 
> `__xray::retryingWriteAll(int, char*, char*)'
>   src/projects/compiler-rt/lib/xray/xray_utils.cc:73: multiple definition of 
> `__xray::readValueFromFile(char const*, long long*)'
>   src/projects/compiler-rt/lib/xray/xray_utils.cc:95: multiple definition of 
> `__xray::getLogFD()'
>
>
> to name a few of the errors.


Fixed in https://reviews.llvm.org/rL319241.

> (related note: Why aren't the unit tests rebuilt if the 
> `libclang_rt.xray-powerpc64le.a` changes?!? After reverting this commit 
> locally for testing, the unit test built just fine. After reapplying the test 
> wasn't rebuilt and I saw no problems...)

Yeah, that one I still haven't figured out 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][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 124717.
dberris added a comment.

- [XRay][compiler-rt][Darwin] Use dynamic initialisation as an alternative


https://reviews.llvm.org/D39114

Files:
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/weak_symbols.txt
  compiler-rt/lib/xray/xray_fdr_logging.cc
  compiler-rt/lib/xray/xray_flags.inc
  compiler-rt/lib/xray/xray_init.cc
  compiler-rt/lib/xray/xray_trampoline_x86_64.S

Index: compiler-rt/lib/xray/xray_trampoline_x86_64.S
===
--- compiler-rt/lib/xray/xray_trampoline_x86_64.S
+++ compiler-rt/lib/xray/xray_trampoline_x86_64.S
@@ -14,10 +14,13 @@
 //===--===//
 
 #include "../builtins/assembly.h"
+#include "../sanitizer_common/sanitizer_asm.h"
+
+
 
 .macro SAVE_REGISTERS
 	subq $192, %rsp
-	.cfi_def_cfa_offset 200
+	CFI_DEF_CFA_OFFSET(200)
 	// At this point, the stack pointer should be aligned to an 8-byte boundary,
 	// because any call instructions that come after this will add another 8
 	// bytes and therefore align it to 16-bytes.
@@ -57,7 +60,7 @@
 	movq	8(%rsp), %r8
 	movq	0(%rsp), %r9
 	addq	$192, %rsp
-	.cfi_def_cfa_offset 8
+	CFI_DEF_CFA_OFFSET(8)
 .endm
 
 .macro ALIGNED_CALL_RAX
@@ -75,21 +78,25 @@
 .endm
 
 	.text
+#if !defined(__APPLE__)
+	.section .text
+#else
+	.section __TEXT,__text
+#endif
 	.file "xray_trampoline_x86.S"
 
 //===--===//
 
-	.globl __xray_FunctionEntry
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionEntry)
 	.align 16, 0x90
-	.type __xray_FunctionEntry,@function
-
-__xray_FunctionEntry:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionEntry)
+ASM_TSAN_SYMBOL(__xray_FunctionEntry):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
 	// This load has to be atomic, it's concurrent with __xray_patch().
 	// On x86/amd64, a simple (type-aligned) MOV instruction is enough.
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq	%rax, %rax
 	je	.Ltmp0
 
@@ -101,28 +108,27 @@
 .Ltmp0:
 	RESTORE_REGISTERS
 	retq
-.Ltmp1:
-	.size __xray_FunctionEntry, .Ltmp1-__xray_FunctionEntry
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionEntry)
+	CFI_ENDPROC
 
 //===--===//
 
-	.globl __xray_FunctionExit
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionExit)
 	.align 16, 0x90
-	.type __xray_FunctionExit,@function
-__xray_FunctionExit:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionExit)
+ASM_TSAN_SYMBOL(__xray_FunctionExit):
+	CFI_STARTPROC
 	// Save the important registers first. Since we're assuming that this
 	// function is only jumped into, we only preserve the registers for
 	// returning.
 	subq	$56, %rsp
-	.cfi_def_cfa_offset 64
+	CFI_DEF_CFA_OFFSET(64)
 	movq  %rbp, 48(%rsp)
 	movupd	%xmm0, 32(%rsp)
 	movupd	%xmm1, 16(%rsp)
 	movq	%rax, 8(%rsp)
 	movq	%rdx, 0(%rsp)
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq %rax,%rax
 	je	.Ltmp2
 
@@ -138,22 +144,21 @@
 	movq	8(%rsp), %rax
 	movq	0(%rsp), %rdx
 	addq	$56, %rsp
-	.cfi_def_cfa_offset 8
+	CFI_DEF_CFA_OFFSET(8)
 	retq
-.Ltmp3:
-	.size __xray_FunctionExit, .Ltmp3-__xray_FunctionExit
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionExit)
+	CFI_ENDPROC
 
 //===--===//
 
-	.global __xray_FunctionTailExit
+	.globl ASM_TSAN_SYMBOL(__xray_FunctionTailExit)
 	.align 16, 0x90
-	.type __xray_FunctionTailExit,@function
-__xray_FunctionTailExit:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_FunctionTailExit)
+ASM_TSAN_SYMBOL(__xray_FunctionTailExit):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq %rax,%rax
 	je	.Ltmp4
 
@@ -165,26 +170,25 @@
 .Ltmp4:
 	RESTORE_REGISTERS
 	retq
-.Ltmp5:
-	.size __xray_FunctionTailExit, .Ltmp5-__xray_FunctionTailExit
-	.cfi_endproc
+	ASM_SIZE(__xray_FunctionTailExit)
+	CFI_ENDPROC
 
 //===--===//
 
-	.globl __xray_ArgLoggerEntry
+	.globl ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry)
 	.align 16, 0x90
-	.type __xray_ArgLoggerEntry,@function
-__xray_ArgLoggerEntry:
-	.cfi_startproc
+	ASM_TYPE_FUNCTION(__xray_ArgLoggerEntry)
+ASM_TSAN_SYMBOL(__xray_ArgLoggerEntry):
+	CFI_STARTPROC
 	SAVE_REGISTERS
 
 	// Again, these function pointer loads must be atomic; MOV is fine.
-	movq	_ZN6__xray13XRayArgLoggerE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray13XRayArgLoggerE)(%rip), %rax
 	testq	%rax, %rax
 	jne	.Larg1entryLog
 
 	// If [arg1 logging handler] not set, defer to no-arg logging.
-	movq	_ZN6__xray19XRayPatchedFunctionE(%rip), %rax
+	movq	ASM_TSAN_SYMBOL(_ZN6__xray19XRayPatchedFunctionE)(%rip), %rax
 	testq	%ra

[PATCH] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.

The -fxray-always-emit-customevents flag instructs clang to always emit
the LLVM IR for calls to the `__xray_customevent(...)` built-in
function. The default behaviour currently respects whether the function
has an `[[clang::xray_never_instrument]]` attribute, and thus not lower
the appropriate IR code for the custom event built-in.

This change allows users calling through to the
`__xray_customevent(...)` built-in to always see those calls lowered to
the corresponding LLVM IR to lay down instrumentation points for these
custom event calls.


https://reviews.llvm.org/D40601

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-always-emit-customevent.cpp

Index: clang/test/CodeGen/xray-always-emit-customevent.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-always-emit-customevent.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-always-emit-customevents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// CHECK-LABEL: @_Z15neverInstrumentv
+[[clang::xray_never_instrument]] void neverInstrument() {
+  static constexpr char kPhase[] = "never";
+  __xray_customevent(kPhase, 5);
+  // CHECK: call void @llvm.xray.customevent(i8*{{.*}}, i32 5)
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -785,6 +785,8 @@
   Opts.InstrumentFunctionEntryBare =
   Args.hasArg(OPT_finstrument_function_entry_bare);
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
+  Opts.XRayAlwaysEmitCustomEvents =
+  Args.hasArg(OPT_fxray_always_emit_customevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
@@ -2503,6 +2505,11 @@
   Opts.XRayInstrument =
   Args.hasFlag(OPT_fxray_instrument, OPT_fnoxray_instrument, false);
 
+  // -fxray-always-emit-customevents
+  Opts.XRayAlwaysEmitCustomEvents =
+  Args.hasFlag(OPT_fxray_always_emit_customevents,
+   OPT_fnoxray_always_emit_customevents, false);
+
   // -fxray-{always,never}-instrument= filenames.
   Opts.XRayAlwaysInstrumentFiles =
   Args.getAllArgValues(OPT_fxray_always_instrument);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -29,6 +29,8 @@
 "-fxray-instruction-threshold=";
 constexpr char XRayAlwaysInstrumentOption[] = "-fxray-always-instrument=";
 constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
+constexpr char XRayAlwaysEmitCustomEventsOption[] =
+"-fxray-always-emit-customevents";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
@@ -63,6 +65,14 @@
 D.Diag(clang::diag::err_drv_invalid_value) << A->getAsString(Args) << S;
 }
 
+// By default, the back-end will not emit the lowering for XRay customevent
+// calls if the function is not instrumented. In the future we will change
+// this default to be the reverse, but in the meantime we're going to
+// introduce the new functionality behind a flag.
+if (Args.hasFlag(options::OPT_fxray_always_emit_customevents,
+ options::OPT_fnoxray_always_emit_customevents, false))
+  XRayAlwaysEmitCustomEvents = true;
+
 // Validate the always/never attribute files. We also make sure that they
 // are treated as actual dependencies.
 for (const auto &Filename :
@@ -91,6 +101,10 @@
 return;
 
   CmdArgs.push_back(XRayInstrumentOption);
+
+  if (XRayAlwaysEmitCustomEvents)
+CmdArgs.push_back(XRayAlwaysEmitCustomEventsOption);
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -1771,6 +1771,10 @@
   /// instrumented with XRay nop sleds.
   bool ShouldXRayInstrumentFunction() const;
 
+  /// AlwaysEmitXRayCustomEvents - Return true if we must unconditionally emit
+  /// XRay custom event handling calls.
+  bool AlwaysEmitXRayCustomEvents() const;
+
   /// Encode an address into a form suitable for use in a function prologue.
   llvm::Constant 

[PATCH] D39114: [XRay][compiler-rt][Darwin] Minimal XRay build support in Darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris closed this revision.
dberris added a comment.

Already commited; closing this revision out now.


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] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 124842.
dberris marked an inline comment as done.
dberris edited the summary of this revision.
dberris added a comment.

Updated description, address review comment(s).


https://reviews.llvm.org/D40601

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-always-emit-customevent.cpp

Index: clang/test/CodeGen/xray-always-emit-customevent.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-always-emit-customevent.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-always-emit-customevents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// CHECK-LABEL: @_Z15neverInstrumentv
+[[clang::xray_never_instrument]] void neverInstrument() {
+  static constexpr char kPhase[] = "never";
+  __xray_customevent(kPhase, 5);
+  // CHECK: call void @llvm.xray.customevent(i8*{{.*}}, i32 5)
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -785,6 +785,8 @@
   Opts.InstrumentFunctionEntryBare =
   Args.hasArg(OPT_finstrument_function_entry_bare);
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
+  Opts.XRayAlwaysEmitCustomEvents =
+  Args.hasArg(OPT_fxray_always_emit_customevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
@@ -2503,6 +2505,11 @@
   Opts.XRayInstrument =
   Args.hasFlag(OPT_fxray_instrument, OPT_fnoxray_instrument, false);
 
+  // -fxray-always-emit-customevents
+  Opts.XRayAlwaysEmitCustomEvents =
+  Args.hasFlag(OPT_fxray_always_emit_customevents,
+   OPT_fnoxray_always_emit_customevents, false);
+
   // -fxray-{always,never}-instrument= filenames.
   Opts.XRayAlwaysInstrumentFiles =
   Args.getAllArgValues(OPT_fxray_always_instrument);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -27,8 +27,6 @@
 constexpr char XRayInstrumentOption[] = "-fxray-instrument";
 constexpr char XRayInstructionThresholdOption[] =
 "-fxray-instruction-threshold=";
-constexpr char XRayAlwaysInstrumentOption[] = "-fxray-always-instrument=";
-constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
@@ -63,6 +61,14 @@
 D.Diag(clang::diag::err_drv_invalid_value) << A->getAsString(Args) << S;
 }
 
+// By default, the back-end will not emit the lowering for XRay customevent
+// calls if the function is not instrumented. In the future we will change
+// this default to be the reverse, but in the meantime we're going to
+// introduce the new functionality behind a flag.
+if (Args.hasFlag(options::OPT_fxray_always_emit_customevents,
+ options::OPT_fnoxray_always_emit_customevents, false))
+  XRayAlwaysEmitCustomEvents = true;
+
 // Validate the always/never attribute files. We also make sure that they
 // are treated as actual dependencies.
 for (const auto &Filename :
@@ -91,17 +97,21 @@
 return;
 
   CmdArgs.push_back(XRayInstrumentOption);
+
+  if (XRayAlwaysEmitCustomEvents)
+CmdArgs.push_back("-fxray-always-emit-customevents");
+
   CmdArgs.push_back(Args.MakeArgString(Twine(XRayInstructionThresholdOption) +
Twine(InstructionThreshold)));
 
   for (const auto &Always : AlwaysInstrumentFiles) {
-SmallString<64> AlwaysInstrumentOpt(XRayAlwaysInstrumentOption);
+SmallString<64> AlwaysInstrumentOpt("-fxray-always-instrument=");
 AlwaysInstrumentOpt += Always;
 CmdArgs.push_back(Args.MakeArgString(AlwaysInstrumentOpt));
   }
 
   for (const auto &Never : NeverInstrumentFiles) {
-SmallString<64> NeverInstrumentOpt(XRayNeverInstrumentOption);
+SmallString<64> NeverInstrumentOpt("-fxray-never-instrument=");
 NeverInstrumentOpt += Never;
 CmdArgs.push_back(Args.MakeArgString(NeverInstrumentOpt));
   }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -1771,6 +1771,10 @@
   /// instrumented with XRay nop sleds.
   bool ShouldXRayInstrumentFunction() const;
 
+  /// AlwaysEmitXR

[PATCH] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: clang/lib/Driver/XRayArgs.cpp:32-33
 constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
+constexpr char XRayAlwaysEmitCustomEventsOption[] =
+"-fxray-always-emit-customevents";
 } // namespace

dblaikie wrote:
> Not clear to me there's a benefit to having these defined as constants versus 
> using the literal directly - other parts of the driver use literals directly 
> & there's are mostly used just once?
Good point. I was following the convention used in the SanitizerArgs 
implementation. I've moved the ones that haven't been repeated in this file 
in-line instead.


https://reviews.llvm.org/D40601



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

[PATCH] D40601: [XRay][clang] Introduce -fxray-always-emit-customevents

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL319388: [XRay][clang] Introduce 
-fxray-always-emit-customevents (authored by dberris).

Changed prior to commit:
  https://reviews.llvm.org/D40601?vs=124842&id=124846#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40601

Files:
  cfe/trunk/include/clang/Basic/LangOptions.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/XRayArgs.h
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/Driver/XRayArgs.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1060,6 +1060,12 @@
   Group, Flags<[CC1Option]>,
   HelpText<"Filename defining the whitelist for imbuing the 'never instrument' XRay attribute.">;
 
+def fxray_always_emit_customevents : Flag<["-"], "fxray-always-emit-customevents">, Group,
+  Flags<[CC1Option]>,
+  HelpText<"Determine whether to always emit __xray_customevent(...) calls even if the function it appears in is not always instrumented.">;
+def fnoxray_always_emit_customevents : Flag<["-"], "fno-xray-always-emit-customevents">, Group,
+  Flags<[CC1Option]>;
+
 def ffine_grained_bitfield_accesses : Flag<["-"],
   "ffine-grained-bitfield-accesses">, Group, Flags<[CC1Option]>,
   HelpText<"Use separate accesses for bitfields with legal widths and alignments.">;
Index: cfe/trunk/include/clang/Driver/XRayArgs.h
===
--- cfe/trunk/include/clang/Driver/XRayArgs.h
+++ cfe/trunk/include/clang/Driver/XRayArgs.h
@@ -24,6 +24,7 @@
   std::vector ExtraDeps;
   bool XRayInstrument = false;
   int InstructionThreshold = 200;
+  bool XRayAlwaysEmitCustomEvents = false;
 
 public:
   /// Parses the XRay arguments from an argument list.
Index: cfe/trunk/include/clang/Basic/LangOptions.def
===
--- cfe/trunk/include/clang/Basic/LangOptions.def
+++ cfe/trunk/include/clang/Basic/LangOptions.def
@@ -272,6 +272,9 @@
"aggressive, 2: more aggressive)")
 
 LANGOPT(XRayInstrument, 1, 0, "controls whether to do XRay instrumentation")
+LANGOPT(XRayAlwaysEmitCustomEvents, 1, 0,
+"controls whether to always emit intrinsic calls to "
+"__xray_customevent(...) builtin.")
 
 BENIGN_LANGOPT(AllowEditorPlaceholders, 1, 0,
"allow editor placeholders in source")
Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -84,6 +84,9 @@
 CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is
///< enabled.
 
+///< Set when -fxray-always-emit-customevents is enabled.
+CODEGENOPT(XRayAlwaysEmitCustomEvents , 1, 0)
+
 ///< Set the minimum number of instructions in a function to determine selective
 ///< XRay instrumentation.
 VALUE_CODEGENOPT(XRayInstructionThreshold , 32, 200)
Index: cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp
===
--- cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp
+++ cfe/trunk/test/CodeGen/xray-always-emit-customevent.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-always-emit-customevents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// CHECK-LABEL: @_Z15neverInstrumentv
+[[clang::xray_never_instrument]] void neverInstrument() {
+  static constexpr char kPhase[] = "never";
+  __xray_customevent(kPhase, 5);
+  // CHECK: call void @llvm.xray.customevent(i8*{{.*}}, i32 5)
+}
Index: cfe/trunk/lib/Driver/XRayArgs.cpp
===
--- cfe/trunk/lib/Driver/XRayArgs.cpp
+++ cfe/trunk/lib/Driver/XRayArgs.cpp
@@ -27,8 +27,6 @@
 constexpr char XRayInstrumentOption[] = "-fxray-instrument";
 constexpr char XRayInstructionThresholdOption[] =
 "-fxray-instruction-threshold=";
-constexpr char XRayAlwaysInstrumentOption[] = "-fxray-always-instrument=";
-constexpr char XRayNeverInstrumentOption[] = "-fxray-never-instrument=";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
@@ -63,6 +61,14 @@
 D.Diag(clang::diag::err_drv_invalid_value) << A->getAsString(Args) << S;
 }
 
+// By default, the back-end will not emit the lowering for XRay customevent
+// calls if the function is not ins

[PATCH] D117929: [XRay] Add support for RISCV

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In D117929#3276843 , @ashwin98 wrote:

> Fixed another lint issue, they should all be done for now hopefully.
>
> @dberris Sorry, I'm a little confused, do you mean I need to update some of 
> the clang tests for XRay instrumentation to test compilation and linking, or 
> add new ones for the llvm-xray tool under llvm/test/tools?

Yes to both. :)

There are already some tests that ensure the supported triples build with XRay 
instrumentation and that we can get the instrumentation map with the tooling on 
binaries generated with clang+lld. Extending those to support risc64 and ensure 
we're able to find the instrumentation with the tooling gives us higher 
confidence that this will work when the patch is accepted.


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

https://reviews.llvm.org/D117929

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

[PATCH] D117929: [XRay] Add support for RISCV

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In D117929#3317813 , @ashwin98 wrote:

> In D117929#3314355 , @ashwin98 
> wrote:
>
>> On including RISCV64 to the check, I see new issues while reading the 
>> instrumentation map. How do I verify that the instrumentation map is being 
>> generated correctly (or if there is a problem with it), and where is the 
>> code that is responsible for the generation of the instrumentation map (is 
>> it in xray_init.cpp)? I'm not sure if this is a RISCV compatibility issue 
>> with the xray tool, or if I've missed something that is causing problems 
>> during the instrumentation map initialization.
>
> I traced the root cause of the issue.  It seems to be stemming from the 
> instrumentation map's relocation handling, specifically, at line 129 of 
> InstrumentationMap.cpp, when we try to extract the load address of the 
> symbol. I believe that part of code was written keeping AArch64 in mind, but 
> I'm not too sure about what changes will need to be made to add RISCV64 
> compatibility, I'm trying to figure it out.

If you can turn the relocations you're emitting in the assembly to be relative 
instead of absolute when building the instrumentation map like in other 
architectures, then the tooling will be able to resolve them. Maybe that helps?


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

https://reviews.llvm.org/D117929

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

[PATCH] D61758: [driver][xray] fix the macOS support checker by supporting -macos triple in addition to -darwin

[Dean Michael Berris via Phabricator via cfe-commits]

dberris accepted this revision.
dberris added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: ributzka.

LGTM -- apologies for the delay, I hope this isn't too late.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61758



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

[PATCH] D87953: [xray] Function coverage groups

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:817
+CurFn->getName().bytes_end());
+  auto Group = crc32(FuncName) % FuncGroups;
+  if (Group != CGM.getCodeGenOpts().XRaySelectedFunctionGroup &&

I'm a little concerned that this is randomly determined. Could we think about 
using a function attribute instead that opts functions into groups by name? Or 
is the intent to have "sampling" in the coverage information instead of full 
coverage?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87953

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

[PATCH] D87953: [xray] Function coverage groups

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

There's two tensions I'm trying to resolve in my mind with this patch:

- Is there a way to make this more deterministic, instead of using a random 
sampling mechanism based on the name?
- This looks like it's a subset of a more general facility to group functions 
by "module" or some logical grouping mechanism. Will this be superseded by a 
feature that allows for example to provide filters in the driver to define 
groupings? If so, wouldn't something like that be more flexible for your needs 
(group names in the IR)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87953

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

[PATCH] D87953: [xray] Function coverage groups

[Dean Michael Berris via Phabricator via cfe-commits]

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

Okay, I think I understand the motivation a little bit more now (i.e. sampling 
seems to be a useful thing to do anyway, regardless of whether we have the 
other facilities).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87953

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

[PATCH] D89441: RFC: Potential fixes to function-instrument=xray-never

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

> Do you think we should fix this by handling the xray-never attribute in 
> XRayInstrumentation, or having CodeGenFunction remove the threshold if 
> xray-never is set?

Good catch, thanks!

I think we should go with the former (handling the case in XRayInstrumentation) 
-- the rationale here is that when we apply the file-based always-instrument 
case, we ignore the "never" attribute (i.e. the "always" case overrides the 
"never" case). We should make sure that this case still works (if we don't have 
cases for that, then we should add those too).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89441

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

[PATCH] D89441: [xray] Honor xray-never function-instrument attribute

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM -- thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89441

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

[PATCH] D77191: [clang][xray] Add xray attributes to functions without decls too

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77191



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

[PATCH] D72890: [xray] Allow instrumenting only function entry and/or only function exit

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM

Do you mind adding x86 tests too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72890



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

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: echristo, kpw, eizan, pelikan.

This change addresses http://llvm.org/PR36926 by allowing users to pick
which instrumentation bundles to use, when instrumenting with XRay. In
particular, the flag `-fxray-instrumentation-bundle=` has four valid
values:

- `all`: the default, which will emit all kinds of instrumentation

points.

- `none`: equivalent to -fnoxray-instrument
- `function-extents`: only emits the entry/exit sleds
- `custom-only`: only emits the custom event sleds


https://reviews.llvm.org/D44970

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/include/clang/Frontend/CodeGenOptions.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-instrumentation-bundles.cpp

Index: clang/test/CodeGen/xray-instrumentation-bundles.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-instrumentation-bundles.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-instrumentation-bundle=none -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-extents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+
+// CHECK-LABEL: alwaysInstrument
+[[clang::xray_always_instrument]] void alwaysInstrument() {
+  static constexpr char kPhase[] = "always";
+  __xray_customevent(kPhase, 6);
+  // CHECK-NOT: call void @llvm.xray.customevent(i8*{{.*}}, i32 6)
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -446,6 +446,21 @@
   }
 }
 
+static CodeGenOptions::XRayInstrumentationPointBundle
+parseXRayInstrumentationBundle(Arg *A, ArgList &Args, DiagnosticsEngine &D) {
+  StringRef V = A->getValue();
+  if (V == "all")
+return CodeGenOptions::XRayInstrumentationPointBundle::XRay_All;
+  if (V == "none")
+return CodeGenOptions::XRayInstrumentationPointBundle::XRay_None;
+  if (V == "function-extents")
+return CodeGenOptions::XRayInstrumentationPointBundle::XRay_FunctionExtents;
+  if (V == "custom-only")
+return CodeGenOptions::XRayInstrumentationPointBundle::XRay_CustomOnly;
+  D.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << V;
+  return CodeGenOptions::XRayInstrumentationPointBundle::XRay_All;
+}
+
 // Set the profile kind for fprofile-instrument.
 static void setPGOInstrumentor(CodeGenOptions &Opts, ArgList &Args,
DiagnosticsEngine &Diags) {
@@ -853,6 +868,11 @@
   Args.hasArg(OPT_fxray_always_emit_customevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
+
+  if (auto A = Args.getLastArg(OPT_fxray_instrumentation_bundle))
+Opts.setXRayInstrumentationBundle(
+parseXRayInstrumentationBundle(A, Args, Diags));
+
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -27,6 +27,8 @@
 constexpr char XRayInstrumentOption[] = "-fxray-instrument";
 constexpr char XRayInstructionThresholdOption[] =
 "-fxray-instruction-threshold=";
+constexpr char XRayInstrumentationBundleOption[] =
+"-fxray-instrumentation-bundle=";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
@@ -50,13 +52,14 @@
 << (std::string(XRayInstrumentOption) + " on " + Triple.str());
   }
 } else if (Triple.getOS() == llvm::Triple::FreeBSD) {
-if (Triple.getArch() != llvm::Triple::x86_64) {
-  D.Diag(diag::err_drv_clang_unsupported)
-  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
-}
+  if (Triple.getArch() != llvm::Triple::x86_64) {
+D.Diag(diag::err_drv_clang_unsupported)
+<< (std::string(XRayInstrumentOption) + " on " + Triple.str());
+  }
 } else {
   D.Diag(diag::err_drv_clang_unsupported)
-  << (std::string(XRayInstrumentOption) + " on non-supported target OS");
+  << (std::string(XRayInstrumentOption) +
+  " on non-supported target OS");
 }
 XRayInstrument = true;
 if (const Arg *A =
@@ -75,6 +78,19 @@
  options::OPT_fnoxray_always_emit_customevents, false))
   XRayAlwaysEmitCustomEvents = true;
 
+// We check the bundle of instrumentation tha

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 140731.
dberris marked 8 inline comments as done.
dberris added a comment.

- fixup: address comments


https://reviews.llvm.org/D44970

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/include/clang/Frontend/CodeGenOptions.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-instrumentation-bundles.cpp

Index: clang/test/CodeGen/xray-instrumentation-bundles.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-instrumentation-bundles.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-instrumentation-bundle=none -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-extents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=none -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,DISABLE %s
+
+// CHECK-LABEL: alwaysInstrument
+[[clang::xray_always_instrument]] void alwaysInstrument() {
+  static constexpr char kPhase[] = "always";
+  __xray_customevent(kPhase, 6);
+  // CHECK-NOT: call void @llvm.xray.customevent(i8*{{.*}}, i32 6)
+  // DISABLE-NOT: call void @llvm.xray.customevent(i8*{{.*}}, i32 6)
+}
+
+// DISABLE-NOT: function-instrument=always
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -446,6 +446,23 @@
   }
 }
 
+static CodeGenOptions::XRayInstrumentationPointBundle
+parseXRayInstrumentationBundle(Arg *A, ArgList &Args, DiagnosticsEngine &D) {
+  StringRef V = A->getValue();
+  auto Bundle =  llvm::StringSwitch(V)
+  .Case("all", CodeGenOptions::XRayInstrumentationPointBundle::XRay_All)
+  .Case("none", CodeGenOptions::XRayInstrumentationPointBundle::XRay_None)
+  .Case("function-extents",
+CodeGenOptions::XRayInstrumentationPointBundle::XRay_Function)
+  .Case("custom-only",
+CodeGenOptions::XRayInstrumentationPointBundle::XRay_CustomOnly)
+  .Default(CodeGenOptions::XRayInstrumentationPointBundle::XRay_All);
+  if (Bundle == CodeGenOptions::XRayInstrumentationPointBundle::XRay_All &&
+  !V.empty() && V != "all")
+D.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << V;
+  return Bundle;
+}
+
 // Set the profile kind for fprofile-instrument.
 static void setPGOInstrumentor(CodeGenOptions &Opts, ArgList &Args,
DiagnosticsEngine &Diags) {
@@ -821,11 +838,20 @@
   Args.hasArg(OPT_finstrument_functions_after_inlining);
   Opts.InstrumentFunctionEntryBare =
   Args.hasArg(OPT_finstrument_function_entry_bare);
-  Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
   Opts.XRayAlwaysEmitCustomEvents =
   Args.hasArg(OPT_fxray_always_emit_customevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
+
+  if (auto A = Args.getLastArg(OPT_fxray_instrumentation_bundle))
+Opts.setXRayInstrumentationBundle(
+parseXRayInstrumentationBundle(A, Args, Diags));
+
+  Opts.XRayInstrumentFunctions =
+  Args.hasArg(OPT_fxray_instrument) &&
+  Opts.getXRayInstrumentationBundle() !=
+  CodeGenOptions::XRayInstrumentationPointBundle::XRay_None;
+
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -27,6 +27,8 @@
 constexpr char XRayInstrumentOption[] = "-fxray-instrument";
 constexpr char XRayInstructionThresholdOption[] =
 "-fxray-instruction-threshold=";
+constexpr char XRayInstrumentationBundleOption[] =
+"-fxray-instrumentation-bundle=";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
@@ -50,10 +52,10 @@
 << (std::string(XRayInstrumentOption) + " on " + Triple.str());
   }
 } else if (Triple.getOS() == llvm::Triple::FreeBSD) {
-if (Triple.getArch() != llvm::Triple::x86_64) {
-  D.Diag(diag::err_drv_clang_unsupported)
-  << (std::string(XRayInstrumentOption) + " on " + Triple.str());
-}
+  if (Triple.getArch() != llvm::Triple::x86_64) {
+D.Diag(diag:

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: clang/include/clang/Frontend/CodeGenOptions.h:110
 
+  enum XRayInstrumentationPointBundle {
+XRay_All, // Always emit all the instrumentation points.

pelikan wrote:
> To me, this feels like a bitfield would be a better match.
> All = Function | Custom
> None = 0
Thought about that, but the ergonomics from the user-side isn't as good. Having 
to know about which kinds of sleds specifically to enable seems much harder to 
explain. Using bundles that we control from the beginning keeps this much 
simpler.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3248
+// custom events.
+{
+  auto XRayBundle = CGM.getCodeGenOpts().getXRayInstrumentationBundle();

echristo wrote:
> I'd probably spell this code like the block above it rather than this.
The codegen options don't work like pointers, and we can't use C++17 if 
initialisers yet. Would have loved this to be:

```
if (auto XRayBundle = ...; XRayBundle == ... || XRayBundle == ...)
  return RValue::getIgnored();
```

Removed the scoping instead.



Comment at: clang/lib/Driver/XRayArgs.cpp:62
+  << (std::string(XRayInstrumentOption) +
+  " on non-supported target OS");
 }

pelikan wrote:
> I would also print the triple.  Something like:
> 
> "-fomit-bugs is not supported on target win64-ppc-none"
> 
> will be much more informative, especially when you collect logs from build 
> machines on lots of architectures (like Linux distro/BSD package builders do).
Probably not today. Good idea though, could be a different patch.


https://reviews.llvm.org/D44970



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

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 140735.
dberris added a comment.

- fixup: Fix tests for better coverage of settings


https://reviews.llvm.org/D44970

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/include/clang/Frontend/CodeGenOptions.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-instrumentation-bundles.cpp

Index: clang/test/CodeGen/xray-instrumentation-bundles.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-instrumentation-bundles.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fxray-instrument -fxray-instrumentation-bundle=none -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,NOCUSTOM %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=function-extents -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,FUNCTION,NOCUSTOM %s
+// RUN: %clang_cc1 -fxray-instrument \
+// RUN: -fxray-instrumentation-bundle=custom-only -x c++ \
+// RUN: -std=c++11 -triple x86_64-unknown-unknown -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK,NOFUNCTION,CUSTOM %s
+
+// CHECK: define void @_Z16alwaysInstrumentv() #[[ALWAYSATTR:[0-9]+]] {
+[[clang::xray_always_instrument]] void alwaysInstrument() {
+  static constexpr char kPhase[] = "always";
+  __xray_customevent(kPhase, 6);
+  // CUSTOM: call void @llvm.xray.customevent(i8*{{.*}}, i32 6)
+  // NOCUSTOM-NOT: call void @llvm.xray.customevent(i8*{{.*}}, i32 6)
+}
+
+// FUNCTION: attributes #[[ALWAYSATTR]] = {{.*}} "function-instrument"="xray-always" {{.*}}
+// NOFUNCTION-NOT: attributes #[[ALWAYSATTR]] = {{.*}} "function-instrument"="xray-always" {{.*}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -446,6 +446,23 @@
   }
 }
 
+static CodeGenOptions::XRayInstrumentationPointBundle
+parseXRayInstrumentationBundle(Arg *A, ArgList &Args, DiagnosticsEngine &D) {
+  StringRef V = A->getValue();
+  auto Bundle =  llvm::StringSwitch(V)
+  .Case("all", CodeGenOptions::XRayInstrumentationPointBundle::XRay_All)
+  .Case("none", CodeGenOptions::XRayInstrumentationPointBundle::XRay_None)
+  .Case("function-extents",
+CodeGenOptions::XRayInstrumentationPointBundle::XRay_Function)
+  .Case("custom-only",
+CodeGenOptions::XRayInstrumentationPointBundle::XRay_CustomOnly)
+  .Default(CodeGenOptions::XRayInstrumentationPointBundle::XRay_All);
+  if (Bundle == CodeGenOptions::XRayInstrumentationPointBundle::XRay_All &&
+  !V.empty() && V != "all")
+D.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << V;
+  return Bundle;
+}
+
 // Set the profile kind for fprofile-instrument.
 static void setPGOInstrumentor(CodeGenOptions &Opts, ArgList &Args,
DiagnosticsEngine &Diags) {
@@ -821,11 +838,18 @@
   Args.hasArg(OPT_finstrument_functions_after_inlining);
   Opts.InstrumentFunctionEntryBare =
   Args.hasArg(OPT_finstrument_function_entry_bare);
-  Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
+
+  Opts.XRayInstrumentFunctions =
+  Args.hasArg(OPT_fxray_instrument);
   Opts.XRayAlwaysEmitCustomEvents =
   Args.hasArg(OPT_fxray_always_emit_customevents);
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_EQ, 200, Diags);
+
+  if (auto A = Args.getLastArg(OPT_fxray_instrumentation_bundle))
+Opts.setXRayInstrumentationBundle(
+parseXRayInstrumentationBundle(A, Args, Diags));
+
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -27,6 +27,8 @@
 constexpr char XRayInstrumentOption[] = "-fxray-instrument";
 constexpr char XRayInstructionThresholdOption[] =
 "-fxray-instruction-threshold=";
+constexpr char XRayInstrumentationBundleOption[] =
+"-fxray-instrumentation-bundle=";
 } // namespace
 
 XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
@@ -50,10 +52,10 @@
 << (std::string(XRayInstrumentOption) + " on " + Triple.str());
   }
 } else if (Triple.getOS() == llvm::Triple::FreeBSD) {
-if (Triple.getArch() != llvm::Triple::x86_64) {
-  D.Diag(diag::err_

[PATCH] D45126: Xray, OpenBSD support

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM -- at some point, it would be good to refactor all these flag settings to 
a single place. Maybe file a bug so that we can track that issue on XRay? If 
you can't do it now, I'd be happy to do it later on.


Repository:
  rC Clang

https://reviews.llvm.org/D45126



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

[PATCH] D45126: Xray, OpenBSD support

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In https://reviews.llvm.org/D45126#1055451, @devnexen wrote:

> In https://reviews.llvm.org/D45126#1055266, @dberris wrote:
>
> > LGTM -- at some point, it would be good to refactor all these flag settings 
> > to a single place. Maybe file a bug so that we can track that issue on 
> > XRay? If you can't do it now, I'd be happy to do it later on.
>
>
> I do not have access to the bug tracker (yet) you can if you wish (I guess it 
> s adding it in the "Common" driver ?


Fixed: http://llvm.org/PR36985


Repository:
  rC Clang

https://reviews.llvm.org/D45126



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

[PATCH] D45126: Xray, OpenBSD support

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.



Comment at: lib/Driver/XRayArgs.cpp:53
+} else if (Triple.getOS() == llvm::Triple::FreeBSD ||
+  Triple.getOS() == llvm::Triple::OpenBSD) {
 if (Triple.getArch() != llvm::Triple::x86_64) {

I just noticed this, you probably want to format this part with clang-format 
(or just move the T of the second line one space, to align with the T on the 
previous line.


Repository:
  rC Clang

https://reviews.llvm.org/D45126



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

[PATCH] D45126: Xray, OpenBSD support

[Dean Michael Berris via Phabricator via cfe-commits]

dberris accepted this revision.
dberris added a comment.

LGTM -- I'll defer to @brad on confirming and landing.


https://reviews.llvm.org/D45126



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

[PATCH] D45243: [XRay][clang] Consolidate runtime and link-time flag processing (NFC)

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: echristo, devnexen.
Herald added a subscriber: emaste.

This change fixes http://llvm.org/PR36985 to define a single place in
CommonArgs.{h,cpp} where XRay runtime flags and link-time dependencies
are processed for all toolchains that support XRay instrumentation. This
is a refactoring of the same functionality spread across multiple
toolchain definitions.


https://reviews.llvm.org/D45243

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp

Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -221,37 +221,6 @@
   // The types are (hopefully) good enough.
 }
 
-static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
-   ArgStringList &CmdArgs) {
-  // Do not add the XRay runtime to shared libraries.
-  if (Args.hasArg(options::OPT_shared))
-return false;
-
-  if (Args.hasFlag(options::OPT_fxray_instrument,
-   options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-whole-archive");
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
-CmdArgs.push_back("-no-whole-archive");
-return true;
-  }
-
-  return false;
-}
-
-static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
-ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
-  CmdArgs.push_back("-lpthread");
-  if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
-CmdArgs.push_back("-lrt");
-  CmdArgs.push_back("-lm");
-
-  if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
-  TC.getTriple().getOS() != llvm::Triple::NetBSD &&
-  TC.getTriple().getOS() != llvm::Triple::OpenBSD)
-CmdArgs.push_back("-ldl");
-}
-
 static const char *getLDMOption(const llvm::Triple &T, const ArgList &Args) {
   switch (T.getArch()) {
   case llvm::Triple::x86:
@@ -496,7 +465,7 @@
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
   if (NeedsXRayDeps)
-linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
+linkXRayRuntimeDeps(ToolChain, CmdArgs);
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -117,30 +117,6 @@
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 
-static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
-   ArgStringList &CmdArgs) {
-  if (Args.hasArg(options::OPT_shared))
-return false;
-
-  if (Args.hasFlag(options::OPT_fxray_instrument,
-   options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-whole-archive");
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
-CmdArgs.push_back("-no-whole-archive");
-return true;
-  }
-  
-  return false;
-}
-
-static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
-ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
-  CmdArgs.push_back("-lrt");
-  CmdArgs.push_back("-lm");
-  CmdArgs.push_back("-lpthread");
-} 
-
 void freebsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output,
const InputInfoList &Inputs,
@@ -275,7 +251,7 @@
 if (NeedsSanitizerDeps)
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 if (NeedsXRayDeps)
-  linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
+  linkXRayRuntimeDeps(ToolChain, CmdArgs);
 // FIXME: For some reason GCC passes -lgcc and -lgcc_s before adding
 // the default system libraries. Just mimic this for now.
 if (Args.hasArg(options::OPT_pg))
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -35,6 +35,12 @@
 void linkSanitizerRuntimeDeps(const ToolChain &TC,
   llvm::opt::ArgStringList &CmdArgs);
 
+bool addXRayRuntime(const ToolChain &TC, const llvm::opt::ArgList &Args,
+llvm::opt::ArgStringList &CmdArgs);
+
+void linkXRayRuntimeDeps(const ToolChain &TC,
+ llvm::opt::ArgStringList &CmdArgs);
+
 void AddRunTimeLibs(const ToolChain &TC, const Driver &D,
 llvm::opt::ArgStringList &CmdArgs,
 const llvm::opt::ArgList &Args);
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
=

[PATCH] D45126: Xray, OpenBSD support

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

Do you need to have someone land this for you, or can you land it yourself? If 
@brad isn't able to get to it anytime soon, then I'm happy for this to land 
either way.


https://reviews.llvm.org/D45126



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

[PATCH] D45243: [XRay][clang] Consolidate runtime and link-time flag processing (NFC)

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added a comment.

In https://reviews.llvm.org/D45243#1056549, @devnexen wrote:

> Do the unit tests have same outcome with these changes ? Otherwise, 
> appreciate the simplification.


Yes, locally, `ninja check-all` on Linux works just fine for me. No idea 
whether this will be the case for the buildbots, but that's easy enough to 
address when I get LGTM for the patch (and it lands, preferably daytime in 
Sydney).


https://reviews.llvm.org/D45243



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

[PATCH] D45126: Xray, OpenBSD support

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC329183: [XRay][clang] Allow clang to build XRay instrumented 
binaries in OpenBSD (authored by dberris, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45126

Files:
  lib/Driver/ToolChains/OpenBSD.cpp
  lib/Driver/XRayArgs.cpp


Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -22,6 +22,29 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
+  if (Args.hasFlag(options::OPT_fxray_instrument,
+   options::OPT_fnoxray_instrument, false)) {
+CmdArgs.push_back("-whole-archive");
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
+CmdArgs.push_back("-no-whole-archive");
+return true;
+  }
+
+  return false;
+}
+
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
+  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back("-lm");
+  CmdArgs.push_back("-lpthread");
+}
+
 void openbsd::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
   const InputInfo &Output,
   const InputInfoList &Inputs,
@@ -180,6 +203,7 @@
 options::OPT_Z_Flag, options::OPT_r});
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
@@ -195,6 +219,10 @@
   CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", 
false));
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 }
+if (NeedsXRayDeps) {
+  CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", 
false));
+  linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
+}
 // FIXME: For some reason GCC passes -lgcc before adding
 // the default system libraries. Just mimic this for now.
 CmdArgs.push_back("-lgcc");
Index: lib/Driver/XRayArgs.cpp
===
--- lib/Driver/XRayArgs.cpp
+++ lib/Driver/XRayArgs.cpp
@@ -49,7 +49,8 @@
 D.Diag(diag::err_drv_clang_unsupported)
 << (std::string(XRayInstrumentOption) + " on " + Triple.str());
   }
-} else if (Triple.getOS() == llvm::Triple::FreeBSD) {
+} else if (Triple.getOS() == llvm::Triple::FreeBSD ||
+   Triple.getOS() == llvm::Triple::OpenBSD) {
 if (Triple.getArch() != llvm::Triple::x86_64) {
   D.Diag(diag::err_drv_clang_unsupported)
   << (std::string(XRayInstrumentOption) + " on " + Triple.str());


Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -22,6 +22,29 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
+  if (Args.hasFlag(options::OPT_fxray_instrument,
+   options::OPT_fnoxray_instrument, false)) {
+CmdArgs.push_back("-whole-archive");
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
+CmdArgs.push_back("-no-whole-archive");
+return true;
+  }
+
+  return false;
+}
+
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
+  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back("-lm");
+  CmdArgs.push_back("-lpthread");
+}
+
 void openbsd::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
   const InputInfo &Output,
   const InputInfoList &Inputs,
@@ -180,6 +203,7 @@
 options::OPT_Z_Flag, options::OPT_r});
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
@@ -195,6 +219,10 @@
   CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", false));
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 }
+if (NeedsXRayDeps) {
+  CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", false));
+  linkXRayRuntimeDeps(ToolCha

[PATCH] D45243: [XRay][clang] Consolidate runtime and link-time flag processing (NFC)

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 141265.
dberris added a comment.

- fixup: Rebase, and re-do OpenBSD specific changes


https://reviews.llvm.org/D45243

Files:
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/OpenBSD.cpp

Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -22,29 +22,6 @@
 using namespace clang;
 using namespace llvm::opt;
 
-static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
-   ArgStringList &CmdArgs) {
-  if (Args.hasArg(options::OPT_shared))
-return false;
-
-  if (Args.hasFlag(options::OPT_fxray_instrument,
-   options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-whole-archive");
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
-CmdArgs.push_back("-no-whole-archive");
-return true;
-  }
-
-  return false;
-}
-
-static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
-ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
-  CmdArgs.push_back("-lm");
-  CmdArgs.push_back("-lpthread");
-}
-
 void openbsd::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
   const InputInfo &Output,
   const InputInfoList &Inputs,
@@ -221,7 +198,7 @@
 }
 if (NeedsXRayDeps) {
   CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", false));
-  linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
+  linkXRayRuntimeDeps(ToolChain, CmdArgs);
 }
 // FIXME: For some reason GCC passes -lgcc before adding
 // the default system libraries. Just mimic this for now.
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -221,37 +221,6 @@
   // The types are (hopefully) good enough.
 }
 
-static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
-   ArgStringList &CmdArgs) {
-  // Do not add the XRay runtime to shared libraries.
-  if (Args.hasArg(options::OPT_shared))
-return false;
-
-  if (Args.hasFlag(options::OPT_fxray_instrument,
-   options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-whole-archive");
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
-CmdArgs.push_back("-no-whole-archive");
-return true;
-  }
-
-  return false;
-}
-
-static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
-ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
-  CmdArgs.push_back("-lpthread");
-  if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
-CmdArgs.push_back("-lrt");
-  CmdArgs.push_back("-lm");
-
-  if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
-  TC.getTriple().getOS() != llvm::Triple::NetBSD &&
-  TC.getTriple().getOS() != llvm::Triple::OpenBSD)
-CmdArgs.push_back("-ldl");
-}
-
 static const char *getLDMOption(const llvm::Triple &T, const ArgList &Args) {
   switch (T.getArch()) {
   case llvm::Triple::x86:
@@ -496,7 +465,7 @@
 linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
   if (NeedsXRayDeps)
-linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
+linkXRayRuntimeDeps(ToolChain, CmdArgs);
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -117,30 +117,6 @@
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 
-static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
-   ArgStringList &CmdArgs) {
-  if (Args.hasArg(options::OPT_shared))
-return false;
-
-  if (Args.hasFlag(options::OPT_fxray_instrument,
-   options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-whole-archive");
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
-CmdArgs.push_back("-no-whole-archive");
-return true;
-  }
-  
-  return false;
-}
-
-static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
-ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
-  CmdArgs.push_back("-lrt");
-  CmdArgs.push_back("-lm");
-  CmdArgs.push_back("-lpthread");
-} 
-
 void freebsd::Linker::ConstructJob(Compilation &C, const JobAction &

[PATCH] D45243: [XRay][clang] Consolidate runtime and link-time flag processing (NFC)

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329372: [XRay][clang] Consolidate runtime and link-time flag 
processing (NFC) (authored by dberris, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45243?vs=141265&id=141273#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45243

Files:
  cfe/trunk/include/clang/Driver/XRayArgs.h
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
  cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp

Index: cfe/trunk/include/clang/Driver/XRayArgs.h
===
--- cfe/trunk/include/clang/Driver/XRayArgs.h
+++ cfe/trunk/include/clang/Driver/XRayArgs.h
@@ -31,6 +31,8 @@
   XRayArgs(const ToolChain &TC, const llvm::opt::ArgList &Args);
   void addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs, types::ID InputType) const;
+
+  bool needsXRayRt() const { return XRayInstrument; }
 };
 
 } // namespace driver
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -31,6 +31,7 @@
 #include "clang/Driver/SanitizerArgs.h"
 #include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Util.h"
+#include "clang/Driver/XRayArgs.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -705,6 +706,33 @@
   return !StaticRuntimes.empty() || !NonWholeStaticRuntimes.empty();
 }
 
+bool tools::addXRayRuntime(const ToolChain&TC, const ArgList &Args, ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
+  if (TC.getXRayArgs().needsXRayRt()) {
+CmdArgs.push_back("-whole-archive");
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
+CmdArgs.push_back("-no-whole-archive");
+return true;
+  }
+
+  return false;
+}
+
+void tools::linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
+  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back("-lpthread");
+  if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
+CmdArgs.push_back("-lrt");
+  CmdArgs.push_back("-lm");
+
+  if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
+  TC.getTriple().getOS() != llvm::Triple::NetBSD &&
+  TC.getTriple().getOS() != llvm::Triple::OpenBSD)
+CmdArgs.push_back("-ldl");
+}
+
 bool tools::areOptimizationsEnabled(const ArgList &Args) {
   // Find the last -O arg and see if it is non-zero.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group))
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
@@ -35,6 +35,12 @@
 void linkSanitizerRuntimeDeps(const ToolChain &TC,
   llvm::opt::ArgStringList &CmdArgs);
 
+bool addXRayRuntime(const ToolChain &TC, const llvm::opt::ArgList &Args,
+llvm::opt::ArgStringList &CmdArgs);
+
+void linkXRayRuntimeDeps(const ToolChain &TC,
+ llvm::opt::ArgStringList &CmdArgs);
+
 void AddRunTimeLibs(const ToolChain &TC, const Driver &D,
 llvm::opt::ArgStringList &CmdArgs,
 const llvm::opt::ArgList &Args);
Index: cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
+++ cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
@@ -117,30 +117,6 @@
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 
-static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
-   ArgStringList &CmdArgs) {
-  if (Args.hasArg(options::OPT_shared))
-return false;
-
-  if (Args.hasFlag(options::OPT_fxray_instrument,
-   options::OPT_fnoxray_instrument, false)) {
-CmdArgs.push_back("-whole-archive");
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
-CmdArgs.push_back("-no-whole-archive");
-return true;
-  }
-  
-  return false;
-}
-
-static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
-ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
-  CmdArgs.push_back("-lrt");
-  CmdArgs.push_back("-lm");
-  CmdArgs.push_back("-lpthread");
-} 
-
 void freebsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output,
const InputInfoList &Inputs,
@@ -275,7 +251,7 @@
 if (NeedsSanitizerDeps)
   linkSanitizerRuntimeDeps(ToolChain, 

[PATCH] D45354: [XRay][clang] Add a flag to enable/disable linking XRay deps explicitly

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: eizan, echristo, chandlerc.

This change introduces `-fxray-link-deps` and `-fnoxray-link-deps`. The
`-fnoxray-link-deps` allows for directly controlling which specific XRay
runtime to link. The default is for clang to link the XRay runtime that
is shipped with the compiler (if there are any), but users may want to
explicitly add the XRay dependencies from other locations or other
means.


https://reviews.llvm.org/D45354

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/Driver/XRayArgs.cpp
  clang/test/Driver/XRay/xray-nolinkdeps.cpp


Index: clang/test/Driver/XRay/xray-nolinkdeps.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-nolinkdeps.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang -v -o /dev/null -fxray-instrument -fnoxray-link-deps %s -### \
+// RUN: 2>&1 | FileCheck --check-prefix DISABLE %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-link-deps %s -### \
+// RUN: 2>&1 | FileCheck --check-prefix ENABLE %s
+// ENABLE: clang_rt.xray
+// DISABLE-NOT: clang_rt.xray
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -76,6 +76,10 @@
  options::OPT_fnoxray_always_emit_customevents, false))
   XRayAlwaysEmitCustomEvents = true;
 
+if (!Args.hasFlag(options::OPT_fxray_link_deps,
+  options::OPT_fnoxray_link_deps, true))
+  XRayRT = false;
+
 // Validate the always/never attribute files. We also make sure that they
 // are treated as actual dependencies.
 for (const auto &Filename :
Index: clang/include/clang/Driver/XRayArgs.h
===
--- clang/include/clang/Driver/XRayArgs.h
+++ clang/include/clang/Driver/XRayArgs.h
@@ -25,14 +25,15 @@
   bool XRayInstrument = false;
   int InstructionThreshold = 200;
   bool XRayAlwaysEmitCustomEvents = false;
+  bool XRayRT = true;
 
 public:
   /// Parses the XRay arguments from an argument list.
   XRayArgs(const ToolChain &TC, const llvm::opt::ArgList &Args);
   void addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs, types::ID InputType) const;
 
-  bool needsXRayRt() const { return XRayInstrument; }
+  bool needsXRayRt() const { return XRayInstrument && XRayRT; }
 };
 
 } // namespace driver
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1107,6 +1107,12 @@
 def fnoxray_always_emit_customevents : Flag<["-"], 
"fno-xray-always-emit-customevents">, Group,
   Flags<[CC1Option]>;
 
+def fxray_link_deps : Flag<["-"], "fxray-link-deps">, Group,
+  Flags<[CC1Option]>,
+  HelpText<"Tells clang to add the link dependencies for XRay.">;
+def fnoxray_link_deps : Flag<["-"], "fnoxray-link-deps">, Group,
+  Flags<[CC1Option]>;
+
 def ffine_grained_bitfield_accesses : Flag<["-"],
   "ffine-grained-bitfield-accesses">, Group, Flags<[CC1Option]>,
   HelpText<"Use separate accesses for bitfields with legal widths and 
alignments.">;


Index: clang/test/Driver/XRay/xray-nolinkdeps.cpp
===
--- /dev/null
+++ clang/test/Driver/XRay/xray-nolinkdeps.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang -v -o /dev/null -fxray-instrument -fnoxray-link-deps %s -### \
+// RUN: 2>&1 | FileCheck --check-prefix DISABLE %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-link-deps %s -### \
+// RUN: 2>&1 | FileCheck --check-prefix ENABLE %s
+// ENABLE: clang_rt.xray
+// DISABLE-NOT: clang_rt.xray
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -76,6 +76,10 @@
  options::OPT_fnoxray_always_emit_customevents, false))
   XRayAlwaysEmitCustomEvents = true;
 
+if (!Args.hasFlag(options::OPT_fxray_link_deps,
+  options::OPT_fnoxray_link_deps, true))
+  XRayRT = false;
+
 // Validate the always/never attribute files. We also make sure that they
 // are treated as actual dependencies.
 for (const auto &Filename :
Index: clang/include/clang/Driver/XRayArgs.h
===
--- clang/include/clang/Driver/XRayArgs.h
+++ clang/include/clang/Driver/XRayArgs.h
@@ -25,14 +25,15 @@
   bool XRayInstrument = false;
   int InstructionThreshold = 200;
   bool XRayAlwaysEmitCustomEvents = false;
+  bool XRayRT = true;
 
 public:
   /// Parses the XRay arguments from an argument list.
   XRayArgs(const ToolChain &TC, const llvm::opt::ArgList &Args);
   void addArgs(const ToolChain &TC,

[PATCH] D45354: [XRay][clang] Add a flag to enable/disable linking XRay deps explicitly

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329376: [XRay][clang] Add a flag to enable/disable linking 
XRay deps explicitly (authored by dberris, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45354?vs=141279&id=141281#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45354

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/XRayArgs.h
  cfe/trunk/lib/Driver/XRayArgs.cpp
  cfe/trunk/test/Driver/XRay/xray-nolinkdeps.cpp


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1107,6 +1107,12 @@
 def fnoxray_always_emit_customevents : Flag<["-"], 
"fno-xray-always-emit-customevents">, Group,
   Flags<[CC1Option]>;
 
+def fxray_link_deps : Flag<["-"], "fxray-link-deps">, Group,
+  Flags<[CC1Option]>,
+  HelpText<"Tells clang to add the link dependencies for XRay.">;
+def fnoxray_link_deps : Flag<["-"], "fnoxray-link-deps">, Group,
+  Flags<[CC1Option]>;
+
 def ffine_grained_bitfield_accesses : Flag<["-"],
   "ffine-grained-bitfield-accesses">, Group, Flags<[CC1Option]>,
   HelpText<"Use separate accesses for bitfields with legal widths and 
alignments.">;
Index: cfe/trunk/include/clang/Driver/XRayArgs.h
===
--- cfe/trunk/include/clang/Driver/XRayArgs.h
+++ cfe/trunk/include/clang/Driver/XRayArgs.h
@@ -25,14 +25,15 @@
   bool XRayInstrument = false;
   int InstructionThreshold = 200;
   bool XRayAlwaysEmitCustomEvents = false;
+  bool XRayRT = true;
 
 public:
   /// Parses the XRay arguments from an argument list.
   XRayArgs(const ToolChain &TC, const llvm::opt::ArgList &Args);
   void addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs, types::ID InputType) const;
 
-  bool needsXRayRt() const { return XRayInstrument; }
+  bool needsXRayRt() const { return XRayInstrument && XRayRT; }
 };
 
 } // namespace driver
Index: cfe/trunk/test/Driver/XRay/xray-nolinkdeps.cpp
===
--- cfe/trunk/test/Driver/XRay/xray-nolinkdeps.cpp
+++ cfe/trunk/test/Driver/XRay/xray-nolinkdeps.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang -v -o /dev/null -fxray-instrument -fnoxray-link-deps %s -### \
+// RUN: 2>&1 | FileCheck --check-prefix DISABLE %s
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-link-deps %s -### \
+// RUN: 2>&1 | FileCheck --check-prefix ENABLE %s
+// ENABLE: clang_rt.xray
+// DISABLE-NOT: clang_rt.xray
Index: cfe/trunk/lib/Driver/XRayArgs.cpp
===
--- cfe/trunk/lib/Driver/XRayArgs.cpp
+++ cfe/trunk/lib/Driver/XRayArgs.cpp
@@ -76,6 +76,10 @@
  options::OPT_fnoxray_always_emit_customevents, false))
   XRayAlwaysEmitCustomEvents = true;
 
+if (!Args.hasFlag(options::OPT_fxray_link_deps,
+  options::OPT_fnoxray_link_deps, true))
+  XRayRT = false;
+
 // Validate the always/never attribute files. We also make sure that they
 // are treated as actual dependencies.
 for (const auto &Filename :


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1107,6 +1107,12 @@
 def fnoxray_always_emit_customevents : Flag<["-"], "fno-xray-always-emit-customevents">, Group,
   Flags<[CC1Option]>;
 
+def fxray_link_deps : Flag<["-"], "fxray-link-deps">, Group,
+  Flags<[CC1Option]>,
+  HelpText<"Tells clang to add the link dependencies for XRay.">;
+def fnoxray_link_deps : Flag<["-"], "fnoxray-link-deps">, Group,
+  Flags<[CC1Option]>;
+
 def ffine_grained_bitfield_accesses : Flag<["-"],
   "ffine-grained-bitfield-accesses">, Group, Flags<[CC1Option]>,
   HelpText<"Use separate accesses for bitfields with legal widths and alignments.">;
Index: cfe/trunk/include/clang/Driver/XRayArgs.h
===
--- cfe/trunk/include/clang/Driver/XRayArgs.h
+++ cfe/trunk/include/clang/Driver/XRayArgs.h
@@ -25,14 +25,15 @@
   bool XRayInstrument = false;
   int InstructionThreshold = 200;
   bool XRayAlwaysEmitCustomEvents = false;
+  bool XRayRT = true;
 
 public:
   /// Parses the XRay arguments from an argument list.
   XRayArgs(const ToolChain &TC, const llvm::opt::ArgList &Args);
   void addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs, types::ID InputType) const;
 
-  bool needsXRayRt() const { return XRayInstrument; }
+  bool needsXRayRt() const { return XRayInstrument && XRayRT; }
 };
 
 } // namespace driver
Index: cfe/trunk/test/Driver/XRay/xray-nolin

[PATCH] D45357: [XRay][llvm+clang] Consolidate attribute list files

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: echristo, vlad.tsyrklevich, eugenis.

This change consolidates the always/never lists that may be provided to
clang to externally control which functions should be XRay instrumented
by imbuing attributes. The files follow the same format as defined in
https://clang.llvm.org/docs/SanitizerSpecialCaseList.html for the
sanitizer blacklist.

We also deprecate the existing `-fxray-instrument-always=` and
`-fxray-instrument-never=` flags, in favour of `-fxray-attr-list=`.

This fixes http://llvm.org/PR34721.


https://reviews.llvm.org/D45357

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/XRayLists.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/XRayLists.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-always-instrument.cpp
  clang/test/CodeGen/xray-attr-list.cpp
  clang/test/CodeGen/xray-imbue-arg1.cpp
  clang/test/CodeGen/xray-never-instrument.cpp
  llvm/docs/XRay.rst
  llvm/docs/XRayExample.rst

Index: llvm/docs/XRayExample.rst
===
--- llvm/docs/XRayExample.rst
+++ llvm/docs/XRayExample.rst
@@ -178,22 +178,22 @@
 To use this feature, you can define one file for the functions to always
 instrument, and another for functions to never instrument. The format of these
 files are exactly the same as the SanitizerLists files that control similar
-things for the sanitizer implementations. For example, we can have two
-different files like below:
+things for the sanitizer implementations. For example:
 
 ::
 
-  # always-instrument.txt
+  # xray-attr-list.txt
   # always instrument functions that match the following filters:
+  [always]
   fun:main
 
-  # never-instrument.txt
   # never instrument functions that match the following filters:
+  [never]
   fun:__cxx_*
 
-Given the above two files we can re-build by providing those two files as
-arguments to clang as ``-fxray-always-instrument=always-instrument.txt`` or
-``-fxray-never-instrument=never-instrument.txt``.
+Given the file above we can re-build by providing it to the
+``-fxray-attr-list=`` flag to clang. You can have multiple files, each defining
+different sets of attribute sets, to be cobined into a single list by clang.
 
 The XRay stack tool
 ---
Index: llvm/docs/XRay.rst
===
--- llvm/docs/XRay.rst
+++ llvm/docs/XRay.rst
@@ -117,6 +117,27 @@
   ; ...
 }
 
+Special Case File
+-
+
+Attributes can be imbued through the use of special case files instead of
+adding them to the original source files. You can use this to mark certain
+functions and classes to be never, always, or instrumented with first-argument
+logging from a file. The file's format is described below:
+
+.. code-block:: bash
+
+# Comments are supported
+[always]
+fun:always_instrument
+fun:log_arg1=arg1 # Log the first argument for the function
+
+[never]
+fun:never_instrument
+
+These files can be provided through the ``-fxray-attr-list=`` flag to clang.
+You may have multiple files loaded through multiple instances of the flag.
+
 XRay Runtime Library
 
 
Index: clang/test/CodeGen/xray-never-instrument.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-never-instrument.cpp
@@ -0,0 +1,24 @@
+// RUN: echo "fun:*foo*" > %t.never-instrument
+// RUN: echo "src:*xray-never-instrument.cpp" >> %t.never-instrument
+// RUN: echo "[never]" > %t.xray-attrlist
+// RUN: echo "fun:*foo*" >> %t.xray-attrlist
+// RUN: echo "src:*xray-never-instrument.cpp" >> %t.xray-attrlist
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 \
+// RUN: -fxray-never-instrument=%t.never-instrument -emit-llvm -o - %s \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 \
+// RUN: -fxray-attr-list=%t.xray-attrlist -emit-llvm -o - %s \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+
+void foo() {}
+
+[[clang::xray_always_instrument]] void bar() {}
+
+void baz() {}
+
+// CHECK: define void @_Z3foov() #[[NEVERATTR:[0-9]+]] {
+// CHECK: define void @_Z3barv() #[[ALWAYSATTR:[0-9]+]] {
+// CHECK: define void @_Z3bazv() #[[NEVERATTR:[0-9]+]] {
+// CHECK: attributes #[[NEVERATTR]] = {{.*}} "function-instrument"="xray-never" {{.*}}
+// CHECK: attributes #[[ALWAYSATTR]] = {{.*}} "function-instrument"="xray-always" {{.*}}
+
Index: clang/test/CodeGen/xray-imbue-arg1.cpp
===
--- clang/test/CodeGen/xray-imbue-arg1.cpp
+++ clang/test/CodeGen/xray-imbue-arg1.cpp
@@ -1,5 +1,12 @@
 // RUN: echo "fun:*arg1*=arg1" >> %t.always-instrument
-// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -fxray

[PATCH] D45357: [XRay][llvm+clang] Consolidate attribute list files

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 141578.
dberris added a comment.

- fixup: fix typo


https://reviews.llvm.org/D45357

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/XRayLists.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/XRayArgs.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/XRayLists.cpp
  clang/lib/Driver/XRayArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/xray-always-instrument.cpp
  clang/test/CodeGen/xray-attr-list.cpp
  clang/test/CodeGen/xray-imbue-arg1.cpp
  clang/test/CodeGen/xray-never-instrument.cpp
  llvm/docs/XRay.rst
  llvm/docs/XRayExample.rst

Index: llvm/docs/XRayExample.rst
===
--- llvm/docs/XRayExample.rst
+++ llvm/docs/XRayExample.rst
@@ -178,22 +178,22 @@
 To use this feature, you can define one file for the functions to always
 instrument, and another for functions to never instrument. The format of these
 files are exactly the same as the SanitizerLists files that control similar
-things for the sanitizer implementations. For example, we can have two
-different files like below:
+things for the sanitizer implementations. For example:
 
 ::
 
-  # always-instrument.txt
+  # xray-attr-list.txt
   # always instrument functions that match the following filters:
+  [always]
   fun:main
 
-  # never-instrument.txt
   # never instrument functions that match the following filters:
+  [never]
   fun:__cxx_*
 
-Given the above two files we can re-build by providing those two files as
-arguments to clang as ``-fxray-always-instrument=always-instrument.txt`` or
-``-fxray-never-instrument=never-instrument.txt``.
+Given the file above we can re-build by providing it to the
+``-fxray-attr-list=`` flag to clang. You can have multiple files, each defining
+different sets of attribute sets, to be combined into a single list by clang.
 
 The XRay stack tool
 ---
Index: llvm/docs/XRay.rst
===
--- llvm/docs/XRay.rst
+++ llvm/docs/XRay.rst
@@ -117,6 +117,27 @@
   ; ...
 }
 
+Special Case File
+-
+
+Attributes can be imbued through the use of special case files instead of
+adding them to the original source files. You can use this to mark certain
+functions and classes to be never, always, or instrumented with first-argument
+logging from a file. The file's format is described below:
+
+.. code-block:: bash
+
+# Comments are supported
+[always]
+fun:always_instrument
+fun:log_arg1=arg1 # Log the first argument for the function
+
+[never]
+fun:never_instrument
+
+These files can be provided through the ``-fxray-attr-list=`` flag to clang.
+You may have multiple files loaded through multiple instances of the flag.
+
 XRay Runtime Library
 
 
Index: clang/test/CodeGen/xray-never-instrument.cpp
===
--- /dev/null
+++ clang/test/CodeGen/xray-never-instrument.cpp
@@ -0,0 +1,24 @@
+// RUN: echo "fun:*foo*" > %t.never-instrument
+// RUN: echo "src:*xray-never-instrument.cpp" >> %t.never-instrument
+// RUN: echo "[never]" > %t.xray-attrlist
+// RUN: echo "fun:*foo*" >> %t.xray-attrlist
+// RUN: echo "src:*xray-never-instrument.cpp" >> %t.xray-attrlist
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 \
+// RUN: -fxray-never-instrument=%t.never-instrument -emit-llvm -o - %s \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 \
+// RUN: -fxray-attr-list=%t.xray-attrlist -emit-llvm -o - %s \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+
+void foo() {}
+
+[[clang::xray_always_instrument]] void bar() {}
+
+void baz() {}
+
+// CHECK: define void @_Z3foov() #[[NEVERATTR:[0-9]+]] {
+// CHECK: define void @_Z3barv() #[[ALWAYSATTR:[0-9]+]] {
+// CHECK: define void @_Z3bazv() #[[NEVERATTR:[0-9]+]] {
+// CHECK: attributes #[[NEVERATTR]] = {{.*}} "function-instrument"="xray-never" {{.*}}
+// CHECK: attributes #[[ALWAYSATTR]] = {{.*}} "function-instrument"="xray-always" {{.*}}
+
Index: clang/test/CodeGen/xray-imbue-arg1.cpp
===
--- clang/test/CodeGen/xray-imbue-arg1.cpp
+++ clang/test/CodeGen/xray-imbue-arg1.cpp
@@ -1,5 +1,12 @@
 // RUN: echo "fun:*arg1*=arg1" >> %t.always-instrument
-// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -fxray-always-instrument=%t.always-instrument -emit-llvm -o - %s -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: echo "[always]" > %t.xray-attrlist
+// RUN: echo "fun:*arg1*=arg1" >> %t.xray-attrlist
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 \
+// RUN: -fxray-always-instrument=%t.always-instrument -emit-llvm -o - %s \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 \
+// RUN: 

[PATCH] D45357: [XRay][llvm+clang] Consolidate attribute list files

[Dean Michael Berris via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL329543: [XRay][llvm+clang] Consolidate attribute list files 
(authored by dberris, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45357?vs=141578&id=141581#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45357

Files:
  cfe/trunk/include/clang/Basic/LangOptions.h
  cfe/trunk/include/clang/Basic/XRayLists.h
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/XRayArgs.h
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/lib/Basic/XRayLists.cpp
  cfe/trunk/lib/Driver/XRayArgs.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/xray-always-instrument.cpp
  cfe/trunk/test/CodeGen/xray-attr-list.cpp
  cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp
  cfe/trunk/test/CodeGen/xray-never-instrument.cpp
  llvm/trunk/docs/XRay.rst
  llvm/trunk/docs/XRayExample.rst

Index: cfe/trunk/include/clang/Basic/XRayLists.h
===
--- cfe/trunk/include/clang/Basic/XRayLists.h
+++ cfe/trunk/include/clang/Basic/XRayLists.h
@@ -26,12 +26,13 @@
 class XRayFunctionFilter {
   std::unique_ptr AlwaysInstrument;
   std::unique_ptr NeverInstrument;
+  std::unique_ptr AttrList;
   SourceManager &SM;
 
 public:
   XRayFunctionFilter(ArrayRef AlwaysInstrumentPaths,
  ArrayRef NeverInstrumentPaths,
- SourceManager &SM);
+ ArrayRef AttrListPaths, SourceManager &SM);
 
   enum class ImbueAttribute {
 NONE,
Index: cfe/trunk/include/clang/Basic/LangOptions.h
===
--- cfe/trunk/include/clang/Basic/LangOptions.h
+++ cfe/trunk/include/clang/Basic/LangOptions.h
@@ -148,13 +148,20 @@
   /// \brief Paths to the XRay "always instrument" files specifying which
   /// objects (files, functions, variables) should be imbued with the XRay
   /// "always instrument" attribute.
+  /// WARNING: This is a deprecated field and will go away in the future.
   std::vector XRayAlwaysInstrumentFiles;
 
   /// \brief Paths to the XRay "never instrument" files specifying which
   /// objects (files, functions, variables) should be imbued with the XRay
   /// "never instrument" attribute.
+  /// WARNING: This is a deprecated field and will go away in the future.
   std::vector XRayNeverInstrumentFiles;
 
+  /// \brief Paths to the XRay attribute list files, specifying which objects
+  /// (files, functions, variables) should be imbued with the appropriate XRay
+  /// attribute(s).
+  std::vector XRayAttrListFiles;
+
   clang::ObjCRuntime ObjCRuntime;
 
   std::string ObjCConstantStringClass;
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1095,11 +1095,15 @@
 def fxray_always_instrument :
   JoinedOrSeparate<["-"], "fxray-always-instrument=">,
   Group, Flags<[CC1Option]>,
-  HelpText<"Filename defining the whitelist for imbuing the 'always instrument' XRay attribute.">;
+  HelpText<"DEPRECATED: Filename defining the whitelist for imbuing the 'always instrument' XRay attribute.">;
 def fxray_never_instrument :
   JoinedOrSeparate<["-"], "fxray-never-instrument=">,
   Group, Flags<[CC1Option]>,
-  HelpText<"Filename defining the whitelist for imbuing the 'never instrument' XRay attribute.">;
+  HelpText<"DEPRECATED: Filename defining the whitelist for imbuing the 'never instrument' XRay attribute.">;
+def fxray_attr_list :
+  JoinedOrSeparate<["-"], "fxray-attr-list=">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Filename defining the list of functions/types for imbuing XRay attributes.">;
 
 def fxray_always_emit_customevents : Flag<["-"], "fxray-always-emit-customevents">, Group,
   Flags<[CC1Option]>,
Index: cfe/trunk/include/clang/Driver/XRayArgs.h
===
--- cfe/trunk/include/clang/Driver/XRayArgs.h
+++ cfe/trunk/include/clang/Driver/XRayArgs.h
@@ -21,6 +21,7 @@
 class XRayArgs {
   std::vector AlwaysInstrumentFiles;
   std::vector NeverInstrumentFiles;
+  std::vector AttrListFiles;
   std::vector ExtraDeps;
   bool XRayInstrument = false;
   int InstructionThreshold = 200;
Index: cfe/trunk/test/CodeGen/xray-attr-list.cpp
===
--- cfe/trunk/test/CodeGen/xray-attr-list.cpp
+++ cfe/trunk/test/CodeGen/xray-attr-list.cpp
@@ -0,0 +1,19 @@
+// RUN: echo "[always]" > %t.xray-attrlist
+// RUN: echo "fun:*always*" >> %t.xray-attrlist
+// RUN: echo "[never]" >> %t.xray-attrlist
+// RUN: echo "fun:*never*" >> %t.xray-attrlist
+// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 \
+// RUN: -fxray-attr-list=%t.xray-attrlist -emit-llvm -o - %s \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+
+void always() {}
+void never() {}
+[[c

[PATCH] D44878: libFuzzer OpenBSD support

[Dean Michael Berris via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D44878



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