Skip to content

Sanitizer test regressions (mostly ASAN) #60394

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

Closed
mgorny opened this issue Jan 30, 2023 · 14 comments
Closed

Sanitizer test regressions (mostly ASAN) #60394

mgorny opened this issue Jan 30, 2023 · 14 comments

Comments

@mgorny
Copy link
Member

mgorny commented Jan 30, 2023

I've noticed them around the beginning of January (I'm sorry for not reporting earlier), they don't seem to related to any change in LLVM (i.e. once they started appearing, they happened with older versions as well):

Failed Tests (39):
  AddressSanitizer-i386-linux :: TestCases/printf-2.c
  AddressSanitizer-i386-linux :: TestCases/printf-3.c
  AddressSanitizer-i386-linux :: TestCases/printf-5.c
  AddressSanitizer-i386-linux :: TestCases/strcat-overlap.cpp
  AddressSanitizer-i386-linux :: TestCases/strcpy-overlap.cpp
  AddressSanitizer-i386-linux :: TestCases/strncat-overlap.cpp
  AddressSanitizer-i386-linux :: TestCases/strncpy-overflow.cpp
  AddressSanitizer-i386-linux :: TestCases/strncpy-overlap.cpp
  AddressSanitizer-i386-linux-dynamic :: TestCases/printf-2.c
  AddressSanitizer-i386-linux-dynamic :: TestCases/printf-3.c
  AddressSanitizer-i386-linux-dynamic :: TestCases/printf-5.c
  AddressSanitizer-i386-linux-dynamic :: TestCases/strcat-overlap.cpp
  AddressSanitizer-i386-linux-dynamic :: TestCases/strcpy-overlap.cpp
  AddressSanitizer-i386-linux-dynamic :: TestCases/strncat-overlap.cpp
  AddressSanitizer-i386-linux-dynamic :: TestCases/strncpy-overflow.cpp
  AddressSanitizer-i386-linux-dynamic :: TestCases/strncpy-overlap.cpp
  AddressSanitizer-x86_64-linux :: TestCases/printf-2.c
  AddressSanitizer-x86_64-linux :: TestCases/printf-3.c
  AddressSanitizer-x86_64-linux :: TestCases/printf-5.c
  AddressSanitizer-x86_64-linux :: TestCases/strcat-overlap.cpp
  AddressSanitizer-x86_64-linux :: TestCases/strcpy-overlap.cpp
  AddressSanitizer-x86_64-linux :: TestCases/strncat-overlap.cpp
  AddressSanitizer-x86_64-linux :: TestCases/strncpy-overflow.cpp
  AddressSanitizer-x86_64-linux :: TestCases/strncpy-overlap.cpp
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/printf-2.c
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/printf-3.c
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/printf-5.c
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/strcat-overlap.cpp
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/strcpy-overlap.cpp
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/strncat-overlap.cpp
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/strncpy-overflow.cpp
  AddressSanitizer-x86_64-linux-dynamic :: TestCases/strncpy-overlap.cpp
  DataFlowSanitizer-x86_64 :: custom.cpp
  DataFlowSanitizer-x86_64 :: origin_unaligned_memtrans.c
  MemorySanitizer-X86_64 :: chained_origin_memcpy.cpp
  MemorySanitizer-X86_64 :: chained_origin_memmove.cpp
  ThreadSanitizer-x86_64 :: exceptions.cpp
  ThreadSanitizer-x86_64 :: fiber_longjmp.cpp
  ThreadSanitizer-x86_64 :: signal_longjmp.cpp

Gentoo Linux amd64
Kernel: 6.1.8-gentoo-dist
glibc: 2.36-r7
systemd: 252.5 (testing is done in nspawn container)

Full log (2.5M): sys-libs:compiler-rt-sanitizers-16.0.0_rc1:20230130-193656.log

@mgorny mgorny added compiler-rt:asan Address sanitizer compiler-rt:tsan Thread sanitizer regression compiler-rt:msan Memory sanitizer labels Jan 30, 2023
@mgorny mgorny added this to the LLVM 16.0.0 Release milestone Jan 30, 2023
@vitalybuka
Copy link
Collaborator

I guess it's not a regression, but something changed in glibc.

