[PATCH] D18164: [tsan] Do not instrument reads/writes to instruction profile counters.

[Vedant Kumar via llvm-commits]

vsk added inline comments.

================
Comment at: lib/ProfileData/InstrProf.cpp:257
@@ +256,3 @@
+      Addr = CE->getOperand(0);
+
+  // Check if global the address is pointing to is in the
----------------
I just realized that this isn't exhaustive. We can end up with loads/stores with non-GEP pointer operands. Consider the following example:

```
% cc -O3 -fprofile-instr-generate -S -emit-llvm -x c - -o -
void foo() {}

int main() {
  for (int I = 0; I < 10; ++I) foo();
}
```

This produces:

```
  %1 = load <2 x i64>, <2 x i64>* bitcast ([2 x i64]* @__profc_main to <2 x i64>*), align 8
  %.promoted3 = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0), align 8
  %2 = add i64 %.promoted3, 10
  %3 = add <2 x i64> %1, <i64 1, i64 10>
  store <2 x i64> %3, <2 x i64>* bitcast ([2 x i64]* @__profc_main to <2 x i64>*), align 8
  store i64 %2, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0), align 8
```

So I think we should add a check like this:

```
if (auto *BCI = dyn_cast<BitCastInst>(Addr))
  Addr = BCI->getOperand(0);
```

================
Comment at: lib/ProfileData/InstrProf.cpp:265
@@ +264,3 @@
+      if (getInstrProfCountersSectionName(isMachO).equals(GV->getSection()))
+        return false;
+    }
----------------
This is fine, but we can avoid constructing the Triple object if you're ok with doing `StringRef(GV->getSection()).endswith(getInstrProfCountersSectionName(/*AddSegment=*/false)`.


http://reviews.llvm.org/D18164