tsan: support C++ exceptions

[dvyukov]

Summary

Work-in-progress, does not work yet.
The idea is to emit a cleanup landing pad for every function that will call __tsan_func_exit in case of exceptions flying by.
Problem one: the cleanup BB does not have predecessors. As the result it is not called in the function that throws.
Problem two: I am not sure how this interacts with functions that have [several nested] try/catch blocks -- the cleanup block must be executed only when we leave function, but not for inner try/catch blocks. So the cleanup block must be somehow "attached" to function exit.
I am looking for advice and/or examples of code that does a similar thing.

[dvyukov]

Chandler, please advise or delegate to somebody else.

[chandlerc]

So this sounds really cool, but yea, I'm totally not the right person to review. =D

Added David and Reid as reviewers. They are *much* more familiar with LLVM's EH at this point than anyone else I suspect, especially as it pertains to C++ exceptions, etc. They're also aware of (and/or planning to make) substantial upcoming changes to LLVM's EH and how Clang uses it for C++ EH.

[dvyukov]

David, Reid,

Please provide some guidance here.

[rnk]

I think the GC lowering code does a similar cleanup insertion, so you can
check that out. Here's a sketch of how to add a cleanup, though.

First, find all call insts that might throw. If there are any, create a new
landingpad for them.

For all landingpads, mark them as having cleanups.

Exit early if there were no calls or landingpads.

Create a new basic block with the cleanup code.

Replace all resume insts with a branch to the new block. If you created a
new lpad, have that branch here too.

Create a phi node in the new block to join together the resume values and
new landing pad value.

Terminate the new block with a resume that uses the phi.

There are some special cases where you can simplify the cfg, but that's the
general idea.

Sent from phone

[dvyukov]

Uh, a bit more involved than I hoped.
Do you mean code in lib/CodeGen/ShadowStackGCLowering.cpp:EscapeEnumerator?
How do I determine whether a Call can throw or not? Do I just assume the worst and replace all calls?
Thanks

[rnk]

Think of 'resume' as 'ret' for exception handling. It means "transfer
control to the next landingpad up the stack", similar to how return
transfers to the return address. You could also insert your code before
every 'resume' if you want to avoid dealing branches, phis, etc.

The main complication is the split between calls and invokes. Effectively,
you need a transform that finds all potentially throwing callsites
(identified with !CallSite->doesNotThrow()) and turns them into invokes
with a trivial landingpad that immediately resumes. Then you can insert
code before the resume, or join all resumes together to avoid code
duplication.

Hope that helps.

[dvyukov]

You could also insert your code before every 'resume' if you want to avoid dealing branches, phis, etc.

I don't think it's that simple. A function may not have any landing pads yet (no dtors/catches).

First, find all call insts that might throw. If there are any, create a new landingpad for them.

What if the code is:

void foo() {

try {
  bar();
} catch (...) {
  ...
}

}

I think it will be wrong to add a __tsan_func_exit cleanup to bar (or extend the existing catch landing pad), because control does not leave the function.
Is it correct?

[dvyukov]

OK, what is the code is:

void foo() {

try {
  bar();
} catch (int) {
  ...
}

}

?
Or even:

void foo() {

try {
  try {
    bar();
  } catch (double) {
    ...
  }
} catch (int) {
  ...
}

}

?

The inner catch should have a resume, but control still does not leave the function.

[dvyukov]

In what sense do you mean control does not leave the function?

In the direct sense. Sometimes control leave the current function on exception, and sometimes it is not. I need to execute __tsan_func_exit only for the latter cases.

Think of 'resume' as 'ret' for exception handling. It means "transfer
control to the next landingpad up the stack", similar to how return
transfers to the return address. You could also insert your code before
every 'resume' if you want to avoid dealing branches, phis, etc.

But it is possible that the next landingpad up the stack is within the same function. In such case I don't need to execute __tsan_func_exit. That's what I tried to show with the examples.

lib/Transforms/Instrumentation/ThreadSanitizer.cpp

if ((Res || HasCalls) && ClInstrumentFuncEntryExit) {
     Value *ReturnAddress = IRB.CreateCall(
         Intrinsic::getDeclaration(F.getParent(), Intrinsic::returnaddress),
         IRB.getInt32(0));
     IRB.CreateCall(TsanFuncEntry, ReturnAddress);
     for (auto RetInst : RetVec) {
       IRBuilder<> IRBRet(RetInst);
       IRBRet.CreateCall(TsanFuncExit, {});
     }
+    // Create cleanup landing pad to call __tsan_func_exit on exceptions.
+    LLVMContext &C = F.getContext();
+    BasicBlock *CleanupBB = BasicBlock::Create(C, "", &F);
+    IRBuilder<> IRBCleanup(CleanupBB);
+    Type *ExnTy =
+        StructType::get(Type::getInt8PtrTy(C), Type::getInt32Ty(C), nullptr);
+    Constant *PersFn = F.getParent()->getOrInsertFunction(
+        "__gcc_personality_v0", FunctionType::get(Type::getInt32Ty(C), true));
+    IRBCleanup.CreateLandingPad(ExnTy, PersFn, 0)->setCleanup(true);
+    IRBCleanup.CreateCall(TsanFuncExit, {});
     Res = true;
   }
   return Res;
 }
 
 bool ThreadSanitizer::instrumentLoadOrStore(Instruction *I,
                                             const DataLayout &DL) {
   IRBuilder<> IRB(I);