[PATCH] New "strict_string_checks" run-time flag

Kostya Serebryany kcc at google.com
Fri Mar 6 18:13:43 PST 2015


Nice!
Once we are done with me I'll ask a second reviewer to check tsan/msan part.


================
Comment at: lib/asan/asan_interceptors.cc:80
@@ +79,3 @@
+  ASAN_READ_RANGE((ctx), (s),                                   \
+    common_flags()->strict_string_checks ? internal_strlen(s) + 1 : (n) )
+
----------------
Interesting.
This code is going to be hot, and internal_strlen(s) is not optimized for speed (and is not expected to be). 
Near some of the places where we now call ASAN_READ_RANGE there are calls to REAL(strlen)(to), so you essentially repeat the call (but using slow internal_strlen). 
Try not to repeat the strlen calls and try to rely on REAL(strlen), which is ~16x faster. 

================
Comment at: lib/sanitizer_common/sanitizer_flags.inc:149
@@ -148,1 +148,3 @@
             "If true, the shadow is not allowed to use huge pages. ")
+COMMON_FLAG(bool, strict_string_checks, true,
+            "If set check that string arguments are properly null-terminated")
----------------
I afraid we should make it false at the first step. 
Once this change is committed (and propagated into our internal base) we'll test with =true to see if it's sane. 

================
Comment at: test/asan/TestCases/atoi_strict.c:2
@@ +1,3 @@
+// Test strict_string_checks option in atoi function
+// RUN: %clang_asan %s -DTEST1 -o %t-1 && not %run %t-1 2>&1 | FileCheck %s --check-prefix=CHECK1
+// RUN: ASAN_OPTIONS=strict_string_checks=false %run %t-1 2>&1
----------------
Good. But if you use a run-time parameter instead of -DTEST_N you will not need to recompile the test multiple times.
Test-time saving and code simplification. 

int main(int argc, char **argv) {
  if (argc != 2) return 1;
  if (!strcmp(argv[1], "foo")) test_foo();
  if (!strcmp(argv[1], "bar")) test_bar();
}

http://reviews.llvm.org/D7123

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list