[llvm-branch-commits] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-02-27 Thread Dan Liew via llvm-branch-commits

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-02-27 Thread Dan Liew via llvm-branch-commits


@@ -172,6 +172,20 @@ def push_dynamic_library_lookup_path(config, new_path):
 # doesn't match config.compiler_rt_libdir then it means we might be testing the
 # compiler's own runtime libraries rather than the ones we just built.
 # Warn about about this and handle appropriately.
+if config.test_standalone_build_libs:
+if config.compiler_id == "Clang":
+# Ensure that we use the just-built libraries when linking by 
overriding
+# the Clang resource directory. However, this also means that we can no
+# longer find the builtin headers from that path, so we explicitly add
+# the builtin headers as an include path.
+resource_dir, _ = get_path_from_clang(
+shlex.split(config.target_cflags) + ["-print-resource-dir"], 
allow_failure=False
+)
+config.target_cflags += f" -nobuiltininc"
+config.target_cflags += f" -I{config.compiler_rt_src_root}/include"
+config.target_cflags += f" -idirafter {resource_dir}/include"
+config.target_cflags += f" -resource-dir={config.compiler_rt_obj_root}"
+config.target_cflags += f" -Wl,--rpath={config.compiler_rt_libdir}"

delcypher wrote:

The flags below create kind of a odd mix

* Some external clang that has its own independent resource directory.
* The external clang is being forced to use a different resource directory than 
what it was shipped with
* The external clang is being forced to use a different set of headers than 
what it was shipped with

I'm not sure that combination is guaranteed to work given that compiler-rt can 
be tightly coupled with clang.

If I've read this code correctly the change you've made is on the common path 
(because `COMPILER_RT_TEST_STANDALONE_BUILD_LIBS` is `ON` by default) but AFAIK 
your change was never needed before because normally you would build 
compiler-rt with a just built clang which I think should mean there's no 
difference between what `-print-resource-dir` reports and 
`config.compiler_rt_obj_root`. Or have I misunderstood something?

https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-02-27 Thread Dan Liew via llvm-branch-commits


@@ -172,6 +172,20 @@ def push_dynamic_library_lookup_path(config, new_path):
 # doesn't match config.compiler_rt_libdir then it means we might be testing the
 # compiler's own runtime libraries rather than the ones we just built.
 # Warn about about this and handle appropriately.
+if config.test_standalone_build_libs:
+if config.compiler_id == "Clang":
+# Ensure that we use the just-built libraries when linking by 
overriding
+# the Clang resource directory. However, this also means that we can no
+# longer find the builtin headers from that path, so we explicitly add
+# the builtin headers as an include path.

delcypher wrote:

Sorry. It's been a while since I looked at this code. If you use a different 
resource directory why can't the compiler use the header files in that resource 
directory?

https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-02-27 Thread Dan Liew via llvm-branch-commits


