[compiler-rt] 4c39f34 - [SanitizerCommon] Print the current value of options when printing out help.

Dan Liew via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 14 14:05:09 PST 2019


Author: Dan Liew
Date: 2019-11-14T14:04:34-08:00
New Revision: 4c39f341996cea2fd8619fc14c8c66ab567744fb

URL: https://github.com/llvm/llvm-project/commit/4c39f341996cea2fd8619fc14c8c66ab567744fb
DIFF: https://github.com/llvm/llvm-project/commit/4c39f341996cea2fd8619fc14c8c66ab567744fb.diff

LOG: [SanitizerCommon] Print the current value of options when printing out help.

Summary:
Previously it wasn't obvious what the default value of various sanitizer
options were. A very close approximation of the "default values" for the
options are the current value of the options at the time of printing the
help output.

In the case that no other options are provided then the current values
are the default values (apart from `help`).

```
ASAN_OPTIONS=help=1 ./program
```

This patch causes the current option values to be printed when the
`help` output is enabled. The original intention for this patch was to append
`(Default: <value>)` to an option's help text. However because this
is technically wrong (and misleading) I've opted to append
`(Current Value: <value>)` instead.

When trying to implement a way of displaying the default value of the
options I tried another solution where the default value used in `*.inc` files
were used to create compile time strings that where used when printing
the help output. This solution was not satisfactory for several reasons:

* Stringifying the default values with the preprocessor did not work very
well in several cases.  Some options contain boolean operators which no
amount of macro expansion can get rid of.
* It was much more invasive than this patch. Every sanitizer had to be changed.
* The settings of `__<sanitizer>_default_options()` are ignored.

For those reasons I opted for the solution in this patch.

rdar://problem/42567204

Reviewers: kubamracek, yln, kcc, dvyukov, vitalybuka, cryptoad, eugenis, samsonov

Subscribers: #sanitizers, llvm-commits

Tags: #sanitizers, #llvm

Differential Revision: https://reviews.llvm.org/D69546

Added: 
    

Modified: 
    compiler-rt/lib/msan/msan.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cpp
    compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h
    compiler-rt/lib/sanitizer_common/sanitizer_flags.cpp
    compiler-rt/test/sanitizer_common/TestCases/options-help.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 6ea63cb2c48f..7095ee1bf20f 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -122,6 +122,10 @@ class FlagHandlerKeepGoing : public FlagHandlerBase {
     *halt_on_error_ = !tmp;
     return true;
   }
