From: Nick Lewycky <nicholas@mxc.ca>
Date: Fri, 8 Nov 2013 23:00:12 +0000 (+0000)
Subject: Remove an incorrect optimization inside Clang's IRGen. Its check to determine
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c56991755e6799f1f733d02f7a98d3159e94c185;p=clang

Remove an incorrect optimization inside Clang's IRGen. Its check to determine
whether we can safely lower a conditional operator to select was insufficient.
I've left a large comment in place to explaining the sort of problems that this
transform can encounter in clang in the hopes of discouraging others from
reimplementing it wrongly again in the future. (The test should also help with
that, but it's easy to work around any single test I might add and think that
your particular implementation doesn't miscompile any code.)


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@194289 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
index f3fd60498c..f3a5387c58 100644
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/lib/CodeGen/CGExprScalar.cpp
@@ -3023,22 +3023,15 @@ Value *ScalarExprEmitter::VisitBinComma(const BinaryOperator *E) {
 /// 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;
-
-  // 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;
+  return E->IgnoreParens()->isEvaluatable(CGF.getContext());
+
+  // 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.
 }
 
 
diff --git a/test/CodeGenCXX/catch-undef-behavior2.cpp b/test/CodeGenCXX/catch-undef-behavior2.cpp
new file mode 100644
index 0000000000..b8b31ca23f
--- /dev/null
+++ b/test/CodeGenCXX/catch-undef-behavior2.cpp
@@ -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;
+}