Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sanitizer] Skip /include/c++/ from summary #78534

Merged

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Jan 18, 2024

std:: usually is not a cause of the bug.

We now display

SUMMARY: AddressSanitizer: allocation-size-too-big path/to/allocator_returns_null.cpp:92:7 in main

instead of

SUMMARY: AddressSanitizer: allocation-size-too-big /usr/lib/../include/c++/13/bits/new_allocator.h:147:27 in std::__new_allocator<char>::allocate(unsigned long, void const*)

/include/c++/ matches both libc++ and libstdc++ include paths.

Created using spr 1.3.4
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 18, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

Changes

std:: usually is not a cause of the bug.


Full diff: https://github.com/llvm/llvm-project/pull/78534.diff

2 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp (+2-1)
  • (modified) compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp (+17-2)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
index 253dc10607a6eb..8438e019591b58 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
@@ -34,7 +34,8 @@ static bool FrameIsInternal(const SymbolizedStack *frame) {
     return true;
   const char *file = frame->info.file;
   const char *module = frame->info.module;
-  if (file && (internal_strstr(file, "/compiler-rt/lib/")))
+  if (file && (internal_strstr(file, "/compiler-rt/lib/") ||
+               internal_strstr(file, "/include/c++/")))
     return true;
   if (module && (internal_strstr(module, "libclang_rt.")))
     return true;
diff --git a/compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp b/compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp
index ca6f637b9a3f50..1e4b0ed520b8a4 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/allocator_returns_null.cpp
@@ -34,17 +34,20 @@
 // RUN:   | FileCheck %s --check-prefix=CHECK-nnCRASH
 // RUN: %env_tool_opts=allocator_may_return_null=1     %run %t new-nothrow 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-NULL
+// RUN: %env_tool_opts=allocator_may_return_null=0 not %run %t vector 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-vCRASH
 
 // TODO(alekseyshl): win32 is disabled due to failing errno tests, fix it there.
 // UNSUPPORTED: ubsan, target={{.*windows-msvc.*}}
 
 #include <assert.h>
 #include <errno.h>
+#include <limits>
+#include <new>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <limits>
-#include <new>
+#include <vector>
 
 int main(int argc, char **argv) {
   assert(argc == 2);
@@ -60,6 +63,8 @@ int main(int argc, char **argv) {
       (3UL << 30) + 1;
 #endif
 
+  std::vector<char> v;
+
   void *x = nullptr;
   if (!strcmp(action, "malloc")) {
     x = malloc(kMaxAllowedMallocSizePlusOne);
@@ -82,6 +87,14 @@ int main(int argc, char **argv) {
     x = operator new(kMaxAllowedMallocSizePlusOne);
   } else if (!strcmp(action, "new-nothrow")) {
     x = operator new(kMaxAllowedMallocSizePlusOne, std::nothrow);
+  } else if (!strcmp(action, "vector")) {
+#if __LP64__ || defined(_WIN64)
+    v.resize(kMaxAllowedMallocSizePlusOne);
+    x = v.data();
+#else
+    // Fake it: 32bit fails early in std.
+    x = malloc(kMaxAllowedMallocSizePlusOne);
+#endif
   } else {
     assert(0);
   }
@@ -117,6 +130,8 @@ int main(int argc, char **argv) {
 // CHECK-nnCRASH: new-nothrow:
 // CHECK-nnCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
 // CHECK-nnCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*allocator_returns_null.cpp.*}} in main
+// CHECK-vCRASH: #{{[0-9]+.*}}allocator_returns_null.cpp
+// CHECK-vCRASH: {{SUMMARY: .*Sanitizer: allocation-size-too-big.*allocator_returns_null.cpp.*}} in main
 
 // CHECK-NULL: {{malloc|calloc|calloc-overflow|realloc|realloc-after-malloc|new-nothrow}}
 // CHECK-NULL: errno: 12, x: 0

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4
@MaskRay
Copy link
Member

MaskRay commented Jan 18, 2024

std:: usually is not a cause of the bug.

Consider elaborating this message.

We now display SUMMARY: AddressSanitizer: allocation-size-too-big path/to/allocator_returns_null.cpp:92:7 in main instead of SUMMARY: AddressSanitizer: allocation-size-too-big /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/new_allocator.h:147:27 in std::__new_allocator<char>::allocate(unsigned long, void const*)
include/c++ matches both libc++ and libstdc++ include paths.

@vitalybuka vitalybuka merged commit ecd4781 into main Jan 18, 2024
4 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/sanitizer-skip-includec-from-summary branch January 18, 2024 20:25
zmodem added a commit that referenced this pull request Jan 19, 2024
The test fails on Darwin, see comment on the PR.

> std:: usually is not a cause of the bug.
>
> We now display
> ```
> SUMMARY: AddressSanitizer: allocation-size-too-big path/to/allocator_returns_null.cpp:92:7 in main
> ```
> instead of
>
> ```
> SUMMARY: AddressSanitizer: allocation-size-too-big /usr/lib/../include/c++/13/bits/new_allocator.h:147:27 in std::__new_allocator<char>::allocate(unsigned long, void const*)
> ```
>
> `/include/c++/` matches both libc++ and libstdc++ include paths.

This reverts commit ecd4781.
@zmodem
Copy link
Collaborator

zmodem commented Jan 19, 2024

@vitalybuka
Copy link
Collaborator Author

Thanks, I guess symbolizer on Darwin somehow strip inlude/c++ path

vitalybuka added a commit that referenced this pull request Jan 20, 2024
@thesamesam
Copy link
Member

Note that on Gentoo, this isn't right either, we have e.g.:

/usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14/bits/new_allocator.h
/usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14/ext/new_allocator.h

@vitalybuka
Copy link
Collaborator Author

Note that on Gentoo, this isn't right either, we have e.g.:

/usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14/bits/new_allocator.h
/usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14/ext/new_allocator.h

I guess it's fine to expand the FrameIsInternal to cover Gentoo as well. it looks quite distinct. It would be nice if someone who can test on platform to create a patch.

For Darwin I don't see what we can do if there is no path at all, hardcode most common c++ headers?

thesamesam added a commit to thesamesam/llvm-project that referenced this pull request Jan 24, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough. Note that the omission of
a trailing slash is intentional to match g++-*.

See llvm#78534 (comment).
llvm-git-migration pushed a commit to llvm-git-migration/llvm-project that referenced this pull request Jan 24, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough.

See llvm#78534 (comment).
thesamesam added a commit that referenced this pull request Jan 27, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough. Note that the omission of
a trailing slash is intentional to match g++-*.

See #78534 (comment).

Reviewed by: mgorny
Closes: #79264

Signed-off-by: Sam James <sam@gentoo.org>
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jan 29, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough. Note that the omission of
a trailing slash is intentional to match g++-*.

See llvm#78534 (comment).

Reviewed by: mgorny
Closes: llvm#79264

Signed-off-by: Sam James <sam@gentoo.org>
(cherry picked from commit e8f882f)
@thesamesam
Copy link
Member

Unfortunately, it's been a while since I've done Darwin (although may be doing a bit more soon), so no ideas for that side yet.

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jan 30, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough. Note that the omission of
a trailing slash is intentional to match g++-*.

See llvm#78534 (comment).

Reviewed by: mgorny
Closes: llvm#79264

Signed-off-by: Sam James <sam@gentoo.org>
(cherry picked from commit e8f882f)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough. Note that the omission of
a trailing slash is intentional to match g++-*.

See llvm#78534 (comment).

Reviewed by: mgorny
Closes: llvm#79264

Signed-off-by: Sam James <sam@gentoo.org>
(cherry picked from commit e8f882f)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough. Note that the omission of
a trailing slash is intentional to match g++-*.

See llvm#78534 (comment).

Reviewed by: mgorny
Closes: llvm#79264

Signed-off-by: Sam James <sam@gentoo.org>
(cherry picked from commit e8f882f)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough. Note that the omission of
a trailing slash is intentional to match g++-*.

See llvm#78534 (comment).

Reviewed by: mgorny
Closes: llvm#79264

Signed-off-by: Sam James <sam@gentoo.org>
(cherry picked from commit e8f882f)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
On Gentoo, libc++ is indeed in /usr/include/c++/*, but libstdc++ is at
e.g. /usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14.

Use '/include/g++' as it should be unique enough. Note that the omission of
a trailing slash is intentional to match g++-*.

See llvm#78534 (comment).

Reviewed by: mgorny
Closes: llvm#79264

Signed-off-by: Sam James <sam@gentoo.org>
(cherry picked from commit e8f882f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants