Skip to content

Commit 23419bf

Browse files
committed
[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
1 parent b239165 commit 23419bf

File tree

4 files changed

+64
-10
lines changed

4 files changed

+64
-10
lines changed

openmp/tools/archer/ompt-tsan.cpp

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,18 @@
1515
#define __STDC_FORMAT_MACROS
1616
#endif
1717

18+
#include <algorithm>
1819
#include <atomic>
1920
#include <cassert>
2021
#include <cstdlib>
2122
#include <cstring>
2223
#include <inttypes.h>
2324
#include <iostream>
25+
#include <list>
2426
#include <mutex>
2527
#include <sstream>
2628
#include <stack>
27-
#include <list>
2829
#include <string>
29-
#include <iostream>
3030
#include <unordered_map>
3131
#include <vector>
3232

@@ -89,17 +89,26 @@ class TsanFlags {
8989
TsanFlags(const char *env) : ignore_noninstrumented_modules(0) {
9090
if (env) {
9191
std::vector<std::string> tokens;
92-
std::string token;
9392
std::string str(env);
94-
std::istringstream iss(str);
95-
while (std::getline(iss, token, ' '))
96-
tokens.push_back(token);
93+
auto end = str.end();
94+
auto it = str.begin();
95+
auto is_sep = [](char c) {
96+
return c == ' ' || c == ',' || c == ':' || c == '\n' || c == '\t' ||
97+
c == '\r';
98+
};
99+
while (it != end) {
100+
auto next_it = std::find_if(it, end, is_sep);
101+
tokens.emplace_back(it, next_it);
102+
it = next_it;
103+
if (it != end) {
104+
++it;
105+
}
106+
}
97107

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

openmp/tools/archer/tests/lit.cfg

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ if 'INTEL_LICENSE_FILE' in os.environ:
9393
# Race Tests
9494
config.substitutions.append(("%libarcher-compile-and-run-race", \
9595
"%libarcher-compile && %libarcher-run-race"))
96+
config.substitutions.append(("%libarcher-compile-and-run-nosuppression", \
97+
"%libarcher-compile && %libarcher-run-nosuppression"))
9698
config.substitutions.append(("%libarcher-compile-and-run", \
9799
"%libarcher-compile && %libarcher-run"))
98100
config.substitutions.append(("%libarcher-cxx-compile-and-run", \
@@ -102,13 +104,15 @@ config.substitutions.append(("%libarcher-cxx-compile", \
102104
config.substitutions.append(("%libarcher-compile", \
103105
"%clang-archer %openmp_flags %archer_flags %flags %s -o %t" + libs))
104106
config.substitutions.append(("%libarcher-run-race", "%suppression %deflake %t 2>&1 | tee %t.log"))
107+
config.substitutions.append(("%libarcher-run-nosuppression", "%nosuppression %t 2>&1 | tee %t.log"))
105108
config.substitutions.append(("%libarcher-run", "%suppression %t 2>&1 | tee %t.log"))
106109
config.substitutions.append(("%clang-archerXX", config.test_cxx_compiler))
107110
config.substitutions.append(("%clang-archer", config.test_c_compiler))
108111
config.substitutions.append(("%openmp_flags", config.test_openmp_flags))
109112
config.substitutions.append(("%archer_flags", config.archer_flags))
110113
config.substitutions.append(("%flags", config.test_flags))
111-
config.substitutions.append(("%suppression", "env TSAN_OPTIONS='ignore_noninstrumented_modules=1'"))
114+
config.substitutions.append(("%nosuppression", "env TSAN_OPTIONS='ignore_noninstrumented_modules=0'"))
115+
config.substitutions.append(("%suppression", "env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1'"))
112116
config.substitutions.append(("%deflake", os.path.join(os.path.dirname(__file__), "deflake.bash")))
113117

114118
config.substitutions.append(("FileCheck", config.test_filecheck))
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* parallel-nosuppression.c -- Archer testcase
3+
*/
4+
5+
//===----------------------------------------------------------------------===//
6+
//
7+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
8+
//
9+
// See tools/archer/LICENSE.txt for details.
10+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
11+
//
12+
//===----------------------------------------------------------------------===//
13+
14+
15+
// RUN: %libarcher-compile-and-run-nosuppression | FileCheck %s
16+
// REQUIRES: tsan
17+
#include <omp.h>
18+
#include <stdio.h>
19+
20+
int main(int argc, char *argv[]) {
21+
int var = 0;
22+
23+
#pragma omp parallel num_threads(2) shared(var)
24+
{
25+
if (omp_get_thread_num() == 1) {
26+
var++;
27+
}
28+
} // implicit barrier
29+
30+
var++;
31+
32+
fprintf(stderr, "DONE\n");
33+
int error = (var != 2);
34+
return error;
35+
}
36+
37+
// CHECK-NOT: ThreadSanitizer: data race
38+
// CHECK-NOT: ThreadSanitizer: reported
39+
// CHECK: Warning: please export TSAN_OPTIONS
40+
// CHECK: DONE

openmp/tools/archer/tests/parallel/parallel-simple.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,5 @@ int main(int argc, char *argv[]) {
3636

3737
// CHECK-NOT: ThreadSanitizer: data race
3838
// CHECK-NOT: ThreadSanitizer: reported
39+
// CHECK-NOT: Warning: please export TSAN_OPTIONS
3940
// CHECK: DONE

0 commit comments

Comments
 (0)