[cfe-commits] r165273 - in /cfe/trunk: lib/CodeGen/CodeGenFunction.cpp test/CodeGen/catch-undef-behavior.c test/CodeGenCXX/catch-undef-behavior.cpp test/CodeGenCXX/return.cpp

Daniel Dunbar daniel at zuster.org
Tue Jan 22 16:15:56 PST 2013


Hi Richard,

I object pretty strongly to using unreachable here for C++ code bases.

The problem is that this can (and has, and will) subtly break code in ways
users might not notice. While I agree we are within our rights to do it,
unlike other situations where we make changes that can break users' code
(like strict aliasing, where it opens up many different optimization
potentials) the optimizations that this is going to open up almost
certainly *will* break the code that triggers this, but not otherwise
improve things. So this change will just force users to edit their code in
order to get the old behavior, without improving anything.

Can we revert to just emitting undef here?

 - Daniel


On Thu, Oct 4, 2012 at 4:52 PM, Richard Smith <richard-llvm at metafoo.co.uk>wrote:

> Author: rsmith
> Date: Thu Oct  4 18:52:29 2012
> New Revision: 165273
>
> URL: http://llvm.org/viewvc/llvm-project?rev=165273&view=rev
> Log:
> If we flow off the end of a value-returning function:
>  - outside C++, return undef (behavior is not undefined unless the value
> is used)
>  - in C++, with -fcatch-undefined-behavior, perform an appropriate trap
>  - in C++, produce an 'unreachable' (behavior is undefined immediately)
>
> Added:
>     cfe/trunk/test/CodeGenCXX/return.cpp
> Modified:
>     cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>     cfe/trunk/test/CodeGen/catch-undef-behavior.c
>     cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=165273&r1=165272&r2=165273&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Oct  4 18:52:29 2012
> @@ -535,6 +535,20 @@
>    else
>      EmitFunctionBody(Args);
>
> +  // C++11 [stmt.return]p2:
> +  //   Flowing off the end of a function [...] results in undefined
> behavior in
> +  //   a value-returning function.
> +  // C11 6.9.1p12:
> +  //   If the '}' that terminates a function is reached, and the value of
> the
> +  //   function call is used by the caller, the behavior is undefined.
> +  if (getContext().getLangOpts().CPlusPlus &&
> !FD->hasImplicitReturnZero() &&
> +      !FD->getResultType()->isVoidType() && Builder.GetInsertBlock()) {
> +    if (CatchUndefined)
> +      EmitCheck(Builder.getFalse());
> +    Builder.CreateUnreachable();
> +    Builder.ClearInsertionPoint();
> +  }
> +
>    // Emit the standard function epilogue.
>    FinishFunction(BodyRange.getEnd());
>
>
> Modified: cfe/trunk/test/CodeGen/catch-undef-behavior.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/catch-undef-behavior.c?rev=165273&r1=165272&r2=165273&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGen/catch-undef-behavior.c (original)
> +++ cfe/trunk/test/CodeGen/catch-undef-behavior.c Thu Oct  4 18:52:29 2012
> @@ -44,3 +44,11 @@
>    // CHECK-NEXT: ret i32 %[[RET]]
>    return a >> b;
>  }
> +
> +// CHECK: @no_return
> +int no_return() {
> +  // Reaching the end of a noreturn function is fine in C.
> +  // CHECK-NOT: call
> +  // CHECK-NOT: unreachable
> +  // CHECK: ret i32
> +}
>
> Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=165273&r1=165272&r2=165273&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Thu Oct  4 18:52:29
> 2012
> @@ -86,3 +86,9 @@
>    // CHECK-NEXT: ret i32 %[[RET]]
>    return a << b;
>  }
> +
> +// CHECK: @_Z9no_return
> +int no_return() {
> +  // CHECK: call void @llvm.trap
> +  // CHECK: unreachable
> +}
>
> Added: cfe/trunk/test/CodeGenCXX/return.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/return.cpp?rev=165273&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/return.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/return.cpp Thu Oct  4 18:52:29 2012
> @@ -0,0 +1,6 @@
> +// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
> +
> +// CHECK: @_Z9no_return
> +int no_return() {
> +  // CHECK: unreachable
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130122/debf816e/attachment.html>


More information about the cfe-commits mailing list