[PATCH] D25321: Fix PR13910: Don't warn that __builtin_unreachable() is unreachable

[Alex L via cfe-commits]

On 6 October 2016 at 15:50, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Oct 6, 2016 at 6:16 AM Alex Lorenz <arphaman at gmail.com> wrote:
>
>> arphaman created this revision.
>> arphaman added reviewers: dblaikie, krememek.
>> arphaman added a subscriber: cfe-commits.
>> arphaman set the repository for this revision to rL LLVM.
>>
>> This patch fixes the issue of clang emitting an unreachable warning when
>> it encounters unreachable `__builtin_unreachable()` statements. It detects
>> references to `__builtin_unreachable` and doesn't emit an unreachable
>> warning for them.
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D25321
>>
>> Files:
>>   lib/Analysis/ReachableCode.cpp
>>   test/Sema/warn-unreachable.c
>>
>>
>> Index: test/Sema/warn-unreachable.c
>> ===================================================================
>> --- test/Sema/warn-unreachable.c
>> +++ test/Sema/warn-unreachable.c
>> @@ -396,3 +396,40 @@
>>    else
>>      calledFun();
>>  }
>> +
>> +int pr13910_foo(int x) {
>> +  if (x == 1)
>> +    return 0;
>> +  else
>> +    return x;
>> +  __builtin_unreachable(); // expected no warning
>> +}
>> +
>> +int pr13910_bar(int x) {
>> +  switch (x) {
>> +  default:
>> +    return x + 1;
>> +  }
>> +  pr13910_foo(x); // expected-warning {{code will never be executed}}
>> +}
>> +
>> +int pr13910_bar2(int x) {
>> +  if (x == 1)
>> +    return 0;
>> +  else
>> +    return x;
>> +  pr13910_foo(x);          // expected-warning {{code will never be
>> executed}}
>> +  __builtin_unreachable(); // expected no warning
>> +  pr13910_foo(x);          // expected-warning {{code will never be
>> executed}}
>>
>
> Seems strange to have two warnings here ^ is that a separate bug?
>

It seems that it might be, yeah. For example, clang emits two unreachable
warnings for the two `foo` calls in this code:

int foo() {
  return 0;
}
int bar(int x) {
  if (x == 0)
    return x + 1;
  else
    return x;
  foo();
  return 0;
  foo();
}

But just one when the `return 0` that's between the two `foo` calls is
removed. It doesn't seem like there's a PR for this issue, I'll file one
later on today.



>
>
>> +}
>> +
>> +void pr13910_noreturn() {
>> +  raze();
>> +  __builtin_unreachable(); // expected no warning
>> +}
>> +
>> +void pr13910_assert() {
>> +  myassert(0 && "unreachable");
>> +  return;
>> +  __builtin_unreachable(); // expected no warning
>> +}
>> Index: lib/Analysis/ReachableCode.cpp
>> ===================================================================
>> --- lib/Analysis/ReachableCode.cpp
>> +++ lib/Analysis/ReachableCode.cpp
>> @@ -58,6 +58,15 @@
>>    return false;
>>  }
>>
>> +static bool isBuiltinUnreachable(const CFGBlock *B, const Stmt *S) {
>> +  if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(S)) {
>> +    const FunctionDecl *FDecl = dyn_cast<FunctionDecl>(DRE->getDecl());
>> +    return FDecl && FDecl->getIdentifier() &&
>> +           FDecl->getBuiltinID() == Builtin::BI__builtin_unreachable;
>> +  }
>> +  return false;
>> +}
>> +
>>  static bool isDeadReturn(const CFGBlock *B, const Stmt *S) {
>>    // Look to see if the current control flow ends with a 'return', and
>> see if
>>    // 'S' is a substatement. The 'return' may not be the last element in
>> the
>> @@ -574,8 +583,7 @@
>>
>>    if (isa<BreakStmt>(S)) {
>>      UK = reachable_code::UK_Break;
>> -  }
>> -  else if (isTrivialDoWhile(B, S)) {
>> +  } else if (isTrivialDoWhile(B, S) || isBuiltinUnreachable(B, S)) {
>>      return;
>>    }
>>    else if (isDeadReturn(B, S)) {
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161006/bc58d036/attachment.html>