From 23419bfd1c8f26617bda47e6d4732dcbfe0c09a3 Mon Sep 17 00:00:00 2001 From: Joachim Protze Date: Thu, 1 Oct 2020 01:01:09 +0200 Subject: [PATCH] [OpenMP][libarcher] Allow all possible argument separators in TSAN_OPTIONS 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 ','. CTest uses ':' to separate sanitizer options by default. The documentation for other sanitizers mentions ':' as separator, but TSAN only lists spaces, which is probably where this mismatch originated. Patch provided by upsj Differential Revision: https://reviews.llvm.org/D87144 --- openmp/tools/archer/ompt-tsan.cpp | 27 ++++++++----- openmp/tools/archer/tests/lit.cfg | 6 ++- .../tests/parallel/parallel-nosuppression.c | 40 +++++++++++++++++++ .../archer/tests/parallel/parallel-simple.c | 1 + 4 files changed, 64 insertions(+), 10 deletions(-) create mode 100644 openmp/tools/archer/tests/parallel/parallel-nosuppression.c diff --git a/openmp/tools/archer/ompt-tsan.cpp b/openmp/tools/archer/ompt-tsan.cpp index d83cf04638d1..a288a2296a5e 100644 --- a/openmp/tools/archer/ompt-tsan.cpp +++ b/openmp/tools/archer/ompt-tsan.cpp @@ -15,18 +15,18 @@ #define __STDC_FORMAT_MACROS #endif +#include #include #include #include #include #include #include +#include #include #include #include -#include #include -#include #include #include @@ -89,17 +89,26 @@ public: TsanFlags(const char *env) : ignore_noninstrumented_modules(0) { if (env) { std::vector 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; + if (it != end) { + ++it; + } + } - for (std::vector::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; } diff --git a/openmp/tools/archer/tests/lit.cfg b/openmp/tools/archer/tests/lit.cfg index ed4ec4d03b69..f064127817d6 100644 --- a/openmp/tools/archer/tests/lit.cfg +++ b/openmp/tools/archer/tests/lit.cfg @@ -93,6 +93,8 @@ if 'INTEL_LICENSE_FILE' in os.environ: # Race Tests config.substitutions.append(("%libarcher-compile-and-run-race", \ "%libarcher-compile && %libarcher-run-race")) +config.substitutions.append(("%libarcher-compile-and-run-nosuppression", \ + "%libarcher-compile && %libarcher-run-nosuppression")) config.substitutions.append(("%libarcher-compile-and-run", \ "%libarcher-compile && %libarcher-run")) config.substitutions.append(("%libarcher-cxx-compile-and-run", \ @@ -102,13 +104,15 @@ config.substitutions.append(("%libarcher-cxx-compile", \ config.substitutions.append(("%libarcher-compile", \ "%clang-archer %openmp_flags %archer_flags %flags %s -o %t" + libs)) config.substitutions.append(("%libarcher-run-race", "%suppression %deflake %t 2>&1 | tee %t.log")) +config.substitutions.append(("%libarcher-run-nosuppression", "%nosuppression %t 2>&1 | tee %t.log")) config.substitutions.append(("%libarcher-run", "%suppression %t 2>&1 | tee %t.log")) config.substitutions.append(("%clang-archerXX", config.test_cxx_compiler)) config.substitutions.append(("%clang-archer", config.test_c_compiler)) config.substitutions.append(("%openmp_flags", config.test_openmp_flags)) config.substitutions.append(("%archer_flags", config.archer_flags)) config.substitutions.append(("%flags", config.test_flags)) -config.substitutions.append(("%suppression", "env TSAN_OPTIONS='ignore_noninstrumented_modules=1'")) +config.substitutions.append(("%nosuppression", "env TSAN_OPTIONS='ignore_noninstrumented_modules=0'")) +config.substitutions.append(("%suppression", "env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1'")) config.substitutions.append(("%deflake", os.path.join(os.path.dirname(__file__), "deflake.bash"))) config.substitutions.append(("FileCheck", config.test_filecheck)) diff --git a/openmp/tools/archer/tests/parallel/parallel-nosuppression.c b/openmp/tools/archer/tests/parallel/parallel-nosuppression.c new file mode 100644 index 000000000000..f0e1cd8b5e46 --- /dev/null +++ b/openmp/tools/archer/tests/parallel/parallel-nosuppression.c @@ -0,0 +1,40 @@ +/* + * parallel-nosuppression.c -- Archer testcase + */ + +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// +// See tools/archer/LICENSE.txt for details. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + + +// RUN: %libarcher-compile-and-run-nosuppression | FileCheck %s +// REQUIRES: tsan +#include +#include + +int main(int argc, char *argv[]) { + int var = 0; + +#pragma omp parallel num_threads(2) shared(var) + { + if (omp_get_thread_num() == 1) { + var++; + } + } // implicit barrier + + var++; + + fprintf(stderr, "DONE\n"); + int error = (var != 2); + return error; +} + +// CHECK-NOT: ThreadSanitizer: data race +// CHECK-NOT: ThreadSanitizer: reported +// CHECK: Warning: please export TSAN_OPTIONS +// CHECK: DONE diff --git a/openmp/tools/archer/tests/parallel/parallel-simple.c b/openmp/tools/archer/tests/parallel/parallel-simple.c index 86f0b5342d8a..5c70ba601b50 100644 --- a/openmp/tools/archer/tests/parallel/parallel-simple.c +++ b/openmp/tools/archer/tests/parallel/parallel-simple.c @@ -36,4 +36,5 @@ int main(int argc, char *argv[]) { // CHECK-NOT: ThreadSanitizer: data race // CHECK-NOT: ThreadSanitizer: reported +// CHECK-NOT: Warning: please export TSAN_OPTIONS // CHECK: DONE