[PATCH] D59919: [Attributor] Deduce "returned" argument attribute

[Nick Lewycky via Phabricator via cfe-commits]

nicholas added a comment.

> CHANGED: build-libcalls   NumNoUnwind 
> 4526 ->   3382 (   -25.276%)

Why did the number of nounwinds drop?




Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:115
+template 
+static void gernericValueTraversal(Value *V, StateTy ,
+   followValueCB_t ,

Typo in "gerneric", should be "generic".



Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:127
+if (Arg.hasReturnedAttr())
+  return gernericValueTraversal(CS.getArgOperand(Arg.getArgNo()), 
State,
+FollowValueCB, VisitValueCB);

LLVM generally has a preference for not recursing like this, it means that the 
amount of stack space we need depends on the input IR and it's hard for a user 
of llvm as a library to foresee or handle an out of stack condition.

Common practice is to structure it as a loop like:
```
SmallVector Worklist;
SmallSet Visited;
Worklist.push_back(V);
do {
  Value *V = Worklist.pop_back_val();
  if (!Visited.insert(V).second)
continue;
  V = V->stripPointerCasts();
  // ...
} while (!Worklist.empty());
```

Also, consider having some sort of loop iteration limit as a safety value 
against runaway compile time.



Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:133
+  // recursion keep a record of the values we followed!
+  if (!FollowValueCB(V, State))
+return;

Offhand, I think placing this after the CS check is incorrect. I haven't tried 
it out, but I expect the testcase that triggers infinite loop to look something 
like this:

```
define i32 @test(i32 %A) {
entry:
  ret i32 0
unreachableblock:
  %B = call i32 @test(i32 %B)
  ret i32 %B
}
```

which should pass the verifier and trigger an infinite loop if you call 
gernericValueTraversal on %B.

Also, if you really need a callback and not just a SmallSet named Visited, I'd 
suggest calling the callback immediately before adding each value to the 
Worklist (or as written not, call it on each value before recursing).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59919/new/

https://reviews.llvm.org/D59919



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48100: Append new attributes to the end of an AttributeList.

[Nick Lewycky via Phabricator via cfe-commits]

nicholas added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:6194
 static SmallVector
 getOrderedEnableIfAttrs(const FunctionDecl *Function) {
   SmallVector Result;

This function shouldn't be necessary any more. All it's doing now is making an 
unnecessary copy.



Comment at: lib/Sema/SemaOverload.cpp:8943
 
-  // FIXME: The next several lines are just
-  // specific_attr_iterator but going in declaration order,
-  // instead of reverse order which is how they're stored in the AST.
   auto Cand1Attrs = getOrderedEnableIfAttrs(Cand1);
   auto Cand2Attrs = getOrderedEnableIfAttrs(Cand2);

This would become "auto Cand1Attrs = Cand1->specific_attrs();" 
but I think you can simplify that even further if you want. To do that you'd 
need to sink the "return Comparison::Worse;" inside the loop that follows it. 
If you don't do that you'll have to switch the calls to Cand1Attrs.size() and 
Cand2Attrs.size() into calls to std::distance, since llvm::iterator_range 
doesn't have a size() method.


Repository:
  rC Clang

https://reviews.llvm.org/D48100



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40850: suppress undefined-template warnings when the pattern is declared in a system header

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky added a comment.

Ping!


https://reviews.llvm.org/D40850



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40850: suppress undefined-template warnings when the pattern is declared in a system header

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky created this revision.

The way to fix an undefined-template warning is to add lines to the header file 
that defines the template pattern. We should suppress the warnings when the 
template pattern is in a system header because we don't expect users to edit 
those.


https://reviews.llvm.org/D40850

Files:
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/SemaTemplate/undefined-template.cpp


Index: test/SemaTemplate/undefined-template.cpp
===
--- test/SemaTemplate/undefined-template.cpp
+++ test/SemaTemplate/undefined-template.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 -Wundefined-func-template 
%s
 
+#if !defined(INCLUDE)
 template  struct C1 {
   static char s_var_1;   // expected-note{{forward declaration of template 
entity is here}}
   static char s_var_2;   // expected-note{{forward declaration of template 
entity is here}}
@@ -142,6 +143,16 @@
   void h(X x) { g(x); } // no warning for use of 'g' despite the 
declaration having been instantiated from a template
 }
 
+#define INCLUDE
+#include "undefined-template.cpp"
+void func_25(SystemHeader *x) {
+  x->meth();
+}
+
 int main() {
   return 0;
 }
