[compiler-rt][rtsan] fseek api interception. #122163

devnexen · 2025-01-08T19:54:38Z

No description provided.

llvmbot · 2025-01-08T19:55:12Z

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

Author: David CARLIER (devnexen)

Changes

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

2 Files Affected:

(modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+59)
(modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp (+62)

diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index 6a5f4b91d11d7e..6b7a6d15f05b64 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -376,6 +376,58 @@ INTERCEPTOR(void, setbuffer, FILE *stream, char *buf, int size) {
 #define RTSAN_MAYBE_INTERCEPT_SETBUFFER
 #endif
 
+#if SANITIZER_INTERCEPT_FSEEK
+INTERCEPTOR(int, fgetpos, FILE *stream, fpos_t *pos) {
+  __rtsan_notify_intercepted_call("fgetpos");
+  return REAL(fgetpos)(stream, pos);
+}
+
+INTERCEPTOR(int, fseek, FILE *stream, long offset, int whence) {
+  __rtsan_notify_intercepted_call("fseek");
+  return REAL(fgetpos)(stream, offset, whence);
+}
+
+INTERCEPTOR(int, fseeko, FILE *stream, long offset, int whence) {
+  __rtsan_notify_intercepted_call("fseeko");
+  return REAL(fgetpos)(stream, offset, whence);
+}
+
+INTERCEPTOR(int, fsetpos, FILE *stream, const fpos_t pos) {
+  __rtsan_notify_intercepted_call("fsetpos");
+  return REAL(fsetpos)(stream, pos);
+}
+
+INTERCEPTOR(long, ftell, FILE *stream) {
+  __rtsan_notify_intercepted_call("ftell");
+  return REAL(ftell)(stream);
+}
+
+INTERCEPTOR(off_t, ftello, FILE *stream) {
+  __rtsan_notify_intercepted_call("ftello");
+  return REAL(ftello)(stream);
+}
+
+INTERCEPTOR(void, rewind, FILE *stream) {
+  __rtsan_notify_intercepted_call("rewind");
+  return REAL(rewind)(stream);
+}
+#define RTSAN_MAYBE_INTERCEPT_FGETPOS INTERCEPT_FUNCTION(fgetpos)
+#define RTSAN_MAYBE_INTERCEPT_FSEEK INTERCEPT_FUNCTION(fseek)
+#define RTSAN_MAYBE_INTERCEPT_FSEEKO INTERCEPT_FUNCTION(fseeko)
+#define RTSAN_MAYBE_INTERCEPT_FSETPOS INTERCEPT_FUNCTION(fsetpos)
+#define RTSAN_MAYBE_INTERCEPT_FTELL INTERCEPT_FUNCTION(ftell)
+#define RTSAN_MAYBE_INTERCEPT_FTELLO INTERCEPT_FUNCTION(ftello)
+#define RTSAN_MAYBE_INTERCEPT_REWIND INTERCEPT_FUNCTION(rewind)
+#else
+#define RTSAN_MAYBE_INTERCEPT_FGETPOS
+#define RTSAN_MAYBE_INTERCEPT_FSEEK
+#define RTSAN_MAYBE_INTERCEPT_FSEEKO
+#define RTSAN_MAYBE_INTERCEPT_FSETPOS
+#define RTSAN_MAYBE_INTERCEPT_FTELL
+#define RTSAN_MAYBE_INTERCEPT_FTELLO
+#define RTSAN_MAYBE_INTERCEPT_REWIND
+#endif
+
 INTERCEPTOR(int, puts, const char *s) {
   __rtsan_notify_intercepted_call("puts");
   return REAL(puts)(s);
@@ -1042,6 +1094,13 @@ void __rtsan::InitializeInterceptors() {
   RTSAN_MAYBE_INTERCEPT_SETVBUF;
   RTSAN_MAYBE_INTERCEPT_SETLINEBUF;
   RTSAN_MAYBE_INTERCEPT_SETBUFFER;
+  RTSAN_MAYBE_INTERCEPT_FGETPOS;
+  RTSAN_MAYBE_INTERCEPT_FSEEK;
+  RTSAN_MAYBE_INTERCEPT_FSEEKO;
+  RTSAN_MAYBE_INTERCEPT_FSETPOS;
+  RTSAN_MAYBE_INTERCEPT_FTELL;
+  RTSAN_MAYBE_INTERCEPT_FTELLO;
+  RTSAN_MAYBE_INTERCEPT_REWIND;
   INTERCEPT_FUNCTION(lseek);
   RTSAN_MAYBE_INTERCEPT_LSEEK64;
   INTERCEPT_FUNCTION(dup);
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
index 5488d3c7e2056c..f27ff1f7a350f0 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
@@ -453,6 +453,68 @@ TEST_F(RtsanFileTest, SetbufferDieWhenRealtime) {
 }
 #endif
 
+#if SANITIZER_INTERCEPT_FSEEK
+TEST_F(RtsanOpenedFileTest, FgetposDieWhenRealtime) {
+  auto Func = [this]() {
+    int pos = fgetpos(GetOpenFile());
+    ASSERT_THAT(pos, Eq(0));
+  };
+
+  ExpectRealtimeDeath(Func, MAYBE_APPEND_64("fgetpos"));
+  ExpectNonRealtimeSurvival(Func);
+}
+
+TEST_F(RtsanOpenedFileTest, FseekDieWhenRealtime) {
+  auto Func = [this]() {
+    int ret = fseek(GetOpenFile(), 0, SEEK_CUR);
+    ASSERT_THAT(ret, Eq(0));
+  };
+
+  ExpectRealtimeDeath(Func, MAYBE_APPEND_64("fseek"));
+  ExpectNonRealtimeSurvival(Func);
+}
+
+TEST_F(RtsanOpenedFileTest, FseekoDieWhenRealtime) {
+  auto Func = [this]() {
+    int ret = fseeko(GetOpenFile(), 0, SEEK_CUR);
+    ASSERT_THAT(ret, Eq(0));
+  };
+
+  ExpectRealtimeDeath(Func, MAYBE_APPEND_64("fseeko"));
+  ExpectNonRealtimeSurvival(Func);
+}
+
+TEST_F(RtsanOpenedFileTest, FtellDieWhenRealtime) {
+  auto Func = [this]() {
+    long ret = ftell(GetOpenFile());
+    ASSERT_THAT(ret, Eq(0));
+  };
+
+  ExpectRealtimeDeath(Func, MAYBE_APPEND_64("ftell"));
+  ExpectNonRealtimeSurvival(Func);
+}
+
+TEST_F(RtsanOpenedFileTest, FtelloDieWhenRealtime) {
+  auto Func = [this]() {
+    off_t ret = ftello(GetOpenFile());
+    ASSERT_THAT(ret, Eq(0));
+  };
+
+  ExpectRealtimeDeath(Func, MAYBE_APPEND_64("ftello"));
+  ExpectNonRealtimeSurvival(Func);
+}
+
+TEST_F(RtsanOpenedFileTest, RewindDieWhenRealtime) {
+  int end = fseek(GetOpenFile(), 0, SEEK_END);
+  auto Func = [this]() {
+    rewind(GetOpenFile());
+  };
+
+  ExpectRealtimeDeath(Func, "rewind");
+  ExpectNonRealtimeSurvival(Func);
+}
+#endif
+
 class RtsanOpenedFileTest : public RtsanFileTest {
 protected:
   void SetUp() override {

github-actions · 2025-01-08T19:58:50Z

✅ With the latest revision this PR passed the C/C++ code formatter.

cjappl · 2025-01-08T22:39:44Z

compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp

+    ASSERT_THAT(pos, Eq(0));
+  };
+
+  ExpectRealtimeDeath(Func, MAYBE_APPEND_64("fgetpos"));


Where are the interceptors for fseek64, fgetpos64, ...64 etc? I'm surprised these tests pass on linux

does not seem there is any fseek64 but fseeko64/ftello64/fgetpos64 and fsetpos64 however ...

cjappl · 2025-01-10T11:11:22Z

compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp

@@ -376,6 +376,95 @@ INTERCEPTOR(void, setbuffer, FILE *stream, char *buf, int size) {
 #define RTSAN_MAYBE_INTERCEPT_SETBUFFER
 #endif

+#if SANITIZER_INTERCEPT_FSEEK


This is more of a question - I don't have the answer. Should all of these be inside this ifdef? Should some of them be outside of it?

For instance, on my mac, I have access to many of these:

FSEEK(3) Library Functions Manual FSEEK(3) NAME fgetpos, fseek, fseeko, fsetpos, ftell, ftello, rewind – reposition a stream

And it seems like these only have support for BSD flavors:

#define SANITIZER_INTERCEPT_FSEEK (SI_NETBSD || SI_FREEBSD)

Even normally flavored linux has many of these https://www.man7.org/linux/man-pages/man3/fseek.3.html

Maybe this macro needs updating?

oh yes weird, I m pretty sure even fuchsia has it too, but setting as SI_POSIX should do at first.

cjappl · 2025-01-10T13:19:37Z

compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h

@@ -593,7 +593,7 @@ SANITIZER_WEAK_IMPORT void *aligned_alloc(__sanitizer::usize __alignment,
 #define SANITIZER_INTERCEPT_SHA1 SI_NETBSD
 #define SANITIZER_INTERCEPT_MD4 SI_NETBSD
 #define SANITIZER_INTERCEPT_RMD160 SI_NETBSD
-#define SANITIZER_INTERCEPT_FSEEK (SI_NETBSD || SI_FREEBSD)
+#define SANITIZER_INTERCEPT_FSEEK SI_POSIX


@fmayer wanted to get your eyes on this change in particular, as it may have ramifications for other sanitizers.

Maybe we should do this change first as a separate PR to see if there is any test fallout, then commit the rest?

Seems fseek is POSIX since 2001

HISTORY top
POSIX.1-2001, C89.

https://man7.org/linux/man-pages/man3/fseek.3.html

But splitting this off SGTM.

yes. to be fair the folk who implemented the interception was a pure BSD guy though. Ok will split soon-ish.

fseek api is POSIX. to also address llvm#122163

to also address llvm#122163

fseek api is POSIX. to also address #122163

davidtrevelyan · 2025-01-14T19:57:16Z

compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h

@@ -593,7 +593,7 @@ SANITIZER_WEAK_IMPORT void *aligned_alloc(__sanitizer::usize __alignment,
 #define SANITIZER_INTERCEPT_SHA1 SI_NETBSD
 #define SANITIZER_INTERCEPT_MD4 SI_NETBSD
 #define SANITIZER_INTERCEPT_RMD160 SI_NETBSD
-#define SANITIZER_INTERCEPT_FSEEK SI_POSIX
+#define SANITIZER_INTERCEPT_FSEEK (SI_NETBSD || SI_FREEBSD)


This line appears to undo the change from https://github.com/llvm/llvm-project/pull/122795/files - is this deliberate?

ah no, good catch !

fmayer · 2025-01-15T18:20:49Z

random driveby: the rtsan_interceptors_posix file doesn't seem to use the preprocessor macro indentation for #if like e.g. asan_interceptors does, which would make the nested conditions easier to read.

devnexen · 2025-01-15T18:23:00Z

If I get what you mean, I tried that earlier but the clang-format did not like it very much.

kstoimenov · 2025-01-15T19:29:28Z

compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp

+}
+
+TEST_F(RtsanOpenedFileTest, RewindDieWhenRealtime) {
+  int end = fseek(GetOpenFile(), 0, SEEK_END);


@devnexen this is causing unused variable warning, which ends up an error in the sanitizer bot. Could you please fix that?

sure, doing it now.

llvmbot added compiler-rt compiler-rt:sanitizer labels Jan 8, 2025

devnexen force-pushed the rtsan_fseek branch from 15bf644 to 5b574aa Compare January 8, 2025 19:59

devnexen requested review from davidtrevelyan and cjappl January 8, 2025 20:01

cjappl reviewed Jan 8, 2025

View reviewed changes

devnexen force-pushed the rtsan_fseek branch from 5b574aa to 3a97637 Compare January 9, 2025 04:40

cjappl reviewed Jan 10, 2025

View reviewed changes

devnexen force-pushed the rtsan_fseek branch 2 times, most recently from 1b0187a to 8ba332c Compare January 10, 2025 12:10

cjappl requested a review from fmayer January 10, 2025 13:18

cjappl reviewed Jan 10, 2025

View reviewed changes

devnexen force-pushed the rtsan_fseek branch from 8ba332c to e0951e5 Compare January 13, 2025 21:28

devnexen added a commit to devnexen/llvm-project that referenced this pull request Jan 13, 2025

[compiler-rt] fseek interception update.

3684e85

fseek api is POSIX. to also address llvm#122163

devnexen mentioned this pull request Jan 13, 2025

[compiler-rt] fseek interception update. #122795

Merged

devnexen added a commit to devnexen/llvm-project that referenced this pull request Jan 14, 2025

[compiler-rt] fseek interception change to enable for all posix systems.

d1e584a

to also address llvm#122163

devnexen added a commit that referenced this pull request Jan 14, 2025

[compiler-rt] fseek interception update. (#122795)

8d9dcd1

fseek api is POSIX. to also address #122163

devnexen force-pushed the rtsan_fseek branch from e0951e5 to 74445a1 Compare January 14, 2025 17:02

davidtrevelyan reviewed Jan 14, 2025

View reviewed changes

devnexen force-pushed the rtsan_fseek branch from 74445a1 to 4bbe488 Compare January 14, 2025 20:02

davidtrevelyan approved these changes Jan 14, 2025

View reviewed changes

[compiler-rt][rtsan] fseek api interception.

ad2982f

devnexen force-pushed the rtsan_fseek branch from 4bbe488 to ad2982f Compare January 15, 2025 13:33

fmayer approved these changes Jan 15, 2025

View reviewed changes

devnexen merged commit c4443a1 into llvm:main Jan 15, 2025
7 checks passed

kstoimenov reviewed Jan 15, 2025

View reviewed changes

[compiler-rt][rtsan] fseek api interception. #122163

[compiler-rt][rtsan] fseek api interception. #122163

Uh oh!

Conversation

devnexen commented Jan 8, 2025

Uh oh!

llvmbot commented Jan 8, 2025

Uh oh!

github-actions bot commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devnexen Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devnexen Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmayer commented Jan 15, 2025

Uh oh!

devnexen commented Jan 15, 2025

Uh oh!

Uh oh!

kstoimenov Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jan 8, 2025 •

edited

Loading

devnexen Jan 9, 2025 •

edited

Loading

devnexen Jan 13, 2025 •

edited

Loading

kstoimenov Jan 15, 2025 •

edited

Loading