[PATCH] D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In https://reviews.llvm.org/D52398#1255148, @hokein wrote:

> Hi, @aaronpuchert, the patch has caused many new warnings in our internal codebase, below is a reduced one. Do you mind reverting the patch?
>
>   // if the condition is not true, CHECK will terminate the program.
>   #define CHECK(condition) (__builtin_expect((condition), true)) ? 1 : 0
>  
>   void foo() {
>       CHECK(mu.TryLock()); 
>       mu.Unlock();
>   }
>


Even before there was another warning in this code: "releasing mutex 'mu' that was not held" on `mu.Unlock()`. Is there an example that didn't show any warnings before?

And can you confirm that the warning also appeared before if you replace `__builtin_expect(x, *)` by `x`? Both are functionally equivalent.

I will have a closer look, but my first feeling is that the problem is //not// that we unwrap __builtin_expect, but instead that we don't consider the ternary operator `?:` in `getTrylockCallExpr`. I'll see if I can fix that.


Repository:
  rL LLVM

https://reviews.llvm.org/D52398