+#else
+#pragma clang system_header
+template  struct SystemHeader { T meth(); };
+#endif
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3808,7 +3808,8 @@
   PendingInstantiations.push_back(
 std::make_pair(Function, PointOfInstantiation));
 } else if (TSK == TSK_ImplicitInstantiation) {
-  if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
+  if (AtEndOfTU && !getDiagnostics().hasErrorOccurred() &&
+  !getSourceManager().isInSystemHeader(PatternDecl->getLocStart())) {
 Diag(PointOfInstantiation, diag::warn_func_template_missing)
   << Function;
 Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
@@ -4338,7 +4339,8 @@
 std::make_pair(Var, PointOfInstantiation));
 } else if (TSK == TSK_ImplicitInstantiation) {
   // Warn about missing definition at the end of translation unit.
-  if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
+  if (AtEndOfTU && !getDiagnostics().hasErrorOccurred() &&
+  !getSourceManager().isInSystemHeader(PatternDecl->getLocStart())) {
 Diag(PointOfInstantiation, diag::warn_var_template_missing)
   << Var;
 Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);


Index: test/SemaTemplate/undefined-template.cpp
===
--- test/SemaTemplate/undefined-template.cpp
+++ test/SemaTemplate/undefined-template.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 -Wundefined-func-template %s
 
+#if !defined(INCLUDE)
 template  struct C1 {
   static char s_var_1;   // expected-note{{forward declaration of template entity is here}}
   static char s_var_2;   // expected-note{{forward declaration of template entity is here}}
@@ -142,6 +143,16 @@
   void h(X x) { g(x); } // no warning for use of 'g' despite the declaration having been instantiated from a template
 }
 
+#define INCLUDE
+#include "undefined-template.cpp"
+void func_25(SystemHeader *x) {
+  x->meth();
+}
+
 int main() {
   return 0;
 }
+#else
+#pragma clang system_header
+template  struct SystemHeader { T meth(); };
+#endif
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3808,7 +3808,8 @@
   PendingInstantiations.push_back(
 std::make_pair(Function, PointOfInstantiation));
 } else if (TSK == TSK_ImplicitInstantiation) {
-  if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
+  if (AtEndOfTU && !getDiagnostics().hasErrorOccurred() &&
+  !getSourceManager().isInSystemHeader(PatternDecl->getLocStart())) {
 Diag(PointOfInstantiation, diag::warn_func_template_missing)
   << Function;
 Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
@@ -4338,7 +4339,8 @@
 std::make_pair(Var, PointOfInstantiation));
 } else if (TSK == TSK_ImplicitInstantiation) {
   // Warn about missing definition at the end of translation unit.
-  if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
+  if (AtEndOfTU && !getDiagnostics().hasErrorOccurred() &&
+  !getSourceManager().isInSystemHeader(PatternDecl->getLocStart())) {
 Diag(PointOfInstantiation, diag::warn_var_template_missing)
   << Var;
 Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D31839: make -Winteger-overflow find overflows in function arguments

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky added a comment.

Ping!


https://reviews.llvm.org/D31839



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31839: make -Winteger-overflow find overflows in function arguments

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky added a comment.

Ping!


https://reviews.llvm.org/D31839



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31839: make -Winteger-overflow find overflows in function arguments

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky updated this revision to Diff 97253.
nlewycky added a comment.

Use an RAII object to always evaluate the arguments, except if 
HandleFunctionCall does it.


https://reviews.llvm.org/D31839

Files:
  lib/AST/ExprConstant.cpp
  test/Sema/integer-overflow.c


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -151,6 +151,14 @@
   uint64_t *b;
   uint64_t b2 = b[4608 * 1024 * 1024] + 1;
 
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 
'int'}} 
+  f0(4608 * 1024 * 1024);
+  f0(4608ul * 1024 * 1024);
+// expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}} 
+  f1(4608 * 1024 * 1024, 4608 * 1024 * 1024);
+// expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}} 
+  f2(4608 * 1024 * 1024, 4608 * 1024 * 1024);
+
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}}
   int j1 = i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024);
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4579,15 +4579,32 @@
   }
 
   bool handleCallExpr(const CallExpr *E, APValue ,
- const LValue *ResultSlot) {
+  const LValue *ResultSlot) {
 const Expr *Callee = E->getCallee()->IgnoreParens();
 QualType CalleeType = Callee->getType();
 
 const FunctionDecl *FD = nullptr;
 LValue *This = nullptr, ThisVal;
 auto Args = llvm::makeArrayRef(E->getArgs(), E->getNumArgs());
 bool HasQualifier = false;
 
+struct EvaluateIgnoredRAII {
+public:
+  EvaluateIgnoredRAII(EvalInfo , llvm::ArrayRef ToEval)
+  : Info(Info), ToEval(ToEval) {}
+  ~EvaluateIgnoredRAII() {
+if (Info.noteFailure()) {
+  for (auto E : ToEval)
+EvaluateIgnoredValue(Info, E);
+}
+  }
+  void cancel() { ToEval = {}; }
+  void drop_front() { ToEval = ToEval.drop_front(); }
+private:
+  EvalInfo 
+  llvm::ArrayRef ToEval;
+} EvalArguments(Info, Args);
+
 // Extract function decl and 'this' pointer from the callee.
 if (CalleeType->isSpecificBuiltinType(BuiltinType::BoundMember)) {
   const ValueDecl *Member = nullptr;
@@ -4637,10 +4654,12 @@
 if (Args.empty())
   return Error(E);
 
-if (!EvaluateObjectArgument(Info, Args[0], ThisVal))
+const Expr *FirstArg = Args[0];
+Args = Args.drop_front();
+EvalArguments.drop_front();
+if (!EvaluateObjectArgument(Info, FirstArg, ThisVal))
   return false;
 This = 
-Args = Args.slice(1);
   } else if (MD && MD->isLambdaStaticInvoker()) {   
 // Map the static invoker for the lambda back to the call operator.
 // Conveniently, we don't have to slice out the 'this' argument (as is
@@ -4692,8 +4711,12 @@
 const FunctionDecl *Definition = nullptr;
 Stmt *Body = FD->getBody(Definition);
 
-if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body) ||
-!HandleFunctionCall(E->getExprLoc(), Definition, This, Args, Body, 
Info,
+if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body))
+  return false;
+
+EvalArguments.cancel();
+
+if (!HandleFunctionCall(E->getExprLoc(), Definition, This, Args, Body, 
Info,
 Result, ResultSlot))
   return false;
 


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -151,6 +151,14 @@
   uint64_t *b;
   uint64_t b2 = b[4608 * 1024 * 1024] + 1;
 
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}} 
+  f0(4608 * 1024 * 1024);
+  f0(4608ul * 1024 * 1024);
+// expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}} 
+  f1(4608 * 1024 * 1024, 4608 * 1024 * 1024);
+// expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}} 
+  f2(4608 * 1024 * 1024, 4608 * 1024 * 1024);
+
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
   int j1 = i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024);
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4579,15 +4579,32 @@
   }
 
   bool handleCallExpr(const CallExpr *E, APValue ,
- const LValue *ResultSlot) {
+  const LValue *ResultSlot) {
 const Expr *Callee = E->getCallee()->IgnoreParens();
 QualType CalleeType = Callee->getType();
 
 const FunctionDecl *FD = nullptr;
 LValue *This = nullptr, ThisVal;
 auto Args = llvm::makeArrayRef(E->getArgs(), 

[PATCH] D32675: in expression evaluator, treat non-literal types as discarded value expressions if EvalInfo says to continue evaluating them

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky created this revision.

Make the integer overflow evaluator continue into expressions with non-literal 
types, notably void.

In passing it fixes a crash attempting to codegen:

  struct A { char x; };
  struct B : virtual A {};
  A  = ((A&)*(B*)0);

which we nearly have a test for except that it casted to void and therefore was 
ignored instead of being evaluated. The existing test 
(test/SemaCXX/cstyle-cast.cpp) is sufficient to cover this case now that we 
don't stop at a void cast.


https://reviews.llvm.org/D32675

Files:
  lib/AST/ExprConstant.cpp
  test/Sema/integer-overflow.c


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -149,16 +149,16 @@
 
 // expected-warning@+2 {{overflow in expression; result is 536870912 with type 
'int'}}
   uint64_t *b;