@@ -571,6 +571,30 @@ string(APPEND COMPILER_RT_TEST_COMPILER_CFLAGS " 
${stdlib_flag}")
 string(REPLACE " " ";" COMPILER_RT_UNITTEST_CFLAGS 
"${COMPILER_RT_TEST_COMPILER_CFLAGS}")
 set(COMPILER_RT_UNITTEST_LINK_FLAGS ${COMPILER_RT_UNITTEST_CFLAGS})
 
+option(COMPILER_RT_TEST_STANDALONE_BUILD_LIBS
+  "When set to ON and testing in a standalone build, test the runtime \
+  libraries built by this standalone build rather than the runtime libraries \
+  shipped with the compiler (used for testing). When set to OFF and testing \
+  in a standalone build, test the runtime libraries shipped with the compiler \
+  (used for testing). This option has no effect if the compiler and this \
+  build are configured to use the same runtime library path."
+  ON)
+if (COMPILER_RT_TEST_STANDALONE_BUILD_LIBS)
+  # Ensure that the unit tests can find the sanitizer headers prior to 
installation.
+  list(APPEND COMPILER_RT_UNITTEST_CFLAGS 
"-I${CMAKE_CURRENT_LIST_DIR}/include")
+  # Ensure that unit tests link against the just-built runtime libraries 
instead
+  # of the ones bundled with the compiler by overriding the resource directory.
+  #
+  if ("${COMPILER_RT_TEST_COMPILER_ID}" MATCHES "Clang")
+list(APPEND COMPILER_RT_UNITTEST_LINK_FLAGS 
"-resource-dir=${CMAKE_CURRENT_BINARY_DIR}")
+  endif()
+  get_compiler_rt_output_dir(${COMPILER_RT_DEFAULT_TARGET_ARCH} output_dir)
+  list(APPEND COMPILER_RT_UNITTEST_LINK_FLAGS "-Wl,-rpath,${output_dir}")
+endif()
+message(WARNING 
"COMPILER_RT_UNITTEST_LINK_FLAGS=${COMPILER_RT_UNITTEST_LINK_FLAGS}, 
COMPILER_RT_TEST_STANDALONE_BUILD_LIBS=${COMPILER_RT_TEST_STANDALONE_BUILD_LIBS}
 COMPILER_RT_TEST_COMPILER_ID=${COMPILER_RT_TEST_COMPILER_ID}")

delcypher wrote:

Is this left over from debugging? This warning message doesn't seem that 
helpful to a developer.

https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-02-27 Thread Dan Liew via llvm-branch-commits

https://github.com/delcypher commented:

What is the `[𝘀𝗽𝗿] initial version` commit message about?

https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-02-27 Thread Dan Liew via llvm-branch-commits


@@ -172,6 +172,20 @@ def push_dynamic_library_lookup_path(config, new_path):
 # doesn't match config.compiler_rt_libdir then it means we might be testing the
 # compiler's own runtime libraries rather than the ones we just built.
 # Warn about about this and handle appropriately.
+if config.test_standalone_build_libs:
+if config.compiler_id == "Clang":
+# Ensure that we use the just-built libraries when linking by 
overriding
+# the Clang resource directory. However, this also means that we can no
+# longer find the builtin headers from that path, so we explicitly add
+# the builtin headers as an include path.
+resource_dir, _ = get_path_from_clang(
+shlex.split(config.target_cflags) + ["-print-resource-dir"], 
allow_failure=False
+)
+config.target_cflags += f" -nobuiltininc"
+config.target_cflags += f" -I{config.compiler_rt_src_root}/include"
+config.target_cflags += f" -idirafter {resource_dir}/include"
+config.target_cflags += f" -resource-dir={config.compiler_rt_obj_root}"
+config.target_cflags += f" -Wl,--rpath={config.compiler_rt_libdir}"

delcypher wrote:

@arichardson To be clear. I'm not necessarily against your change but I'm a bit 
concerned about your change being on the default path given that it's not 
normally necessary.

https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-03-12 Thread Dan Liew via llvm-branch-commits

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-03-12 Thread Dan Liew via llvm-branch-commits

https://github.com/delcypher approved this pull request.

Thanks for addressing my feedback. I have a minor nit that you can address if 
you feel its worth it. When landing please make sure you squash your commits 
into a single commit and rewrite the commit message to be something that 
conforms to LLVM's style for commit messages.

https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] [compiler-rt] Allow running tests without installing first (PR #83088)

2024-03-12 Thread Dan Liew via llvm-branch-commits


@@ -168,10 +169,45 @@ def push_dynamic_library_lookup_path(config, new_path):
 r"/i386(?=-[^/]+$)", "/x86_64", config.compiler_rt_libdir
 )
 
+
+# Check if the test compiler resource dir matches the local build directory
+# (which happens with -DLLVM_ENABLE_PROJECTS=clang;compiler-rt) or if we are
+# using an installed clang to test compiler-rt standalone. In the latter case
+# we may need to override the resource dir to match the path of the just-built
+# compiler-rt libraries.
+test_cc_resource_dir, _ = get_path_from_clang(
+shlex.split(config.target_cflags) + ["-print-resource-dir"], 
allow_failure=True
+)
+# Normalize the path for comparison
+if test_cc_resource_dir is not None:
+test_cc_resource_dir = os.path.realpath(test_cc_resource_dir)
+if lit_config.debug:
+lit_config.note(f"Resource dir for {config.clang} is 
{test_cc_resource_dir}")
+local_build_resource_dir = os.path.realpath(config.compiler_rt_output_dir)
+if test_cc_resource_dir != local_build_resource_dir:
+if config.test_standalone_build_libs and config.compiler_id == "Clang":

delcypher wrote:

Nit: If `config.test_standalone_build_libs` and `config.compiler_id` `!=` 
`Clang` we may want to emit a warning/error about an unsupported compiler.

https://github.com/llvm/llvm-project/pull/83088
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [compiler-rt] dd922bc - [LSan] Introduce a callback mechanism to allow adding data reachable from ThreadContexts to the frontier.

2021-01-22 Thread Dan Liew via llvm-branch-commits

Author: Dan Liew
Date: 2021-01-22T19:26:02-08:00
New Revision: dd922bc2a62163cef442646974324943c551725e

URL: 
https://github.com/llvm/llvm-project/commit/dd922bc2a62163cef442646974324943c551725e
DIFF: 
https://github.com/llvm/llvm-project/commit/dd922bc2a62163cef442646974324943c551725e.diff

LOG: [LSan] Introduce a callback mechanism to allow adding data reachable from 
ThreadContexts to the frontier.

This mechanism is intended to provide a way to treat the `arg` pointer
of a created (but not yet started) thread as reachable. In future
patches this will be implemented in `GetAdditionalThreadContextPtrs`.

A separate implementation of `GetAdditionalThreadContextPtrs` exists
for ASan and LSan runtimes because they need to be implemented
differently in future patches.

rdar://problem/63537240

Differential Revision: https://reviews.llvm.org/D95183

Added: 


Modified: 
compiler-rt/lib/asan/asan_allocator.cpp
compiler-rt/lib/lsan/lsan_allocator.cpp
compiler-rt/lib/lsan/lsan_common.cpp
compiler-rt/lib/lsan/lsan_common.h

Removed: 




diff  --git a/compiler-rt/lib/asan/asan_allocator.cpp 
b/compiler-rt/lib/asan/asan_allocator.cpp
index 58b496a3ca4b..4da697835870 100644
--- a/compiler-rt/lib/asan/asan_allocator.cpp
+++ b/compiler-rt/lib/asan/asan_allocator.cpp
@@ -1183,6 +1183,16 @@ IgnoreObjectResult IgnoreObjectLocked(const void *p) {
   m->lsan_tag = __lsan::kIgnored;
   return kIgnoreObjectSuccess;
 }
+
+void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs) {
+  // This function can be used to treat memory reachable from `tctx` as live.
+  // This is useful for threads that have been created but not yet started.
+
+  // This is currently a no-op because the ASan `pthread_create()` interceptor
+  // blocks until the child thread starts which keeps the thread's `arg` 
pointer
+  // live.
+}
+
 }  // namespace __lsan
 
 // -- Interface  {{{1

