[compiler-rt] [win/asan] Ensure errno gets set correctly for strtol (PR #109258)

via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 19 04:01:47 PDT 2024


https://github.com/zmodem updated https://github.com/llvm/llvm-project/pull/109258

>From cd842922ef4f7a11c9dd46e196ac8b7215d7c5fe Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Wed, 18 Sep 2024 15:54:54 +0200
Subject: [PATCH 01/10] [win/asan] Set errno in the strtol interceptor (#34485)

---
 compiler-rt/lib/asan/asan_interceptors.cpp    | 20 +++++++++++++++-
 compiler-rt/lib/asan/tests/asan_str_test.cpp  | 23 +++++++++++++++++++
 .../lib/sanitizer_common/sanitizer_errno.cpp  |  1 +
 .../sanitizer_common/sanitizer_errno_codes.h  |  1 +
 4 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp
index 74af2e65e9bfa6..ad8d28747186bc 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors.cpp
@@ -650,8 +650,26 @@ static ALWAYS_INLINE auto StrtolImpl(void *ctx, Fn real, const char *nptr,
       return StrtolImpl(ctx, REAL(func), nptr, endptr, base);                \
     }
 
-INTERCEPTOR_STRTO_BASE(long, strtol)
 INTERCEPTOR_STRTO_BASE(long long, strtoll)
+#if SANITIZER_WINDOWS
+INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
+  // REAL(strol) may be ntdll!strol, which doesn't set errno. Instead,
+  // re-use the strtoll interceptor and do the range check ourselves.
+  COMPILER_CHECK(sizeof(long) == sizeof(i32));
+  long long result = strtoll(nptr, endptr, base);
+  if (result > INT32_MAX) {
+    errno = errno_ERANGE;
+    return INT32_MAX;
+  }
+  if (result < INT32_MIN) {
+    errno = errno_ERANGE;
+    return INT32_MIN;
+  }
+  return (long)result;
+}
+#else
+INTERCEPTOR_STRTO_BASE(long, strtol)
+#endif
 
 #  if SANITIZER_GLIBC
 INTERCEPTOR_STRTO_BASE(long, __isoc23_strtol)
diff --git a/compiler-rt/lib/asan/tests/asan_str_test.cpp b/compiler-rt/lib/asan/tests/asan_str_test.cpp
index 8b1c1dc459d74f..ae5c55ab571cc6 100644
--- a/compiler-rt/lib/asan/tests/asan_str_test.cpp
+++ b/compiler-rt/lib/asan/tests/asan_str_test.cpp
@@ -638,3 +638,26 @@ TEST(AddressSanitizer, StrtolOOBTest) {
   RunStrtolOOBTest(&CallStrtol);
 }
 #endif
+
+#if defined(_WIN32)
+TEST(AddressSanitizer, StrtolOverflow) {
+  long res;
+  errno = 0;
+  res = Ident(strtol("2147483647", NULL, 0));
+  EXPECT_EQ(errno, 0);
+  EXPECT_EQ(res, 2147483647);
+
+  res = Ident(strtol("2147483648", NULL, 0));
+  EXPECT_EQ(errno, ERANGE);
+  EXPECT_EQ(res, 2147483647);
+
+  errno = 0;
+  res = Ident(strtol("-2147483648", NULL, 0));
+  EXPECT_EQ(errno, 0);
+  EXPECT_EQ(res, -2147483648);
+
+  res = Ident(strtol("-2147483649", NULL, 0));
+  EXPECT_EQ(errno, ERANGE);
+  EXPECT_EQ(res, -2147483648);
+}
+#endif
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_errno.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_errno.cpp
index cbadf4d924a7e0..a7cdf3256757c7 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_errno.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_errno.cpp
@@ -23,6 +23,7 @@ namespace __sanitizer {
 COMPILER_CHECK(errno_ENOMEM == ENOMEM);
 COMPILER_CHECK(errno_EBUSY == EBUSY);
 COMPILER_CHECK(errno_EINVAL == EINVAL);
+COMPILER_CHECK(errno_ERANGE == ERANGE);
 
 // EOWNERDEAD is not present in some older platforms.
 #if defined(EOWNERDEAD)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_errno_codes.h b/compiler-rt/lib/sanitizer_common/sanitizer_errno_codes.h
index 3917b2817f2e99..9e6e71ec80c15c 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_errno_codes.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_errno_codes.h
@@ -24,6 +24,7 @@ namespace __sanitizer {
 #define errno_ENOMEM 12
 #define errno_EBUSY 16
 #define errno_EINVAL 22
+#define errno_ERANGE 34
 #define errno_ENAMETOOLONG 36
 #define errno_ENOSYS 38
 

>From 0df92864d7c5bf3600baedb5c2cba3eb5f291795 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Wed, 18 Sep 2024 16:16:27 +0200
Subject: [PATCH 02/10] build fix

---
 compiler-rt/lib/asan/asan_interceptors.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp
index ad8d28747186bc..e55c007ad153a0 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors.cpp
@@ -655,7 +655,7 @@ INTERCEPTOR_STRTO_BASE(long long, strtoll)
 INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
   // REAL(strol) may be ntdll!strol, which doesn't set errno. Instead,
   // re-use the strtoll interceptor and do the range check ourselves.
-  COMPILER_CHECK(sizeof(long) == sizeof(i32));
+  COMPILER_CHECK(sizeof(long) == sizeof(u32));
   long long result = strtoll(nptr, endptr, base);
   if (result > INT32_MAX) {
     errno = errno_ERANGE;

>From 0dbdc9501d9c08325b3959f64350b15b5ad6d2bc Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Wed, 18 Sep 2024 16:30:30 +0200
Subject: [PATCH 03/10] actually do the interception stuff

---
 compiler-rt/lib/asan/asan_interceptors.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp
index e55c007ad153a0..a035e5fd79bf40 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors.cpp
@@ -656,7 +656,12 @@ INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
   // REAL(strol) may be ntdll!strol, which doesn't set errno. Instead,
   // re-use the strtoll interceptor and do the range check ourselves.
   COMPILER_CHECK(sizeof(long) == sizeof(u32));
-  long long result = strtoll(nptr, endptr, base);
+
+  void *ctx;
+  ASAN_INTERCEPTOR_ENTER(ctx, strtol);
+  AsanInitFromRtl();
+  long long result = StrtolImpl(ctx, REAL(strtoll), nptr, endptr, base);
+
   if (result > INT32_MAX) {
     errno = errno_ERANGE;
     return INT32_MAX;

>From fb9f413e234472a446690fb6ea4dc000740edd37 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Wed, 18 Sep 2024 17:26:33 +0200
Subject: [PATCH 04/10] hacks

---
 compiler-rt/lib/asan/asan_interceptors.cpp   | 6 +++++-
 compiler-rt/lib/asan/tests/asan_str_test.cpp | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp
index a035e5fd79bf40..3d00887eb3d63a 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors.cpp
@@ -663,7 +663,11 @@ INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
   long long result = StrtolImpl(ctx, REAL(strtoll), nptr, endptr, base);
 
   if (result > INT32_MAX) {
-    errno = errno_ERANGE;
+    // XXX Use strtoll to set errno.
+    (void)REAL(strtoll)("9999999999999999999999999", nullptr, 0);
+    //errno = errno_ERANGE;
+    Report("errno: %d %p\n", errno, &errno); // XXX hack hack
+    CHECK(errno == 34);
     return INT32_MAX;
   }
   if (result < INT32_MIN) {
diff --git a/compiler-rt/lib/asan/tests/asan_str_test.cpp b/compiler-rt/lib/asan/tests/asan_str_test.cpp
index ae5c55ab571cc6..fcbfded8479589 100644
--- a/compiler-rt/lib/asan/tests/asan_str_test.cpp
+++ b/compiler-rt/lib/asan/tests/asan_str_test.cpp
@@ -641,6 +641,8 @@ TEST(AddressSanitizer, StrtolOOBTest) {
 
 #if defined(_WIN32)
 TEST(AddressSanitizer, StrtolOverflow) {
+  EXPECT_EQ(&errno, nullptr); // XXX: what is our errno?
+
   long res;
   errno = 0;
   res = Ident(strtol("2147483647", NULL, 0));

>From 39b1a6f9238172ce4d3221a3d4fbf9a2b36dad2b Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 19 Sep 2024 09:50:37 +0200
Subject: [PATCH 05/10] tidy up the interceptor

---
 compiler-rt/lib/asan/asan_interceptors.cpp | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp
index 3d00887eb3d63a..afde42e677a62d 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors.cpp
@@ -651,23 +651,21 @@ static ALWAYS_INLINE auto StrtolImpl(void *ctx, Fn real, const char *nptr,
     }
 
 INTERCEPTOR_STRTO_BASE(long long, strtoll)
+
 #if SANITIZER_WINDOWS
 INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
   // REAL(strol) may be ntdll!strol, which doesn't set errno. Instead,
-  // re-use the strtoll interceptor and do the range check ourselves.
+  // call REAL(strtoll) and do the range check ourselves.
   COMPILER_CHECK(sizeof(long) == sizeof(u32));
 
   void *ctx;
   ASAN_INTERCEPTOR_ENTER(ctx, strtol);
   AsanInitFromRtl();
+
   long long result = StrtolImpl(ctx, REAL(strtoll), nptr, endptr, base);
 
   if (result > INT32_MAX) {
-    // XXX Use strtoll to set errno.
-    (void)REAL(strtoll)("9999999999999999999999999", nullptr, 0);
-    //errno = errno_ERANGE;
-    Report("errno: %d %p\n", errno, &errno); // XXX hack hack
-    CHECK(errno == 34);
+    errno = errno_ERANGE;
     return INT32_MAX;
   }
   if (result < INT32_MIN) {

>From d10f68bd7550605d95aa05aa35061810c6a4f5b1 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 19 Sep 2024 10:03:51 +0200
Subject: [PATCH 06/10] don't intercept strtol in the static thunk

---
 compiler-rt/lib/asan/asan_win_static_runtime_thunk.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/compiler-rt/lib/asan/asan_win_static_runtime_thunk.cpp b/compiler-rt/lib/asan/asan_win_static_runtime_thunk.cpp
index dec50a5e1d4d9e..82aaadb9db1038 100644
--- a/compiler-rt/lib/asan/asan_win_static_runtime_thunk.cpp
+++ b/compiler-rt/lib/asan/asan_win_static_runtime_thunk.cpp
@@ -63,10 +63,13 @@ INTERCEPT_LIBRARY_FUNCTION_ASAN(strpbrk);
 INTERCEPT_LIBRARY_FUNCTION_ASAN(strspn);
 INTERCEPT_LIBRARY_FUNCTION_ASAN(strstr);
 INTERCEPT_LIBRARY_FUNCTION_ASAN(strtok);
-INTERCEPT_LIBRARY_FUNCTION_ASAN(strtol);
 INTERCEPT_LIBRARY_FUNCTION_ASAN(wcslen);
 INTERCEPT_LIBRARY_FUNCTION_ASAN(wcsnlen);
 
+// Note: Don't intercept strtol(l). They are supposed to set errno for out-of-
+// range values, but since the ASan runtime is linked against the dynamic CRT,
+// its errno is different from the one in the current module.
+
 #  if defined(_MSC_VER) && !defined(__clang__)
 #    pragma warning(pop)
 #  endif

>From 7b997f234346a8866ed8fe94b7ddab095f9e1236 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 19 Sep 2024 10:15:31 +0200
Subject: [PATCH 07/10] make tests pass (unsupport strtol_strict.c with the
 static runtime)

---
 compiler-rt/lib/asan/tests/asan_str_test.cpp    | 2 --
 compiler-rt/test/asan/TestCases/strtol_strict.c | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/lib/asan/tests/asan_str_test.cpp b/compiler-rt/lib/asan/tests/asan_str_test.cpp
index fcbfded8479589..ae5c55ab571cc6 100644
--- a/compiler-rt/lib/asan/tests/asan_str_test.cpp
+++ b/compiler-rt/lib/asan/tests/asan_str_test.cpp
@@ -641,8 +641,6 @@ TEST(AddressSanitizer, StrtolOOBTest) {
 
 #if defined(_WIN32)
 TEST(AddressSanitizer, StrtolOverflow) {
-  EXPECT_EQ(&errno, nullptr); // XXX: what is our errno?
-
   long res;
   errno = 0;
   res = Ident(strtol("2147483647", NULL, 0));
diff --git a/compiler-rt/test/asan/TestCases/strtol_strict.c b/compiler-rt/test/asan/TestCases/strtol_strict.c
index bc30c87b18cb30..aa37af07b63dfc 100644
--- a/compiler-rt/test/asan/TestCases/strtol_strict.c
+++ b/compiler-rt/test/asan/TestCases/strtol_strict.c
@@ -23,6 +23,9 @@
 // RUN: %env_asan_opts=strict_string_checks=true not %run %t test7 2>&1 | FileCheck %s --check-prefix=CHECK7
 // REQUIRES: shadow-scale-3
 
+// On Windows, strtol cannot be intercepted when statically linked against the CRT.
+// UNSUPPORTED: win32-static-asan
+
 #include <assert.h>
 #include <stdlib.h>
 #include <string.h>

>From 493d31d17df36ed1a82b51caa6a386312cebdfcd Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 19 Sep 2024 10:30:34 +0200
Subject: [PATCH 08/10] run the test on all platforms, add comment

---
 compiler-rt/lib/asan/tests/asan_str_test.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/compiler-rt/lib/asan/tests/asan_str_test.cpp b/compiler-rt/lib/asan/tests/asan_str_test.cpp
index ae5c55ab571cc6..8a3314f2297bf6 100644
--- a/compiler-rt/lib/asan/tests/asan_str_test.cpp
+++ b/compiler-rt/lib/asan/tests/asan_str_test.cpp
@@ -639,7 +639,7 @@ TEST(AddressSanitizer, StrtolOOBTest) {
 }
 #endif
 
-#if defined(_WIN32)
+// Check that errno gets set correctly on overflow.
 TEST(AddressSanitizer, StrtolOverflow) {
   long res;
   errno = 0;
@@ -660,4 +660,3 @@ TEST(AddressSanitizer, StrtolOverflow) {
   EXPECT_EQ(errno, ERANGE);
   EXPECT_EQ(res, -2147483648);
 }
-#endif

>From 2a8678496ee5029b2d687e069f6b9c0892c794a1 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 19 Sep 2024 10:51:44 +0200
Subject: [PATCH 09/10] the test is for 32-bit

---
 compiler-rt/lib/asan/tests/asan_str_test.cpp | 40 ++++++++++----------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/compiler-rt/lib/asan/tests/asan_str_test.cpp b/compiler-rt/lib/asan/tests/asan_str_test.cpp
index 8a3314f2297bf6..b28143db860f5e 100644
--- a/compiler-rt/lib/asan/tests/asan_str_test.cpp
+++ b/compiler-rt/lib/asan/tests/asan_str_test.cpp
@@ -639,24 +639,26 @@ TEST(AddressSanitizer, StrtolOOBTest) {
 }
 #endif
 
-// Check that errno gets set correctly on overflow.
 TEST(AddressSanitizer, StrtolOverflow) {
-  long res;
-  errno = 0;
-  res = Ident(strtol("2147483647", NULL, 0));
-  EXPECT_EQ(errno, 0);
-  EXPECT_EQ(res, 2147483647);
-
-  res = Ident(strtol("2147483648", NULL, 0));
-  EXPECT_EQ(errno, ERANGE);
-  EXPECT_EQ(res, 2147483647);
-
-  errno = 0;
-  res = Ident(strtol("-2147483648", NULL, 0));
-  EXPECT_EQ(errno, 0);
-  EXPECT_EQ(res, -2147483648);
-
-  res = Ident(strtol("-2147483649", NULL, 0));
-  EXPECT_EQ(errno, ERANGE);
-  EXPECT_EQ(res, -2147483648);
+  if (sizeof(long) == 4) {
+    // Check that errno gets set correctly on 32-bit strtol overflow.
+    long res;
+    errno = 0;
+    res = Ident(strtol("2147483647", NULL, 0));
+    EXPECT_EQ(errno, 0);
+    EXPECT_EQ(res, 2147483647);
+
+    res = Ident(strtol("2147483648", NULL, 0));
+    EXPECT_EQ(errno, ERANGE);
+    EXPECT_EQ(res, 2147483647);
+
+    errno = 0;
+    res = Ident(strtol("-2147483648", NULL, 0));
+    EXPECT_EQ(errno, 0);
+    EXPECT_EQ(res, -2147483648);
+
+    res = Ident(strtol("-2147483649", NULL, 0));
+    EXPECT_EQ(errno, ERANGE);
+    EXPECT_EQ(res, -2147483648);
+  }
 }

>From 70dbdbe4e30345a759d30b6c36400a85f0e70cd7 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 19 Sep 2024 13:01:22 +0200
Subject: [PATCH 10/10] format

---
 compiler-rt/lib/asan/asan_interceptors.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/compiler-rt/lib/asan/asan_interceptors.cpp b/compiler-rt/lib/asan/asan_interceptors.cpp
index afde42e677a62d..9531f96c84a833 100644
--- a/compiler-rt/lib/asan/asan_interceptors.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors.cpp
@@ -652,7 +652,7 @@ static ALWAYS_INLINE auto StrtolImpl(void *ctx, Fn real, const char *nptr,
 
 INTERCEPTOR_STRTO_BASE(long long, strtoll)
 
-#if SANITIZER_WINDOWS
+#  if SANITIZER_WINDOWS
 INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
   // REAL(strol) may be ntdll!strol, which doesn't set errno. Instead,
   // call REAL(strtoll) and do the range check ourselves.
@@ -674,9 +674,9 @@ INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
   }
   return (long)result;
 }
-#else
+#  else
 INTERCEPTOR_STRTO_BASE(long, strtol)
-#endif
+#  endif
 
 #  if SANITIZER_GLIBC
 INTERCEPTOR_STRTO_BASE(long, __isoc23_strtol)



More information about the llvm-commits mailing list