+  bool Format(char *buffer, uptr size) final {
+    const char *keep_going_str = (*halt_on_error_) ? "false" : "true";
+    return FormatString(buffer, size, keep_going_str);
+  }
 };
 
 static void RegisterMsanFlags(FlagParser *parser, Flags *f) {

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cpp
index 1e2bc6652617..9e274268bf2a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cpp
@@ -56,9 +56,16 @@ char *FlagParser::ll_strndup(const char *s, uptr n) {
 }
 
 void FlagParser::PrintFlagDescriptions() {
+  char buffer[128];
+  buffer[sizeof(buffer) - 1] = '\0';
   Printf("Available flags for %s:\n", SanitizerToolName);
-  for (int i = 0; i < n_flags_; ++i)
-    Printf("\t%s\n\t\t- %s\n", flags_[i].name, flags_[i].desc);
+  for (int i = 0; i < n_flags_; ++i) {
+    bool truncated = !(flags_[i].handler->Format(buffer, sizeof(buffer)));
+    CHECK_EQ(buffer[sizeof(buffer) - 1], '\0');
+    const char *truncation_str = truncated ? " Truncated" : "";
+    Printf("\t%s\n\t\t- %s (Current Value%s: %s)\n", flags_[i].name,
+           flags_[i].desc, truncation_str, buffer);
+  }
 }
 
 void FlagParser::fatal_error(const char *err) {

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h b/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h
index c24ad25626ba..fac5dff34633 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h
@@ -22,9 +22,23 @@ namespace __sanitizer {
 class FlagHandlerBase {
  public:
   virtual bool Parse(const char *value) { return false; }
+  // Write the C string representation of the current value (truncated to fit)
+  // into the buffer of size `size`. Returns false if truncation occurred and
+  // returns true otherwise.
+  virtual bool Format(char *buffer, uptr size) {
+    if (size > 0)
+      buffer[0] = '\0';
+    return false;
+  }
 
  protected:
   ~FlagHandlerBase() {}
+
+  inline bool FormatString(char *buffer, uptr size, const char *str_to_use) {
+    uptr num_symbols_should_write =
+        internal_snprintf(buffer, size, "%s", str_to_use);
+    return num_symbols_should_write < size;
+  }
 };
 
 template <typename T>
@@ -34,6 +48,7 @@ class FlagHandler : public FlagHandlerBase {
  public:
   explicit FlagHandler(T *t) : t_(t) {}
   bool Parse(const char *value) final;
+  bool Format(char *buffer, uptr size) final;
 };
 
 inline bool ParseBool(const char *value, bool *b) {
@@ -59,6 +74,11 @@ inline bool FlagHandler<bool>::Parse(const char *value) {
   return false;
 }
 
+template <>
+inline bool FlagHandler<bool>::Format(char *buffer, uptr size) {
+  return FormatString(buffer, size, *t_ ? "true" : "false");
+}
+
 template <>
 inline bool FlagHandler<HandleSignalMode>::Parse(const char *value) {
   bool b;
@@ -75,12 +95,23 @@ inline bool FlagHandler<HandleSignalMode>::Parse(const char *value) {
   return false;
 }
 
+template <>
+inline bool FlagHandler<HandleSignalMode>::Format(char *buffer, uptr size) {
+  uptr num_symbols_should_write = internal_snprintf(buffer, size, "%d", *t_);
+  return num_symbols_should_write < size;
+}
+
 template <>
 inline bool FlagHandler<const char *>::Parse(const char *value) {
   *t_ = value;
   return true;
 }
 
+template <>
+inline bool FlagHandler<const char *>::Format(char *buffer, uptr size) {
+  return FormatString(buffer, size, *t_);
+}
+
 template <>
 inline bool FlagHandler<int>::Parse(const char *value) {
   const char *value_end;
@@ -90,6 +121,12 @@ inline bool FlagHandler<int>::Parse(const char *value) {
   return ok;
 }
 
+template <>
+inline bool FlagHandler<int>::Format(char *buffer, uptr size) {
+  uptr num_symbols_should_write = internal_snprintf(buffer, size, "%d", *t_);
+  return num_symbols_should_write < size;
+}
+
 template <>
 inline bool FlagHandler<uptr>::Parse(const char *value) {
   const char *value_end;
@@ -99,6 +136,12 @@ inline bool FlagHandler<uptr>::Parse(const char *value) {
   return ok;
 }
 
+template <>
+inline bool FlagHandler<uptr>::Format(char *buffer, uptr size) {
+  uptr num_symbols_should_write = internal_snprintf(buffer, size, "%p", *t_);
+  return num_symbols_should_write < size;
+}
+
 template <>
 inline bool FlagHandler<s64>::Parse(const char *value) {
   const char *value_end;
@@ -108,6 +151,12 @@ inline bool FlagHandler<s64>::Parse(const char *value) {
   return ok;
 }
 
+template <>
+inline bool FlagHandler<s64>::Format(char *buffer, uptr size) {
+  uptr num_symbols_should_write = internal_snprintf(buffer, size, "%lld", *t_);
+  return num_symbols_should_write < size;
+}
+
 class FlagParser {
   static const int kMaxFlags = 200;
   struct Flag {

diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_flags.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_flags.cpp
index 66a0a5579ed3..684ee1e0b999 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_flags.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_flags.cpp
@@ -75,11 +75,13 @@ void SubstituteForFlagValue(const char *s, char *out, uptr out_size) {
 class FlagHandlerInclude : public FlagHandlerBase {
   FlagParser *parser_;
   bool ignore_missing_;
+  const char *original_path_;
 
  public:
   explicit FlagHandlerInclude(FlagParser *parser, bool ignore_missing)
-      : parser_(parser), ignore_missing_(ignore_missing) {}
+      : parser_(parser), ignore_missing_(ignore_missing), original_path_("") {}
   bool Parse(const char *value) final {
+    original_path_ = value;
     if (internal_strchr(value, '%')) {
       char *buf = (char *)MmapOrDie(kMaxPathLength, "FlagHandlerInclude");
       SubstituteForFlagValue(value, buf, kMaxPathLength);
@@ -89,6 +91,12 @@ class FlagHandlerInclude : public FlagHandlerBase {
     }
     return parser_->ParseFile(value, ignore_missing_);
   }
+  bool Format(char *buffer, uptr size) {
+    // Note `original_path_` isn't actually what's parsed due to `%`
+    // substitutions. Printing the substituted path would require holding onto
+    // mmap'ed memory.
+    return FormatString(buffer, size, original_path_);
+  }
 };
 
 void RegisterIncludeFlags(FlagParser *parser, CommonFlags *cf) {

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/options-help.cpp b/compiler-rt/test/sanitizer_common/TestCases/options-help.cpp
index 913377db70f1..ca1cf25e7085 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/options-help.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/options-help.cpp
@@ -1,8 +1,43 @@
 // RUN: %clangxx -O0 %s -o %t
-// RUN: %env_tool_opts=help=1 %run %t 2>&1 | FileCheck %s
+// RUN: %env_tool_opts=help=1,include_if_exists=___some_path_that_does_not_exist___  %run %t 2>&1 | FileCheck %s
+// RUN: %env_tool_opts=help=1,symbolize=0 %run %t 2>&1 | FileCheck --check-prefix=CHECK-CV %s
+// RUN: %env_tool_opts=help=1,sancov_path=/long/path/that/requires/truncation/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaB \
+// RUN:   %run %t 2>&1 | FileCheck --check-prefix=CHECK-TRUNCATION %s
 
 int main() {
 }
 
 // CHECK: Available flags for {{.*}}Sanitizer:
-// CHECK: handle_segv
+
+//
+// Bool option
+// CHECK: {{^[ \t]+symbolize$}}
+// CHECK-NEXT: (Current Value: true)
+//
+// String option
+// CHECK: {{^[ \t]+log_path$}}
+// CHECK-NEXT: (Current Value: {{.+}})
+//
+// int option
+// CHECK: {{^[ \t]+verbosity$}}
+// CHECK-NEXT: (Current Value: {{-?[0-9]+}})
+//
+// HandleSignalMode option
+// CHECK: {{^[ \t]+handle_segv$}}
+// CHECK-NEXT: (Current Value: {{0|1|2}})
+//
+// uptr option
+// CHECK: {{^[ \t]+mmap_limit_mb$}}
+// CHECK-NEXT: (Current Value: 0x{{[0-9a-fA-F]+}})
+//
+// FlagHandlerInclude option
+// CHECK: include_if_exists
+// CHECK-NEXT: (Current Value: ___some_path_that_does_not_exist___)
+
+// Test we show the current value and not the default.
+// CHECK-CV: {{^[ \t]+symbolize$}}
+// CHECK-CV-NEXT: (Current Value: false)
+
+// Test truncation of long paths.
+// CHECK-TRUNCATION: sancov_path
+// CHECK-TRUNCATION-NEXT: (Current Value Truncated: /long/path/that/requires/truncation/aaa{{a+}})


        


More information about the llvm-commits mailing list