This is an archive of the discontinued LLVM Phabricator instance.

[msan] Add stat-family interceptors on Linux
ClosedPublic

Authored by foobarrior on Oct 18 2021, 4:03 AM.

Details

Summary

Add following interceptors on Linux: stat, lstat, fstat, fstatat.

This fixes use-of-uninitialized value on platforms with GLIBC 2.33+.
In particular: Arch Linux, Ubuntu hirsute/impish.

The tests should have also been failing during the release on the mentioned platforms, but I cannot find any related discussion.

Most likely, the regression was introduced by glibc commit 8ed005daf0ab03e14250032:
all stat-family functions are now exported as shared functions.

Before, some of them (namely stat, lstat, fstat, fstatat) were provided as a part of libc_noshared.a and called their __xstat dopplegangers. This is still true for Debian Sid and earlier Ubuntu's. stat interceptors may be safely provided for them, no problem with that.

Closes https://github.com/google/sanitizers/issues/1452.
See also https://jira.mariadb.org/browse/MDEV-24841

Diff Detail

Event Timeline

foobarrior created this revision.Oct 18 2021, 4:03 AM
foobarrior requested review of this revision.Oct 18 2021, 4:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 18 2021, 4:03 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
foobarrior edited the summary of this revision. (Show Details)Oct 18 2021, 5:08 AM
eugenis added inline comments.Oct 18 2021, 2:04 PM
compiler-rt/lib/msan/msan_interceptors.cpp
711

Shouldn't this be "!SANITIZER_FREEBSD && !SANITIZER_NETBSD" ?

foobarrior updated this revision to Diff 380689.EditedOct 19 2021, 7:22 AM

Fix debian build.

__fxstatat interceptor now resides together with fstatat (just like fstat/__fxstat)

INTERCEPT_FUNCTION -> MSAN_INTERCEPT_FUNC to report interception errors

MSAN_INTERCEPT_FUNC(__fxstatat64)

foobarrior added inline comments.Oct 19 2021, 7:27 AM
compiler-rt/lib/msan/msan_interceptors.cpp
711

yes, stupid mistake, sorry

Please apply the clang-format lint changes in the comments.

compiler-rt/lib/msan/msan_interceptors.cpp
708

Please add _MAYBE_ to the macro name for consistency.

compiler-rt/test/msan/fstat.cpp
1 ↗(On Diff #380690)

why is -fPIC necessary?

Add _MAYBE_ where necessary.
Remove accidentally added -fPIC

Should I fix lint checks from the review? They seem to be incorrect from my point of view

compiler-rt/test/msan/fstat.cpp
1 ↗(On Diff #380690)

That snuck in accidentally, while i was testing. Removing

Reformat the code after eugenis comment is noticed

Reformat the code a bit less (no newlines after if's)

foobarrior edited the summary of this revision. (Show Details)Oct 20 2021, 6:01 AM
eugenis accepted this revision.Oct 20 2021, 4:30 PM

Should I fix lint checks from the review? They seem to be incorrect from my point of view

Yes please. Otherwise they will simply pop up in someone else's patch. We do what clang-format tell us to do :)

LGTM. Do you need me to land this change for you? I can reformat before commit, too.

This revision is now accepted and ready to land.Oct 20 2021, 4:30 PM

Yes, please. I have no commit rights.

This revision was automatically updated to reflect the committed changes.
vitalybuka added inline comments.
compiler-rt/lib/msan/msan_interceptors.cpp
661

This condition is inconvenient
I have false msan reports on 2.33 because it's compiled on older glibc

foobarrior added inline comments.Dec 14 2021, 8:25 PM
compiler-rt/lib/msan/msan_interceptors.cpp
661

Hi Vitaly!
What's your setup?
And can you provide an exmple of false positive?

MaskRay added inline comments.
compiler-rt/lib/msan/msan_interceptors.cpp
661

If __GLIBC_PREREQ is not defined (e.g. Linux musl), there will be a preprocessor error like:

  • Clang: error: function-like macro '__GLIBC_PREREQ' is not defined
  • GCC: error: missing binary operator before token "("

Looks like @thesamesam is trying to fix this for Linux musl.