diff  --git a/compiler-rt/lib/lsan/lsan_allocator.cpp 
b/compiler-rt/lib/lsan/lsan_allocator.cpp
index d86c3921395c..70422957e6f3 100644
--- a/compiler-rt/lib/lsan/lsan_allocator.cpp
+++ b/compiler-rt/lib/lsan/lsan_allocator.cpp
@@ -309,6 +309,16 @@ IgnoreObjectResult IgnoreObjectLocked(const void *p) {
 return kIgnoreObjectInvalid;
   }
 }
+
+void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs) {
+  // This function can be used to treat memory reachable from `tctx` as live.
+  // This is useful for threads that have been created but not yet started.
+
+  // This is currently a no-op because the LSan `pthread_create()` interceptor
+  // blocks until the child thread starts which keeps the thread's `arg` 
pointer
+  // live.
+}
+
 } // namespace __lsan
 
 using namespace __lsan;

diff  --git a/compiler-rt/lib/lsan/lsan_common.cpp 
b/compiler-rt/lib/lsan/lsan_common.cpp
index ab7500ce32cf..d5b4132b24d5 100644
--- a/compiler-rt/lib/lsan/lsan_common.cpp
+++ b/compiler-rt/lib/lsan/lsan_common.cpp
@@ -253,6 +253,27 @@ extern "C" SANITIZER_WEAK_ATTRIBUTE void 
__libc_iterate_dynamic_tls(
 pid_t, void (*cb)(void *, void *, uptr, void *), void *);
 #endif
 
+static void ProcessThreadRegistry(Frontier *frontier) {
+  InternalMmapVector ptrs;
+  GetThreadRegistryLocked()->RunCallbackForEachThreadLocked(
+  GetAdditionalThreadContextPtrs, &ptrs);
+
+  for (uptr i = 0; i < ptrs.size(); ++i) {
+void *ptr = reinterpret_cast(ptrs[i]);
+uptr chunk = PointsIntoChunk(ptr);
+if (!chunk)
+  continue;
+LsanMetadata m(chunk);
+if (!m.allocated())
+  continue;
+
+// Mark as reachable and add to frontier.
+LOG_POINTERS("Treating pointer %p from ThreadContext as reachable\n", ptr);
+m.set_tag(kReachable);
+frontier->push_back(chunk);
+  }
+}
+
 // Scans thread data (stacks and TLS) for heap pointers.
 static void ProcessThreads(SuspendedThreadsList const &suspended_threads,
Frontier *frontier) {
@@ -364,6 +385,9 @@ static void ProcessThreads(SuspendedThreadsList const 
&suspended_threads,
 #endif
 }
   }
+
+  // Add pointers reachable from ThreadContexts
+  ProcessThreadRegistry(frontier);
 }
 
 #endif  // SANITIZER_FUCHSIA

