[PATCH] D48052: [sanitizer] Don't treat colon before slash as a flag separator

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 11 13:52:40 PDT 2018


rnk created this revision.
rnk added reviewers: vitalybuka, dvyukov.
Herald added a subscriber: kubamracek.

Users often have path escaping bugs in their ASan options. They do
things like this
ASAN_OPTIONS=foo=1:bar=0:symbolizer_path=%s
... where %s is an absolute path. They may have project rules that
require paths to not contain spaces, so they get away with this on
Linux. Then they come to Windows, and the substituted options look like:
symbolizer_path=C:/path/to/llvm-symbolizer.exe
This doesn't work because we have chosen colon to be an option
separator.

Many users have hit this, it affects the Go race detector, and it
affects Chromium Windows ASan usage. Try to make their life a little
easier by not treating colon before slash as a flag separator.

Fixes https://github.com/google/sanitizers/issues/884


https://reviews.llvm.org/D48052

Files:
  compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cc
  compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h
  compiler-rt/lib/sanitizer_common/tests/sanitizer_flags_test.cc


Index: compiler-rt/lib/sanitizer_common/tests/sanitizer_flags_test.cc
===================================================================
--- compiler-rt/lib/sanitizer_common/tests/sanitizer_flags_test.cc
+++ compiler-rt/lib/sanitizer_common/tests/sanitizer_flags_test.cc
@@ -177,4 +177,17 @@
   EXPECT_STREQ("path/two", cf.log_path);
 }
 
+TEST(SanitizerCommon, WindowsPath) {
+  FlagParser parser;
+  const char *p1 = "";
+  const char *p2 = "";
+  RegisterFlag(&parser, "path1", kFlagDesc, &p1);
+  RegisterFlag(&parser, "path2", kFlagDesc, &p2);
+  parser.ParseString("path1=C:/win/path.exe:path2=C:\\win\\path.exe");
+  ReportUnrecognizedFlags();
+
+  EXPECT_STREQ(p1, "C:/win/path.exe");
+  EXPECT_STREQ(p2, "C:\\win\\path.exe");
+}
+
 }  // namespace __sanitizer
Index: compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h
===================================================================
--- compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h
+++ compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.h
@@ -121,7 +121,7 @@
 
  private:
   void fatal_error(const char *err);
-  bool is_space(char c);
+  bool is_space();
   void skip_whitespace();
   void parse_flags();
   void parse_flag();
Index: compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cc
===================================================================
--- compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cc
+++ compiler-rt/lib/sanitizer_common/sanitizer_flag_parser.cc
@@ -67,18 +67,27 @@
   Die();
 }
 
-bool FlagParser::is_space(char c) {
-  return c == ' ' || c == ',' || c == ':' || c == '\n' || c == '\t' ||
-         c == '\r';
+bool FlagParser::is_space() {
+  char c = buf_[pos_];
+  if (c == ' ' || c == ',' || c == '\n' || c == '\t' || c == '\r')
+    return true;
+
+  // Treat ':' as an option separator unless it is immediately followed by a
+  // path separator. This avoids common portability issues when an unquoted
+  // Windows path is used, as in 'symbolizer_path=C:/foo/llvm-symbolizer.exe'.
+  // Since no option names should start with / or \, it should be unambiguous.
+  if (c == ':')
+    return buf_[pos_ + 1] != '\\' && buf_[pos_ + 1] != '/';
+  return false;
 }
 
 void FlagParser::skip_whitespace() {
-  while (is_space(buf_[pos_])) ++pos_;
+  while (is_space()) ++pos_;
 }
 
 void FlagParser::parse_flag() {
   uptr name_start = pos_;
-  while (buf_[pos_] != 0 && buf_[pos_] != '=' && !is_space(buf_[pos_])) ++pos_;
+  while (buf_[pos_] != 0 && buf_[pos_] != '=' && !is_space()) ++pos_;
   if (buf_[pos_] != '=') fatal_error("expected '='");
   char *name = ll_strndup(buf_ + name_start, pos_ - name_start);
 
@@ -91,8 +100,8 @@
     value = ll_strndup(buf_ + value_start + 1, pos_ - value_start - 1);
     ++pos_; // consume the closing quote
   } else {
-    while (buf_[pos_] != 0 && !is_space(buf_[pos_])) ++pos_;
-    if (buf_[pos_] != 0 && !is_space(buf_[pos_]))
+    while (buf_[pos_] != 0 && !is_space()) ++pos_;
+    if (buf_[pos_] != 0 && !is_space())
       fatal_error("expected separator or eol");
     value = ll_strndup(buf_ + value_start, pos_ - value_start);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48052.150836.patch
Type: text/x-patch
Size: 3112 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180611/24d3b355/attachment.bin>


More information about the llvm-commits mailing list