On 5 November 2013 17:30, Rafael Espíndola <[email protected]>wrote:
> The test passes even with the code change reverted.
>
Bah! Thanks for noticing. The testcase worked in C++ mode but not C mode.
Adding it to test/CodeGenCXX/catch-undef-behavior.cpp doesn't work well
either because of ordering dependencies in that test.
Updated patch attached!
Nick
On 5 November 2013 16:54, Nick Lewycky <[email protected]> wrote:
> > When lowering "cond ? X : Y" we do some safety checks to see whether we
> can
> > instead emit both X and Y and use a llvm select instruction to choose
> > between them. This code is insufficiently safe, and introducing loads
> into a
> > program that didn't load is a bad idea. For example, it could be TLS. It
> > could be a non-volatile auto in another function that isn't the current
> > function (think lambdas).
> >
> > Don't do this here. LLVM knows how to do this properly.
> >
> > Patch attached, please review!
> >
> > Nick
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > [email protected]
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
>
Index: test/CodeGenCXX/catch-undef-behavior2.cpp
===================================================================
--- test/CodeGenCXX/catch-undef-behavior2.cpp (revision 0)
+++ test/CodeGenCXX/catch-undef-behavior2.cpp (revision 0)
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++11 -fsanitize=signed-integer-overflow,integer-divide-by-zero,float-divide-by-zero,shift,unreachable,return,vla-bound,alignment,null,vptr,object-size,float-cast-overflow,bool,enum,array-bounds,function -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+
+bool GetOptionalBool(bool *value);
+bool GetBool(bool default_value) {
+ // CHECK-LABEL: @_Z7GetBoolb
+ // CHECK-NOT: select
+ bool value;
+ return GetOptionalBool(&value) ? value : default_value;
+}
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp (revision 194115)
+++ lib/CodeGen/CGExprScalar.cpp (working copy)
@@ -3023,22 +3023,15 @@
/// flow into selects in some cases.
static bool isCheapEnoughToEvaluateUnconditionally(const Expr *E,
CodeGenFunction &CGF) {
- E = E->IgnoreParens();
-
// Anything that is an integer or floating point constant is fine.
- if (E->isEvaluatable(CGF.getContext()))
- return true;
+ return E->IgnoreParens()->isEvaluatable(CGF.getContext());
- // Non-volatile automatic variables too, to get "cond ? X : Y" where
- // X and Y are local variables.
- if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
- if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl()))
- if (VD->hasLocalStorage() && !(CGF.getContext()
- .getCanonicalType(VD->getType())
- .isVolatileQualified()))
- return true;
-
- return false;
+ // Even non-volatile automatic variables can't be evaluated unconditionally.
+ // Referencing a thread_local may cause non-trivial initialization work to
+ // occur. If we're inside a lambda and one of the variables is from the scope
+ // outside the lambda, that function may have returned already. Reading its
+ // locals is a bad idea. Also, these reads may introduce races there didn't
+ // exist in the source-level program.
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