diff  --git a/compiler-rt/lib/lsan/lsan_common.h 
b/compiler-rt/lib/lsan/lsan_common.h
index 05f380d4a5fa..b0ae6f020b63 100644
--- a/compiler-rt/lib/lsan/lsan_common.h
+++ b/compiler-rt/lib/lsan/lsan_common.h
@@ -50,6 +50,7 @@
 namespace __sanitizer {
 class FlagParser;
 class ThreadRegistry;
+class ThreadContextBase;
 struct DTLS;
 }
 
@@ -142,6 +143,7 @@ InternalMmapVector const *GetRootRegions();
 void ScanRootRegion(Frontier *frontier, RootRegion const ®ion,
 uptr region_begin, uptr region_end, bool is_readable);
 void ForEachExtraStackRangeCb(uptr begin, uptr end, void* arg);
+void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs);
 // Run stoptheworld while holding any 

[llvm-branch-commits] [compiler-rt] 596d534 - [ASan] Stop blocking child thread progress from parent thread in `pthread_create` interceptor.

2021-01-22 Thread Dan Liew via llvm-branch-commits

Author: Dan Liew
Date: 2021-01-22T23:34:43-08:00
New Revision: 596d534ac3524052df210be8d3c01a33b2260a42

URL: 
https://github.com/llvm/llvm-project/commit/596d534ac3524052df210be8d3c01a33b2260a42
DIFF: 
https://github.com/llvm/llvm-project/commit/596d534ac3524052df210be8d3c01a33b2260a42.diff

LOG: [ASan] Stop blocking child thread progress from parent thread in 
`pthread_create` interceptor.

Previously in ASan's `pthread_create` interceptor we would block in the
`pthread_create` interceptor waiting for the child thread to start.

Unfortunately this has bad performance characteristics because the OS
scheduler doesn't know the relationship between the parent and child
thread (i.e. the parent thread cannot make progress until the child
thread makes progress) and may make the wrong scheduling decision which
stalls progress.

It turns out that ASan didn't use to block in this interceptor but was
changed to do so to try to address
http://llvm.org/bugs/show_bug.cgi?id=21621/.

In that bug the problem being addressed was a LeakSanitizer false
positive. That bug concerns a heap object being passed
as `arg` to `pthread_create`. If:

* The calling thread loses a live reference to the object (e.g.
  `pthread_create` finishes and the thread no longer has a live
  reference to the object).
* Leak checking is triggered.
* The child thread has not yet started (once it starts it will have a
  live reference).

then the heap object will incorrectly appear to be leaked.

This bug is covered by the 
`lsan/TestCases/leak_check_before_thread_started.cpp` test case.

In b029c5101fb49b3577a1c322f42ef9fc616f25bf ASan was changed to block
in `pthread_create()` until the child thread starts so that `arg` is
kept alive for the purposes of leaking check.

While this change "works" its problematic due to the performance
problems it causes. The change is also completely unnecessary if leak
checking is disabled (via detect_leaks runtime option or
CAN_SANITIZE_LEAKS compile time config).

This patch does two things:

1. Takes a different approach to solving the leak false positive by
   making LSan's leak checking mechanism treat the `arg` pointer of
   created but not started threads as reachable.  This is done by
   implementing the `ForEachRegisteredThreadContextCb` callback for
   ASan.

2. Removes the blocking behaviour in the ASan `pthread_create`
   interceptor.

rdar://problem/63537240

Differential Revision: https://reviews.llvm.org/D95184

Added: 


Modified: 
compiler-rt/lib/asan/asan_allocator.cpp
compiler-rt/lib/asan/asan_interceptors.cpp
compiler-rt/lib/asan/asan_thread.cpp
compiler-rt/lib/asan/asan_thread.h

Removed: 




diff  --git a/compiler-rt/lib/asan/asan_allocator.cpp 
b/compiler-rt/lib/asan/asan_allocator.cpp
index 4da697835870..cd97b37652f8 100644
--- a/compiler-rt/lib/asan/asan_allocator.cpp
+++ b/compiler-rt/lib/asan/asan_allocator.cpp
@@ -1185,12 +1185,30 @@ IgnoreObjectResult IgnoreObjectLocked(const void *p) {
 }
 
 void GetAdditionalThreadContextPtrs(ThreadContextBase *tctx, void *ptrs) {
-  // This function can be used to treat memory reachable from `tctx` as live.
-  // This is useful for threads that have been created but not yet started.
-
-  // This is currently a no-op because the ASan `pthread_create()` interceptor
-  // blocks until the child thread starts which keeps the thread's `arg` 
pointer
-  // live.
+  // Look for the arg pointer of threads that have been created or are running.
+  // This is necessary to prevent false positive leaks due to the AsanThread
+  // holding the only live reference to a heap object.  This can happen because
+  // the `pthread_create()` interceptor doesn't wait for the child thread to
+  // start before returning and thus loosing the the only live reference to the
+  // heap object on the stack.
+
+  __asan::AsanThreadContext *atctx =
+  reinterpret_cast<__asan::AsanThreadContext *>(tctx);
+  __asan::AsanThread *asan_thread = atctx->thread;
+
+  // Note ThreadStatusRunning is required because there is a small window where
+  // the thread status switches to `ThreadStatusRunning` but the `arg` pointer
+  // still isn't on the stack yet.
+  if (atctx->status != ThreadStatusCreated &&
+  atctx->status != ThreadStatusRunning)
+return;
+
+  uptr thread_arg = reinterpret_cast(asan_thread->get_arg());
+  if (!thread_arg)
+return;
+
+  auto ptrsVec = reinterpret_cast *>(ptrs);
+  ptrsVec->push_back(thread_arg);
 }
 
 }  // namespace __lsan

