[tsan] Refine fstat{,64} interceptors #86625

MaskRay · 2024-03-26T04:31:23Z

In glibc versions before 2.33. libc_nonshared.a defines
__fxstat/__fxstat64 but there is no fstat/fstat64. glibc 2.33 added
fstat/fstat64 and obsoleted __fxstat/__fxstat64. Ports added after
2.33 do not provide __fxstat/__fxstat64, so our fstat/fstat64
interceptors using __fxstat/__fxstat64 interceptors would lead to
runtime failures on such ports (LoongArch and certain RISC-V ports).

Similar to https://reviews.llvm.org/D118423, refine the conditions that
we define fstat{,64} interceptors. fstat is supported by musl/*BSD
while fstat64 is glibc only.

Created using spr 1.3.5-bogner

llvmbot · 2024-03-26T04:31:56Z

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

Author: Fangrui Song (MaskRay)

Changes

In glibc versions before 2.33. libc_nonshared.a defines
__fxstat/__fxstat64 but there is no fstat/fstat64. glibc 2.33 added
fstat/fstat64 and obsoleted __fxstat/__fxstat64. Ports added after
2.33 do not provide __fxstat/__fxstat64, so our fstat/fstat64
interceptors using __fxstat/__fxstat64 interceptors would lead to
runtime failures on such ports (LoongArch and certain RISC-V ports).

Similar to https://reviews.llvm.org/D118423, refine the conditions that
we define fstat{,64} interceptors. fstat is supported by musl/*BSD
while fstat64 is glibc only.

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

1 Files Affected:

(modified) compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp (+18-25)

diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 8ffc703b05eace..50161fc02dfad8 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -14,6 +14,7 @@
 
 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_errno.h"
+#include "sanitizer_common/sanitizer_glibc_version.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_platform_limits_netbsd.h"
@@ -1613,47 +1614,40 @@ TSAN_INTERCEPTOR(int, __fxstat, int version, int fd, void *buf) {
     FdAccess(thr, pc, fd);
   return REAL(__fxstat)(version, fd, buf);
 }
-#define TSAN_MAYBE_INTERCEPT___FXSTAT TSAN_INTERCEPT(__fxstat)
+
+TSAN_INTERCEPTOR(int, __fxstat64, int version, int fd, void *buf) {
+  SCOPED_TSAN_INTERCEPTOR(__fxstat64, version, fd, buf);
+  if (fd > 0)
+    FdAccess(thr, pc, fd);
+  return REAL(__fxstat64)(version, fd, buf);
+}
+#define TSAN_MAYBE_INTERCEPT___FXSTAT TSAN_INTERCEPT(__fxstat); TSAN_INTERCEPT(__fxstat64)
 #else
 #define TSAN_MAYBE_INTERCEPT___FXSTAT
 #endif
 
+#if !SANITIZER_GLIBC || __GLIBC_PREREQ(2, 33)
 TSAN_INTERCEPTOR(int, fstat, int fd, void *buf) {
-#if SANITIZER_GLIBC
-  SCOPED_TSAN_INTERCEPTOR(__fxstat, 0, fd, buf);
-  if (fd > 0)
-    FdAccess(thr, pc, fd);
-  return REAL(__fxstat)(0, fd, buf);
-#else
   SCOPED_TSAN_INTERCEPTOR(fstat, fd, buf);
   if (fd > 0)
     FdAccess(thr, pc, fd);
   return REAL(fstat)(fd, buf);
-#endif
-}
-
-#if SANITIZER_GLIBC
-TSAN_INTERCEPTOR(int, __fxstat64, int version, int fd, void *buf) {
-  SCOPED_TSAN_INTERCEPTOR(__fxstat64, version, fd, buf);
-  if (fd > 0)
-    FdAccess(thr, pc, fd);
-  return REAL(__fxstat64)(version, fd, buf);
 }
-#define TSAN_MAYBE_INTERCEPT___FXSTAT64 TSAN_INTERCEPT(__fxstat64)
+#  define TSAN_MAYBE_INTERCEPT_FSTAT TSAN_INTERCEPT(fstat)
 #else
-#define TSAN_MAYBE_INTERCEPT___FXSTAT64
+#  define TSAN_MAYBE_INTERCEPT_FSTAT
 #endif
 
-#if SANITIZER_GLIBC
+#if __GLIBC_PREREQ(2, 33)
 TSAN_INTERCEPTOR(int, fstat64, int fd, void *buf) {
-  SCOPED_TSAN_INTERCEPTOR(__fxstat64, 0, fd, buf);
+  SCOPED_TSAN_INTERCEPTOR(fstat64, fd, buf);
   if (fd > 0)
     FdAccess(thr, pc, fd);
-  return REAL(__fxstat64)(0, fd, buf);
+  return REAL(fstat64)(fd, buf);
 }
-#define TSAN_MAYBE_INTERCEPT_FSTAT64 TSAN_INTERCEPT(fstat64)
+#  define TSAN_MAYBE_INTERCEPT_FSTAT64 TSAN_INTERCEPT(fstat64)
 #else
-#define TSAN_MAYBE_INTERCEPT_FSTAT64
+#  define TSAN_MAYBE_INTERCEPT_FSTAT64
 #endif
 
 TSAN_INTERCEPTOR(int, open, const char *name, int oflag, ...) {
@@ -2963,10 +2957,9 @@ void InitializeInterceptors() {
 
   TSAN_INTERCEPT(pthread_once);
 
-  TSAN_INTERCEPT(fstat);
   TSAN_MAYBE_INTERCEPT___FXSTAT;
+  TSAN_MAYBE_INTERCEPT_FSTAT;
   TSAN_MAYBE_INTERCEPT_FSTAT64;
-  TSAN_MAYBE_INTERCEPT___FXSTAT64;
   TSAN_INTERCEPT(open);
   TSAN_MAYBE_INTERCEPT_OPEN64;
   TSAN_INTERCEPT(creat);

github-actions · 2024-03-26T04:34:08Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff ce73b1672a6053d5974dc2342881aac02efe2dbb fa19d5db5f7696dcfebcbbdae2cd5d8343ecbf9f -- compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

View the diff from clang-format here.

diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 50161fc02d..96e81baf3b 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -12,27 +12,27 @@
 // sanitizer_common/sanitizer_common_interceptors.inc
 //===----------------------------------------------------------------------===//
 
+#include <stdarg.h>
+
+#include "interception/interception.h"
 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_errno.h"
 #include "sanitizer_common/sanitizer_glibc_version.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_linux.h"
+#include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_platform_limits_netbsd.h"
 #include "sanitizer_common/sanitizer_platform_limits_posix.h"
-#include "sanitizer_common/sanitizer_placement_new.h"
 #include "sanitizer_common/sanitizer_posix.h"
 #include "sanitizer_common/sanitizer_stacktrace.h"
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
-#include "interception/interception.h"
+#include "tsan_fd.h"
 #include "tsan_interceptors.h"
 #include "tsan_interface.h"
+#include "tsan_mman.h"
 #include "tsan_platform.h"
-#include "tsan_suppressions.h"
 #include "tsan_rtl.h"
-#include "tsan_mman.h"
-#include "tsan_fd.h"
-
-#include <stdarg.h>
+#include "tsan_suppressions.h"
 
 using namespace __tsan;
 
@@ -1621,7 +1621,9 @@ TSAN_INTERCEPTOR(int, __fxstat64, int version, int fd, void *buf) {
     FdAccess(thr, pc, fd);
   return REAL(__fxstat64)(version, fd, buf);
 }
-#define TSAN_MAYBE_INTERCEPT___FXSTAT TSAN_INTERCEPT(__fxstat); TSAN_INTERCEPT(__fxstat64)
+#  define TSAN_MAYBE_INTERCEPT___FXSTAT \
+    TSAN_INTERCEPT(__fxstat);           \
+    TSAN_INTERCEPT(__fxstat64)
 #else
 #define TSAN_MAYBE_INTERCEPT___FXSTAT
 #endif

wangleiat

Tested OK on LoongArch. Prior to this, the following errors will be prompted:

ThreadSanitizer:DEADLYSIGNAL
==752233==ERROR: ThreadSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000000000 bp 0x7ffffbdfe080 sp 0x7ffffbdfe010 T752233)
==752233==Hint: pc points to the zero page.
==752233==The signal is caused by a READ memory access.
==752233==Hint: address points to the zero page.

LGTM, except for code formatting.
Thanks.

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

MaskRay · 2024-03-26T21:11:19Z

Thanks for the testing and review.

I've checked our internal systems and this patch seems good.

I will wait a bit to ensure things are stable and consider release/18.x backport if it's still accepting updates.

MaskRay · 2024-04-01T22:05:29Z

/cherry-pick d5224b7

In glibc versions before 2.33. `libc_nonshared.a` defines `__fxstat/__fxstat64` but there is no `fstat/fstat64`. glibc 2.33 added `fstat/fstat64` and obsoleted `__fxstat/__fxstat64`. Ports added after 2.33 do not provide `__fxstat/__fxstat64`, so our `fstat/fstat64` interceptors using `__fxstat/__fxstat64` interceptors would lead to runtime failures on such ports (LoongArch and certain RISC-V ports). Similar to https://reviews.llvm.org/D118423, refine the conditions that we define fstat{,64} interceptors. `fstat` is supported by musl/*BSD while `fstat64` is glibc only. (cherry picked from commit d5224b7)

llvmbot · 2024-04-01T22:10:32Z

/pull-request #87286

[𝘀𝗽𝗿] initial version

fa19d5d

Created using spr 1.3.5-bogner

llvmbot added compiler-rt compiler-rt:tsan Thread sanitizer compiler-rt:sanitizer labels Mar 26, 2024

MaskRay requested review from dvyukov and vitalybuka March 26, 2024 04:31

MaskRay requested review from wangleiat and SixWeining March 26, 2024 04:32

MaskRay mentioned this pull request Mar 26, 2024

[tsan] Refine fstat{,64} interceptors on GLIBC #85612

Closed

wangleiat approved these changes Mar 26, 2024

View reviewed changes

vitalybuka reviewed Mar 26, 2024

View reviewed changes

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp Show resolved Hide resolved

vitalybuka approved these changes Mar 26, 2024

View reviewed changes

MaskRay merged commit d5224b7 into main Mar 26, 2024
7 of 8 checks passed

MaskRay deleted the users/MaskRay/spr/tsan-refine-fstat64-interceptors branch March 26, 2024 21:09

MaskRay added this to the LLVM 18.X Release milestone Apr 1, 2024

pointhex mentioned this pull request May 7, 2024

getStyleDiagHandler #91314

Closed

aemerson mentioned this pull request May 9, 2024

release/18.x: [AArc64][GlobalISel] Fix legalizer assert for G_INSERT_VECTOR_ELT - manual merge #91672

Merged

This was referenced Jun 20, 2024

[compiler-rt] [libtsan] fix fstat{,64} interceptors on glibc #75578

Closed

fstat TSan interceptor not behaving correctly on POWER #75346

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tsan] Refine fstat{,64} interceptors #86625

[tsan] Refine fstat{,64} interceptors #86625

MaskRay commented Mar 26, 2024

llvmbot commented Mar 26, 2024

github-actions bot commented Mar 26, 2024

wangleiat left a comment

MaskRay commented Mar 26, 2024 •

edited

Loading

MaskRay commented Apr 1, 2024

llvmbot commented Apr 1, 2024

[tsan] Refine fstat{,64} interceptors #86625

[tsan] Refine fstat{,64} interceptors #86625

Conversation

MaskRay commented Mar 26, 2024

llvmbot commented Mar 26, 2024

github-actions bot commented Mar 26, 2024

wangleiat left a comment

Choose a reason for hiding this comment

MaskRay commented Mar 26, 2024 • edited Loading

MaskRay commented Apr 1, 2024

llvmbot commented Apr 1, 2024

MaskRay commented Mar 26, 2024 •

edited

Loading