-  uint64_t b2 = b[4608 * 1024 * 1024] + 1;
+  (void)b[4608 * 1024 * 1024];
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}}
-  int j1 = i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024);
+  (void)(i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024));
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 
'int'}}
-  int j2 = -(4608 * 1024 * 1024);
+  (void)(-(4608 * 1024 * 1024));
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 
'int'}}
-  uint64_t j3 = b[4608 * 1024 * 1024];
+  (void)b[4608 * 1024 * 1024];
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -2169,6 +2169,9 @@
   if (!Base->isVirtual())
 return HandleLValueDirectBase(Info, E, Obj, DerivedDecl, BaseDecl);
 
+  if (!Obj.checkNullPointer(Info, E, CSK_Base))
+return false;
+
   SubobjectDesignator  = Obj.Designator;
   if (D.Invalid)
 return false;
@@ -9913,8 +9916,11 @@
   if (E->getType().isNull())
 return false;
 
-  if (!CheckLiteralType(Info, E))
+  if (!CheckLiteralType(Info, E)) {
+if (Info.noteFailure())
+  EvaluateIgnoredValue(Info, E);
 return false;
+  }
 
   if (!::Evaluate(Result, Info, E))
 return false;


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -149,16 +149,16 @@
 
 // expected-warning@+2 {{overflow in expression; result is 536870912 with type 'int'}}
   uint64_t *b;
-  uint64_t b2 = b[4608 * 1024 * 1024] + 1;
+  (void)b[4608 * 1024 * 1024];
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
-  int j1 = i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024);
+  (void)(i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024));
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
-  int j2 = -(4608 * 1024 * 1024);
+  (void)(-(4608 * 1024 * 1024));
 
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
-  uint64_t j3 = b[4608 * 1024 * 1024];
+  (void)b[4608 * 1024 * 1024];
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -2169,6 +2169,9 @@
   if (!Base->isVirtual())
 return HandleLValueDirectBase(Info, E, Obj, DerivedDecl, BaseDecl);
 
+  if (!Obj.checkNullPointer(Info, E, CSK_Base))
+return false;
+
   SubobjectDesignator  = Obj.Designator;
   if (D.Invalid)
 return false;
@@ -9913,8 +9916,11 @@
   if (E->getType().isNull())
 return false;
 
-  if (!CheckLiteralType(Info, E))
+  if (!CheckLiteralType(Info, E)) {
+if (Info.noteFailure())
+  EvaluateIgnoredValue(Info, E);
 return false;
+  }
 
   if (!::Evaluate(Result, Info, E))
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32412: analyze all kinds of expressions for integer overflow

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky updated this revision to Diff 97179.
nlewycky added a comment.

[No changes, just full context this time.]


https://reviews.llvm.org/D32412