diff  --git a/compiler-rt/lib/asan/asan_interceptors.cpp 
b/compiler-rt/lib/asan/asan_interceptors.cpp
index b19cf25c7cd0..cd07d51878b1 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors.cpp
@@ -189,20 +189,11 @@ DECLARE_REAL_AND_INTERCEPTOR(void, free, void *)
 #include "sanitizer_common/sanitizer_common_syscalls.inc"
 #include "sanitizer_common/sanitizer_syscalls_netb

[llvm-branch-commits] [compiler-rt] 757b93b - [ASan] Fix broken Windows build due to 596d534ac3524052df210be8d3c01a33b2260a42.

2021-01-23 Thread Dan Liew via llvm-branch-commits

Author: Dan Liew
Date: 2021-01-23T09:09:06-08:00
New Revision: 757b93bb7b384038a8dec35433f78f5c7c2ef8b0

URL: 
https://github.com/llvm/llvm-project/commit/757b93bb7b384038a8dec35433f78f5c7c2ef8b0
DIFF: 
https://github.com/llvm/llvm-project/commit/757b93bb7b384038a8dec35433f78f5c7c2ef8b0.diff

LOG: [ASan] Fix broken Windows build due to 
596d534ac3524052df210be8d3c01a33b2260a42.

In that change I forgot to update the call to
`AsanThread::ThreadStart()` in `asan_win.cpp`.

Added: 


Modified: 
compiler-rt/lib/asan/asan_win.cpp

Removed: 