@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Jan 31, 2023
@tru tru moved this from Needs Triage to Needs Fix in LLVM Release Status Jan 31, 2023
@mgorny
Copy link
Member Author

mgorny commented Jan 31, 2023

I thought that it was maybe the kernel but 5.15.90 fails the same (unless it was a kernel change backported to 5.15 ;-)).

@mgorny
Copy link
Member Author

mgorny commented Jan 31, 2023

I have seen the same set of failures on 2023-01-12 (on top of 6dc85bd) but almost all tests passed on 2023-01-08 (with the same commit). I did not upgrade glibc during that period.

@mgorny
Copy link
Member Author

mgorny commented Jan 31, 2023

I see the same failures outside the nspawn container, so it's not that either. I've checked install logs for the aforementioned period and I don't see anything obvious that could have caused this — so most likely it was some kernel change that was backported to 5.15. Unfortunately, I don't have the old kernel binpkgs around anymore.

@thesamesam
Copy link
Member

Posted https://reviews.llvm.org/D143322. We may want to file another bug for the remaining 3 issues I get (no idea if mgorny gets them yet).

@mgorny
Copy link
Member Author

mgorny commented Feb 4, 2023

I can confirm that the patch fixes all test failures for me.

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Feb 4, 2023
Without this, if hardening measures like FORTIFY_SOURCE are are in
/etc/clang/*.cfg, many sanitizer tests will die before the sanitizer
can trap the problem being tested, because e.g. the _chk variants
of common functions will abort first.

This gets the number of failing tests down from 42->3 for me (and the
remaining 3 are unrelated).

See: 52ce677
See: 136f778
Closes: llvm/llvm-project#60394

Reviewed By: MaskRay

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

(cherry picked from commit 8ab7625)
@MaskRay
Copy link
Member

MaskRay commented Feb 4, 2023

For posterity, the issues were due to Gentoo's -D_FORTIFY_SOURCE=2 in a configuration file.

See google/sanitizers#247
When _FORTIFY_SOURCE is enabled, some library functions are redirected to *_chk.
Interceptors don't provide *_chk (with an exception https://reviews.llvm.org/D40951), so we just end up with fewer detected errors (e.g. asan, tsan) or false positives (msan).

Added this to my https://maskray.me/blog/2023-01-08-all-about-sanitizer-interceptors

@EugeneZelenko EugeneZelenko added compiler-rt test-suite and removed compiler-rt:asan Address sanitizer compiler-rt:tsan Thread sanitizer compiler-rt:msan Memory sanitizer labels Feb 5, 2023
@tru tru reopened this Feb 5, 2023
@tru
Copy link
Collaborator

tru commented Feb 5, 2023

I am guessing this fix should be picked into the release branch as well? Run the cherry-pick command in that case, reopen until we have decision on that.

@tru tru moved this from Needs Fix to Needs Pull Request in LLVM Release Status Feb 5, 2023
@thesamesam
Copy link
Member

Ah, I filed a separate issue for that: #60533.

@tru
Copy link
Collaborator

tru commented Feb 5, 2023

Ah nice I just merged that one as well. Thanks for the fix!

@tru tru moved this from Needs Pull Request to Done in LLVM Release Status Feb 5, 2023
@thesamesam
Copy link
Member

Thank you!

CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Feb 6, 2023
Without this, if hardening measures like FORTIFY_SOURCE are are in
/etc/clang/*.cfg, many sanitizer tests will die before the sanitizer
can trap the problem being tested, because e.g. the _chk variants
of common functions will abort first.

This gets the number of failing tests down from 42->3 for me (and the
remaining 3 are unrelated).

See: 52ce677
See: 136f77805fd89cd30e69b3d1204fbf7efedd9a12
Closes: llvm/llvm-project#60394

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D143322
skatrak pushed a commit to skatrak/llvm-project-rocm that referenced this issue Feb 10, 2023
Without this, if hardening measures like FORTIFY_SOURCE are are in
/etc/clang/*.cfg, many sanitizer tests will die before the sanitizer
can trap the problem being tested, because e.g. the _chk variants
of common functions will abort first.

This gets the number of failing tests down from 42->3 for me (and the
remaining 3 are unrelated).

See: 52ce677
See: 136f778
Closes: llvm/llvm-project#60394

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D143322
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Feb 11, 2023
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 8, 2024
Without this, if hardening measures like FORTIFY_SOURCE are are in
/etc/clang/*.cfg, many sanitizer tests will die before the sanitizer
can trap the problem being tested, because e.g. the _chk variants
of common functions will abort first.

This gets the number of failing tests down from 42->3 for me (and the
remaining 3 are unrelated).

See: 52ce677
See: 136f778
Closes: llvm/llvm-project#60394

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D143322
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 14, 2024
Without this, if hardening measures like FORTIFY_SOURCE are are in
/etc/clang/*.cfg, many sanitizer tests will die before the sanitizer
can trap the problem being tested, because e.g. the _chk variants
of common functions will abort first.

This gets the number of failing tests down from 42->3 for me (and the
remaining 3 are unrelated).

See: 52ce677
See: 136f778
Closes: llvm/llvm-project#60394

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D143322
@mstorsjo
Copy link
Member

Unfortunately, this fix in 8ab7625, flat out disabling any default config files, break other potential use cases.

In llvm-mingw, I set a bunch of custom defaults for the mingw cross compilation targets (-rtlib=compiler-rt -unwindlib=libunwind -stdlib=libc++ -fuse-ld=lld). These defaults could in theory be set in a couple different ways:

  • The options can be passed explicitly by using a wrapper script/executable - this is what I currently do
  • The options can be hardcoded in Clang by building clang with options like -DCLANG_DEFAULT_RTLIB=compiler-rt. This is terrible, as this Clang binary can't be used for any other targets where these options don't fit.
  • The options could be set in target specific config files, so that any way that clang (or related tools like clangd, clang-scan-deps etc) is invoked for such a target, it picks up these right defaults

I tried switching llvm-mingw over to the third option, in mstorsjo/llvm-mingw@6939aee and mstorsjo/llvm-mingw@4bff029. However, then running the compiler-rt tests fail, because of CLANG_NO_DEFAULT_CONFIG=1 from 8ab7625 - see e.g. https://github.com/mstorsjo/llvm-mingw/actions/runs/11319240849/job/31483785448.

So for the cases where the system Clang uses a global config that adds an extra option that we want to skip (to make a more "vanilla" compilation), like Gentoo's -D_FORTIFY_SOURCE=2, running tests with CLANG_NO_DEFAULT_CONFIG=1 works fine. But the compiler default config files can also be essential options for the compiler to work at all - and then this env variable is breaking things.

I'm not entirely sure how to best deal with this. I mean, I guess we could add an option, for when configuring compiler-rt for running tests, for the user to pick whether to override clang config files or not. But this is rather obscure...

Or should we have some sort of cmake configure level test, to see whether the compiler still works if we set CLANG_NO_DEFAULT_CONFIG=1, and only set it in the testsuite if seems to work and not break the compiler?

mstorsjo added a commit to mstorsjo/llvm-project that referenced this issue Oct 23, 2024
…configs

The use of CLANG_NO_DEFAULT_CONFIG in the tests was added because
some Linux distributions had a global default config file, that
added flags relating to hardening, which interfere with the
sanitizer tests. By setting CLANG_NO_DEFAULT_CONFIG, the global
default config files that are found are ignored, and the sanitizers
get the expected default compiler behaviour.

(This was llvm#60394, which
was fixed in 8ab7625.)

However, some toolchains may rely on default config files for
mandatory parts required for functioning at all - setting things
like sysroots, -rtlib, -unwindlib, -stdlib, -fuse-ld etc. In such
a case we can't forcibly disable any default config, because it
will break the otherwise working toolchain.

Add a test for whether the compiler works while passing
--no-default-config to it. If the option is accepted and the
toolchain still works while that is set, set CLANG_NO_DEFAULT_CONFIG
while running tests.

(This adds a little bit of inconsistency, as we're testing for the
command line option, while using the environment variable. However
doing compile testing with an environment variable isn't quite as
easily doable, and passing an extra command line flag to all
compile commands while testing, is a bit clumsy - therefore this
inconsistency.)
@mstorsjo
Copy link
Member

I'm not entirely sure how to best deal with this. I mean, I guess we could add an option, for when configuring compiler-rt for running tests, for the user to pick whether to override clang config files or not. But this is rather obscure...

Or should we have some sort of cmake configure level test, to see whether the compiler still works if we set CLANG_NO_DEFAULT_CONFIG=1, and only set it in the testsuite if seems to work and not break the compiler?

I posted a PR that tries to fix this now, in #113491.

mstorsjo added a commit to mstorsjo/llvm-project that referenced this issue Oct 23, 2024
…configs

The use of CLANG_NO_DEFAULT_CONFIG in the tests was added because
some Linux distributions had a global default config file, that
added flags relating to hardening, which interfere with the
sanitizer tests. By setting CLANG_NO_DEFAULT_CONFIG, the global
default config files that are found are ignored, and the sanitizers
get the expected default compiler behaviour.

(This was llvm#60394, which
was fixed in 8ab7625.)

However, some toolchains may rely on default config files for
mandatory parts required for functioning at all - setting things
like sysroots, -rtlib, -unwindlib, -stdlib, -fuse-ld etc. In such
a case we can't forcibly disable any default config, because it
will break the otherwise working toolchain.

Add a test for whether the compiler works while passing
--no-default-config to it. If the option is accepted and the
toolchain still works while that is set, set CLANG_NO_DEFAULT_CONFIG
while running tests.

(This adds a little bit of inconsistency, as we're testing for the
command line option, while using the environment variable. However
doing compile testing with an environment variable isn't quite as
easily doable, and passing an extra command line flag to all
compile commands while testing, is a bit clumsy - therefore this
inconsistency.)
@thesamesam
Copy link
Member

I'm sorry that I missed your earlier comment!

mstorsjo added a commit that referenced this issue Oct 24, 2024
…configs (#113491)

The use of CLANG_NO_DEFAULT_CONFIG in the tests was added because some
Linux distributions had a global default config file, that added flags
relating to hardening, which interfere with the sanitizer tests. By
setting CLANG_NO_DEFAULT_CONFIG, the global default config files that
are found are ignored, and the sanitizers get the expected default
compiler behaviour.

(This was #60394, which was
fixed in 8ab7625.)

However, some toolchains may rely on default config files for mandatory
parts required for functioning at all - setting things like sysroots,
-rtlib, -unwindlib, -stdlib, -fuse-ld etc. In such a case we can't
forcibly disable any default config, because it will break the otherwise
working toolchain.

Add a test for whether the compiler works while passing
--no-default-config to it. If the option is accepted and the toolchain
still works while that is set, set CLANG_NO_DEFAULT_CONFIG while running
tests.

(This adds a little bit of inconsistency, as we're testing for the
command line option, while using the environment variable. However doing
compile testing with an environment variable isn't quite as easily
doable, and passing an extra command line flag to all compile commands
while testing, is a bit clumsy - therefore this inconsistency.)
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
…configs (llvm#113491)

The use of CLANG_NO_DEFAULT_CONFIG in the tests was added because some
Linux distributions had a global default config file, that added flags
relating to hardening, which interfere with the sanitizer tests. By
setting CLANG_NO_DEFAULT_CONFIG, the global default config files that
are found are ignored, and the sanitizers get the expected default
compiler behaviour.

(This was llvm#60394, which was
fixed in 8ab7625.)

However, some toolchains may rely on default config files for mandatory
parts required for functioning at all - setting things like sysroots,
-rtlib, -unwindlib, -stdlib, -fuse-ld etc. In such a case we can't
forcibly disable any default config, because it will break the otherwise
working toolchain.

Add a test for whether the compiler works while passing
--no-default-config to it. If the option is accepted and the toolchain
still works while that is set, set CLANG_NO_DEFAULT_CONFIG while running
tests.

(This adds a little bit of inconsistency, as we're testing for the
command line option, while using the environment variable. However doing
compile testing with an environment variable isn't quite as easily
doable, and passing an extra command line flag to all compile commands
while testing, is a bit clumsy - therefore this inconsistency.)
tru pushed a commit to llvmbot/llvm-project that referenced this issue Nov 12, 2024
…configs (llvm#113491)

The use of CLANG_NO_DEFAULT_CONFIG in the tests was added because some
Linux distributions had a global default config file, that added flags
relating to hardening, which interfere with the sanitizer tests. By
setting CLANG_NO_DEFAULT_CONFIG, the global default config files that
are found are ignored, and the sanitizers get the expected default
compiler behaviour.

(This was llvm#60394, which was
fixed in 8ab7625.)

However, some toolchains may rely on default config files for mandatory
parts required for functioning at all - setting things like sysroots,
-rtlib, -unwindlib, -stdlib, -fuse-ld etc. In such a case we can't
forcibly disable any default config, because it will break the otherwise
working toolchain.

Add a test for whether the compiler works while passing
--no-default-config to it. If the option is accepted and the toolchain
still works while that is set, set CLANG_NO_DEFAULT_CONFIG while running
tests.

(This adds a little bit of inconsistency, as we're testing for the
command line option, while using the environment variable. However doing
compile testing with an environment variable isn't quite as easily
doable, and passing an extra command line flag to all compile commands
while testing, is a bit clumsy - therefore this inconsistency.)

(cherry picked from commit a14a83d)
tru pushed a commit to llvmbot/llvm-project that referenced this issue Nov 15, 2024
…configs (llvm#113491)

The use of CLANG_NO_DEFAULT_CONFIG in the tests was added because some
Linux distributions had a global default config file, that added flags
relating to hardening, which interfere with the sanitizer tests. By
setting CLANG_NO_DEFAULT_CONFIG, the global default config files that
are found are ignored, and the sanitizers get the expected default
compiler behaviour.

(This was llvm#60394, which was
fixed in 8ab7625.)

However, some toolchains may rely on default config files for mandatory
parts required for functioning at all - setting things like sysroots,
-rtlib, -unwindlib, -stdlib, -fuse-ld etc. In such a case we can't
forcibly disable any default config, because it will break the otherwise
working toolchain.

Add a test for whether the compiler works while passing
--no-default-config to it. If the option is accepted and the toolchain
still works while that is set, set CLANG_NO_DEFAULT_CONFIG while running
tests.

(This adds a little bit of inconsistency, as we're testing for the
command line option, while using the environment variable. However doing
compile testing with an environment variable isn't quite as easily
doable, and passing an extra command line flag to all compile commands
while testing, is a bit clumsy - therefore this inconsistency.)

(cherry picked from commit a14a83d)
nikic pushed a commit to rust-lang/llvm-project that referenced this issue Nov 20, 2024
…configs (llvm#113491)

The use of CLANG_NO_DEFAULT_CONFIG in the tests was added because some
Linux distributions had a global default config file, that added flags
relating to hardening, which interfere with the sanitizer tests. By
setting CLANG_NO_DEFAULT_CONFIG, the global default config files that
are found are ignored, and the sanitizers get the expected default
compiler behaviour.

(This was llvm#60394, which was
fixed in 8ab7625.)

However, some toolchains may rely on default config files for mandatory
parts required for functioning at all - setting things like sysroots,
-rtlib, -unwindlib, -stdlib, -fuse-ld etc. In such a case we can't
forcibly disable any default config, because it will break the otherwise
working toolchain.

Add a test for whether the compiler works while passing
--no-default-config to it. If the option is accepted and the toolchain
still works while that is set, set CLANG_NO_DEFAULT_CONFIG while running
tests.

(This adds a little bit of inconsistency, as we're testing for the
command line option, while using the environment variable. However doing
compile testing with an environment variable isn't quite as easily
doable, and passing an extra command line flag to all compile commands
while testing, is a bit clumsy - therefore this inconsistency.)

(cherry picked from commit a14a83d)
arichardson pushed a commit to CTSRD-CHERI/compiler-rt that referenced this issue Jan 28, 2025
Without this, if hardening measures like FORTIFY_SOURCE are are in
/etc/clang/*.cfg, many sanitizer tests will die before the sanitizer
can trap the problem being tested, because e.g. the _chk variants
of common functions will abort first.

This gets the number of failing tests down from 42->3 for me (and the
remaining 3 are unrelated).

See: 52ce6776cf98e993c6ec04ae54b52e1354fff917
See: 136f77805fd89cd30e69b3d1204fbf7efedd9a12
Closes: llvm/llvm-project#60394

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D143322
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

7 participants