Files:
  include/clang/Sema/Sema.h
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  test/OpenMP/distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/distribute_simd_aligned_messages.cpp
  test/OpenMP/for_simd_aligned_messages.cpp
  test/OpenMP/parallel_for_simd_aligned_messages.cpp
  test/OpenMP/simd_aligned_messages.cpp
  test/OpenMP/target_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/target_simd_aligned_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
  test/OpenMP/taskloop_simd_aligned_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/teams_distribute_simd_aligned_messages.cpp
  test/Sema/integer-overflow.c

Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -152,7 +152,13 @@
   uint64_t b2 = b[4608 * 1024 * 1024] + 1;
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
-  (void)((i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024)) + 1);
+  int j1 = i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024);
+
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
+  int j2 = -(4608 * 1024 * 1024);
+
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
+  uint64_t j3 = b[4608 * 1024 * 1024];
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
Index: test/OpenMP/teams_distribute_simd_aligned_messages.cpp
===
--- test/OpenMP/teams_distribute_simd_aligned_messages.cpp
+++ test/OpenMP/teams_distribute_simd_aligned_messages.cpp
@@ -123,9 +123,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target
Index: test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
===
--- test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
+++ test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
@@ -123,9 +123,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target
Index: test/OpenMP/taskloop_simd_aligned_messages.cpp
===
--- test/OpenMP/taskloop_simd_aligned_messages.cpp
+++ test/OpenMP/taskloop_simd_aligned_messages.cpp
@@ -107,9 +107,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
   #pragma omp taskloop simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
Index: test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
===
--- test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
+++ test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
@@ -110,9 +110,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target teams distribute simd aligned // expected-error {{expected '(' after 'aligned'}}
Index: test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
===
--- test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
+++ test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
@@ -110,9 +110,8 @@
 template 

[PATCH] D32412: analyze all kinds of expressions for integer overflow

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky added a comment.

> If we're now catching integer overflow in more cases, please add some 
> relevant testcases.

Both more and fewer. More because we no longer have a whitelist of three kinds 
of expressions that we recurse into. Fewer because we no longer call 
IgnoreParenCasts() on the full-expression so "(void)(4608 * 1024 * 1024);" used 
to get a warning but now doesn't. The plan to fix this is a patch to call 
EvaluateIgnoredValue on non-literal types, which will get us that warning back.

I've added two cases to test/Sema/integer-overflow.c and changed one to 
demonstrate this patch. It previously had an additional + on it just to trigger 
the int overflow checking.

> I have an unsubstantiated performance concern: we've seen this overflow 
> checking having a visible effect on compile times in LNT before

I haven't observed slowdown like I did with my previous attempt at this change 
in https://reviews.llvm.org/D31839, but yes we may need to back this patch out 
if it causes problems.


https://reviews.llvm.org/D32412



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32412: analyze all kinds of expressions for integer overflow

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky updated this revision to Diff 97178.
nlewycky edited the summary of this revision.
nlewycky added a comment.

Rebase. Now that ObjCBoxedExpr change is in, we can remove 
Sema::CheckForIntOverflow entirely.


https://reviews.llvm.org/D32412

Files:
  include/clang/Sema/Sema.h
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  test/OpenMP/distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/distribute_simd_aligned_messages.cpp
  test/OpenMP/for_simd_aligned_messages.cpp
  test/OpenMP/parallel_for_simd_aligned_messages.cpp
  test/OpenMP/simd_aligned_messages.cpp
  test/OpenMP/target_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/target_simd_aligned_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
  test/OpenMP/taskloop_simd_aligned_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/teams_distribute_simd_aligned_messages.cpp
  test/Sema/integer-overflow.c

Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -152,8 +152,14 @@
   uint64_t b2 = b[4608 * 1024 * 1024] + 1;
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
-  (void)((i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024)) + 1);
+  int j1 = i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024);
 
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
+  int j2 = -(4608 * 1024 * 1024);
+
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
+  uint64_t j3 = b[4608 * 1024 * 1024];
+
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
 }
Index: test/OpenMP/teams_distribute_simd_aligned_messages.cpp
===
--- test/OpenMP/teams_distribute_simd_aligned_messages.cpp
+++ test/OpenMP/teams_distribute_simd_aligned_messages.cpp
@@ -123,9 +123,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target
Index: test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
===
--- test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
+++ test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
@@ -123,9 +123,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target
Index: test/OpenMP/taskloop_simd_aligned_messages.cpp
===
--- test/OpenMP/taskloop_simd_aligned_messages.cpp
+++ test/OpenMP/taskloop_simd_aligned_messages.cpp
@@ -107,9 +107,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
   #pragma omp taskloop simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
Index: test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
===
--- test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
+++ test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
@@ -110,9 +110,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target teams distribute simd aligned // expected-error {{expected '(' after 'aligned'}}
Index: test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
===
--- test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
+++ 

[PATCH] D32410: change the way the expr evaluator handles objcboxedexpr

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky updated this revision to Diff 97173.
nlewycky added a comment.

If the boxing method can't be constexpr then we can never evaluate it for a 
constant value, which means that we should unconditionally return Error, and 
use noteFailure to decide whether to visit the subexpr.


https://reviews.llvm.org/D32410

Files:
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjCXX/boxing.mm