diff  --git a/compiler-rt/lib/asan/asan_win.cpp 
b/compiler-rt/lib/asan/asan_win.cpp
index 8044ae16ff9b..1577c83cf994 100644
--- a/compiler-rt/lib/asan/asan_win.cpp
+++ b/compiler-rt/lib/asan/asan_win.cpp
@@ -134,7 +134,7 @@ INTERCEPTOR(int, _except_handler4, void *a, void *b, void 
*c, void *d) {
 static thread_return_t THREAD_CALLING_CONV asan_thread_start(void *arg) {
   AsanThread *t = (AsanThread *)arg;
   SetCurrentThread(t);
-  return t->ThreadStart(GetTid(), /* signal_thread_is_registered */ nullptr);
+  return t->ThreadStart(GetTid());
 }
 
 INTERCEPTOR_WINAPI(HANDLE, CreateThread, LPSECURITY_ATTRIBUTES security,



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


[llvm-branch-commits] [clang] 0e3f038 - [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.

2021-04-28 Thread Dan Liew via llvm-branch-commits

Author: Dan Liew
Date: 2021-04-28T18:58:55-07:00
New Revision: 0e3f038261be4799d0d09e70e165f526e182b0cf

URL: 
https://github.com/llvm/llvm-project/commit/0e3f038261be4799d0d09e70e165f526e182b0cf
DIFF: 
https://github.com/llvm/llvm-project/commit/0e3f038261be4799d0d09e70e165f526e182b0cf.diff

LOG: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` 
suffix.

Renaming the option is based on discussions in https://reviews.llvm.org/D101122.

It is normally not a good idea to rename driver flags but this flag is
new enough and obscure enough that it is very unlikely to have adopters.

While we're here also drop the `` metavar. It's not necessary and
is actually inconsistent with the documentation in
`clang/docs/ClangCommandLineReference.rst`.

Differential Revision: https://reviews.llvm.org/D101491

Added: 
clang/test/Driver/fsanitize-address-destructor.c

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td
clang/lib/Driver/SanitizerArgs.cpp
clang/test/CodeGen/asan-destructor-kind.cpp
clang/test/Driver/darwin-asan-mkernel-kext.c

Removed: 
clang/test/Driver/fsanitize-address-destructor-kind.c



diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 24e0040a4f50d..6b67dc07982bd 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -870,7 +870,7 @@ Enable use-after-scope detection in AddressSanitizer
 
 Enable ODR indicator globals to avoid false ODR violation reports in partially 
sanitized programs at the cost of an increase in binary size
 
-.. option:: -fsanitize-address-destructor-kind=
+.. option:: -fsanitize-address-destructor=
 
 Set the kind of module destructors emitted by AddressSanitizer instrumentation.
 These destructors are emitted to unregister instrumented global variables when

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index eaebed5978368..f30e08a2930f5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1540,8 +1540,7 @@ defm sanitize_address_use_odr_indicator : BoolOption<"f", 
"sanitize-address-use-
   NegFlag>,
   Group;
 def sanitize_address_destructor_kind_EQ
-: Joined<["-"], "fsanitize-address-destructor-kind=">,
-  MetaVarName<"">,
+: Joined<["-"], "fsanitize-address-destructor=">,
   Flags<[CC1Option]>,
   HelpText<"Set destructor type used in ASan instrumentation">,
   Group,

diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index 0f9f5d87696c4..de6f5796d6ec8 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1098,7 +1098,7 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const 
llvm::opt::ArgList &Args,
   // Only pass the option to the frontend if the user requested,
   // otherwise the frontend will just use the codegen default.
   if (AsanDtorKind != llvm::AsanDtorKind::Invalid) {
-CmdArgs.push_back(Args.MakeArgString("-fsanitize-address-destructor-kind=" 
+
+CmdArgs.push_back(Args.MakeArgString("-fsanitize-address-destructor=" +
  AsanDtorKindToString(AsanDtorKind)));
   }
 

diff  --git a/clang/test/CodeGen/asan-destructor-kind.cpp 
b/clang/test/CodeGen/asan-destructor-kind.cpp
index 567482b726435..8d70b663f67a2 100644
--- a/clang/test/CodeGen/asan-destructor-kind.cpp
+++ b/clang/test/CodeGen/asan-destructor-kind.cpp
@@ -1,9 +1,9 @@
 // Frontend rejects invalid option
 // RUN: not %clang_cc1 -fsanitize=address \
-// RUN:   -fsanitize-address-destructor-kind=bad_arg -emit-llvm -o - \
+// RUN:   -fsanitize-address-destructor=bad_arg -emit-llvm -o - \
 // RUN:   -triple x86_64-apple-macosx10.15 %s 2>&1 | \
 // RUN:   FileCheck %s --check-prefixes=CHECK-BAD-ARG
-// CHECK-BAD-ARG: invalid value 'bad_arg' in 
'-fsanitize-address-destructor-kind=bad_arg'
+// CHECK-BAD-ARG: invalid value 'bad_arg' in 
'-fsanitize-address-destructor=bad_arg'
 
 // Default is global dtor
 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-apple-macosx10.15 \
@@ -16,12 +16,12 @@
 
 // Explictly ask for global dtor
 // RUN: %clang_cc1 -fsanitize=address \
-// RUN:   -fsanitize-address-destructor-kind=global -emit-llvm -o - \
+// RUN:   -fsanitize-address-destructor=global -emit-llvm -o - \
 // RUN:   -triple x86_64-apple-macosx10.15 -fno-legacy-pass-manager %s | \
 // RUN:   FileCheck %s --check-prefixes=CHECK-GLOBAL-DTOR
 //
 // RUN: %clang_cc1 -fsanitize=address \
-// RUN:   -fsanitize-address-destructor-kind=global -emit-llvm -o - \
+// RUN:   -fsanitize-address-destructor=global -emit-llvm -o - \
 // RUN:   -triple x86_64-apple-macosx10.15 -flegacy-pass-manager %s | \
 // RUN:   FileCheck %s --check-prefixes=CHECK-GLOBAL-DTOR
 
@@ -30,12 +30,12 @@
 
 // Explictly ask for no

[llvm-branch-commits] [llvm-branch] r322287 - [docs] Add JFS as an external project built againt LLVM 6.0.

2018-01-11 Thread Dan Liew via llvm-branch-commits
Author: delcypher
Date: Thu Jan 11 08:24:04 2018
New Revision: 322287

URL: http://llvm.org/viewvc/llvm-project?rev=322287&view=rev
Log:
[docs] Add JFS as an external project built againt LLVM 6.0.

Modified:
llvm/branches/release_60/docs/ReleaseNotes.rst

Modified: llvm/branches/release_60/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/llvm/branches/release_60/docs/ReleaseNotes.rst?rev=322287&r1=322286&r2=322287&view=diff
==
--- llvm/branches/release_60/docs/ReleaseNotes.rst (original)
+++ llvm/branches/release_60/docs/ReleaseNotes.rst Thu Jan 11 08:24:04 2018
@@ -122,7 +122,19 @@ Changes to the C API
 External Open Source Projects Using LLVM 6
 ==
 
-* A project...
+JFS - JIT Fuzzing Solver
+
+
+`JFS `_ is an experimental constraint solver
+designed to investigate using coverage guided fuzzing as an incomplete strategy
+for solving boolean, BitVector, and floating-point constraints.
+It is built on top of LLVM, Clang, LibFuzzer, and Z3.
+
+The solver works by generating a C++ program where the reachability of an
+`abort()` statement is equivalent to finding a satisfying assignment to the
+constraints. This program is then compiled by Clang with `SanitizerCoverage
+`_
+instrumentation and then fuzzed using :doc:`LibFuzzer `.
 
 
 Additional Information


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


[llvm-branch-commits] [compiler-rt] 8678afc - [Symbolizers] On Darwin compute function offset when possible.

2019-10-28 Thread Dan Liew via llvm-branch-commits

Author: Dan Liew
Date: 2019-10-28T23:41:24-07:00
New Revision: 8678afce2c8cb2b392a02459350023093ab7eb17

URL: 
https://github.com/llvm/llvm-project/commit/8678afce2c8cb2b392a02459350023093ab7eb17
DIFF: 
https://github.com/llvm/llvm-project/commit/8678afce2c8cb2b392a02459350023093ab7eb17.diff

LOG: [Symbolizers] On Darwin compute function offset when possible.

Summary:
The sanitizer symbolizers support printing the function offset
(difference between pc and function start) of a stackframe using the
`%q` format specifier.

Unfortunately this didn't actually work because neither the atos
or dladdr symbolizer set the `AddressInfo::function_offset` field.

This patch teaches both symbolizers to try to compute the function
offset. In the case of the atos symbolizer, atos might not report the
function offset (e.g. it reports a source location instead) so in this
case it fallsback to using `dladdr()` to compute the function offset.

Two test cases are included.

rdar://problem/56695185

Reviewers: kubamracek, yln

Subscribers: #sanitizers, llvm-commits

Tags: #sanitizers, #llvm

Differential Revision: https://reviews.llvm.org/D69549

Added: 

compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-atos.cpp

compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-dladdr.cpp

Modified: 
compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp

Removed: 




diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp 
b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
index a619ed092f0b..2e06a513e714 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
@@ -31,6 +31,9 @@ bool DlAddrSymbolizer::SymbolizePC(uptr addr, SymbolizedStack 
*stack) {
   Dl_info info;
   int result = dladdr((const void *)addr, &info);
   if (!result) return false;
+
+  CHECK(addr >= reinterpret_cast(info.dli_saddr));
+  stack->info.function_offset = addr - reinterpret_cast(info.dli_saddr);
   const char *demangled = DemangleSwiftAndCXX(info.dli_sname);
   if (!demangled) return false;
   stack->info.function = internal_strdup(demangled);
@@ -145,12 +148,29 @@ bool AtosSymbolizer::SymbolizePC(uptr addr, 
SymbolizedStack *stack) {
   const char *buf = process_->SendCommand(command);
   if (!buf) return false;
   uptr line;
+  uptr start_address = AddressInfo::kUnknown;
   if (!ParseCommandOutput(buf, addr, &stack->info.function, 
&stack->info.module,
-  &stack->info.file, &line, nullptr)) {
+  &stack->info.file, &line, &start_address)) {
 process_ = nullptr;
 return false;
   }
   stack->info.line = (int)line;
+
+  // Compute the function offset.
+  uptr function_offset = AddressInfo::kUnknown;
+  if (start_address != AddressInfo::kUnknown) {
+CHECK(addr >= start_address);
+function_offset = addr - start_address;
+  } else {
+// Fallback to dladdr() to get function offset if atos doesn't report it.
+Dl_info info;
+int result = dladdr((const void *)addr, &info);
+if (result) {
+  CHECK(addr >= reinterpret_cast(info.dli_saddr));
+  function_offset = addr - reinterpret_cast(info.dli_saddr);
+}
+  }
+  stack->info.function_offset = function_offset;
   return true;
 }
 

diff  --git 
a/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-atos.cpp
 
b/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-atos.cpp
new file mode 100644
index ..d7685c727f33
--- /dev/null
+++ 
b/compiler-rt/test/sanitizer_common/TestCases/Darwin/symbolizer-function-offset-atos.cpp
@@ -0,0 +1,47 @@
+// The no-debug case should cause atos to report the function offset so this 
should test that path.
+// RUN: rm -rf %t-no-debug.dSYM
+// RUN: %clangxx %s -g0 -O0 -o %t-no-debug
+// RUN: %env_tool_opts=verbosity=2,stack_trace_format='"function_name:%f 
function_offset:%q"' %run %t-no-debug > %t-no-debug.output 2>&1
+// RUN: FileCheck -input-file=%t-no-debug.output %s
+// RUN: FileCheck -check-prefix=BADADDR -input-file=%t-no-debug.output %s
+
+// The debug info case should cause atos to not report the function offset so 
this should test the dladdr() fallback path.
+// RUN: %clangxx %s -g -O0 -o %t-with-debug
+// RUN: %env_tool_opts=verbosity=2,stack_trace_format='"function_name:%f 
function_offset:%q"' %run %t-with-debug > %t-with-debug.output 2>&1
+// RUN: FileCheck -input-file=%t-with-debug.output %s
+// RUN: FileCheck -check-prefix=BADADDR -input-file=%t-with-debug.output %s
+#include 
+#include 
+
+void baz() {
+  printf("Do stuff in baz\n");
+  __sanitizer_print_stack_trace();
+}
+
+void bar() {
+  printf("Do stuff in bar\n");
+  baz();
+}
+
+void foo() {
+  printf("Do stuff in foo\n");
+  bar();
+}
+
+int main() {
+  printf("Do stuff in main\n");
+  foo();
+  

[llvm-branch-commits] [clang] [clang] [sanitizer] add pseudofunction to indicate array-bounds check (PR #128977)

2025-03-05 Thread Dan Liew via llvm-branch-commits

delcypher wrote:

@fmayer The usual approach for indicating instrumentation in Clang is to use 
opt-remarks. This is the approach we use for `-fbounds-safety`. 

In `-fbounds-safety` we embed "trap reasons" in debug info so that debuggers 
and symbolication tools can better understand the reason for trapping.

What's the reason for using debug info, instead of opt-remarks here?

https://github.com/llvm/llvm-project/pull/128977
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [clang] [sanitizer] add pseudofunction to indicate array-bounds check (PR #128977)

2025-03-05 Thread Dan Liew via llvm-branch-commits

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/128977
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [clang] [sanitizer] add pseudofunction to indicate array-bounds check (PR #128977)

2025-03-05 Thread Dan Liew via llvm-branch-commits

https://github.com/delcypher approved this pull request.

Thanks for explaining the purpose.

Regarding the "more easily see why we crashed." please be aware [I have a GSoC 
proposal to basically do 
this](https://discourse.llvm.org/t/clang-gsoc-2025-usability-improvements-for-trapping-undefined-behavior-sanitizer/84568)
 using the `createTrapFailureMessageFor`. So if possible please don't tackle 
what I describe in the proposal before a GSoC student has had a chance to do 
this. To be clear what you've done in this PR is different from I'm proposing 
so they don't conflict.

Also

* Please give a chance for Clang Debug Info contributors to look over this (CC 
@adrian-prantl) before landing this.
* When possible add as reviewers previous people who worked on the code. In 
this particular case I believe this was @ahatanak 

https://github.com/llvm/llvm-project/pull/128977
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [clang] [sanitizer] add pseudofunction to indicate array-bounds check (PR #128977)

2025-03-05 Thread Dan Liew via llvm-branch-commits


@@ -3598,6 +3598,14 @@ llvm::DIMacroFile 
*CGDebugInfo::CreateTempMacroFile(llvm::DIMacroFile *Parent,
   return DBuilder.createTempMacroFile(Parent, Line, FName);
 }
 
+llvm::DILocation *CGDebugInfo::CreateSyntheticInline(llvm::DebugLoc Location,
+ StringRef FuncName) {
+  llvm::DISubprogram *TrapSP =

delcypher wrote:

Nit. The name `TrapSP` doesn't make much sense here given this function isn't 
specifically for traps.

https://github.com/llvm/llvm-project/pull/128977
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [clang] [sanitizer] add pseudofunction to indicate array-bounds check (PR #128977)

2025-03-05 Thread Dan Liew via llvm-branch-commits


@@ -635,6 +635,13 @@ class CGDebugInfo {
   llvm::DILocation *CreateTrapFailureMessageFor(llvm::DebugLoc TrapLocation,
 StringRef Category,
 StringRef FailureMsg);
+  /// Create a debug location from `Location` that adds an artificial inline
+  /// frame where the frame name is FuncName
+  ///
+  /// This is used to indiciate instructions that come from compiler
+  /// instrumentation.
+  llvm::DILocation *CreateSyntheticInline(llvm::DebugLoc Location,

delcypher wrote:

Nit. Maybe call it `CreateSyntheticInlineAt` ? Those who know more about Clang 
debug info generation (e.g. @adrian-prantl @felipepiovezan @Michael137 ) might 
have ideas on a better name though.

https://github.com/llvm/llvm-project/pull/128977
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [clang] [sanitizer] add pseudofunction to indicate array-bounds check (PR #128977)

2025-03-05 Thread Dan Liew via llvm-branch-commits

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/128977
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits