[Openmp-commits] [PATCH] D87144: Allow all possible argument separators in TSAN_OPTIONS

[Tobias Ribizel via Phabricator via Openmp-commits]

upsj created this revision.
upsj added a reviewer: OpenMP.
upsj added projects: OpenMP, Sanitizers.
Herald added a subscriber: openmp-commits.
upsj requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Currently, the parser used to tokenize the TSAN_OPTIONS in libomp uses
only spaces as separators, even though TSAN in compiler-rt supports
other separators like ':' or ','. This causes TSAN used with OpenMP to
spuriously display the following warning:

Warning: please export TSAN_OPTIONS='ignore_noninstrumented_modules=1'
to avoid false positive reports from the OpenMP runtime!

when TSAN_OPTIONS contains multiple arguments and
ignore_noninstrumented_modules follows a non-space separator.

CTest uses ':' to separate sanitizer options by default, which is
where I encountered this issue first in practice.
The documentation for other sanitizers mentions ':' as separator,
but TSAN only lists spaces, which is probably where this mismatch originated.

----------

I wasn't sure where to best add tests for this bugfix, so I wanted to leave it at this stage until I can get some pointers in the right direction :)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87144

Files:
  openmp/tools/archer/ompt-tsan.cpp


Index: openmp/tools/archer/ompt-tsan.cpp
===================================================================
--- openmp/tools/archer/ompt-tsan.cpp
+++ openmp/tools/archer/ompt-tsan.cpp
@@ -15,6 +15,7 @@
 #define __STDC_FORMAT_MACROS
 #endif

+#include <algorithm>
 #include <atomic>
 #include <cassert>
 #include <cstdlib>
@@ -89,17 +90,23 @@
   TsanFlags(const char *env) : ignore_noninstrumented_modules(0) {
     if (env) {
       std::vector<std::string> tokens;
-      std::string token;
       std::string str(env);
-      std::istringstream iss(str);
-      while (std::getline(iss, token, ' '))
-        tokens.push_back(token);
+      auto end = str.end();
+      auto it = str.begin();
+      auto is_sep = [](char c) {
+        return c == ' ' || c == ',' || c == ':' || c == '\n' || c == '\t' ||
+               c == '\r';
+      };
+      while (it != end) {
+        auto next_it = std::find_if(it, end, is_sep);
+        tokens.emplace_back(it, next_it);
+        it = next_it + 1;
+      }

-      for (std::vector<std::string>::iterator it = tokens.begin();
-           it != tokens.end(); ++it) {
+      for (const auto &token : tokens) {
         // we are interested in ignore_noninstrumented_modules to print a
         // warning
-        if (sscanf(it->c_str(), "ignore_noninstrumented_modules=%d",
+        if (sscanf(token.c_str(), "ignore_noninstrumented_modules=%d",
                    &ignore_noninstrumented_modules))
           continue;
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87144.289947.patch
Type: text/x-patch
Size: 1478 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20200904/0d8b1247/attachment-0001.bin>