Index: test/CodeGenObjCXX/boxing.mm
===
--- test/CodeGenObjCXX/boxing.mm
+++ test/CodeGenObjCXX/boxing.mm
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s | 
FileCheck %s
+
+@interface NSNumber
++ (id)numberWithInt:(int)n;
+@end
+
+int n = 1;
+int m = (@(n++), 0);
+
+// CHECK: define {{.*}} @__cxx_global_var_init()
+// CHECK: load i32, i32* @n
+// CHECK: store i32 %{{.*}}, i32* @n
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -595,7 +595,6 @@
 break;
   }
 }
-CheckForIntOverflow(ValueExpr);
 // FIXME:  Do I need to do anything special with BoolTy expressions?
 
 // Look for the appropriate method within NSNumber.
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -9882,6 +9882,9 @@
 
 if (auto InitList = dyn_cast(E))
   Exprs.append(InitList->inits().begin(), InitList->inits().end());
+
+if (isa(E))
+  E->IgnoreParenCasts()->EvaluateForOverflow(Context);
   } while (!Exprs.empty());
 }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5481,8 +5481,11 @@
   bool VisitUnaryAddrOf(const UnaryOperator *E);
   bool VisitObjCStringLiteral(const ObjCStringLiteral *E)
   { return Success(E); }
-  bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E)
-  { return Success(E); }
+  bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E) {
+if (Info.noteFailure())
+  EvaluateIgnoredValue(Info, E->getSubExpr());
+return Error(E);
+  }
   bool VisitAddrLabelExpr(const AddrLabelExpr *E)
   { return Success(E); }
   bool VisitCallExpr(const CallExpr *E);


Index: test/CodeGenObjCXX/boxing.mm
===
--- test/CodeGenObjCXX/boxing.mm
+++ test/CodeGenObjCXX/boxing.mm
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s | FileCheck %s
+
+@interface NSNumber
++ (id)numberWithInt:(int)n;
+@end
+
+int n = 1;
+int m = (@(n++), 0);
+
+// CHECK: define {{.*}} @__cxx_global_var_init()
+// CHECK: load i32, i32* @n
+// CHECK: store i32 %{{.*}}, i32* @n
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -595,7 +595,6 @@
 break;
   }
 }
-CheckForIntOverflow(ValueExpr);
 // FIXME:  Do I need to do anything special with BoolTy expressions?
 
 // Look for the appropriate method within NSNumber.
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -9882,6 +9882,9 @@
 
 if (auto InitList = dyn_cast(E))
   Exprs.append(InitList->inits().begin(), InitList->inits().end());
+
+if (isa(E))
+  E->IgnoreParenCasts()->EvaluateForOverflow(Context);
   } while (!Exprs.empty());
 }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5481,8 +5481,11 @@
   bool VisitUnaryAddrOf(const UnaryOperator *E);
   bool VisitObjCStringLiteral(const ObjCStringLiteral *E)
   { return Success(E); }
-  bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E)
-  { return Success(E); }
+  bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E) {
+if (Info.noteFailure())
+  EvaluateIgnoredValue(Info, E->getSubExpr());
+return Error(E);
+  }
   bool VisitAddrLabelExpr(const AddrLabelExpr *E)
   { return Success(E); }
   bool VisitCallExpr(const CallExpr *E);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32455: detect integer overflow inside arms of conditional operator with non-constant expression

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky created this revision.
Herald added subscribers: rengolin, aemerson.

Descend into both the true and false expressions of a ConditionalOperator when 
the condition can't be evaluated and we're in an evaluation-mode that says we 
should continue evaluating.


https://reviews.llvm.org/D32455

Files:
  lib/AST/ExprConstant.cpp
  test/Sema/integer-overflow.c


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -148,6 +148,9 @@
   a[4608 * 1024 * 1024] = 1i;
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}}
+  (void)((i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024)) + 1);
+
+// expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
 }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4418,8 +4418,14 @@
   bool HandleConditionalOperator(const ConditionalOperator *E) {
 bool BoolResult;
 if (!EvaluateAsBooleanCondition(E->getCond(), BoolResult, Info)) {
-  if (Info.checkingPotentialConstantExpression() && Info.noteFailure())
+  if (Info.checkingPotentialConstantExpression() && Info.noteFailure()) {
 CheckPotentialConstantConditional(E);
+return false;
+  }
+  if (Info.noteFailure()) {
+StmtVisitorTy::Visit(E->getTrueExpr());
+StmtVisitorTy::Visit(E->getFalseExpr());
+  }
   return false;
 }
 


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -148,6 +148,9 @@
   a[4608 * 1024 * 1024] = 1i;
 
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
+  (void)((i ? (4608 * 1024 * 1024) : (4608 * 1024 * 1024)) + 1);
+
+// expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
 }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4418,8 +4418,14 @@
   bool HandleConditionalOperator(const ConditionalOperator *E) {
 bool BoolResult;
 if (!EvaluateAsBooleanCondition(E->getCond(), BoolResult, Info)) {
-  if (Info.checkingPotentialConstantExpression() && Info.noteFailure())
+  if (Info.checkingPotentialConstantExpression() && Info.noteFailure()) {
 CheckPotentialConstantConditional(E);
+return false;
+  }
+  if (Info.noteFailure()) {
+StmtVisitorTy::Visit(E->getTrueExpr());
+StmtVisitorTy::Visit(E->getFalseExpr());
+  }
   return false;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32412: analyze all kinds of expressions for integer overflow

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky created this revision.

Remove clang::Sema::CheckForIntOverflow(E) by calling into 
E->EvaluateForOverflow instead. CheckForIntOverflow implemented a whitelist of 
top-level expressions to check, currently BinaryOperator and InitListExpr.

Two changes are made to avoid regression with the existing test suite.
test/SemaCXX/eval-sizeof-dependent-type.cpp has an example of a value-dependent 
InitListExpr. Handle value-dependent initializers in init list exprs.
test/Sema/integer-overflow.c tests that initializers are checked for overflow, 
which was conditionally disabled for performance. Allow checking for overflow 
of init list exprs in the performance check.


https://reviews.llvm.org/D32412

Files:
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  test/OpenMP/distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/distribute_simd_aligned_messages.cpp
  test/OpenMP/for_simd_aligned_messages.cpp
  test/OpenMP/parallel_for_simd_aligned_messages.cpp
  test/OpenMP/simd_aligned_messages.cpp
  test/OpenMP/target_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/target_simd_aligned_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
  test/OpenMP/taskloop_simd_aligned_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/teams_distribute_simd_aligned_messages.cpp

Index: test/OpenMP/teams_distribute_simd_aligned_messages.cpp
===
--- test/OpenMP/teams_distribute_simd_aligned_messages.cpp
+++ test/OpenMP/teams_distribute_simd_aligned_messages.cpp
@@ -123,9 +123,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target
Index: test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
===
--- test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
+++ test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
@@ -123,9 +123,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target
Index: test/OpenMP/taskloop_simd_aligned_messages.cpp
===
--- test/OpenMP/taskloop_simd_aligned_messages.cpp
+++ test/OpenMP/taskloop_simd_aligned_messages.cpp
@@ -107,9 +107,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
   #pragma omp taskloop simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
Index: test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
===
--- test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
+++ test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
@@ -110,9 +110,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target teams distribute simd aligned // expected-error {{expected '(' after 'aligned'}}
Index: test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
===
--- test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
+++ test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
@@ -110,9 +110,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma 

[PATCH] D32410: change the way objcboxedexpr is handled

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky created this revision.

Make ObjCBoxedExpr less of a special case, teach the expression evaluator to 
handle it in general, sometimes descending through to its subexpr. Remove the 
code that called CheckForIntOverflow from outside BuildObjCBoxedExpr, leaving 
its only caller CheckCompletedExpr.

To make the existing tests continue to work, we also need to whitelist 
ObjCBoxedExpr as a top-level expression in CheckForIntOverflow. Ultimately that 
we shouldn't need to whitelist, but one piece at a time.


https://reviews.llvm.org/D32410

Files:
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExprObjC.cpp


Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -595,7 +595,6 @@
 break;
   }
 }
-CheckForIntOverflow(ValueExpr);
 // FIXME:  Do I need to do anything special with BoolTy expressions?
 
 // Look for the appropriate method within NSNumber.
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -9882,6 +9882,9 @@
 
 if (auto InitList = dyn_cast(E))
   Exprs.append(InitList->inits().begin(), InitList->inits().end());
+
+if (isa(E))
+  E->IgnoreParenCasts()->EvaluateForOverflow(Context);
   } while (!Exprs.empty());
 }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4466,6 +4466,8 @@
 { return StmtVisitorTy::Visit(E->getSubExpr()); }
   bool VisitUnaryPlus(const UnaryOperator *E)
 { return StmtVisitorTy::Visit(E->getSubExpr()); }
+  bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E)
+{ return StmtVisitorTy::Visit(E->getSubExpr()); }
   bool VisitChooseExpr(const ChooseExpr *E)
 { return StmtVisitorTy::Visit(E->getChosenSubExpr()); }
   bool VisitGenericSelectionExpr(const GenericSelectionExpr *E)
@@ -5471,7 +5473,26 @@
   bool VisitObjCStringLiteral(const ObjCStringLiteral *E)
   { return Success(E); }
   bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E)
-  { return Success(E); }
+  {
+// Should we continue evaluation after determining the result value?
+switch (Info.EvalMode) {
+case EvalInfo::EM_PotentialConstantExpression:
+case EvalInfo::EM_PotentialConstantExpressionUnevaluated:
+case EvalInfo::EM_EvaluateForOverflow:
+case EvalInfo::EM_IgnoreSideEffects:
+  if (!E->getSubExpr()->isValueDependent()) {
+APValue Discard;
+Evaluate(Discard, Info, E->getSubExpr());
+  }
+
+case EvalInfo::EM_ConstantExpression:
+case EvalInfo::EM_ConstantExpressionUnevaluated:
+case EvalInfo::EM_ConstantFold:
+case EvalInfo::EM_OffsetFold:
+  return Success(E);
+}
+  llvm_unreachable("Missed EvalMode case");
+  }
   bool VisitAddrLabelExpr(const AddrLabelExpr *E)
   { return Success(E); }
   bool VisitCallExpr(const CallExpr *E);


Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -595,7 +595,6 @@
 break;
   }
 }
-CheckForIntOverflow(ValueExpr);
 // FIXME:  Do I need to do anything special with BoolTy expressions?
 
 // Look for the appropriate method within NSNumber.
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -9882,6 +9882,9 @@
 
 if (auto InitList = dyn_cast(E))
   Exprs.append(InitList->inits().begin(), InitList->inits().end());
+
+if (isa(E))
+  E->IgnoreParenCasts()->EvaluateForOverflow(Context);
   } while (!Exprs.empty());
 }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -4466,6 +4466,8 @@
 { return StmtVisitorTy::Visit(E->getSubExpr()); }
   bool VisitUnaryPlus(const UnaryOperator *E)
 { return StmtVisitorTy::Visit(E->getSubExpr()); }
+  bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E)
+{ return StmtVisitorTy::Visit(E->getSubExpr()); }
   bool VisitChooseExpr(const ChooseExpr *E)
 { return StmtVisitorTy::Visit(E->getChosenSubExpr()); }
   bool VisitGenericSelectionExpr(const GenericSelectionExpr *E)
@@ -5471,7 +5473,26 @@
   bool VisitObjCStringLiteral(const ObjCStringLiteral *E)
   { return Success(E); }
   bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E)
-  { return Success(E); }
+  {
+// Should we continue evaluation after determining the result value?
+switch (Info.EvalMode) {
+case EvalInfo::EM_PotentialConstantExpression:
+case EvalInfo::EM_PotentialConstantExpressionUnevaluated:
+   

[PATCH] D31839: make -Winteger-overflow find overflows in function arguments

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky added a comment.

In https://reviews.llvm.org/D31839#724551, @ahatanak wrote:

> OK, thanks for looking into it. Warnings for ObjCMessageExpr can probably be 
> implemented in a separate patch.
>
> It looks like clang still doesn't issue overflow warnings when the called 
> functions have a void return. Should we try to fix it in this patch too?
>
>   void foo(int);
>  
>   void test0() {
> foo(4068 * 1024 * 1024); // no warnings
>   }
>


That testcase and the ObjCMessageExpr can go together in another patch where we 
fix visiting of non-literal-type expressions. This patch is really about 
inconsistent visiting of the arguments of a CallExpr.

There's a problem with this patch, we sometimes revisit nodes leading to 
exponential time. I've written a fix to that locally but it's not upstreamable 
quality yet.


https://reviews.llvm.org/D31839



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31839: make -Winteger-overflow find overflows in function arguments

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky added a comment.

In https://reviews.llvm.org/D31839#722763, @ahatanak wrote:

> Is it possible to fix ObjCMessageExpr too while you are in here?


I looked into this, but it turns out to be different enough to belong in a 
separate patch. An ObjCMessageExpr has void type which means that we bail very 
early in the expression evaluator since void isn't a literal type. I think the 
original design of the code was to turn an Expr* into an APValue, and as we 
push it past that original purpose we're going to need to restructure it a bit.

> I think clang should issue a warning when compiling the following code:
> 
>   @protocol NSObject
>   @end
>   
>   @interface NSObject
>   @end
>   
>   @interface C1 : NSObject
>   - (void)foo:(int)i;
>   @end
>   @implementation C1
>   - (void)foo:(int)i {
>   }
>   @end
>   
>   void test1(C1 *c) {
> [c foo:(4068 * 1024 * 1024)];
>   }




https://reviews.llvm.org/D31839



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31839: make -Winteger-overflow find overflows in function arguments

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky created this revision.

When checkingForOverflow(), look through call arguments (and the callee itself 
if calculated).

Make the statement visitor go through ObjCBoxedExpr by default because it has a 
single subexpr node. Don't do that when looking for a pointer object, stop 
because the boxed expr is the pointer.


https://reviews.llvm.org/D31839

Files:
  include/clang/Sema/Sema.h
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExprObjC.cpp
  test/OpenMP/distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/distribute_simd_aligned_messages.cpp
  test/OpenMP/for_simd_aligned_messages.cpp
  test/OpenMP/parallel_for_simd_aligned_messages.cpp
  test/OpenMP/simd_aligned_messages.cpp
  test/OpenMP/target_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/target_simd_aligned_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
  test/OpenMP/taskloop_simd_aligned_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
  test/OpenMP/teams_distribute_simd_aligned_messages.cpp
  test/SemaCXX/integer-overflow.cpp

Index: test/SemaCXX/integer-overflow.cpp
===
--- test/SemaCXX/integer-overflow.cpp
+++ test/SemaCXX/integer-overflow.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only -std=gnu++98 -triple x86_64-pc-linux-gnu
+// RUN: %clang_cc1 %s -verify -std=gnu++98 -triple x86_64-pc-linux-gnu
 typedef unsigned long long uint64_t;
 typedef unsigned int uint32_t;
 
@@ -166,6 +166,9 @@
   uint64_t a[10];
   a[4608 * 1024 * 1024] = 1i;
 
+// expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
+  f0(4608 * 1024 * 1024);
+
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with type 'int'}}
   return ((4608 * 1024 * 1024) + ((uint64_t)(4608 * 1024 * 1024)));
 }
Index: test/OpenMP/teams_distribute_simd_aligned_messages.cpp
===
--- test/OpenMP/teams_distribute_simd_aligned_messages.cpp
+++ test/OpenMP/teams_distribute_simd_aligned_messages.cpp
@@ -123,9 +123,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target
Index: test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
===
--- test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
+++ test/OpenMP/teams_distribute_parallel_for_simd_aligned_messages.cpp
@@ -123,9 +123,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target
Index: test/OpenMP/taskloop_simd_aligned_messages.cpp
===
--- test/OpenMP/taskloop_simd_aligned_messages.cpp
+++ test/OpenMP/taskloop_simd_aligned_messages.cpp
@@ -107,9 +107,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
   #pragma omp taskloop simd aligned // expected-error {{expected '(' after 'aligned'}}
   for (I k = 0; k < argc; ++k) ++k;
Index: test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
===
--- test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
+++ test/OpenMP/target_teams_distribute_simd_aligned_messages.cpp
@@ -110,9 +110,8 @@
 template int foomain(I argc, C **argv) {
   I e(argc);
   I g(argc);
-  int i; // expected-note {{declared here}} expected-note {{'i' defined here}}
-  // expected-note@+2 {{declared here}}
-  // expected-note@+1 {{reference to 'i' is not a constant expression}}
+  int i; // expected-note {{'i' defined here}}
+  // expected-note@+1 {{declared here}}
   int  = i;
 
 #pragma omp target teams distribute simd aligned // expected-error {{expected '(' after 'aligned'}}
Index: test/OpenMP/target_teams_distribute_parallel_for_simd_aligned_messages.cpp
===
--- 

[PATCH] D29915: emit constant expression for new expression array size if it is one

[Nick Lewycky via Phabricator via cfe-commits]

nlewycky created this revision.

When the new expr's array size is an ICE, emit it as a constant expression.

This bypasses integer sanitization checks which are redundant on the expression 
since it's been checked by Sema. Fixes a clang codegen assertion on "void 
test() { new int[0+1]{0}; }" when building with 
-fsanitize=signed-integer-overflow.


https://reviews.llvm.org/D29915

Files:
  lib/CodeGen/CGExprCXX.cpp
  test/CodeGenCXX/new-array-init.cpp


Index: test/CodeGenCXX/new-array-init.cpp
===
--- test/CodeGenCXX/new-array-init.cpp
+++ test/CodeGenCXX/new-array-init.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -triple i386-unknown-unknown %s -emit-llvm -o - 
| FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple i386-unknown-unknown %s -emit-llvm 
-fsanitize=signed-integer-overflow -o - | FileCheck --check-prefix=SIO %s
 
 // CHECK: @[[ABC4:.*]] = {{.*}} constant [4 x i8] c"abc\00"
 // CHECK: @[[ABC15:.*]] = {{.*}} constant [15 x i8] c"abc\00\00\00\00
@@ -116,3 +117,9 @@
   struct Aggr { int a, b; };
   new Aggr[n] { 1, 2, 3 };
 }
+
+// SIO-LABEL: define void @_Z14constexpr_testv
+void constexpr_test() {
+  // SIO: call i8* @_Zna{{.}}(i32 4)
+  new int[0+1]{0};
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -659,7 +659,10 @@
   // Emit the array size expression.
   // We multiply the size of all dimensions for NumElements.
   // e.g for 'int[2][3]', ElemType is 'int' and NumElements is 6.
-  numElements = CGF.EmitScalarExpr(e->getArraySize());
+  numElements = CGF.CGM.EmitConstantExpr(e->getArraySize(),
+ CGF.getContext().getSizeType(), );
+  if (!numElements)
+numElements = CGF.EmitScalarExpr(e->getArraySize());
   assert(isa(numElements->getType()));
 
   // The number of elements can be have an arbitrary integer type;


Index: test/CodeGenCXX/new-array-init.cpp
===
--- test/CodeGenCXX/new-array-init.cpp
+++ test/CodeGenCXX/new-array-init.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -triple i386-unknown-unknown %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple i386-unknown-unknown %s -emit-llvm -fsanitize=signed-integer-overflow -o - | FileCheck --check-prefix=SIO %s
 
 // CHECK: @[[ABC4:.*]] = {{.*}} constant [4 x i8] c"abc\00"
 // CHECK: @[[ABC15:.*]] = {{.*}} constant [15 x i8] c"abc\00\00\00\00
@@ -116,3 +117,9 @@
   struct Aggr { int a, b; };
   new Aggr[n] { 1, 2, 3 };
 }
+
+// SIO-LABEL: define void @_Z14constexpr_testv
+void constexpr_test() {
+  // SIO: call i8* @_Zna{{.}}(i32 4)
+  new int[0+1]{0};
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -659,7 +659,10 @@
   // Emit the array size expression.
   // We multiply the size of all dimensions for NumElements.
   // e.g for 'int[2][3]', ElemType is 'int' and NumElements is 6.
-  numElements = CGF.EmitScalarExpr(e->getArraySize());
+  numElements = CGF.CGM.EmitConstantExpr(e->getArraySize(),
+ CGF.getContext().getSizeType(), );
+  if (!numElements)
+numElements = CGF.EmitScalarExpr(e->getArraySize());
   assert(isa(numElements->getType()));
 
   // The number of elements can be have an arbitrary integer type;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits