[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

FYI the fix for the 1-bit APSInt issue is in 
https://reviews.llvm.org/D35450#change-ifYnQ3IlVso


https://reviews.llvm.org/D45517



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

[PATCH] D33308: [analyzer]: Improve test handling with multiple constraint managers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@dcoughlin @zaks.anna @NoQ @xazax.hun Ping, I'd appreciate it if I could get a 
review for this (https://reviews.llvm.org/D33308), 
https://reviews.llvm.org/D28955, https://reviews.llvm.org/D28953, and 
https://reviews.llvm.org/D28954. Rebasing and fixing up these commits is fairly 
time consuming.


https://reviews.llvm.org/D33308



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

[PATCH] D33308: [analyzer]: Improve test handling with multiple constraint managers

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305480: [analyzer]: Improve test handling with multiple 
constraint managers (authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D33308?vs=99394&id=102682#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33308

Files:
  cfe/trunk/test/Analysis/analyzer_test.py


Index: cfe/trunk/test/Analysis/analyzer_test.py
===
--- cfe/trunk/test/Analysis/analyzer_test.py
+++ cfe/trunk/test/Analysis/analyzer_test.py
@@ -5,24 +5,39 @@
 class AnalyzerTest(lit.formats.ShTest):
 
 def execute(self, test, litConfig):
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=range')
+results = []
 
-if result.code == lit.Test.FAIL:
-return result
+# Parse any test requirements ('REQUIRES: ')
+saved_test = test
+lit.TestRunner.parseIntegratedTestScript(test)
+
+if 'z3' not in test.requires:
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=range'))
+
+if results[-1].code == lit.Test.FAIL:
+return results[-1]
 
 # If z3 backend available, add an additional run line for it
 if test.config.clang_staticanalyzer_z3 == '1':
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=z3 
-DANALYZER_CM_Z3'))
 
-return result
+# Combine all result outputs into the last element
+for x in results:
+if x != results[-1]:
+results[-1].output = x.output + results[-1].output
+
+if results:
+return results[-1]
+return lit.Test.Result(lit.Test.UNSUPPORTED,
+"Test requires the following unavailable features: z3")
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
 saved_substitutions = list(test.config.substitutions)
 test.config.substitutions.append(('%analyze', substitution))
 result = lit.TestRunner.executeShTest(test, litConfig,
-  self.execute_external)
+self.execute_external)
 test.config.substitutions = saved_substitutions
 
 return result


Index: cfe/trunk/test/Analysis/analyzer_test.py
===
--- cfe/trunk/test/Analysis/analyzer_test.py
+++ cfe/trunk/test/Analysis/analyzer_test.py
@@ -5,24 +5,39 @@
 class AnalyzerTest(lit.formats.ShTest):
 
 def execute(self, test, litConfig):
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=range')
+results = []
 
-if result.code == lit.Test.FAIL:
-return result
+# Parse any test requirements ('REQUIRES: ')
+saved_test = test
+lit.TestRunner.parseIntegratedTestScript(test)
+
+if 'z3' not in test.requires:
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=range'))
+
+if results[-1].code == lit.Test.FAIL:
+return results[-1]
 
 # If z3 backend available, add an additional run line for it
 if test.config.clang_staticanalyzer_z3 == '1':
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3'))
 
-return result
+# Combine all result outputs into the last element
+for x in results:
+if x != results[-1]:
+results[-1].output = x.output + results[-1].output
+
+if results:
+return results[-1]
+return lit.Test.Result(lit.Test.UNSUPPORTED,
+"Test requires the following unavailable features: z3")
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
 saved_substitutions = list(test.config.substitutions)
 test.config.substitutions.append(('%analyze', substitution))
 result = lit.TestRunner.executeShTest(test, litConfig,
-  self.execute_external)
+self.execute_external)
 test.config.substitutions = saved_substitutions
 
 return result
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:356
QualType ResultTy) {
-  if (!State->isTainted(RHS) && !State->isTainted(LHS))
-return UnknownVal();

zaks.anna wrote:
> I am concerned that removing the guard will regress performance in the 
> vanilla case. (Note that Z3 support as well as taint are not on by default.)
> 
> I am curious how much of the regression you've measured could be gained back 
> if we make this conditional.
To clarify, the changes made in this patch aren't specific to Z3 support, 
especially simplifying `SymbolCast` and `IntSymExpr`. With the exception of 
`PR24184.cpp` and `plist-macros.cpp`, all testcases pass with both the default 
and Z3 constraint managers. However, creating additional constraints does have 
performance overhead, and it may be useful to consider the parameters for 
gating this functionality.

On a combined execution (Range + Z3) through the testcases, except the two 
mentioned above, the runtime is 327 sec with this patch applied, and 195 sec 
without this patch applied. On a separate execution through the testcases with 
only the Z3 constraint manager, I get runtimes 320 and 191, respectively.

For testing purposes, I also tried the following code, which has combined 
runtime 311 sec, but loses the accuracy improvements with the Range constraint 
manager on `bitwise-ops.c`, `conditional-path-notes.c`, `explain-svals.cpp`, 
and `std-c-library-functions.c`.

```
ConstraintManager &CM = getStateManager().getConstraintManager();
if (!State->isTainted(RHS) && !State->isTainted(LHS) && !CM.isZ3())
```



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:363
   // instead of generating an Unknown value and propagate the taint info to it.
-  const unsigned MaxComp = 1; // 10 28X
 

zaks.anna wrote:
> Reducing the MaxComp is going to regress taint analysis..
> 
> > I've updated this revision to account for the recent SVal simplification 
> > commit by @NoQ, 
> 
> Which commit introduced the regression?
> 
> > but now there is an exponential blowup problem that prevents testcase 
> > PR24184.cpp from terminating, 
> 
> What triggers the regression? Removing the if statement above? Does the 
> regression only effect the Z3 "mode" (I am guessing not since you said "due 
> to an interaction between Simplifier::VisitNonLocSymbolVal() and 
> SValBuilder::makeSymExprValNN()")? 
> 
> Reducing the MaxComp is going to regress taint analysis..

I think the original intention was to increase `MaxComp`, not decrease it, but 
I will restore the original value here.

> What triggers the regression? Removing the if statement above? Does the 
> regression only effect the Z3 "mode"

No, the regression isn't specifically due to this code, but with @NoQ 's patch 
for `SVal` simplification (rL300178), and this commit extending it to handle 
`SymbolCast` and `IntSymExpr`, the cast of `ST *` used in the loop of case 3 of 
PR24184.cpp becomes "simplified" (inlined) repeatedly on each recursive 
iteration through `Simplifier::VisitNonLocSymbolVal()` -> 
`SValBuilder::makeSymExprValNN()`, causing a big slowdown in runtime 
performance.

The naive way to prevent it is to drop `MaxComp` (but this isn't reasonable, 
since it needs to be absurdly low, e.g. `10`). Alternatively, simplification 
for `SymbolCast` can be dropped from this commit (but it will eliminate some of 
the other analysis improvements), or, most likely, introduce another parameter 
to reduce recursion between these two functions.


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> Can we drop computing these for some expressions that we know the 
> RangeConstraintManager will not utilize?

It's possible, though I'm not sure what the actual limitations of the 
RangeConstraintManager are, since there are a lot of intermediate steps that 
attempt to transform unsupported expressions into supported ones.

> We could implement the TODO described below and possibly also lower the 
> MaxComp value. This means that instead of keeping a complex expression and 
> constraints on every symbol used in that expression, we would conjure a new 
> symbol and associate a new constrain derived from the expression with it. 
> (Would this strategy also work for the Z3 case?)

I think it's feasible, but would probably require some more to code to handle 
`SymbolConjured` (right now all `SymbolData` are treated as variables).

> Would it help to decrease 100 to 10 here?

Yes, changing it to 10 eliminates the excessive recursion; combined runtime 
drops to 282 sec on the testcases (~8 sec for Range only, ~271 sec for Z3 only; 
doesn't sum due to separate executions). This appears to be the most 
straightforward solution.

> (2) With RangeConstraintManager, simplification is not entirely idempotent: 
> we may simplify a symbol further when one of its sub-symbols gets constrained 
> to a constant in a new state. However, apart from that, we might be able to 
> avoid re-simplifying the same symbol by caching results based on the (symbol, 
> state's constraint data) pair. Probably it may work with Z3 as well.

Yeah, that would also fix this issue. In general, I think there's plenty of 
room for improvements from caching, especially for queries to e.g. 
`getSymVal()`.


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 103239.
ddcc added a comment.

Rebase, decrease simplification complexity


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
 
 
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col25
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-N

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I forgot to mention that the only remaining test failure is on 
`plist-macros.cpp`; there is a `Assuming condition is true` path note that only 
appears with the RangeConstraintManager but not with Z3ConstraintManager, and I 
can't `#ifdef` it because the annotations are checked by `FileCheck`.


https://reviews.llvm.org/D28953



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D35450#1116535, @george.karpenkov wrote:

> @ddcc Hi, are you still interested in landing the fixes associated with this 
> patch? I can take a look as I'm currently reviewing 
> https://reviews.llvm.org/D45517, but it is likely that the patch would need 
> to be changed substantially before it could land.


@george.karpenkov Yeah, I've got this and a couple of other patches still 
awaiting review. If it's easier, I can also split out the APSInt fix into a 
separate patch.


https://reviews.llvm.org/D35450



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

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added reviewers: george.karpenkov, NoQ.
Herald added subscribers: a.sidorin, szepet, xazax.hun.

Clang does not have a corresponding QualType for a 1-bit APSInt, so use the 
BoolTy and extend the APSInt. Split from https://reviews.llvm.org/D35450.


Repository:
  rC Clang

https://reviews.llvm.org/D47603

Files:
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp

Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -987,6 +987,10 @@
   // TODO: Refactor to put elsewhere
   QualType getAPSIntType(const llvm::APSInt &Int) const;
 
+  // Fix the input APSInt if it is has a bitwidth of 1, and set the type.
+  const llvm::APSInt &fixAPSInt(const llvm::APSInt &Int, llvm::APSInt &NewInt,
+QualType *Ty = nullptr) const;
+
   // Perform implicit type conversion on binary symbolic expressions.
   // May modify all input parameters.
   // TODO: Refactor to use built-in conversion functions
@@ -1037,28 +1041,29 @@
   QualType RetTy;
   // The expression may be casted, so we cannot call getZ3DataExpr() directly
   Z3Expr Exp = getZ3Expr(Sym, &RetTy);
-
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType LTy;
+  llvm::APSInt NewFromInt;
+  Z3Expr FromExp = Z3Expr::fromAPSInt(fixAPSInt(From, NewFromInt, isSignedIntegerOrEnumerationType()));
 }
 
 ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State,
@@ -1145,8 +1150,8 @@
 
 const llvm::APSInt *Z3ConstraintManager::getSymVal(ProgramStateRef State,
SymbolRef Sym) const {
-  BasicValueFactory &BV = getBasicVals();
-  ASTContext &Ctx = BV.getContext();
+  BasicValueFactory &BVF = getBasicVals();
+  ASTContext &Ctx = BVF.getContext();
 
   if (const SymbolData *SD = dyn_cast(Sym)) {
 QualType Ty = Sym->getType();
@@ -1180,7 +1185,7 @@
   return nullptr;
 
 // This is the only solution, store it
-return &BV.getValue(Value);
+return &BVF.getValue(Value);
   } else if (const SymbolCast *SC = dyn_cast(Sym)) {
 SymbolRef CastSym = SC->getOperand();
 QualType CastTy = SC->getType();
@@ -1191,7 +1196,7 @@
 const llvm::APSInt *Value;
 if (!(Value = getSymVal(State, CastSym)))
   return nullptr;
-return &BV.Convert(SC->getType(), *Value);
+return &BVF.Convert(SC->getType(), *Value);
   } else if (const BinarySymExpr *BSE = dyn_cast(Sym)) {
 const llvm::APSInt *LHS, *RHS;
 if (const SymIntExpr *SIE = dyn_cast(BSE)) {
@@ -1215,7 +1220,7 @@
 QualType LTy = getAPSIntType(*LHS), RTy = getAPSIntType(*RHS);
 doIntTypeConversion(
 ConvertedLHS, LTy, ConvertedRHS, RTy);
-return BV.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
+return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
   }
 
   llvm_unreachable("Unsupported expression to get symbol value!");
@@ -1342,13 +1347,13 @@
   BinaryOperator::Opcode Op = BSE->getOpcode();
 
   if (const SymIntExpr *SIE = dyn_cast(BSE)) {
-RTy = getAPSIntType(SIE->getRHS());
+llvm::APSInt NewRInt;
 Z3Expr LHS = getZ3SymExpr(SIE->getLHS(), getRHS());
+Z3Expr RHS = Z3Expr::fromAPSInt(fixAPSInt(SIE->getRHS(), NewRInt, &RTy));
 return getZ3BinExpr(LHS, LTy, Op, RHS, RTy, RetTy);
   } else if (const IntSymExpr *ISE = dyn_cast(BSE)) {
-LTy = getAPSIntType(ISE->getLHS());
-Z3Expr LHS = Z3Expr::fromAPSInt(ISE->getLHS());
+llvm::APSInt NewLInt;
+Z3Expr LHS = Z3Expr::fromAPSInt(fixAPSInt(ISE->getLHS

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D47603#1118138, @vlad.tsyrklevich wrote:

> In https://reviews.llvm.org/D47603#1118106, @george.karpenkov wrote:
>
> > Would it be possible to add tests? I know we have very few unit tests, but 
> > I assume you could actually use an integration test to exercise this path?
>
>
> I tested this change and it fixes PR37622. There's a simple crash reproducer 
> included there.


Cool, thanks for the repro! It's been long enough since I've touched this code 
that I don't recall the original failing testcase. I'll add the test to this 
revision.




Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1044
   Z3Expr Exp = getZ3Expr(Sym, &RetTy);
-
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType LTy;
+  llvm::APSInt NewFromInt;

george.karpenkov wrote:
> What does `L` stand for here? It's confusing because `L/R` usually stand for 
> left/right-hand-side in this context.
They correspond to the inferred left/right hand-side inferred types, but inside 
the subsequent Z3Expr `LHS` and `RHS` variables. This is confusing, so I'll 
rename them to `FromTy` and `ToTy`.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1154
+  BasicValueFactory &BVF = getBasicVals();
+  ASTContext &Ctx = BVF.getContext();
 

george.karpenkov wrote:
> that's a separate change, but OK
Yeah, this is one of several small miscellaneous changes that didn't make the 
original commit. It seemed a bit excessive to open separate revisions for each, 
so I've just been merging them into the next patch. I'm not sure which is 
preferable?



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1426
+*Ty = getAPSIntType(Int);
+  return Int;
+}

george.karpenkov wrote:
> It's redundant to mutate the argument passed by reference and also return it.
> Could we take a single `APSInt` parameter by value and return 
> `std::pair` ?
Sure.


Repository:
  rC Clang

https://reviews.llvm.org/D47603



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

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 149356.
ddcc added a comment.

Add test, address comments


Repository:
  rC Clang

https://reviews.llvm.org/D47603

Files:
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/apsint.c

Index: test/Analysis/apsint.c
===
--- /dev/null
+++ test/Analysis/apsint.c
@@ -0,0 +1,7 @@
+// REQUIRES: z3
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux-gnu -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+_Bool a() {
+  return !({ a(); });
+}
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -987,6 +987,9 @@
   // TODO: Refactor to put elsewhere
   QualType getAPSIntType(const llvm::APSInt &Int) const;
 
+  // Get the QualTy for the input APSInt, and fix it if it has a bitwidth of 1.
+  std::pair fixAPSInt(const llvm::APSInt &Int) const;
+
   // Perform implicit type conversion on binary symbolic expressions.
   // May modify all input parameters.
   // TODO: Refactor to use built-in conversion functions
@@ -1038,27 +1041,31 @@
   // The expression may be casted, so we cannot call getZ3DataExpr() directly
   Z3Expr Exp = getZ3Expr(Sym, &RetTy);
 
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType FromTy;
+  llvm::APSInt NewFromInt;
+  std::tie(NewFromInt, FromTy) = fixAPSInt(From);
+  Z3Expr FromExp = Z3Expr::fromAPSInt(NewFromInt);
 
   // Construct single (in)equality
   if (From == To)
 return assumeZ3Expr(State, Sym,
 getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE,
- FromExp, RTy, nullptr));
+ FromExp, FromTy, nullptr));
 
+  QualType ToTy;
+  llvm::APSInt NewToInt;
+  std::tie(NewToInt, ToTy) = fixAPSInt(To);
+  Z3Expr ToExp = Z3Expr::fromAPSInt(NewToInt);
+  assert(FromTy == ToTy && "Range values have different types!");
   // Construct two (in)equalities, and a logical and/or
-  Z3Expr LHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr);
+  Z3Expr LHS = getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp,
+FromTy, nullptr);
   Z3Expr RHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr);
+  getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, ToTy, nullptr);
   return assumeZ3Expr(
   State, Sym,
-  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy));
+  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS,
+RetTy->isSignedIntegerOrEnumerationType()));
 }
 
 ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State,
@@ -1145,8 +1152,8 @@
 
 const llvm::APSInt *Z3ConstraintManager::getSymVal(ProgramStateRef State,
SymbolRef Sym) const {
-  BasicValueFactory &BV = getBasicVals();
-  ASTContext &Ctx = BV.getContext();
+  BasicValueFactory &BVF = getBasicVals();
+  ASTContext &Ctx = BVF.getContext();
 
   if (const SymbolData *SD = dyn_cast(Sym)) {
 QualType Ty = Sym->getType();
@@ -1180,7 +1187,7 @@
   return nullptr;
 
 // This is the only solution, store it
-return &BV.getValue(Value);
+return &BVF.getValue(Value);
   } else if (const SymbolCast *SC = dyn_cast(Sym)) {
 SymbolRef CastSym = SC->getOperand();
 QualType CastTy = SC->getType();
@@ -1191,7 +1198,7 @@
 const llvm::APSInt *Value;
 if (!(Value = getSymVal(State, CastSym)))
   return nullptr;
-return &BV.Convert(SC->getType(), *Value);
+return &BVF.Convert(SC->getType(), *Value);
   } else if (const BinarySymExpr *BSE = dyn_cast(Sym)) {
 const llvm::APSInt *LHS, *RHS;
 if (const SymIntExpr *SIE = dyn_cast(BSE)) {
@@ -1215,7 +1222,7 @@
 QualType LTy = getAPSIntType(*LHS), RTy = getAPSIntType(*RHS);
 doIntTypeConversion(
 ConvertedLHS, LTy, ConvertedRHS, RTy);
-return BV.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
+return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
   }
 
   llvm_unreachable("Unsupported expression to get symbol value!");
@@ -1342,13 +1349,15 @@
   BinaryOperator::Opcode Op = BSE->getOpcode();
 
   if (const SymIntExpr *SIE = dyn_cast(BSE)) {
-RTy = getAPSIntType(SIE->getRHS());
 Z3Expr LHS = getZ3SymExpr(SIE->getLHS(), getRHS());
+llvm::APSInt NewRInt;
+std::tie(NewRInt, RTy) = fixAPSInt(SIE->getRHS());
+Z3Expr RHS = Z3Expr::fromAPSInt(NewRInt);
 return

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333704: [analyzer] fix bug with 1-bit APSInt types in 
Z3ConstraintManager (authored by ddcc, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47603

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  cfe/trunk/test/Analysis/apsint.c

Index: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -987,6 +987,9 @@
   // TODO: Refactor to put elsewhere
   QualType getAPSIntType(const llvm::APSInt &Int) const;
 
+  // Get the QualTy for the input APSInt, and fix it if it has a bitwidth of 1.
+  std::pair fixAPSInt(const llvm::APSInt &Int) const;
+
   // Perform implicit type conversion on binary symbolic expressions.
   // May modify all input parameters.
   // TODO: Refactor to use built-in conversion functions
@@ -1038,27 +1041,31 @@
   // The expression may be casted, so we cannot call getZ3DataExpr() directly
   Z3Expr Exp = getZ3Expr(Sym, &RetTy);
 
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType FromTy;
+  llvm::APSInt NewFromInt;
+  std::tie(NewFromInt, FromTy) = fixAPSInt(From);
+  Z3Expr FromExp = Z3Expr::fromAPSInt(NewFromInt);
 
   // Construct single (in)equality
   if (From == To)
 return assumeZ3Expr(State, Sym,
 getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE,
- FromExp, RTy, nullptr));
+ FromExp, FromTy, nullptr));
 
+  QualType ToTy;
+  llvm::APSInt NewToInt;
+  std::tie(NewToInt, ToTy) = fixAPSInt(To);
+  Z3Expr ToExp = Z3Expr::fromAPSInt(NewToInt);
+  assert(FromTy == ToTy && "Range values have different types!");
   // Construct two (in)equalities, and a logical and/or
-  Z3Expr LHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr);
+  Z3Expr LHS = getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp,
+FromTy, nullptr);
   Z3Expr RHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr);
+  getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, ToTy, nullptr);
   return assumeZ3Expr(
   State, Sym,
-  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy));
+  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS,
+RetTy->isSignedIntegerOrEnumerationType()));
 }
 
 ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State,
@@ -1145,8 +1152,8 @@
 
 const llvm::APSInt *Z3ConstraintManager::getSymVal(ProgramStateRef State,
SymbolRef Sym) const {
-  BasicValueFactory &BV = getBasicVals();
-  ASTContext &Ctx = BV.getContext();
+  BasicValueFactory &BVF = getBasicVals();
+  ASTContext &Ctx = BVF.getContext();
 
   if (const SymbolData *SD = dyn_cast(Sym)) {
 QualType Ty = Sym->getType();
@@ -1180,7 +1187,7 @@
   return nullptr;
 
 // This is the only solution, store it
-return &BV.getValue(Value);
+return &BVF.getValue(Value);
   } else if (const SymbolCast *SC = dyn_cast(Sym)) {
 SymbolRef CastSym = SC->getOperand();
 QualType CastTy = SC->getType();
@@ -1191,7 +1198,7 @@
 const llvm::APSInt *Value;
 if (!(Value = getSymVal(State, CastSym)))
   return nullptr;
-return &BV.Convert(SC->getType(), *Value);
+return &BVF.Convert(SC->getType(), *Value);
   } else if (const BinarySymExpr *BSE = dyn_cast(Sym)) {
 const llvm::APSInt *LHS, *RHS;
 if (const SymIntExpr *SIE = dyn_cast(BSE)) {
@@ -1215,7 +1222,7 @@
 QualType LTy = getAPSIntType(*LHS), RTy = getAPSIntType(*RHS);
 doIntTypeConversion(
 ConvertedLHS, LTy, ConvertedRHS, RTy);
-return BV.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
+return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
   }
 
   llvm_unreachable("Unsupported expression to get symbol value!");
@@ -1342,13 +1349,15 @@
   BinaryOperator::Opcode Op = BSE->getOpcode();
 
   if (const SymIntExpr *SIE = dyn_cast(BSE)) {
-RTy = getAPSIntType(SIE->getRHS());
 Z3Expr LHS = getZ3SymExpr(SIE->getLHS(), getRHS());
+llvm::APSInt NewRInt;
+std::tie(NewRInt, RTy) = fixAPSInt(SIE->getRHS());
+Z3Expr RHS = Z3Expr::fromAPSInt(NewRInt);
 return getZ3BinExpr(LHS, LTy, Op, RHS, RTy, RetTy);
   } else if (const IntSymExpr *ISE = dyn_cast(BSE)) {
-LTy = getAPSIntType(ISE->getLHS

[PATCH] D47617: [Analyzer] Fix Z3ConstraintManager crash (PR37646)

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D47617#1119257, @george.karpenkov wrote:

> LGTM with a nit on a test name.


Same.

In https://reviews.llvm.org/D47617#1119268, @NoQ wrote:

> Also does this test need to be z3-specific? We would also not like to crash 
> here without z3.


I had the same though originally about the `REQUIRES` line. Since this code 
path is very specific to Z3ConstraintManager, it didn't seem useful to run the 
test on normal buildbots. But I have no preference either way.


Repository:
  rC Clang

https://reviews.llvm.org/D47617



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

[PATCH] D47726: [Analyzer][Z3] Test fixes for Z3 constraint manager

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a subscriber: dcoughlin.
ddcc added a comment.

In https://reviews.llvm.org/D47726#1121395, @george.karpenkov wrote:

> - Do all tests for Z3 run when LLVM is configured to use Z3? I'm not sure if 
> that's the right thing: do all tests start to take 10x time to run once Z3 is 
> enabled?


The test approach is what @dcoughlin suggested in 
https://reviews.llvm.org/D28952, where the tests are run using each constraint 
manager that is available. At that time, he measured an increase in test time 
from 25s to 90s (see https://reviews.llvm.org/D28952#686284).

> - Is anyone interested in running a buildbot running those?

@dcoughlin mentioned that he (Apple?) would set up a buildbot for this, but I 
don't know if that happened.


Repository:
  rC Clang

https://reviews.llvm.org/D47726



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

[PATCH] D47726: [Analyzer][Z3] Test fixes for Z3 constraint manager

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> @ddcc  To be completely honest, I see a few design issues with the current 
> implementation of Z3 backend,
>  the main one being that it checks satisfiability after every single exploded 
> node.
>  To the best of my knowledge, reasonable scalability would not be achieved 
> with such an approach,
>  and I'm not sure how feasible would it be to change it without rewriting 
> most of the checkers.

I agree, though a number of these are limitations in CSA, and not specifically 
the backend.

> Thus we currently do not plan to set up a Z3 bot, but if you wish to maintain 
> we certainly can provide pointers on how this can be done.

I don't plan to set one up either. Just compiling clang/llvm is already very 
resource intensive.

> What do you think if we introduce `ninja check-clang-analyzer` to run the 
> analyzer tests, and `ninja check-clang-analyzer-z3` to run all the analyzer 
> tests with Z3?
>  [there might be another ninja target for running all analyzer tests, I just 
> don't remember which one. But at the end of the day there should be away to 
> run analyzer tests without Z3 even when Z3 is linked in]

Sounds good.


Repository:
  rC Clang

https://reviews.llvm.org/D47726



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> Sorry for the massive delay, but I just updated the `FindZ3` script to 
> retrieve the version from the lib. I changed it to use `try_run` instead of 
> `try_compile` so we can get the version number.
>
> I tried to use @brzycki code to get the version from the header, however, 
> it's not working for Z3 4.8.4. In Z3 4.8.3 the FULL_VERSION is a nice `"Z3 
> 4.8.3.0"` but in version 4.8.4 it's `"Z3 4.8.4.10272 d6df51951f4c master 
> z3-4.8.4"` and cmake fails with the following message:


Thanks for making the changes! Is this being parsed from e.g. 
`/usr/include/z3_version.h`? I checked their code, and I have a local build of 
z3 4.8.5.0, but I'm not seeing those changes to the version string:

  #define Z3_FULL_VERSION"Z3 4.8.5.0"


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

The only relevant commit that I can find is 
https://github.com/Z3Prover/z3/commit/2cb4223979cc94e2ebc4e49a9e83adbdcd2b6979 
, but it first landed in z3 4.6.0. It looks like it's specific to CMake though, 
so is it different if you use the python build? I haven't tried the CMake build.


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> 2. Instead of parsing `Z3_FULL_VERSION`, we can parse `Z3_MAJOR_VERSION`, 
> `Z3_MINOR_VERSION` and `Z3_BUILD_NUMBER` which are also available in the same 
> header.


Sounds like this might be the way to go, since the format seems to be more 
stable.


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1431935 , @delcypher wrote:

> Would one of you be able to file a bug against Z3 to fix this? I am no longer 
> in a position to contribute to Z3 so I can't do this.


I've opened https://github.com/Z3Prover/z3/issues/2184 .

In D54978#1431936 , @delcypher wrote:

> This output means you built Z3 from source that was not in a git repository. 
> In this case the header file should look the same for both Z3's CMake and 
> Python build systems.


That's strange, I have been building from a git repository.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> 2. Instead of parsing `Z3_FULL_VERSION`, we can parse `Z3_MAJOR_VERSION`, 
> `Z3_MINOR_VERSION` and `Z3_BUILD_NUMBER` which are also available in the same 
> header.


Since the differences in version string depending on the build system are 
intended behavior, it seems (2) would be preferable?


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: llvm/cmake/modules/FindZ3.cmake:92
+
+  set(Z3_VERSION_STRING ${Z3_MAJOR}.${Z3_MINOR}.${Z3_MAJOR})
+  unset(z3_version_str)

Should be ${Z3_MAJOR}.${Z3_MINOR}.${Z3_BUILD_NUMBER}


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1433646 , @mikhail.ramalho 
wrote:

> To fix that, I changed the script slightly: we first try to dinamically get 
> the Z3's version, if we fail and we are cross compiling, then we try to parse 
> the headers. Right now, it should just work but I'd like to add a message to 
> warn the user that we found the lib but we don't know if it'll link.


Thanks, sounds good to me.

In D54978#1394613 , @thakis wrote:

> mikhail.ramalho: My guess is that we need to pass on LLVM_OPTIMIZED_TABLEGEN 
> to the child cmake invocation in 
> http://llvm-cs.pcc.me.uk/cmake/modules/CrossCompile.cmake#50 (like we pass on 
> a few other variables) to fix this.


Do you know if this configuration builds fine now?


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added subscribers: delcypher, ddcc.
ddcc added a comment.

The likely reason for this versioning problem is that the current versioning 
implementation in FindZ3.cmake is best-effort only: among other conditions, if 
the z3 binary is available, it will execute it and parse out the version number 
from standard output, otherwise, it fails silently. This is because upstream Z3 
doesn't define the API version in a header file, and uses a homebrew 
python-based build system that also doesn't export the version. I believe 
@delcypher 's CMake-based build system for upstream Z3 might solve this 
problem, but I haven't looked at it in a long time, and it it appears to be 
stalled ( https://github.com/Z3Prover/z3/issues/986 ).

I also agree that more notice about this patch would have been appreciated; I 
didn't hear it until I read LLVM weekly today.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1395268 , @brzycki wrote:

> I think I found why others are struggling to recreate this issue. I did not 
> have the package `z3/bionic` installed. This provides the `/usr/bin/z3` 
> executable which `llvm/cmake/modules/FindZ3.cmake` relies upon to determine 
> the correct `Z3_VERSION_STRING`.


Yeah, that's what I meant by best-effort only. Due to upstream Z3's design, 
without the binary, there is no easy way to retrieve the current installed 
version, so when I originally wrote the Z3 integration, it seemed fine to let 
the version check pass. Given the issues that have come up, it might make more 
sense to at least emit a warning, or explicitly fail if the version can't be 
determined, and prompt the user to install the binary.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1395425 , @brzycki wrote:

> This works for everything I could throw at it. If you think it's reasonable I 
> can open another ticket and have the code reviewed as a separate fix.


Sounds good to me, although I think it'd be good to emit a warning or at least 
a message about assuming the version due to a missing executable.

In D54978#1395561 , @brzycki wrote:

> I looked at the Z3 source and they do have a `z3_version.h` file now and was 
> added in version 4.4.2.0. You may be able to use the header directly, but I'm 
> not sure how `find_package()` parses for library versions and if this is 
> useful or not.  The generated header is named `src/util/version.h` in this 
> initial commit:
>  
> https://github.com/Z3Prover/z3/commit/251527603df01904f70ed884f8695fedab5caed9


You're right, it looks like it was originally internal-only, but them became 
exposed as part of the interface with the second commit, starting around the 
release of z3 4.8.1. It's been a while since I've used CMake, but perhaps 
something like this:

  if(Z3_INCLUDE_DIR AND EXISTS "${Z3_INCLUDE_DIR }/z3_version.h")
file(STRINGS "${Z3_INCLUDE_DIR }/z3_version.h" z3_version_str REGEX 
"^#define[\t ]+Z3_FULL_VERSION[\t ]+\".*\"")
  
string(REGEX REPLACE "^.*Z3_FULL_VERSION[\t ]+\"Z3 ([0-9\.]+)\".*$" "\\1" 
Z3_VERSION_STRING "${z3_version_str}")
unset(z3_version_str)
  endif()



In D54978#1395476 , @mikhail.ramalho 
wrote:

> I'm wondering if we can remove the binary requirement all together: is it 
> possible to run a small program that would return EXIT_SUCCESS if the library 
> is the correct version?


I see other projects do something similar; e.g. 
https://github.com/SRI-CSL/sally/blob/master/cmake/FindZ3.cmake#L20 . I'm less 
fond of that approach because it involves even more moving parts, but then 
again, parsing C header files with regular expressions isn't particularly 
robust either.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1396403 , @brzycki wrote:

> That is almost exactly what I was thinking about this morning. I'd prefer the 
> following since it's more robust for older versions:


The old `version.h` header isn't externally exposed, only the newer 
`z3_version.h` header starting with version 4.8.1. I built a copy of 4.7.1 from 
source, and I don't see it, so unfortunately I think the second check for 
`version.h` is superfluous. Instead, I think it'd be ok to still query the 
executable as the primary, then fallback to this header as the secondary?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1397316 , @mikhail.ramalho 
wrote:

> I just sent the first prototype of the solution. All the magic happens inside 
> the `CHECK_CXX_SOURCE_RUNS` call.


I think hardcoding the version into the binary is too brittle. Instead, your 
program can just print out the current z3 version (much like the current 
execution of the z3 binary), and the remaining logic can remain in CMake, with 
the fallbacks as suggested above by @brzycki.


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

https://reviews.llvm.org/D54978



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364
   if (symLHS && symRHS &&
-  (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
+  (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
 return makeNonLoc(symLHS, Op, symRHS, ResultTy);

zaks.anna wrote:
> As a follow up to the previous version of this patch, I do not think we 
> should set the default complexity limit to 1.
> 
> What is the relation between this limit and the limit in 
> VisitNonLocSymbolVal? If they are related, would it be worthwhile turning 
> these into an analyzer option?
To clarify, the current version of this patch does not modify the `MaxComp` 
parameter.

My understanding is that `MaxComp` is the upper bound for building a new 
`NonLoc` symbol from two children, based on the sum of the number of child 
symbols (complexity) across both children.

In contrast, the limit in `VisitNonLocSymbolVal` (@NoQ, correct me if I'm 
wrong), is the upper bound for recursively evaluating and inlining a `NonLoc` 
symbol, triggered from `simplifySVal` by `evalBinOpNN`. Note that these two 
latter functions indirectly call each other recursively (through `evalBinOp`), 
causing the previous runtime blowup. Furthermore, each call to 
`computeComplexity`will re-iterate through all child symbols of the current 
symbol, but only the first complexity check at the root symbol is actually 
necessary, because by definition the complexity of a child symbol at each 
recursive call is monotonically decreasing.

I think it'd be useful to allow both to be configurable, but I don't see a 
direct relationship between the two.


https://reviews.llvm.org/D35450



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364
   if (symLHS && symRHS &&
-  (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
+  (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
 return makeNonLoc(symLHS, Op, symRHS, ResultTy);

zaks.anna wrote:
> ddcc wrote:
> > zaks.anna wrote:
> > > As a follow up to the previous version of this patch, I do not think we 
> > > should set the default complexity limit to 1.
> > > 
> > > What is the relation between this limit and the limit in 
> > > VisitNonLocSymbolVal? If they are related, would it be worthwhile turning 
> > > these into an analyzer option?
> > To clarify, the current version of this patch does not modify the `MaxComp` 
> > parameter.
> > 
> > My understanding is that `MaxComp` is the upper bound for building a new 
> > `NonLoc` symbol from two children, based on the sum of the number of child 
> > symbols (complexity) across both children.
> > 
> > In contrast, the limit in `VisitNonLocSymbolVal` (@NoQ, correct me if I'm 
> > wrong), is the upper bound for recursively evaluating and inlining a 
> > `NonLoc` symbol, triggered from `simplifySVal` by `evalBinOpNN`. Note that 
> > these two latter functions indirectly call each other recursively (through 
> > `evalBinOp`), causing the previous runtime blowup. Furthermore, each call 
> > to `computeComplexity`will re-iterate through all child symbols of the 
> > current symbol, but only the first complexity check at the root symbol is 
> > actually necessary, because by definition the complexity of a child symbol 
> > at each recursive call is monotonically decreasing.
> > 
> > I think it'd be useful to allow both to be configurable, but I don't see a 
> > direct relationship between the two.
> > To clarify, the current version of this patch does not modify the MaxComp 
> > parameter.
> 
> I know. Also, currently, it is only used in the unsupported taint tracking 
> mode and only for tainted symbols. I would like a different smaller default 
> for all expressions.
Ok. I can make both configurable as part of this patch, with a new default of 
10 for `VisitNonLocSymbolVal`. But I've never used the taint tracking mode, so 
I don't know what would be a reasonable default for `MaxComp`.


https://reviews.llvm.org/D35450



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 113505.
ddcc added a comment.

Rebase, make complexity limits configurable


https://reviews.llvm.org/D35450

Files:
  include/clang/AST/Expr.h
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/loop-unrolling.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,7 +67,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
+  if (index - 1 == 0)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
@@ -87,7 +87,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
+  if (index - 1L == 0L)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
@@ -117,7 +117,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
+  if (index - 1UL == 0L)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc marked 5 inline comments as done.
ddcc added a comment.

All testcases pass, except the issue with `range_casts.c`. The cause is still 
the range intersection discussed in https://reviews.llvm.org/D35450#810469.


https://reviews.llvm.org/D35450



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 113619.
ddcc added a comment.

Rebase, factor out floating-point changes, fix Z3 type bug, support general 
APSInt comparison


https://reviews.llvm.org/D28955

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/PR3991.m
  test/Analysis/dead-stores.m
  test/Analysis/explain-svals.cpp
  test/Analysis/malloc.c
  test/Analysis/misc-ps-eager-assume.m
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -146,7 +146,7 @@
 void test_isgraph_isprint(int x) {
   char y = x;
   if (isgraph(y))
-clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
+clang_analyzer_eval(isprint(y)); // expected-warning{{TRUE}}
 }
 
 int isdigit(int);
Index: test/Analysis/misc-ps-eager-assume.m
===
--- test/Analysis/misc-ps-eager-assume.m
+++ test/Analysis/misc-ps-eager-assume.m
@@ -1,5 +1,4 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks %s -analyzer-eagerly-assume
-// expected-no-diagnostics
 
 // Delta-reduced header stuff (needed for test cases).
 typedef signed char BOOL;
@@ -56,7 +55,7 @@
 void handle_symbolic_cast_in_condition(void) {
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
 
-  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"];
+  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"]; // expected-warning {{Assignment of a non-Boolean value}}
   NSMutableArray* array = needsAnArray ? [[NSMutableArray alloc] init] : 0;
   if(needsAnArray)
 [array release];
Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1656,13 +1656,13 @@
 void testOffsetPassedToStrlen() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
+  size_t length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
 }
 
 void testOffsetPassedToStrlenThenFree() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string);
+  size_t length = strlen(string);
   free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}}
 }
 
@@ -1705,7 +1705,7 @@
 }
 
 char *dupstrNoWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocNoWarn(len + 1);
   strcpy(p, s); // no-warning
   return p;
@@ -1721,7 +1721,7 @@
 }
 
 char *dupstrWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocWarn(len + 1);
   strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
   return p;
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -41,11 +41,19 @@
 
 void test_2(char *ptr, int ext) {
   clang_analyzer_explain((void *) "asdf"); // expected-warning-re^pointer to element of type 'char' with index 0 of string literal "asdf"$
+#ifdef ANALYZER_CM_Z3
+  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^cast of type 'int' of metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+#else
   clang_analyzer_explain(strlen(ptr)); // expected-warning-re^metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+#endif
   clang_analyzer_explain(conjure()); // expected-warning-re^symbol of type 'int' conjured at statement 'conjure\(\)'$
   clang_analyzer_explain(glob); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob'$
   clang_analyzer_explain(glob_ptr); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob_ptr'$
+#ifdef ANALYZER_CM_Z3
+  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^cast of type 'int' of extent of pointee of argument 'ptr'$
+#else
   clang_analyzer_explain(clang_analyzer_getExtent(

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@dcoughlin No, all three patches are separate. I have been testing them with 
each applied incrementally onto the previous, with the order trunk, 
https://reviews.llvm.org/D35450, https://reviews.llvm.org/D28954, then 
https://reviews.llvm.org/D28955 (this). But since these are a lot of 
dependencies, I've refactored this patch to apply directly on top of 
https://reviews.llvm.org/D35450, though the floating-point specific changes 
will need to be merged into https://reviews.llvm.org/D28954. Also, I think 
https://reviews.llvm.org/D28954 is waiting for review.


https://reviews.llvm.org/D28955



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@NoQ Does the proposal in https://reviews.llvm.org/D28955#652465 satisfy your 
concern?


https://reviews.llvm.org/D28955



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

[PATCH] D77705: [Driver] Forward pass plugin arguments to gold

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added a reviewer: tejohnson.
Herald added a project: clang.

Support forwarding `-fplugin` and `-fpass-plugin` arguments for loading pass 
plugins

Depends on: D77704 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77705

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -474,10 +474,18 @@
 ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
 if (Path.empty() || llvm::sys::fs::is_directory(Path))
   llvm::sys::path::append(Path, "default.profdata");
-CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") 
+
- Path));
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") + Path));
   }
 
+  // Forward arguments for loading plugins (old/new PM)
+  for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ))
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=load=") + A->getValue()));
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-plugin-opt=load-pass-plugin=") + A->getValue()));
+
   // Need this flag to turn on new pass manager via Gold plugin.
   if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
options::OPT_fno_experimental_new_pass_manager,


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -474,10 +474,18 @@
 ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
 if (Path.empty() || llvm::sys::fs::is_directory(Path))
   llvm::sys::path::append(Path, "default.profdata");
-CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") +
- Path));
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") + Path));
   }
 
+  // Forward arguments for loading plugins (old/new PM)
+  for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ))
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=load=") + A->getValue()));
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-plugin-opt=load-pass-plugin=") + A->getValue()));
+
   // Need this flag to turn on new pass manager via Gold plugin.
   if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
options::OPT_fno_experimental_new_pass_manager,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77705: [Driver] Forward pass plugin arguments to gold

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Originally, I tried forwarding the `-Xclang -load` arguments, but couldn't 
access the `options::OPT_plugin` arguments from the argument list. I'm not 
familiar with tablegen and argument processing, there was some issue with the 
group not being available causing the arguments to be skipped when filtering 
the argument list, so I ended up reusing the `OPT_fplugin` and 
`OPT_fpass_plugin` arguments instead. As far as I can tell, `OPT_fplugin` seems 
to work for both Clang and LLVM plugins, but I haven't used the former.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77705



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

[PATCH] D77704: [gold] Add support for loading pass plugins

[Dominic Chen via Phabricator via cfe-commits]

ddcc marked an inline comment as done.
ddcc added a comment.

Thanks, seems to be working now for statically-built passes on a default LLVM 
build without `LLVM_LINK_LLVM_DYLIB`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77704



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

[PATCH] D77704: [gold] Add support for loading pass plugins

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 259981.
ddcc added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Support statically-built passes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77704

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  llvm/tools/gold/CMakeLists.txt
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/gold/gold.exports


Index: llvm/tools/gold/gold.exports
===
--- llvm/tools/gold/gold.exports
+++ /dev/null
@@ -1 +0,0 @@
-onload
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Object/Error.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -207,6 +208,12 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // List of new PM pass plugins
+  static std::vector pass_plugins;
+  // Path to ourselves, and whether we have been reloaded for pass plugin 
symbol
+  // resolution
+  static std::string self;
+  static bool self_reloaded = false;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -300,6 +307,30 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt.consume_front("self=")) {
+  // Store our own path, in case we need to be reloaded
+  self = std::string(opt);
+} else if (opt.consume_front("load=")) {
+  std::string Error, Path(opt);
+
+  // This plugin is loaded by gold under RTLD_LOCAL, which means that our
+  // LLVM symbols are not available to subsequent pass plugins. Since this
+  // will break statically-built pass plugins, we reload with RTLD_GLOBAL.
+  if (!self_reloaded) {
+if (self.size()) {
+  if (sys::DynamicLibrary::LoadLibraryPermanently(self.c_str(), 
&Error))
+message(LDPL_ERROR, "Unable to reload LLVM gold plugin: %s",
+Error.c_str());
+  else
+self_reloaded = true;
+} else
+  message(LDPL_ERROR, "Unable to retrieve path to LLVM gold plugin!");
+  }
+
+  if (sys::DynamicLibrary::LoadLibraryPermanently(Path.c_str(), &Error))
+message(LDPL_ERROR, "Unable to load plugin: %s", Error.c_str());
+} else if (opt.consume_front("load-pass-plugin=")) {
+  pass_plugins.push_back(opt.data());
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -935,6 +966,8 @@
   Conf.UseNewPM = options::new_pass_manager;
   // Debug new pass manager if requested
   Conf.DebugPassManager = options::debug_pass_manager;
+  // Pass plugins to load
+  Conf.PassPlugins = std::move(options::pass_plugins);
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
Index: llvm/tools/gold/CMakeLists.txt
===
--- llvm/tools/gold/CMakeLists.txt
+++ llvm/tools/gold/CMakeLists.txt
@@ -1,5 +1,3 @@
-set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/gold.exports)
-
 if( LLVM_ENABLE_PIC AND LLVM_BINUTILS_INCDIR )
   include_directories( ${LLVM_BINUTILS_INCDIR} )
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -379,6 +379,9 @@
 Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Plugin));
+
+// Pass in our own path in case we need to reload
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=self=") + Plugin));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to


Index: llvm/tools/gold/gold.exports
===
--- llvm/tools/gold/gold.exports
+++ /dev/null
@@ -1 +0,0 @@
-onload
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Object/Error.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -207,6 +208,12 @@
   static std::str

[PATCH] D77704: [gold] Add support for loading pass plugins

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In order to reload the gold plugin, I'm modifying the Clang driver to pass in 
our own path as a separate argument, which is the most generic approach. 
Another method would be to use e.g. `dladdr()` to grab our own path from one of 
our exported functions, but that method appears to be a glibc extension which 
isn't cross-platform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77704



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

[PATCH] D77704: [gold] Add support for loading pass plugins

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

ping, any feedback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77704



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

[PATCH] D132003: [clang][ARM][NFC] Clean up signed conversion and undefined macros in builtin header

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 458581.
ddcc marked an inline comment as done.
ddcc added a comment.

Fix typo in __revsh


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132003

Files:
  clang/lib/Headers/arm_acle.h

Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -64,7 +64,7 @@
 }
 #endif
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __dbg(t) __builtin_arm_dbg(t)
 #endif
 
@@ -82,7 +82,7 @@
 /* 8.6.1 Data prefetch */
 #define __pld(addr) __pldx(0, 0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __pldx(access_kind, cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, access_kind, 1)
 #else
@@ -93,7 +93,7 @@
 /* 8.6.2 Instruction prefetch */
 #define __pli(addr) __plix(0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __plix(cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, 0, 0)
 #else
@@ -140,17 +140,17 @@
 /* CLZ */
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __clz(uint32_t __t) {
-  return __builtin_clz(__t);
+  return (uint32_t)__builtin_clz(__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
 __clzl(unsigned long __t) {
-  return __builtin_clzl(__t);
+  return (unsigned long)__builtin_clzl(__t);
 }
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __clzll(uint64_t __t) {
-  return __builtin_clzll(__t);
+  return (uint64_t)__builtin_clzll(__t);
 }
 
 /* CLS */
@@ -201,7 +201,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rev16ll(uint64_t __t) {
-  return (((uint64_t)__rev16(__t >> 32)) << 32) | __rev16(__t);
+  return (((uint64_t)__rev16(__t >> 32)) << 32) | (uint64_t)__rev16((uint32_t)__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
@@ -216,7 +216,7 @@
 /* REVSH */
 static __inline__ int16_t __attribute__((__always_inline__, __nodebug__))
 __revsh(int16_t __t) {
-  return __builtin_bswap16(__t);
+  return (int16_t)__builtin_bswap16((uint16_t)__t);
 }
 
 /* RBIT */
@@ -227,7 +227,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rbitll(uint64_t __t) {
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
   return (((uint64_t)__builtin_arm_rbit(__t)) << 32) |
  __builtin_arm_rbit(__t >> 32);
 #else
@@ -247,7 +247,7 @@
 /*
  * 9.3 16-bit multiplications
  */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__,__nodebug__))
 __smulbb(int32_t __a, int32_t __b) {
   return __builtin_arm_smulbb(__a, __b);
@@ -281,13 +281,13 @@
  * intrinsics are implemented and the flag is enabled.
  */
 /* 9.4.1 Width-specified saturation intrinsics */
-#if __ARM_FEATURE_SAT
+#if defined(__ARM_FEATURE_SAT) && __ARM_FEATURE_SAT
 #define __ssat(x, y) __builtin_arm_ssat(x, y)
 #define __usat(x, y) __builtin_arm_usat(x, y)
 #endif
 
 /* 9.4.2 Saturating addition and subtraction intrinsics */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __qadd(int32_t __t, int32_t __v) {
   return __builtin_arm_qadd(__t, __v);
@@ -305,7 +305,7 @@
 #endif
 
 /* 9.4.3 Accumultating multiplications */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smlabb(int32_t __a, int32_t __b, int32_t __c) {
   return __builtin_arm_smlabb(__a, __b, __c);
@@ -334,13 +334,13 @@
 
 
 /* 9.5.4 Parallel 16-bit saturation */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 #define __ssat16(x, y) __builtin_arm_ssat16(x, y)
 #define __usat16(x, y) __builtin_arm_usat16(x, y)
 #endif
 
 /* 9.5.5 Packing and unpacking */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 typedef int32_t int8x4_t;
 typedef int32_t int16x2_t;
 typedef uint32_t uint8x4_t;
@@ -365,7 +365,7 @@
 #endif
 
 /* 9.5.6 Parallel selection */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ uint8x4_t __attribute__((__always_inline__, __nodebug__))
 __sel(uint8x4_t __a, uint8x4_t __b) {
   return __builtin_arm_sel(__a, __b);
@@ -373,7 +373,7 @@
 #endif
 
 /* 9.5.7 Parallel 8-bit addition and subtraction */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int8x4_t __attribute__((__always_inline__, __nodebug__))
 __qadd8(int8x4_t __a, int8x4_t __b) {
   return __builtin_arm_qadd8(__a, __b);
@@ -425,7 +

[PATCH] D132003: [clang][ARM][NFC] Clean up signed conversion and undefined macros in builtin header

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Yup, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132003

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

[PATCH] D132003: [clang][ARM][NFC] Clean up signed conversion and undefined macros in builtin header

[Dominic Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGac77b3fde120: [clang][ARM][NFC] Clean up signed conversion 
and undefined macros in builtin… (authored by ddcc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132003

Files:
  clang/lib/Headers/arm_acle.h

Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -64,7 +64,7 @@
 }
 #endif
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __dbg(t) __builtin_arm_dbg(t)
 #endif
 
@@ -82,7 +82,7 @@
 /* 8.6.1 Data prefetch */
 #define __pld(addr) __pldx(0, 0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __pldx(access_kind, cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, access_kind, 1)
 #else
@@ -93,7 +93,7 @@
 /* 8.6.2 Instruction prefetch */
 #define __pli(addr) __plix(0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __plix(cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, 0, 0)
 #else
@@ -140,17 +140,17 @@
 /* CLZ */
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __clz(uint32_t __t) {
-  return __builtin_clz(__t);
+  return (uint32_t)__builtin_clz(__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
 __clzl(unsigned long __t) {
-  return __builtin_clzl(__t);
+  return (unsigned long)__builtin_clzl(__t);
 }
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __clzll(uint64_t __t) {
-  return __builtin_clzll(__t);
+  return (uint64_t)__builtin_clzll(__t);
 }
 
 /* CLS */
@@ -201,7 +201,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rev16ll(uint64_t __t) {
-  return (((uint64_t)__rev16(__t >> 32)) << 32) | __rev16(__t);
+  return (((uint64_t)__rev16(__t >> 32)) << 32) | (uint64_t)__rev16((uint32_t)__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
@@ -216,7 +216,7 @@
 /* REVSH */
 static __inline__ int16_t __attribute__((__always_inline__, __nodebug__))
 __revsh(int16_t __t) {
-  return __builtin_bswap16(__t);
+  return (int16_t)__builtin_bswap16((uint16_t)__t);
 }
 
 /* RBIT */
@@ -227,7 +227,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rbitll(uint64_t __t) {
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
   return (((uint64_t)__builtin_arm_rbit(__t)) << 32) |
  __builtin_arm_rbit(__t >> 32);
 #else
@@ -247,7 +247,7 @@
 /*
  * 9.3 16-bit multiplications
  */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__,__nodebug__))
 __smulbb(int32_t __a, int32_t __b) {
   return __builtin_arm_smulbb(__a, __b);
@@ -281,13 +281,13 @@
  * intrinsics are implemented and the flag is enabled.
  */
 /* 9.4.1 Width-specified saturation intrinsics */
-#if __ARM_FEATURE_SAT
+#if defined(__ARM_FEATURE_SAT) && __ARM_FEATURE_SAT
 #define __ssat(x, y) __builtin_arm_ssat(x, y)
 #define __usat(x, y) __builtin_arm_usat(x, y)
 #endif
 
 /* 9.4.2 Saturating addition and subtraction intrinsics */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __qadd(int32_t __t, int32_t __v) {
   return __builtin_arm_qadd(__t, __v);
@@ -305,7 +305,7 @@
 #endif
 
 /* 9.4.3 Accumultating multiplications */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smlabb(int32_t __a, int32_t __b, int32_t __c) {
   return __builtin_arm_smlabb(__a, __b, __c);
@@ -334,13 +334,13 @@
 
 
 /* 9.5.4 Parallel 16-bit saturation */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 #define __ssat16(x, y) __builtin_arm_ssat16(x, y)
 #define __usat16(x, y) __builtin_arm_usat16(x, y)
 #endif
 
 /* 9.5.5 Packing and unpacking */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 typedef int32_t int8x4_t;
 typedef int32_t int16x2_t;
 typedef uint32_t uint8x4_t;
@@ -365,7 +365,7 @@
 #endif
 
 /* 9.5.6 Parallel selection */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ uint8x4_t __attribute__((__always_inline__, __nodebug__))
 __sel(uint8x4_t __a, uint8x4_t __b) {
   return __builtin_arm_sel(__a, __b);
@@ -373,7 +373,7 @@
 #endif
 
 /* 9.5.7 Parallel 8-bit addition and subtraction */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int8x4_t __a

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added reviewers: efriedma, rnk, aaron.ballman.
Herald added a project: All.
ddcc requested review of this revision.
Herald added a project: clang.

While debugging module support using -Wsystem-headers, we discovered that if
-Werror, and -Wundef or -Wmacro-redefined are specified, they can cause errors
to be generated in these builtin headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h


Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -91,6 +91,9 @@
  * defined if there exists an exact-width type of equal or greater width.
  */
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmacro-redefined"
+
 #ifdef __INT64_TYPE__
 # ifndef __int8_t_defined /* glibc sys/types.h also defines int64_t*/
 typedef __INT64_TYPE__ int64_t;
@@ -857,5 +860,7 @@
 #define WINT_WIDTH   __WINT_WIDTH__
 #endif
 
+#pragma GCC diagnostic pop
+
 #endif /* __STDC_HOSTED__ */
 #endif /* __CLANG_STDINT_H */
Index: clang/lib/Headers/stddef.h
===
--- clang/lib/Headers/stddef.h
+++ clang/lib/Headers/stddef.h
@@ -98,7 +98,8 @@
 #endif /* defined(__need_NULL) */
 
 #if defined(__need_STDDEF_H_misc)
-#if __STDC_VERSION__ >= 201112L || __cplusplus >= 201103L
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) ||  
\
+(defined(__cplusplus) && __cplusplus >= 201103L)
 #include "__stddef_max_align_t.h"
 #endif
 #define offsetof(t, d) __builtin_offsetof(t, d)


Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -91,6 +91,9 @@
  * defined if there exists an exact-width type of equal or greater width.
  */
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmacro-redefined"
+
 #ifdef __INT64_TYPE__
 # ifndef __int8_t_defined /* glibc sys/types.h also defines int64_t*/
 typedef __INT64_TYPE__ int64_t;
@@ -857,5 +860,7 @@
 #define WINT_WIDTH   __WINT_WIDTH__
 #endif
 
+#pragma GCC diagnostic pop
+
 #endif /* __STDC_HOSTED__ */
 #endif /* __CLANG_STDINT_H */
Index: clang/lib/Headers/stddef.h
===
--- clang/lib/Headers/stddef.h
+++ clang/lib/Headers/stddef.h
@@ -98,7 +98,8 @@
 #endif /* defined(__need_NULL) */
 
 #if defined(__need_STDDEF_H_misc)
-#if __STDC_VERSION__ >= 201112L || __cplusplus >= 201103L
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) ||  \
+(defined(__cplusplus) && __cplusplus >= 201103L)
 #include "__stddef_max_align_t.h"
 #endif
 #define offsetof(t, d) __builtin_offsetof(t, d)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 449118.
ddcc added a comment.

Undef macros instead, handle other header files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/__clang_cuda_builtin_vars.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Headers/__clang_hip_cmath.h
  clang/lib/Headers/__clang_hip_math.h
  clang/lib/Headers/cuda_wrappers/algorithm
  clang/lib/Headers/cuda_wrappers/complex
  clang/lib/Headers/cuda_wrappers/new
  clang/lib/Headers/float.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/openmp_wrappers/new
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdatomic.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/velintrin.h

Index: clang/lib/Headers/velintrin.h
===
--- clang/lib/Headers/velintrin.h
+++ clang/lib/Headers/velintrin.h
@@ -13,7 +13,7 @@
 typedef double __vr __attribute__((__vector_size__(2048)));
 
 // Vector mask registers
-#if __STDC_VERSION__ >= 199901L
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // For C99
 typedef _Bool __vm__attribute__((ext_vector_type(256)));
 typedef _Bool __vm256 __attribute__((ext_vector_type(256)));
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -13,7 +13,7 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
-#if __STDC_VERSION__ > 201710L &&  \
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
including the header file in C2x is also deprecated and we do not want to
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -96,13 +96,21 @@
 typedef __INT64_TYPE__ int64_t;
 # endif /* __int8_t_defined */
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t
 # define __uint_least64_t uint64_t
+# undef __int_least32_t
 # define __int_least32_t int64_t
+# undef __uint_least32_t
 # define __uint_least32_t uint64_t
+# undef __int_least16_t
 # define __int_least16_t int64_t
+# undef __uint_least16_t
 # define __uint_least16_t uint64_t
+# undef __int_least8_t
 # define __int_least8_t int64_t
+# undef __uint_least8_t
 # define __uint_least8_t uint64_t
 #endif /* __INT64_TYPE__ */
 
@@ -120,11 +128,17 @@
 typedef uint56_t uint_least56_t;
 typedef int56_t int_fast56_t;
 typedef uint56_t uint_fast56_t;
+# undef __int_least32_t
 # define __int_least32_t int56_t
+# undef __uint_least32_t
 # define __uint_least32_t uint56_t
+# undef __int_least16_t
 # define __int_least16_t int56_t
+# undef __uint_least16_t
 # define __uint_least16_t uint56_t
+# undef __int_least8_t
 # define __int_least8_t int56_t
+# undef __uint_least8_t
 # define __uint_least8_t uint56_t
 #endif /* __INT56_TYPE__ */
 
@@ -136,11 +150,17 @@
 typedef uint48_t uint_least48_t;
 typedef int48_t int_fast48_t;
 typedef uint48_t uint_fast48_t;
+# undef __int_least32_t
 # define __int_least32_t int48_t
+# undef __uint_least32_t
 # define __uint_least32_t uint48_t
+# undef __int_least16_t
 # define __int_least16_t int48_t
+# undef __uint_least16_t
 # define __uint_least16_t uint48_t
+# undef __int_least8_t
 # define __int_least8_t int48_t
+# undef __uint_least8_t
 # define __uint_least8_t uint48_t
 #endif /* __INT48_TYPE__ */
 
@@ -152,11 +172,17 @@
 typedef uint40_t uint_least40_t;
 typedef int40_t int_fast40_t;
 typedef uint40_t uint_fast40_t;
+# undef __int_least32_t
 # define __int_least32_t int40_t
+# undef __uint_least32_t
 # define __uint_least32_t uint40_t
+# undef __int_least16_t
 # define __int_least16_t int40_t
+# undef __uint_least16_t
 # define __uint_least16_t uint40_t
+# undef __int_least8_t
 # define __int_least8_t int40_t
+# undef __uint_least8_t
 # define __uint_least8_t uint40_t
 #endif /* __INT40_TYPE__ */
 
@@ -172,11 +198,17 @@
 typedef __UINT32_TYPE__ uint32_t;
 # endif /* __uint32_t_defined */
 
+# undef __int_least32_t
 # define __int_least32_t int32_t
+# undef __uint_least32_t
 # define __uint_least32_t uint32_t
+# undef __int_least16_t
 # define __int_least16_t int32_t
+# undef __uint_least16_t
 # define __uint_least16_t uint32_t
+# undef __int_least8_t
 # define __int_least8_t int32_t
+# undef __uint_least8_t
 # define __uint_least8_t uint32_t
 #endif /* __INT32_TYPE__ */
 
@@ -194,9 +226,13 @@
 typedef uint24_t uint_least24_t;
 typedef int24_t int_fast24_t;
 typedef uint24_t uint_fast24_t;
+# undef __int_least16_t
 # define __int_least1

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_builtin_vars.h:37
 
-#if __cplusplus >= 201103L
+#if defined(__cplusplus) && __cplusplus >= 201103L
 #define __DELETE =delete

tra wrote:
> Are there actual use cases where CUDA headers are used from a non-C++ 
> compilation? I do not think they will compile with non-C++ compilation at all.
> 
> I'm curious how we managed to run into a situation where CUDA headers may be 
> used w/o C++. 
> 
> I think a better fix here and other CUDA headers may be to add 
> ```
> #if !defined(__cplusplus)
> #error CUDA headers must be used from a CUDA/C++ compilation.
> #endif
> ```
Sure, I can do that. I'm not familiar with CUDA or its headers, so I previously 
just updated them all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

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

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 449403.
ddcc added a comment.

Error out on undef __cplusplus in CUDA headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/__clang_cuda_builtin_vars.h
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Headers/__clang_hip_cmath.h
  clang/lib/Headers/__clang_hip_math.h
  clang/lib/Headers/cuda_wrappers/algorithm
  clang/lib/Headers/cuda_wrappers/complex
  clang/lib/Headers/cuda_wrappers/new
  clang/lib/Headers/float.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/openmp_wrappers/new
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdatomic.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/velintrin.h

Index: clang/lib/Headers/velintrin.h
===
--- clang/lib/Headers/velintrin.h
+++ clang/lib/Headers/velintrin.h
@@ -13,7 +13,7 @@
 typedef double __vr __attribute__((__vector_size__(2048)));
 
 // Vector mask registers
-#if __STDC_VERSION__ >= 199901L
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // For C99
 typedef _Bool __vm__attribute__((ext_vector_type(256)));
 typedef _Bool __vm256 __attribute__((ext_vector_type(256)));
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -13,7 +13,7 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
-#if __STDC_VERSION__ > 201710L &&  \
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
including the header file in C2x is also deprecated and we do not want to
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -96,13 +96,21 @@
 typedef __INT64_TYPE__ int64_t;
 # endif /* __int8_t_defined */
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t
 # define __uint_least64_t uint64_t
+# undef __int_least32_t
 # define __int_least32_t int64_t
+# undef __uint_least32_t
 # define __uint_least32_t uint64_t
+# undef __int_least16_t
 # define __int_least16_t int64_t
+# undef __uint_least16_t
 # define __uint_least16_t uint64_t
+# undef __int_least8_t
 # define __int_least8_t int64_t
+# undef __uint_least8_t
 # define __uint_least8_t uint64_t
 #endif /* __INT64_TYPE__ */
 
@@ -120,11 +128,17 @@
 typedef uint56_t uint_least56_t;
 typedef int56_t int_fast56_t;
 typedef uint56_t uint_fast56_t;
+# undef __int_least32_t
 # define __int_least32_t int56_t
+# undef __uint_least32_t
 # define __uint_least32_t uint56_t
+# undef __int_least16_t
 # define __int_least16_t int56_t
+# undef __uint_least16_t
 # define __uint_least16_t uint56_t
+# undef __int_least8_t
 # define __int_least8_t int56_t
+# undef __uint_least8_t
 # define __uint_least8_t uint56_t
 #endif /* __INT56_TYPE__ */
 
@@ -136,11 +150,17 @@
 typedef uint48_t uint_least48_t;
 typedef int48_t int_fast48_t;
 typedef uint48_t uint_fast48_t;
+# undef __int_least32_t
 # define __int_least32_t int48_t
+# undef __uint_least32_t
 # define __uint_least32_t uint48_t
+# undef __int_least16_t
 # define __int_least16_t int48_t
+# undef __uint_least16_t
 # define __uint_least16_t uint48_t
+# undef __int_least8_t
 # define __int_least8_t int48_t
+# undef __uint_least8_t
 # define __uint_least8_t uint48_t
 #endif /* __INT48_TYPE__ */
 
@@ -152,11 +172,17 @@
 typedef uint40_t uint_least40_t;
 typedef int40_t int_fast40_t;
 typedef uint40_t uint_fast40_t;
+# undef __int_least32_t
 # define __int_least32_t int40_t
+# undef __uint_least32_t
 # define __uint_least32_t uint40_t
+# undef __int_least16_t
 # define __int_least16_t int40_t
+# undef __uint_least16_t
 # define __uint_least16_t uint40_t
+# undef __int_least8_t
 # define __int_least8_t int40_t
+# undef __uint_least8_t
 # define __uint_least8_t uint40_t
 #endif /* __INT40_TYPE__ */
 
@@ -172,11 +198,17 @@
 typedef __UINT32_TYPE__ uint32_t;
 # endif /* __uint32_t_defined */
 
+# undef __int_least32_t
 # define __int_least32_t int32_t
+# undef __uint_least32_t
 # define __uint_least32_t uint32_t
+# undef __int_least16_t
 # define __int_least16_t int32_t
+# undef __uint_least16_t
 # define __uint_least16_t uint32_t
+# undef __int_least8_t
 # define __int_least8_t int32_t
+# undef __uint_least8_t
 # define __uint_least8_t uint32_t
 #endif /* __INT32_TYPE__ */
 
@@ -194,9 +226,13 @@
 typedef uint24_t uint_least24_t;
 typedef int24_t int_fast24_t;
 typedef uint24_t uint_fast24_t;
+# unde

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: clang/lib/Headers/__clang_hip_cmath.h:381
 // decltype is only available in C++11 and above.
-#if __cplusplus >= 201103L
+#if defined(__cplusplus) && __cplusplus >= 201103L
 // __hip_promote

tra wrote:
> HIP headers are also not expected to compile w/o C++ and we may want a 
> `#error`here, too. 
> 
> Actually, I'm not even sure if we shopuld/need to add `#if 
> defined(__cplusplus)`in files that are clearly C++ -- undefined macro warning 
> will be the least of one's worries if they attempt to compile it w/o C++.
> 
> Perhaps it would make sense to limit these changes to the headers intended to 
> be used from both C and C++? Both CUDA and HIP are expected to never be 
> included from a non-C++ compilation and will never trigger the undefined 
> macro warning on `__cplusplus`.
> 
> @yaxunl - FYI.
Sure, I'm fine with leaving those headers alone since I'm not familiar with 
them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

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

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 449458.
ddcc added a comment.

Drop changes to CUDA/HIP/OpenMP headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/float.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdatomic.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/velintrin.h

Index: clang/lib/Headers/velintrin.h
===
--- clang/lib/Headers/velintrin.h
+++ clang/lib/Headers/velintrin.h
@@ -13,7 +13,7 @@
 typedef double __vr __attribute__((__vector_size__(2048)));
 
 // Vector mask registers
-#if __STDC_VERSION__ >= 199901L
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // For C99
 typedef _Bool __vm__attribute__((ext_vector_type(256)));
 typedef _Bool __vm256 __attribute__((ext_vector_type(256)));
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -13,7 +13,7 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
-#if __STDC_VERSION__ > 201710L &&  \
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
including the header file in C2x is also deprecated and we do not want to
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -96,13 +96,21 @@
 typedef __INT64_TYPE__ int64_t;
 # endif /* __int8_t_defined */
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t
 # define __uint_least64_t uint64_t
+# undef __int_least32_t
 # define __int_least32_t int64_t
+# undef __uint_least32_t
 # define __uint_least32_t uint64_t
+# undef __int_least16_t
 # define __int_least16_t int64_t
+# undef __uint_least16_t
 # define __uint_least16_t uint64_t
+# undef __int_least8_t
 # define __int_least8_t int64_t
+# undef __uint_least8_t
 # define __uint_least8_t uint64_t
 #endif /* __INT64_TYPE__ */
 
@@ -120,11 +128,17 @@
 typedef uint56_t uint_least56_t;
 typedef int56_t int_fast56_t;
 typedef uint56_t uint_fast56_t;
+# undef __int_least32_t
 # define __int_least32_t int56_t
+# undef __uint_least32_t
 # define __uint_least32_t uint56_t
+# undef __int_least16_t
 # define __int_least16_t int56_t
+# undef __uint_least16_t
 # define __uint_least16_t uint56_t
+# undef __int_least8_t
 # define __int_least8_t int56_t
+# undef __uint_least8_t
 # define __uint_least8_t uint56_t
 #endif /* __INT56_TYPE__ */
 
@@ -136,11 +150,17 @@
 typedef uint48_t uint_least48_t;
 typedef int48_t int_fast48_t;
 typedef uint48_t uint_fast48_t;
+# undef __int_least32_t
 # define __int_least32_t int48_t
+# undef __uint_least32_t
 # define __uint_least32_t uint48_t
+# undef __int_least16_t
 # define __int_least16_t int48_t
+# undef __uint_least16_t
 # define __uint_least16_t uint48_t
+# undef __int_least8_t
 # define __int_least8_t int48_t
+# undef __uint_least8_t
 # define __uint_least8_t uint48_t
 #endif /* __INT48_TYPE__ */
 
@@ -152,11 +172,17 @@
 typedef uint40_t uint_least40_t;
 typedef int40_t int_fast40_t;
 typedef uint40_t uint_fast40_t;
+# undef __int_least32_t
 # define __int_least32_t int40_t
+# undef __uint_least32_t
 # define __uint_least32_t uint40_t
+# undef __int_least16_t
 # define __int_least16_t int40_t
+# undef __uint_least16_t
 # define __uint_least16_t uint40_t
+# undef __int_least8_t
 # define __int_least8_t int40_t
+# undef __uint_least8_t
 # define __uint_least8_t uint40_t
 #endif /* __INT40_TYPE__ */
 
@@ -172,11 +198,17 @@
 typedef __UINT32_TYPE__ uint32_t;
 # endif /* __uint32_t_defined */
 
+# undef __int_least32_t
 # define __int_least32_t int32_t
+# undef __uint_least32_t
 # define __uint_least32_t uint32_t
+# undef __int_least16_t
 # define __int_least16_t int32_t
+# undef __uint_least16_t
 # define __uint_least16_t uint32_t
+# undef __int_least8_t
 # define __int_least8_t int32_t
+# undef __uint_least8_t
 # define __uint_least8_t uint32_t
 #endif /* __INT32_TYPE__ */
 
@@ -194,9 +226,13 @@
 typedef uint24_t uint_least24_t;
 typedef int24_t int_fast24_t;
 typedef uint24_t uint_fast24_t;
+# undef __int_least16_t
 # define __int_least16_t int24_t
+# undef __uint_least16_t
 # define __uint_least16_t uint24_t
+# undef __int_least8_t
 # define __int_least8_t int24_t
+# undef __uint_least8_t
 # define __uint_least8_t uint24_t
 #endif /* __INT24_TYPE__ */
 
@@ -205,9 +241,13 @@
 typedef __INT16_TYPE__ int16_t;
 #endif /* __int8_t_defined */
 typedef __UINT16_TYPE__ uint16_t;
+# undef

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: clang/lib/Headers/stdint.h:99-100
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t

iana wrote:
> iana wrote:
> > What are you seeing that's defining `__int_least64_t` and all these other 
> > ones by the time you get here? It's surprising to me that so much 
> > preprocessor state exists when you hit this point considering that this 
> > file doesn't include anything else. Is this some kind of artifact with how 
> > the OS module map is making a module for stdint.h?
> Oh I see, it's not these ones that are the problem, it's the redefinitions 
> below. I guess it's a bigger change, but I wonder if flipping the order would 
> be cleaner? e.g.
> ```
> #ifdef __INT32_TYPE__
> # define __int_least32_t int32_t
> #endif
> 
> #ifdef __INT64_TYPE__
> # ifndef __int_least32_t
> #  define __int_least32_t int64_t
> # endif
> #endif
> ```
> 
> I guess it's an extra line per type doing it that way, but maybe it's more 
> obvious? (Maybe I just don't do a lot of preprocessor programming, but 
> `#undef` feels like a code smell to me)
I'm a little reluctant to change this file too much since it's tedious and 
error-prone, but yeah if we can agree on a solution, I can go ahead and make 
the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

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

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbf19005714b: [clang][Headers] Avoid compiler warnings in 
builtin headers (authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D130800?vs=449458&id=449844#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/float.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdatomic.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/velintrin.h

Index: clang/lib/Headers/velintrin.h
===
--- clang/lib/Headers/velintrin.h
+++ clang/lib/Headers/velintrin.h
@@ -13,7 +13,7 @@
 typedef double __vr __attribute__((__vector_size__(2048)));
 
 // Vector mask registers
-#if __STDC_VERSION__ >= 199901L
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // For C99
 typedef _Bool __vm__attribute__((ext_vector_type(256)));
 typedef _Bool __vm256 __attribute__((ext_vector_type(256)));
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -13,7 +13,7 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
-#if __STDC_VERSION__ > 201710L &&  \
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
including the header file in C2x is also deprecated and we do not want to
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -96,13 +96,21 @@
 typedef __INT64_TYPE__ int64_t;
 # endif /* __int8_t_defined */
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t
 # define __uint_least64_t uint64_t
+# undef __int_least32_t
 # define __int_least32_t int64_t
+# undef __uint_least32_t
 # define __uint_least32_t uint64_t
+# undef __int_least16_t
 # define __int_least16_t int64_t
+# undef __uint_least16_t
 # define __uint_least16_t uint64_t
+# undef __int_least8_t
 # define __int_least8_t int64_t
+# undef __uint_least8_t
 # define __uint_least8_t uint64_t
 #endif /* __INT64_TYPE__ */
 
@@ -120,11 +128,17 @@
 typedef uint56_t uint_least56_t;
 typedef int56_t int_fast56_t;
 typedef uint56_t uint_fast56_t;
+# undef __int_least32_t
 # define __int_least32_t int56_t
+# undef __uint_least32_t
 # define __uint_least32_t uint56_t
+# undef __int_least16_t
 # define __int_least16_t int56_t
+# undef __uint_least16_t
 # define __uint_least16_t uint56_t
+# undef __int_least8_t
 # define __int_least8_t int56_t
+# undef __uint_least8_t
 # define __uint_least8_t uint56_t
 #endif /* __INT56_TYPE__ */
 
@@ -136,11 +150,17 @@
 typedef uint48_t uint_least48_t;
 typedef int48_t int_fast48_t;
 typedef uint48_t uint_fast48_t;
+# undef __int_least32_t
 # define __int_least32_t int48_t
+# undef __uint_least32_t
 # define __uint_least32_t uint48_t
+# undef __int_least16_t
 # define __int_least16_t int48_t
+# undef __uint_least16_t
 # define __uint_least16_t uint48_t
+# undef __int_least8_t
 # define __int_least8_t int48_t
+# undef __uint_least8_t
 # define __uint_least8_t uint48_t
 #endif /* __INT48_TYPE__ */
 
@@ -152,11 +172,17 @@
 typedef uint40_t uint_least40_t;
 typedef int40_t int_fast40_t;
 typedef uint40_t uint_fast40_t;
+# undef __int_least32_t
 # define __int_least32_t int40_t
+# undef __uint_least32_t
 # define __uint_least32_t uint40_t
+# undef __int_least16_t
 # define __int_least16_t int40_t
+# undef __uint_least16_t
 # define __uint_least16_t uint40_t
+# undef __int_least8_t
 # define __int_least8_t int40_t
+# undef __uint_least8_t
 # define __uint_least8_t uint40_t
 #endif /* __INT40_TYPE__ */
 
@@ -172,11 +198,17 @@
 typedef __UINT32_TYPE__ uint32_t;
 # endif /* __uint32_t_defined */
 
+# undef __int_least32_t
 # define __int_least32_t int32_t
+# undef __uint_least32_t
 # define __uint_least32_t uint32_t
+# undef __int_least16_t
 # define __int_least16_t int32_t
+# undef __uint_least16_t
 # define __uint_least16_t uint32_t
+# undef __int_least8_t
 # define __int_least8_t int32_t
+# undef __uint_least8_t
 # define __uint_least8_t uint32_t
 #endif /* __INT32_TYPE__ */
 
@@ -194,9 +226,13 @@
 typedef uint24_t uint_least24_t;
 typedef int24_t int_fast24_t;
 typedef uint24_t uint_fast24_t;
+# undef __int_least16_t
 # define __int_least16_t int24_t
+# undef __uint_least16_t
 # define __uint_least16_t uint24_t
+# undef __int_least8_t
 # define __int_least8_t int24

[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added reviewers: aaron.ballman, iana.
Herald added a project: All.
ddcc requested review of this revision.
Herald added a project: clang.

Undefined macros evaluate to zero, so when checking for a smaller value,
we need to include the case when the macro is undefined.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131213

Files:
  clang/lib/Headers/stdbool.h


Index: clang/lib/Headers/stdbool.h
===
--- clang/lib/Headers/stdbool.h
+++ clang/lib/Headers/stdbool.h
@@ -23,7 +23,7 @@
 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__)
 /* Define _Bool as a GNU extension. */
 #define _Bool bool
-#if defined(__cplusplus) && __cplusplus < 201103L
+#if !defined(__cplusplus) || __cplusplus < 201103L
 /* For C++98, define bool, false, true as a GNU extension. */
 #define bool bool
 #define false false


Index: clang/lib/Headers/stdbool.h
===
--- clang/lib/Headers/stdbool.h
+++ clang/lib/Headers/stdbool.h
@@ -23,7 +23,7 @@
 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__)
 /* Define _Bool as a GNU extension. */
 #define _Bool bool
-#if defined(__cplusplus) && __cplusplus < 201103L
+#if !defined(__cplusplus) || __cplusplus < 201103L
 /* For C++98, define bool, false, true as a GNU extension. */
 #define bool bool
 #define false false
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I missed line 19, yeah that makes sense. @iana is that ok with you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131213

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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@NoQ ping


https://reviews.llvm.org/D35450



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

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added a reviewer: ostannard.
Herald added a subscriber: dang.
Herald added a project: clang.
ddcc requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88349

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1975,7 +1975,7 @@
 def fvirtual_function_elimination : Flag<["-"], 
"fvirtual-function-elimination">, Group,
   Flags<[CoreOption, CC1Option]>,
   HelpText<"Enables dead virtual function elimination optimization. Requires 
-flto=full">;
-def fno_virtual_function_elimination : Flag<["-"], 
"fno-virtual-function_elimination">, Group,
+def fno_virtual_function_elimination : Flag<["-"], 
"fno-virtual-function-elimination">, Group,
   Flags<[CoreOption]>;
 
 def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>,


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1975,7 +1975,7 @@
 def fvirtual_function_elimination : Flag<["-"], "fvirtual-function-elimination">, Group,
   Flags<[CoreOption, CC1Option]>,
   HelpText<"Enables dead virtual function elimination optimization. Requires -flto=full">;
-def fno_virtual_function_elimination : Flag<["-"], "fno-virtual-function_elimination">, Group,
+def fno_virtual_function_elimination : Flag<["-"], "fno-virtual-function-elimination">, Group,
   Flags<[CoreOption]>;
 
 def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

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

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 296782.
ddcc added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

Files:
  clang/include/clang/Driver/Options.td
  clang/test/CodeGenCXX/virtual-function-elimination.cpp


Index: clang/test/CodeGenCXX/virtual-function-elimination.cpp
===
--- clang/test/CodeGenCXX/virtual-function-elimination.cpp
+++ clang/test/CodeGenCXX/virtual-function-elimination.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -flto -flto-unit 
-fvirtual-function-elimination -fwhole-program-vtables -emit-llvm -o - %s | 
FileCheck %s
-
+// RUN: %clang -target x86_64-unknown-linux -flto 
-fno-virtual-function-elimination -fwhole-program-vtables -S -emit-llvm -o - %s 
| FileCheck %s -check-prefix=NOVFE
 
 struct __attribute__((visibility("default"))) A {
   virtual void foo();
@@ -8,9 +8,13 @@
 void test_1(A *p) {
   // A has default visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_1P1A
+// NOVFE-LABEL: define dso_local void @_Z6test_1P1A
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -22,9 +26,13 @@
 void test_2(B *p) {
   // B has public LTO visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_2P1B
+// NOVFE-LABEL: define dso_local void @_Z6test_2P1B
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -37,10 +45,14 @@
 void test_3(C *p) {
   // C has hidden visibility, so we generate type.checked.load to allow VFE.
 // CHECK-LABEL: define void @_Z6test_3P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_3P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 0, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[LOAD:%.+]] = load void (%struct.C*)*, void (%struct.C*)** {{%.+}}, 
align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[LOAD]](
   p->foo();
 }
 
@@ -48,10 +60,14 @@
   // When using type.checked.load, we pass the vtable offset to the intrinsic,
   // rather than adding it to the pointer with a GEP.
 // CHECK-LABEL: define void @_Z6test_4P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_4P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 8, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[LOAD:%.+]] = load void (%struct.C*)*, void (%struct.C*)** {{%.+}}, 
align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[LOAD]](
   p->bar();
 }
 
@@ -64,12 +80,17 @@
   // function pointer to the intrinsic, this information would be lost. No
   // codegen changes on the non-virtual side.
 // CHECK-LABEL: define void @_Z6test_5P1CMS_FvvE(
+// NOVFE-LABEL: define dso_local void @_Z6test_5P1CMS_FvvE(
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr i8, i8* %vtable, i64 {{%.+}}
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* 
[[FN_PTR_ADDR]], i32 0, metadata !"_ZTSM1CFvvE.virtual")
+// NOVFE-NOT: call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, i32 0, 
metadata !"_ZTSM1CFvvE.virtual")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 
 // CHECK: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
+// NOVFE: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
 // CHECK: call void [[PHI]](
+// NOVFE: call void [[PHI]](
   (p->*q)();
 }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 296826.
ddcc added a comment.

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

Files:
  clang/test/CodeGenCXX/virtual-function-elimination.cpp


Index: clang/test/CodeGenCXX/virtual-function-elimination.cpp
===
--- clang/test/CodeGenCXX/virtual-function-elimination.cpp
+++ clang/test/CodeGenCXX/virtual-function-elimination.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -flto -flto-unit 
-fvirtual-function-elimination -fwhole-program-vtables -emit-llvm -o - %s | 
FileCheck %s
-
+// RUN: %clang -target x86_64-unknown-linux -flto 
-fvirtual-function-elimination -fno-virtual-function-elimination 
-fwhole-program-vtables -S -emit-llvm -o - %s | FileCheck %s -check-prefix=NOVFE
 
 struct __attribute__((visibility("default"))) A {
   virtual void foo();
@@ -8,9 +8,13 @@
 void test_1(A *p) {
   // A has default visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_1P1A
+// NOVFE-LABEL: define dso_local void @_Z6test_1P1A
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -22,9 +26,13 @@
 void test_2(B *p) {
   // B has public LTO visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_2P1B
+// NOVFE-LABEL: define dso_local void @_Z6test_2P1B
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -37,10 +45,14 @@
 void test_3(C *p) {
   // C has hidden visibility, so we generate type.checked.load to allow VFE.
 // CHECK-LABEL: define void @_Z6test_3P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_3P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 0, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -48,10 +60,14 @@
   // When using type.checked.load, we pass the vtable offset to the intrinsic,
   // rather than adding it to the pointer with a GEP.
 // CHECK-LABEL: define void @_Z6test_4P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_4P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 8, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->bar();
 }
 
@@ -64,12 +80,17 @@
   // function pointer to the intrinsic, this information would be lost. No
   // codegen changes on the non-virtual side.
 // CHECK-LABEL: define void @_Z6test_5P1CMS_FvvE(
+// NOVFE-LABEL: define dso_local void @_Z6test_5P1CMS_FvvE(
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr i8, i8* %vtable, i64 {{%.+}}
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* 
[[FN_PTR_ADDR]], i32 0, metadata !"_ZTSM1CFvvE.virtual")
+// NOVFE-NOT: call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, i32 0, 
metadata !"_ZTSM1CFvvE.virtual")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 
 // CHECK: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
+// NOVFE: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
 // CHECK: call void [[PHI]](
+// NOVFE: call void [[PHI]](
   (p->*q)();
 }


Index: clang/test/CodeGenCXX/virtual-function-elimination.cpp
===
--- clang

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Hmm, looks like this was already fixed by 
e5158b52730d323bb8cd2cba6dc6c89b90cba452 
. I guess 
I'll just commit the test then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

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

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc10248829357: Add test for disabling Dead Virtual Function 
Elimination (authored by ddcc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

Files:
  clang/test/CodeGenCXX/virtual-function-elimination.cpp


Index: clang/test/CodeGenCXX/virtual-function-elimination.cpp
===
--- clang/test/CodeGenCXX/virtual-function-elimination.cpp
+++ clang/test/CodeGenCXX/virtual-function-elimination.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -flto -flto-unit 
-fvirtual-function-elimination -fwhole-program-vtables -emit-llvm -o - %s | 
FileCheck %s
-
+// RUN: %clang -target x86_64-unknown-linux -flto 
-fvirtual-function-elimination -fno-virtual-function-elimination 
-fwhole-program-vtables -S -emit-llvm -o - %s | FileCheck %s -check-prefix=NOVFE
 
 struct __attribute__((visibility("default"))) A {
   virtual void foo();
@@ -8,9 +8,13 @@
 void test_1(A *p) {
   // A has default visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_1P1A
+// NOVFE-LABEL: define dso_local void @_Z6test_1P1A
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -22,9 +26,13 @@
 void test_2(B *p) {
   // B has public LTO visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_2P1B
+// NOVFE-LABEL: define dso_local void @_Z6test_2P1B
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -37,10 +45,14 @@
 void test_3(C *p) {
   // C has hidden visibility, so we generate type.checked.load to allow VFE.
 // CHECK-LABEL: define void @_Z6test_3P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_3P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 0, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -48,10 +60,14 @@
   // When using type.checked.load, we pass the vtable offset to the intrinsic,
   // rather than adding it to the pointer with a GEP.
 // CHECK-LABEL: define void @_Z6test_4P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_4P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 8, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->bar();
 }
 
@@ -64,12 +80,17 @@
   // function pointer to the intrinsic, this information would be lost. No
   // codegen changes on the non-virtual side.
 // CHECK-LABEL: define void @_Z6test_5P1CMS_FvvE(
+// NOVFE-LABEL: define dso_local void @_Z6test_5P1CMS_FvvE(
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr i8, i8* %vtable, i64 {{%.+}}
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* 
[[FN_PTR_ADDR]], i32 0, metadata !"_ZTSM1CFvvE.virtual")
+// NOVFE-NOT: call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, i32 0, 
metadata !"_ZTSM1CFvvE.virtual")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 
 // CHECK: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
+// NOVFE: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
 // CHECK: call void [[PHI]](
+// NOVFE: call void [[PHI]

[PATCH] D132003: [clang][ARM][NFC] Clean up signed conversion and undefined macros in builtin header

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added reviewers: tmatheson, javed.absar, SjoerdMeijer.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
ddcc requested review of this revision.
Herald added a project: clang.

These warnings were identified while debugging modules with Wsystem-headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132003

Files:
  clang/lib/Headers/arm_acle.h

Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -64,7 +64,7 @@
 }
 #endif
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __dbg(t) __builtin_arm_dbg(t)
 #endif
 
@@ -82,7 +82,7 @@
 /* 8.6.1 Data prefetch */
 #define __pld(addr) __pldx(0, 0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __pldx(access_kind, cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, access_kind, 1)
 #else
@@ -93,7 +93,7 @@
 /* 8.6.2 Instruction prefetch */
 #define __pli(addr) __plix(0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __plix(cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, 0, 0)
 #else
@@ -140,17 +140,17 @@
 /* CLZ */
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __clz(uint32_t __t) {
-  return __builtin_clz(__t);
+  return (uint32_t)__builtin_clz(__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
 __clzl(unsigned long __t) {
-  return __builtin_clzl(__t);
+  return (unsigned long)__builtin_clzl(__t);
 }
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __clzll(uint64_t __t) {
-  return __builtin_clzll(__t);
+  return (uint64_t)__builtin_clzll(__t);
 }
 
 /* CLS */
@@ -201,7 +201,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rev16ll(uint64_t __t) {
-  return (((uint64_t)__rev16(__t >> 32)) << 32) | __rev16(__t);
+  return (((uint64_t)__rev16(__t >> 32)) << 32) | (uint64_t)__rev16((uint32_t)__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
@@ -216,7 +216,7 @@
 /* REVSH */
 static __inline__ int16_t __attribute__((__always_inline__, __nodebug__))
 __revsh(int16_t __t) {
-  return __builtin_bswap16(__t);
+  return (int16_t)__builtin_bswap16((int16_t)__t);
 }
 
 /* RBIT */
@@ -227,7 +227,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rbitll(uint64_t __t) {
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
   return (((uint64_t)__builtin_arm_rbit(__t)) << 32) |
  __builtin_arm_rbit(__t >> 32);
 #else
@@ -247,7 +247,7 @@
 /*
  * 9.3 16-bit multiplications
  */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__,__nodebug__))
 __smulbb(int32_t __a, int32_t __b) {
   return __builtin_arm_smulbb(__a, __b);
@@ -281,13 +281,13 @@
  * intrinsics are implemented and the flag is enabled.
  */
 /* 9.4.1 Width-specified saturation intrinsics */
-#if __ARM_FEATURE_SAT
+#if defined(__ARM_FEATURE_SAT) && __ARM_FEATURE_SAT
 #define __ssat(x, y) __builtin_arm_ssat(x, y)
 #define __usat(x, y) __builtin_arm_usat(x, y)
 #endif
 
 /* 9.4.2 Saturating addition and subtraction intrinsics */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __qadd(int32_t __t, int32_t __v) {
   return __builtin_arm_qadd(__t, __v);
@@ -305,7 +305,7 @@
 #endif
 
 /* 9.4.3 Accumultating multiplications */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smlabb(int32_t __a, int32_t __b, int32_t __c) {
   return __builtin_arm_smlabb(__a, __b, __c);
@@ -334,13 +334,13 @@
 
 
 /* 9.5.4 Parallel 16-bit saturation */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 #define __ssat16(x, y) __builtin_arm_ssat16(x, y)
 #define __usat16(x, y) __builtin_arm_usat16(x, y)
 #endif
 
 /* 9.5.5 Packing and unpacking */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 typedef int32_t int8x4_t;
 typedef int32_t int16x2_t;
 typedef uint32_t uint8x4_t;
@@ -365,7 +365,7 @@
 #endif
 
 /* 9.5.6 Parallel selection */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ uint8x4_t __attribute__((__always_inline__, __nodebug__))
 __sel(uint8x4_t __a, uint8x4_t __b) {
   return __builtin_arm_sel(__a, __b);
@@ -373,7 +373,7 @@
 #endif
 
 /* 9.5.7 Parallel 8-bit addition and subtraction */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int8x4_t __attribute__((__always_

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I don't have the time to rebase and retest this currently, so it might be 
better for someone else to take over this patch. Unfortunately, it's been long 
enough that I don't remember the details of these changes.


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

https://reviews.llvm.org/D28955



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:60
+// Set timeout to 15000ms = 15s
+Z3_set_param_value(Config, "timeout", "15000");
+  }

xazax.hun wrote:
> Sorry for being a bit late in the party, I was wondering whether this timeout 
> can be a source of non-determinism. The constraint solver might get lucky or 
> unlucky based on the load or the underlying hardware to solve some of the 
> constraints. We might end up with different results over different runs which 
> can be confusing for the users. Is there any other way, to set something like 
> a timeout, like limiting the number of steps inside the solver?
I believe Z3 uses various heuristics internally, so the exact execution trace 
can differ between multiple executions. However, I don't believe that this 
would be a common issue, since queries on fixed-width bitvector are decidable. 
The 15 sec timeout is intended to prevent a single query from stalling the 
static analyzer; I've only experienced this with complex queries over 
floating-point types (e.g. D28953 + D28954 + D28955), which can degenerate to 
bitblasting. I don't have the exact numbers right now, but given that the 200+ 
tests in the testsuite complete in ~90 secs with this patch, the average time 
per test is < 0.5 sec, which includes tens if not hundreds of individual SMT 
queries. As far as I know, this timeout parameter is the only support 
configuration parameter. I'd also like to point out that this solver isn't used 
unless the user both builds with it and specifies it at runtime.


https://reviews.llvm.org/D28952



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93587.
ddcc marked 4 inline comments as done.
ddcc added a comment.

Use direct bitcasting instead of string conversion for APFloat casting, add 
reference counting for Z3_sort, eliminate some duplicate code


https://reviews.llvm.org/D28952

Files:
  CMakeLists.txt
  cmake/modules/FindZ3.cmake
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Core/Analyses.def
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/expr-inspection.c
  test/Analysis/lit.local.cfg
  test/Analysis/unsupported-types.c
  test/lit.cfg
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -18,6 +18,7 @@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clang_staticanalyzer_z3 = "@CLANG_ANALYZER_WITH_Z3@"
 config.clang_examples = @CLANG_BUILD_EXAMPLES@
 config.enable_shared = @ENABLE_SHARED@
 config.enable_backtrace = @ENABLE_BACKTRACES@
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -361,6 +361,9 @@
 if config.clang_staticanalyzer:
 config.available_features.add("staticanalyzer")
 
+if config.clang_staticanalyzer_z3 == '1':
+config.available_features.add("z3")
+
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
 config.available_features.add('crash-recovery')
Index: test/Analysis/unsupported-types.c
===
--- /dev/null
+++ test/Analysis/unsupported-types.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-unknown-linux -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple powerpc64-linux-gnu -verify %s
+
+#define _Complex_I  (__extension__ 1.0iF)
+
+void clang_analyzer_eval(int);
+
+void complex_float(double _Complex x, double _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void complex_int(int _Complex x, int _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void longdouble_float(long double x, long double y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 0.0L && y != 1.0L)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 1.0L); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -10,6 +10,10 @@
 if result.code == lit.Test.FAIL:
 return result
 
+# If z3 backend available, add an additional run line for it
+if test.config.clang_staticanalyzer_z3 == '1':
+result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+
 return result
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -19,4 +19,4 @@
 
 // CHECK: Expressions:
 // CHECK-NEXT: clang_analyzer_printState : &code{clang_analyzer_printState}
-// CHECK-NEXT: Ranges are empty.
+// CHECK-NEXT: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -0,0 +1,1606 @@
+//== Z3ConstraintManager.cpp *- C++ -*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/Static

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Thanks for the feedback! My main constraint is that the results from the 
floating-point analysis weren't very interesting (see #652894 <#652894>), so 
I'm not actively working on further development.

> FYI I've been working on floating point support in KLEE and have extended it 
> to support floating point (note however only the Z3 backend actually supports 
> consuming floating point constraints). I've not yet open sourced what I've 
> done as I'm not entirely happy with the design but if there is interest we 
> could see if we could figure out a way of pulling klee::Expr (and the solver 
> bits) out of KLEE to make a standalone library. Note there is a project 
> called metaSMT that uses template meta programming to give the same interface 
> to multiple solvers. KLEE does support it but I'm not familiar with it.

I agree that it'd be useful to move to a more generic SMT interface, 
potentially SMT-LIB instead of something specific to a solver, but this is more 
of a long-term goal at the moment.

> KLEE has a few optimization ideas that you could consider implementing that 
> certainly help in the context of symbolic execution...

Likewise, I think adding a constraint cache and implementing performance 
optimizations would be the next step to getting this enabled by default, and 
perhaps deprecating the range constraint manager, but the implementation is a 
little tricky because there's state also being stored in the `ProgramState` 
object. Since those are reference counted, there's a little more work involved 
than just adding an intermediate layer with e.g. an `std::map`. But again, it's 
not something I have the time for at the moment.




Comment at: CMakeLists.txt:188
 
+find_package(Z3 4.5)
+

delcypher wrote:
> delcypher wrote:
> > @ddcc It is of course up to you but I recently [[ added support for using 
> > `libz3` directly | added support for using `libz3` directly ]] from CMake. 
> > via it's own CMake config package. You only get this if Z3 was built with 
> > CMake so you might not want this restriction.  This feature has only just 
> > landed though and might not be sufficient for your needs.  If you take a 
> > look at Z3's example projects they are now built with this mechanism when 
> > building with CMake.
> > 
> > If you are interested I'd be more than happy to work with you to get this 
> > feature ready for your needs in upstream Z3 so you can use it here.
> Sorry that URL should be https://github.com/Z3Prover/z3/pull/926
I think this is useful, and upstream z3 has been in need of better packaging. 
But until it's used by default over the current python script and the z3 folks 
actively maintain it, I'm a little hesitant to depend on it.



Comment at: include/clang/Config/config.h.cmake:42
+/* Define if we have z3 and want to build it */
+#cmakedefine CLANG_ANALYZER_WITH_Z3 ${CLANG_ANALYZER_WITH_Z3}
+

delcypher wrote:
> Do you really want such a specific name? How about 
> `CLANG_ANALYSER_HAS_SMT_SOLVER`?
There's been a bit of back and forth over this name in previous revisions, so 
I'm a little hesitant to change it. In essence, there are two CMake variables, 
one for whether z3 is present, and another for whether the user requested to 
build with z3, which are exported to a single C-level definition that is true 
iff both CMake variables are true.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:70
+public:
+  static Z3_context ZC;
+

delcypher wrote:
> @ddc
> This decision of having a global context might come back to bite you. 
> Especially if you switch to doing parallel solving in the future. This is why 
> KLEE's `Z3ASTHandle` and `Z3SortHandle` store the context so it's possible to 
> use different context.
I agree that it is an inherent limitation, but since the static analyzer itself 
is only single-threaded anyway, the entire analyzer will need significant 
changes before this becomes a problem.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:81
+
+class Z3Expr {
+  friend class Z3Model;

delcypher wrote:
> @ddcc 
> [[ 
> https://github.com/klee/klee/blob/1f13e9dbf9db2095b6612a47717c2b86e4aaba72/lib/Solver/Z3Builder.h#L20
>  | In KLEE I have something similar to represent Z3Expr ]] called Z3ASTHandle 
> and Z3SortHandle for doing manual reference counting. You might want to take 
> a look at. I don't see you doing reference counting on sorts here so I think 
> you might be leaking memory.
> 
> We also have a handy `dump()` method on `Z3ASTHandle` and `Z3SortHandle` for 
> debugging.
Originally, my understanding was that reference counting wasn't necessary for 
`Z3_sort` objects, based on the API documentation, and the lack of 
`Z3_sort_inc_ref()`/`Z3_sort_dec_ref()` functions. But, taking another look at 
the z3 source code, it seems that `Z3_sort` is casted directly to `Z3_ast`,  so 
I thin

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93588.
ddcc added a comment.

Fix erroneous comment


https://reviews.llvm.org/D28952

Files:
  CMakeLists.txt
  cmake/modules/FindZ3.cmake
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Core/Analyses.def
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/expr-inspection.c
  test/Analysis/lit.local.cfg
  test/Analysis/unsupported-types.c
  test/lit.cfg
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -18,6 +18,7 @@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clang_staticanalyzer_z3 = "@CLANG_ANALYZER_WITH_Z3@"
 config.clang_examples = @CLANG_BUILD_EXAMPLES@
 config.enable_shared = @ENABLE_SHARED@
 config.enable_backtrace = @ENABLE_BACKTRACES@
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -361,6 +361,9 @@
 if config.clang_staticanalyzer:
 config.available_features.add("staticanalyzer")
 
+if config.clang_staticanalyzer_z3 == '1':
+config.available_features.add("z3")
+
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
 config.available_features.add('crash-recovery')
Index: test/Analysis/unsupported-types.c
===
--- /dev/null
+++ test/Analysis/unsupported-types.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-unknown-linux -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple powerpc64-linux-gnu -verify %s
+
+#define _Complex_I  (__extension__ 1.0iF)
+
+void clang_analyzer_eval(int);
+
+void complex_float(double _Complex x, double _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void complex_int(int _Complex x, int _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void longdouble_float(long double x, long double y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 0.0L && y != 1.0L)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 1.0L); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -10,6 +10,10 @@
 if result.code == lit.Test.FAIL:
 return result
 
+# If z3 backend available, add an additional run line for it
+if test.config.clang_staticanalyzer_z3 == '1':
+result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+
 return result
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -19,4 +19,4 @@
 
 // CHECK: Expressions:
 // CHECK-NEXT: clang_analyzer_printState : &code{clang_analyzer_printState}
-// CHECK-NEXT: Ranges are empty.
+// CHECK-NEXT: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -0,0 +1,1606 @@
+//== Z3ConstraintManager.cpp *- C++ -*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h"
+
+#include "clang/Config/config.h

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93589.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -69,7 +69,7 @@
   static int stat;
   clang_analyzer_explain(x + 1); // expected-warning-re^\(argument 'x'\) \+ 1$
   clang_analyzer_explain(1 + y); // expected-warning-re^\(argument 'y'\) \+ 1$
-  clang_analyzer_explain(x + y); // expected-warning-re^unknown value$
+  clang_analyzer_explain(x + y); // expected-warning-re^\(argument 'x'\) \+ \(argument 'y'\)$
   clang_analyzer_explain(z); // expected-warning-re^undefined value$
   clang_analyzer_explain(&z); // expected-warning-re^pointer to local variable 'z'$
   clang_analyzer_explain(stat); // expected-warning-re^signed 32-bit integer '0'$
Index: test/Analysis/conditional-path-notes.c
===
--- test/Analysis/conditional-path-notes.c
+++ test/Analysis/conditional-path-notes.c
@@ -77,7 +77,8 @@
 
 void testNonDiagnosableBranchArithmetic(int a, int b) {
   if (a - b) {
-// expected-note@-1 {{Taking true branch}}
+// expected-note@-1 {{Assuming the condition is true}}
+// expected-note@-2 {{Taking true branch}}
 *(volatile int *)0 = 1; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1 {{Dereference of null pointer}}
   }
@@ -1573,12 +1574,75 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line79
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col11
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
@@ -1594,25 +1658,25 @@
 // CHECK-NEXT: start
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHEC

[PATCH] D28954: [analyzer] Add support for symbolic float expressions

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93590.
ddcc added a comment.

Rebase, update tests, fix bugs


https://reviews.llvm.org/D28954

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/FloatingPointMath.cpp
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/diagnostics/macros.cpp
  test/Analysis/float-rules.c
  test/Analysis/float.c
  test/Analysis/inline.cpp
  test/Analysis/lit.local.cfg
  test/Analysis/operator-calls.cpp

Index: test/Analysis/operator-calls.cpp
===
--- test/Analysis/operator-calls.cpp
+++ test/Analysis/operator-calls.cpp
@@ -81,8 +81,8 @@
   void test(int coin) {
 // Force a cache-out when we try to conjure a temporary region for the operator call.
 // ...then, don't crash.
-clang_analyzer_eval(+(coin ? getSmallOpaque() : getSmallOpaque())); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(+(coin ? getLargeOpaque() : getLargeOpaque())); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(+(coin ? getSmallOpaque() : getSmallOpaque())); // expected-warning{{TRUE}}
+clang_analyzer_eval(+(coin ? getLargeOpaque() : getLargeOpaque())); // expected-warning{{TRUE}}
   }
 }
 
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -1,20 +1,35 @@
+import lit.Test
 import lit.TestRunner
 import sys
 
 # Custom format class for static analyzer tests
 class AnalyzerTest(lit.formats.ShTest, object):
 
 def execute(self, test, litConfig):
-result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=range')
+results = []
 
-if result.code == lit.Test.FAIL:
-return result
+# Parse any test requirements ('REQUIRES: ')
+saved_test = test
+lit.TestRunner.parseIntegratedTestScript(test)
+
+if 'z3' not in test.requires:
+results.append(self.executeWithAnalyzeSubstitution(saved_test, litConfig, '-analyzer-constraints=range'))
+
+if results[-1].code == lit.Test.FAIL:
+return results[-1]
 
 # If z3 backend available, add an additional run line for it
 if test.config.clang_staticanalyzer_z3 == '1':
-result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+results.append(self.executeWithAnalyzeSubstitution(saved_test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3'))
 
-return result
+# Combine all result outputs into the last element
+for x in results:
+if x != results[-1]:
+results[-1].output = x.output + results[-1].output
+
+if results:
+return results[-1]
+return lit.Test.Result(lit.Test.UNSUPPORTED, "Test requires the following unavailable features: z3")
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
 saved_substitutions = list(test.config.substitutions)
Index: test/Analysis/inline.cpp
===
--- test/Analysis/inline.cpp
+++ test/Analysis/inline.cpp
@@ -285,11 +285,11 @@
   }
 
   void testFloatReference() {
-clang_analyzer_eval(defaultFloatReference(1) == -1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(defaultFloatReference() == -42); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(defaultFloatReference(1) == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(defaultFloatReference() == -42); // expected-warning{{TRUE}}
 
-clang_analyzer_eval(defaultFloatReferenceZero(1) == -1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(defaultFloatReferenceZero() == 0); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(defaultFloatReferenceZero(1) == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(defaultFloatReferenceZero() == 0); // expected-warning{{TRUE}}
   }
 
 

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93591.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D28955

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/dead-stores.m
  test/Analysis/explain-svals.cpp
  test/Analysis/malloc.c
  test/Analysis/misc-ps-eager-assume.m
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -146,7 +146,7 @@
 void test_isgraph_isprint(int x) {
   char y = x;
   if (isgraph(y))
-clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
+clang_analyzer_eval(isprint(y)); // expected-warning{{TRUE}}
 }
 
 int isdigit(int);
Index: test/Analysis/misc-ps-eager-assume.m
===
--- test/Analysis/misc-ps-eager-assume.m
+++ test/Analysis/misc-ps-eager-assume.m
@@ -1,5 +1,4 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks %s -analyzer-eagerly-assume
-// expected-no-diagnostics
 
 // Delta-reduced header stuff (needed for test cases).
 typedef signed char BOOL;
@@ -56,7 +55,7 @@
 void handle_symbolic_cast_in_condition(void) {
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
 
-  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"];
+  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"]; // expected-warning {{Assignment of a non-Boolean value}}
   NSMutableArray* array = needsAnArray ? [[NSMutableArray alloc] init] : 0;
   if(needsAnArray)
 [array release];
Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1656,13 +1656,13 @@
 void testOffsetPassedToStrlen() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
+  size_t length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
 }
 
 void testOffsetPassedToStrlenThenFree() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string);
+  size_t length = strlen(string);
   free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}}
 }
 
@@ -1705,7 +1705,7 @@
 }
 
 char *dupstrNoWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocNoWarn(len + 1);
   strcpy(p, s); // no-warning
   return p;
@@ -1721,7 +1721,7 @@
 }
 
 char *dupstrWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocWarn(len + 1);
   strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
   return p;
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -41,11 +41,11 @@
 
 void test_2(char *ptr, int ext) {
   clang_analyzer_explain((void *) "asdf"); // expected-warning-re^pointer to element of type 'char' with index 0 of string literal "asdf"$
-  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^cast of type 'int' of metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
   clang_analyzer_explain(conjure()); // expected-warning-re^symbol of type 'int' conjured at statement 'conjure\(\)'$
   clang_analyzer_explain(glob); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob'$
   clang_analyzer_explain(glob_ptr); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob_ptr'$
-  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^extent of pointee of argument 'ptr'$
+  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^cast of type 'int' of extent of pointee of argument 'ptr'$
   int *x = new int[ext];
   clang_analyzer_explain(x); // expected-warning-re^pointer to element of type 'int' with index 0 of heap segment that starts at symbol of type 'int \*' conjured at statement 'new int \[ext\]'$
   // Sic! What gets computed is the extent of the element-region.
Index: test/Analysis/dead-stores.m
===

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: cmake/modules/FindZ3.cmake:3
+# in the find_path() and find_library() calls
+find_package(PkgConfig QUIET)
+PKG_CHECK_MODULES(PC_Z3 QUIET libz3)

delcypher wrote:
> @ddcc Seeing as you don't want to use the new upstream Z3 CMake package 
> config files I'll try to review this.
> 
> Upstream Z3 does not come with pkg-config files for the native library so I'm 
> wondering where you expect this to work. Does a Linux distro add their own 
> `.pc` files?
> 
> It looks like you only use these paths as hints so this so this looks like 
> it'll work even without the pkg-config files.
See below.



Comment at: cmake/modules/FindZ3.cmake:5
+PKG_CHECK_MODULES(PC_Z3 QUIET libz3)
+set(Z3_DEFINITIONS ${PC_LIBZ3_CFLAGS_OTHER})
+

delcypher wrote:
> @ddcc This CMake variable is set but never used. Also based on the name it 
> suggests that they are compiler definitions rather than other compiler flags. 
> Does `_CFLAGS_OTHER` have those semantics? It's unclear from 
> https://cmake.org/cmake/help/v3.7/module/FindPkgConfig.html#command:pkg_check_modules
>  what they are supposed to be.
> 
> To consume these flags you could add `target_compile_definitions()` and 
> `target_compile_options()` to all users of Z3. A more elegant way of doing 
> this is to create an imported library target (e.g. `z3::libz3`) and set 
> compile definitions, compile options and include directories on this imported 
> target with `INTERFACE` visibility so that these usage requirements 
> implicitly propagate to all users of `z3::libz3`.
I'm not very familiar with CMake, so I based it off of the FindLibXml2.cmake 
from the upstream CMake repository. The `pkg-config` part isn't used currently, 
but I left it in case z3 does get a proper package.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:619
+
+llvm::APSInt Int = llvm::APSInt(Float.bitcastToAPInt(), true);
+Z3Expr Z3Int = Z3Expr::fromAPSInt(Int);

delcypher wrote:
> @ddcc Why use APSInt here? Why not APInt, we are looking at raw bits and 
> don't want to interpret the most significant bit in a special way.
Since the rest of the code already handles `APSInt`, I just reused that instead 
of implementing another method for `APInt`. The overhead is one additional bool 
used to store the sign, which doesn't make much difference, since it needs to 
be specified anyway with `APInt::toString*()`.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:668
+default:
+  llvm_unreachable("Unsupported sort to integer!");
+case Z3_BV_SORT: {

delcypher wrote:
> Is `Z3_FLOATING_POINT_SORT` possible in your implementation?
I don't think so. Things change around a bit with D28954, but by the time this 
method is called, the casting should have already been handled elsewhere.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:674
+  // are the same size.
+  Z3_get_numeral_uint64(Z3Context::ZC, AST,
+reinterpret_cast<__uint64 *>(&Value));

The only problem I see now is that if a IEEEquad is fed in, it will generate a 
128-bit bitvector, which will be truncated at this point. But the z3 API 
doesn't support retrieving a rational into anything larger than 64-bit, so 
perhaps converting it to string is the way to go here.


https://reviews.llvm.org/D28952



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93974.
ddcc added a comment.

Fix support for 128-bit APInt creation, drop pkg-config from CMake module


https://reviews.llvm.org/D28952

Files:
  CMakeLists.txt
  cmake/modules/FindZ3.cmake
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Core/Analyses.def
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/expr-inspection.c
  test/Analysis/lit.local.cfg
  test/Analysis/unsupported-types.c
  test/lit.cfg
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -18,6 +18,7 @@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clang_staticanalyzer_z3 = "@CLANG_ANALYZER_WITH_Z3@"
 config.clang_examples = @CLANG_BUILD_EXAMPLES@
 config.enable_shared = @ENABLE_SHARED@
 config.enable_backtrace = @ENABLE_BACKTRACES@
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -361,6 +361,9 @@
 if config.clang_staticanalyzer:
 config.available_features.add("staticanalyzer")
 
+if config.clang_staticanalyzer_z3 == '1':
+config.available_features.add("z3")
+
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
 config.available_features.add('crash-recovery')
Index: test/Analysis/unsupported-types.c
===
--- /dev/null
+++ test/Analysis/unsupported-types.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-unknown-linux -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple powerpc64-linux-gnu -verify %s
+
+#define _Complex_I  (__extension__ 1.0iF)
+
+void clang_analyzer_eval(int);
+
+void complex_float(double _Complex x, double _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void complex_int(int _Complex x, int _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void longdouble_float(long double x, long double y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 0.0L && y != 1.0L)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 1.0L); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -10,6 +10,10 @@
 if result.code == lit.Test.FAIL:
 return result
 
+# If z3 backend available, add an additional run line for it
+if test.config.clang_staticanalyzer_z3 == '1':
+result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+
 return result
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -19,4 +19,4 @@
 
 // CHECK: Expressions:
 // CHECK-NEXT: clang_analyzer_printState : &code{clang_analyzer_printState}
-// CHECK-NEXT: Ranges are empty.
+// CHECK-NEXT: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -0,0 +1,1618 @@
+//== Z3ConstraintManager.cpp *- C++ -*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleCon

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299463: [analyzer] Add new Z3 constraint manager backend 
(authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D28952?vs=93974&id=94107#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28952

Files:
  cfe/trunk/CMakeLists.txt
  cfe/trunk/cmake/modules/FindZ3.cmake
  cfe/trunk/include/clang/Config/config.h.cmake
  cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  cfe/trunk/test/Analysis/expr-inspection.c
  cfe/trunk/test/Analysis/lit.local.cfg
  cfe/trunk/test/Analysis/unsupported-types.c
  cfe/trunk/test/lit.cfg
  cfe/trunk/test/lit.site.cfg.in

Index: cfe/trunk/include/clang/Config/config.h.cmake
===
--- cfe/trunk/include/clang/Config/config.h.cmake
+++ cfe/trunk/include/clang/Config/config.h.cmake
@@ -38,6 +38,9 @@
 /* Define if we have libxml2 */
 #cmakedefine CLANG_HAVE_LIBXML ${CLANG_HAVE_LIBXML}
 
+/* Define if we have z3 and want to build it */
+#cmakedefine CLANG_ANALYZER_WITH_Z3 ${CLANG_ANALYZER_WITH_Z3}
+
 /* Define if we have sys/resource.h (rlimits) */
 #cmakedefine CLANG_HAVE_RLIMITS ${CLANG_HAVE_RLIMITS}
 
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
@@ -22,6 +22,7 @@
 #endif
 
 ANALYSIS_CONSTRAINTS(RangeConstraints, "range", "Use constraint tracking of concrete value ranges", CreateRangeConstraintManager)
+ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver", CreateZ3ConstraintManager)
 
 #ifndef ANALYSIS_DIAGNOSTICS
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -184,6 +184,9 @@
 CreateRangeConstraintManager(ProgramStateManager &statemgr,
  SubEngine *subengine);
 
+std::unique_ptr
+CreateZ3ConstraintManager(ProgramStateManager &statemgr, SubEngine *subengine);
+
 } // end GR namespace
 
 } // end clang namespace
Index: cfe/trunk/cmake/modules/FindZ3.cmake
===
--- cfe/trunk/cmake/modules/FindZ3.cmake
+++ cfe/trunk/cmake/modules/FindZ3.cmake
@@ -0,0 +1,28 @@
+find_path(Z3_INCLUDE_DIR NAMES z3.h
+   PATH_SUFFIXES libz3
+   )
+
+find_library(Z3_LIBRARIES NAMES z3 libz3
+   )
+
+find_program(Z3_EXECUTABLE z3)
+
+if(Z3_INCLUDE_DIR AND Z3_EXECUTABLE)
+execute_process (COMMAND ${Z3_EXECUTABLE} -version
+  OUTPUT_VARIABLE libz3_version_str
+  ERROR_QUIET
+  OUTPUT_STRIP_TRAILING_WHITESPACE)
+
+string(REGEX REPLACE "^Z3 version ([0-9.]+)" "\\1"
+   Z3_VERSION_STRING "${libz3_version_str}")
+unset(libz3_version_str)
+endif()
+
+# handle the QUIETLY and REQUIRED arguments and set Z3_FOUND to TRUE if
+# all listed variables are TRUE
+include(FindPackageHandleStandardArgs)
+FIND_PACKAGE_HANDLE_STANDARD_ARGS(Z3
+  REQUIRED_VARS Z3_LIBRARIES Z3_INCLUDE_DIR
+  VERSION_VAR Z3_VERSION_STRING)
+
+mark_as_advanced(Z3_INCLUDE_DIR Z3_LIBRARIES)
Index: cfe/trunk/test/lit.site.cfg.in
===
--- cfe/trunk/test/lit.site.cfg.in
+++ cfe/trunk/test/lit.site.cfg.in
@@ -18,6 +18,7 @@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clang_staticanalyzer_z3 = "@CLANG_ANALYZER_WITH_Z3@"
 config.clang_examples = @CLANG_BUILD_EXAMPLES@
 config.enable_shared = @ENABLE_SHARED@
 config.enable_backtrace = @ENABLE_BACKTRACES@
Index: cfe/trunk/test/lit.cfg
===
--- cfe/trunk/test/lit.cfg
+++ cfe/trunk/test/lit.cfg
@@ -361,6 +361,9 @@
 if config.clang_staticanalyzer:
 config.available_features.add("staticanalyzer")
 
+if config.clang_staticanalyzer_z3 == '1':
+config.available_features.add("z3")
+
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
 config.available_features.add('crash-recovery')
Index: cfe/trunk/test/Analysis/unsupported-types.c
===
--- cfe/trunk/test/Analysis/unsupported-types.c
+++ cfe/trunk/test/Analysis/unsupported-ty

[PATCH] D31756: [cmake] Support Gentoo install for z3

[Dominic Chen via Phabricator via cfe-commits]

ddcc accepted this revision.
ddcc added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D31756



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

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 85136.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D26061

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.h

Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -7,12 +7,12 @@
 //
 //===--===//
 //
-//  This file defines SimpleConstraintManager, a class that holds code shared
-//  between BasicConstraintManager and RangeConstraintManager.
+//  This file defines SimpleConstraintManager, a class that provides a
+//  simplified constraint manager interface, compared to ConstraintManager.
 //
 //===--===//
 
-#include "SimpleConstraintManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -23,48 +23,6 @@
 
 SimpleConstraintManager::~SimpleConstraintManager() {}
 
-bool SimpleConstraintManager::canReasonAbout(SVal X) const {
-  Optional SymVal = X.getAs();
-  if (SymVal && SymVal->isExpression()) {
-const SymExpr *SE = SymVal->getSymbol();
-
-if (const SymIntExpr *SIE = dyn_cast(SE)) {
-  switch (SIE->getOpcode()) {
-  // We don't reason yet about bitwise-constraints on symbolic values.
-  case BO_And:
-  case BO_Or:
-  case BO_Xor:
-return false;
-  // We don't reason yet about these arithmetic constraints on
-  // symbolic values.
-  case BO_Mul:
-  case BO_Div:
-  case BO_Rem:
-  case BO_Shl:
-  case BO_Shr:
-return false;
-  // All other cases.
-  default:
-return true;
-  }
-}
-
-if (const SymSymExpr *SSE = dyn_cast(SE)) {
-  if (BinaryOperator::isComparisonOp(SSE->getOpcode())) {
-// We handle Loc <> Loc comparisons, but not (yet) NonLoc <> NonLoc.
-if (Loc::isLocType(SSE->getLHS()->getType())) {
-  assert(Loc::isLocType(SSE->getRHS()->getType()));
-  return true;
-}
-  }
-}
-
-return false;
-  }
-
-  return true;
-}
-
 ProgramStateRef SimpleConstraintManager::assume(ProgramStateRef State,
 DefinedSVal Cond,
 bool Assumption) {
@@ -92,23 +50,6 @@
   return State;
 }
 
-ProgramStateRef
-SimpleConstraintManager::assumeAuxForSymbol(ProgramStateRef State,
-SymbolRef Sym, bool Assumption) {
-  BasicValueFactory &BVF = getBasicVals();
-  QualType T = Sym->getType();
-
-  // None of the constraint solvers currently support non-integer types.
-  if (!T->isIntegralOrEnumerationType())
-return State;
-
-  const llvm::APSInt &zero = BVF.getValue(0, T);
-  if (Assumption)
-return assumeSymNE(State, Sym, zero, zero);
-  else
-return assumeSymEQ(State, Sym, zero, zero);
-}
-
 ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef State,
NonLoc Cond,
bool Assumption) {
@@ -118,7 +59,8 @@
   if (!canReasonAbout(Cond)) {
 // Just add the constraint to the expression without trying to simplify.
 SymbolRef Sym = Cond.getAsSymExpr();
-return assumeAuxForSymbol(State, Sym, Assumption);
+assert(Sym);
+return assumeSymUnsupported(State, Sym, Assumption);
   }
 
   switch (Cond.getSubKind()) {
@@ -129,51 +71,7 @@
 nonloc::SymbolVal SV = Cond.castAs();
 SymbolRef Sym = SV.getSymbol();
 assert(Sym);
-
-// Handle SymbolData.
-if (!SV.isExpression()) {
-  return assumeAuxForSymbol(State, Sym, Assumption);
-
-  // Handle symbolic expression.
-} else if (const SymIntExpr *SE = dyn_cast(Sym)) {
-  // We can only simplify expressions whose RHS is an integer.
-
-  BinaryOperator::Opcode Op = SE->getOpcode();
-  if (BinaryOperator::isComparisonOp(Op)) {
-if (!Assumption)
-  Op = BinaryOperator::negateComparisonOp(Op);
-
-return assumeSymRel(State, SE->getLHS(), Op, SE->getRHS());
-  }
-
-} else if (const SymSymExpr *SSE = dyn_cast(Sym)) {
-  // Translate "a != b" to "(b - a) != 0

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 85139.
ddcc added a comment.

Fix rebase


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -69,7 +69,7 @@
   static int stat;
   clang_analyzer_explain(x + 1); // expected-warning-re^\(argument 'x'\) \+ 1$
   clang_analyzer_explain(1 + y); // expected-warning-re^\(argument 'y'\) \+ 1$
-  clang_analyzer_explain(x + y); // expected-warning-re^unknown value$
+  clang_analyzer_explain(x + y); // expected-warning-re^\(argument 'x'\) \+ \(argument 'y'\)$
   clang_analyzer_explain(z); // expected-warning-re^undefined value$
   clang_analyzer_explain(&z); // expected-warning-re^pointer to local variable 'z'$
   clang_analyzer_explain(stat); // expected-warning-re^signed 32-bit integer '0'$
Index: test/Analysis/conditional-path-notes.c
===
--- test/Analysis/conditional-path-notes.c
+++ test/Analysis/conditional-path-notes.c
@@ -77,7 +77,8 @@
 
 void testNonDiagnosableBranchArithmetic(int a, int b) {
   if (a - b) {
-// expected-note@-1 {{Taking true branch}}
+// expected-note@-1 {{Assuming the condition is true}}
+// expected-note@-2 {{Taking true branch}}
 *(volatile int *)0 = 1; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1 {{Dereference of null pointer}}
   }
@@ -1573,12 +1574,75 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line79
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col11
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
@@ -1594,25 +1658,25 @@
 // CHECK-NEXT: start
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // 

[PATCH] D28954: [analyzer] Add support for symbolic float expressions

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
Herald added a subscriber: mgorny.

With the Z3 constraint manager, symbolic floating-point constraints can also be 
reasoned about. This commit includes a basic floating-point checker for domain 
errors with math functions.


https://reviews.llvm.org/D28954

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/FloatingPointMath.cpp
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  test/Analysis/diagnostics/macros.cpp
  test/Analysis/float-rules.c
  test/Analysis/float.c
  test/Analysis/inline.cpp
  test/Analysis/operator-calls.cpp
  tools/z3-constraint-manager/Z3ConstraintManager.cpp

Index: tools/z3-constraint-manager/Z3ConstraintManager.cpp
===
--- tools/z3-constraint-manager/Z3ConstraintManager.cpp
+++ tools/z3-constraint-manager/Z3ConstraintManager.cpp
@@ -77,6 +77,12 @@
 
 Z3_ast Assign = Z3_model_get_const_interp(Z3Expr::ZC, Model, Func);
 Z3_sort Sort = Z3_get_sort(Z3Expr::ZC, Assign);
+if (Z3_get_sort_kind(Z3Expr::ZC, Sort) == Z3_FLOATING_POINT_SORT) {
+  llvm::APFloat Float(llvm::APFloat::Bogus());
+  if (!Z3Expr::toAPFloat(Sort, Assign, Float, false))
+return false;
+  return BasicValueFactory::Convert(Int, Float);
+}
 return Z3Expr::toAPSInt(Sort, Assign, Int, true);
   }
 
@@ -88,6 +94,12 @@
 
 Z3_ast Assign = Z3_model_get_const_interp(Z3Expr::ZC, Model, Func);
 Z3_sort Sort = Z3_get_sort(Z3Expr::ZC, Assign);
+if (Z3_get_sort_kind(Z3Expr::ZC, Sort) != Z3_FLOATING_POINT_SORT) {
+  llvm::APSInt Int;
+  if (!Z3Expr::toAPSInt(Sort, Assign, Int, false))
+return false;
+  return BasicValueFactory::Convert(Float, Int);
+}
 return Z3Expr::toAPFloat(Sort, Assign, Float, true);
   }
 
@@ -617,8 +629,8 @@
 }; // end class Z3Expr
 
 void Z3ErrorHandler(Z3_context Context, Z3_error_code Error) {
-  llvm::report_fatal_error("Z3 error: " +
-   llvm::Twine(Z3_get_error_msg_ex(Context, Error)));
+  llvm::report_fatal_error(
+  "Z3 error: " + llvm::Twine(Z3_get_error_msg_ex(Context, Error)));
 }
 
 Z3_context Z3Expr::ZC;
@@ -660,6 +672,9 @@
   const llvm::APSInt *getSymVal(ProgramStateRef State,
 SymbolRef Sym) const override;
 
+  const llvm::APFloat *getSymFloatVal(ProgramStateRef State,
+  SymbolRef Sym) const override;
+
   ProgramStateRef removeDeadBindings(ProgramStateRef St,
  SymbolReaper &SymReaper) override;
 
@@ -740,6 +755,10 @@
   // Helper functions.
   //===--===//
 
+  // Recover the QualType of an APFloat.
+  // TODO: Refactor to put elsewhere
+  QualType getAPFloatType(const llvm::APFloat &Float) const;
+
   // Recover the QualType of an APSInt.
   // TODO: Refactor to put elsewhere
   QualType getAPSIntType(const llvm::APSInt &Int) const;
@@ -765,6 +784,11 @@
   void doFloatTypeConversion(T &LHS, QualType getLHS();
   } else if (const IntSymExpr *ISE = dyn_cast(BSE)) {
 Sym = ISE->getRHS();
+  } else if (const SymFloatExpr *SFE = dyn_cast(BSE)) {
+Sym = SFE->getLHS();
+  } else if (const FloatSymExpr *FSE = dyn_cast(BSE)) {
+Sym = FSE->getRHS();
   } else if (const SymSymExpr *SSM = dyn_cast(BSE)) {
 return canReasonAbout(nonloc::SymbolVal(SSM->getLHS())) &&
canReasonAbout(nonloc::Sy

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.

With the Z3 constraint manager, symbolic extension and truncation of variables 
can be fully reasoned about.


https://reviews.llvm.org/D28955

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/bool-assignment.c
  test/Analysis/dead-stores.m
  test/Analysis/explain-svals.cpp
  test/Analysis/malloc.c
  test/Analysis/misc-ps-eager-assume.m
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -146,7 +146,7 @@
 void test_isgraph_isprint(int x) {
   char y = x;
   if (isgraph(y))
-clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
+clang_analyzer_eval(isprint(y)); // expected-warning{{TRUE}}
 }
 
 int isdigit(int);
Index: test/Analysis/misc-ps-eager-assume.m
===
--- test/Analysis/misc-ps-eager-assume.m
+++ test/Analysis/misc-ps-eager-assume.m
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 %z3_cc1 -analyze -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks %s -analyzer-eagerly-assume
-// expected-no-diagnostics
 
 // Delta-reduced header stuff (needed for test cases).
 typedef signed char BOOL;
@@ -56,7 +55,7 @@
 void handle_symbolic_cast_in_condition(void) {
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
 
-  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"];
+  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"]; // expected-warning {{Assignment of a non-Boolean value}}
   NSMutableArray* array = needsAnArray ? [[NSMutableArray alloc] init] : 0;
   if(needsAnArray)
 [array release];
Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1656,13 +1656,13 @@
 void testOffsetPassedToStrlen() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
+  size_t length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
 }
 
 void testOffsetPassedToStrlenThenFree() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string);
+  size_t length = strlen(string);
   free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}}
 }
 
@@ -1705,7 +1705,7 @@
 }
 
 char *dupstrNoWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocNoWarn(len + 1);
   strcpy(p, s); // no-warning
   return p;
@@ -1721,7 +1721,7 @@
 }
 
 char *dupstrWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocWarn(len + 1);
   strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
   return p;
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -41,11 +41,11 @@
 
 void test_2(char *ptr, int ext) {
   clang_analyzer_explain((void *) "asdf"); // expected-warning-re^pointer to element of type 'char' with index 0 of string literal "asdf"$
-  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^cast of type 'int' of metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
   clang_analyzer_explain(conjure()); // expected-warning-re^symbol of type 'int' conjured at statement 'conjure\(\)'$
   clang_analyzer_explain(glob); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob'$
   clang_analyzer_explain(glob_ptr); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob_ptr'$
-  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^extent of pointee of argument 'ptr'$
+  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^cast of type 'int' of extent of pointee of argument 'ptr'$
   int *x = new int[ext];
   clang_analyzer_explain(x); // expected-warning-re^pointer to element of type 'int' with index 0 of heap segment that starts at symbol of type 'int \*' conjured at statement 'new int \[ext\]'$}

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

This is a fairly large patch, but the core of it is the actual implementation 
of the Z3 constraint solver backend. In its current form, the backend is 
implemented as a Clang plugin,  but I also have a git commit that has it inside 
the static analyzer. The non-plugin approach makes testing easier, by avoiding 
the introduction of new `%z3` and %z3_cc1` variables in `lit.cfg` and modifying 
all the testcases to load in the Z3 constraint manager plugin.

1. The testcase `Analysis/unsupported-types.c` currently fails, because when 
using the constraint manager as a Clang plugin, somehow a copy of 
`llvm::APFloat` is getting compiled into the shared library. Thus, comparisons 
of `&TI.getLongDoubleFormat()` against `&llvm::APFloat::x87DoubleExtended()` or 
`&llvm::APFloat::PPCDoubleDouble()` are always failing, which causes a 
subsequent assertion error. When testing the non-plugin implementation, it 
works fine.
2. The testcase `Analysis/reference.cpp` currently fails, because the 
`RangeConstraintManager` has an internal optimization that assumes references 
are known to be non-zero (see RangeConstraintManager.cpp:422). This 
optimization is probably in the wrong place, and should be moved into the 
analyzer and out of the constraint solver.
3. The testcase `Analysis/ptr-arith.c` currently fails, because handling of 
`ptrdiff_t` is somewhat tricky. This constraint manager interface maps 
`ptrdiff_t` to a signed bitvector in Z3, and pointers to unsigned bitvectors in 
Z3. However, since this testcase compares the pointer values both directly and 
indirectly via `ptrdiff_t`, if `a < b` is true using a signed comparison, the 
same is not necessarily true using an unsigned comparison.
4. Because of the divergence in capabilities between this and the existing 
constraint manager, the follow-up child patches that add support for additional 
features will probably cause problems for the existing constraint manager. I 
haven't fully tested that, but there is an issue of how to update the testsuite 
results without duplicating most of the existing testcases for each constraint 
manager backend.
5. Since this constraint manager uses Z3, the plugin links against the Z3 
shared library. I believe this should be fine since Z3 is under a MIT license, 
but I'm not really familiar with any licensing issues.
6. This plugin requires Z3 4.5.0 or later. I'm not sure if this is available in 
any distribution package repositories, so it may be necessary to build from 
source.


https://reviews.llvm.org/D28952



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

[PATCH] D28954: [analyzer] Add support for symbolic float expressions

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

- This patch introduces a virtual overloaded `llvm::APFloat getSymVal()` for 
returning the value of known floating-point constants. In the long term, both 
`getSymVal()` functions should probably be refactored to return something like 
a `llvm::Con
- There are also long-term issues with the flexibility of the `SymExpr` class. 
Currently, this patch adds two `SymFloatExpr` and `FloatSymExpr` subclasses, 
but this isn't expressive enough for certain types of symbolic queries. For 
example, the current implementation checks whether a floating-point value is 
`NaN` by comparing whether `V == V`, which should only be false for `NaN`. It 
would be better to add support for special queries for whether a value is NaN, 
positive infinity, negative infinity, etc.


https://reviews.llvm.org/D28954



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Do you want me to replace this version of the patch with one that omits the 
test case changes? The underlying git commit for just the Z3 constraint manager 
implementation is 
https://github.com/ddcc/clang/commit/e1414d300882c1459f461424d3e89d1613ecf03c , 
and 
https://github.com/ddcc/clang/commit/fb7d558be6492f11a3793f011f364098ddcc9711 
is the commit that converts it to a plugin and modifies all the testcases.

I looked around for a generic smt-lib interface earlier, but couldn't find much 
available, and since I wanted floating-point arithmetic support, I ended up 
just directly using the Z3 C interface (the C++ interface uses exceptions, 
which causes problems). As far as I know, CVC4 doesn't have built-in 
floating-point support. But longer term, I'd agree that this backend should be 
made more generic.

I'm not sure what's the easiest way to maintain backwards support with 
RangeConstraintManager. Right now, I've modified `lit.cfg` to substitute `%z3` 
and `%z3_cc1` (for `clang` or `clang -cc1`) with the appropriate argument to 
load the plugin, if the `z3` feature is available. Otherwise, they are 
substituted with just the empty string. Another approach would be to do this 
all in `llvm-lit` without modifying the testcases, but then there'd need to be 
some sort of path-based matching to determine when the argument to load the 
plugin should be injected (e.g. only for `test/Analysis`). However, if this 
constraint manager were built into clang instead of as a plugin, then this 
issue is moot.

Another issue is that some testcases will have different results depending on 
the constraint manager in use, but I don't believe it's possible to change the 
expected testcase output based on the presence of a feature flag. Unless this 
changes, there'll need to be (mostly) duplicate testcases for each constraint 
manager.

Furthermore, this and the child patches will cause the static analyzer to 
generate more complex constraints at runtime. But, I'm not sure if just having 
RangeConstraintManager ignore unsupported constraints will be sufficient, from 
a performance and correctness perspective.


https://reviews.llvm.org/D28952



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

When I was testing this patch, it was on top of both 
https://reviews.llvm.org/D28952 and https://reviews.llvm.org/D28953. For 
`malloc.c`, the change on line 1708 from `int` to `size_t` is necessary to 
prevent a false positive warning at line 1710. The other three changes don't 
affect the testcase output, but I probably made the changes at the same time. 
Do you want those other three pulled out separately?


https://reviews.llvm.org/D28955



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> For such test cases i'd consider something like this:
> 
>   // RUN: ... -analyzer-constraints=range -DRANGE ...
>   // RUN: ... -analyzer-constraints=z3 ...
>   
>   #ifdef RANGE
>   foo(); // expected-warning{{}}
>   #else
>   foo(); // no-warning
>   #endif

Would this be assuming that the test setup has Z3 installed and both constraint 
managers available? I see that `llvm-lit` supports a `REQUIRES` directive, but 
I don't know if that can be scoped to just one `RUN`.

> Yep, the analyzer core has a lot of code that says "we denote this value with 
> an incorrect symbol here, because even if we provided the correct symbol it 
> would be too hard for the RangeConstraintManager to understand". The most 
> obvious place is the integral cast code, which leaves the symbol intact 
> *most* of the time (all the time until recently).
> 
> I think the right way to address these issues is introduce an API in the base 
> ConstraintManager interface to ask the solver if he implements a certain 
> feature or is capable of dealing with a particular kind of symbol, and change 
> our behavior accordingly. We already have one function that was intended to 
> do something like this - `ConstraintManager::canReasonAbout()`; not sure if 
> it's working or broken.

`canReasonAbout()` works fine, but in its current form, requires parsing each 
constraint to determine whether its supported or not. In 
https://reviews.llvm.org/D26061, I've pushed the implementation from 
SimpleConstraintManager into RangeConstraintManager itself, since 
Z3ConstraintManager will also inherit from SimpleConstraintManager but with a 
different feature set. However, even with these changes, `canReasonAbout()` is 
only called from within SimpleConstraintManager; the feedback doesn't go all 
the way back to e.g. SimpleSValBuilder to dynamically change the constraint 
complexity at runtime.


https://reviews.llvm.org/D28952



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> We should have expected-warning on 64-bit targets (where `size_t` easily 
> overflows `int`) and no-warning on 32-bit targets (where they are of the same 
> size and the fix for the original issue 
> https://llvm.org/bugs/show_bug.cgi?id=16558 applies). I think we should have 
> two run-lines for this test, with two concrete targets specified; it'd be 
> great to actually have other tests in this file undergo such trial.

To clarify, you're asking for something like the following, instead of changing 
from `int` to `size_t`?

  diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
  index 42deb9f..80e4184 100644
  --- a/test/Analysis/malloc.c
  +++ b/test/Analysis/malloc.c
  @@ -1,4 +1,5 @@
  -// RUN: %clang_cc1 %z3_cc1 -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,debug.ExprInspection
 -analyzer-store=region -verify %s
  +// RUN: %clang_cc1 %z3_cc1 -triple i386-unknown-linux -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,debug.ExprInspection
 -analyzer-store=region -verify %s
  +// RUN: %clang_cc1 %z3_cc1 -triple x86_64-unknown-linux -Dx86_64 -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,debug.ExprInspection
 -analyzer-store=region -verify %s
  
   #include "Inputs/system-header-simulator.h"
  
  @@ -1705,9 +1706,13 @@ void *smallocNoWarn(size_t size) {
   }
  
   char *dupstrNoWarn(const char *s) {
  -  const size_t len = strlen(s);
  +  const int len = strlen(s);
 char *p = (char*) smallocNoWarn(len + 1);
  -  strcpy(p, s); // no-warning
  +#ifdef x86_64
  +  strcpy(p, s); // expected-warning{{String copy function overflows 
destination buffer}}
  +#else
  +  strcpy(p, s); // no warning
  +#endif
 return p;
   }


https://reviews.llvm.org/D28955



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 85313.
ddcc added a comment.

Convert to plugin, add move/assignment constructor, avoid Z3_simplify(), use 
Z3_mk_simple_solver(), generate logical and of all constraints in solver


https://reviews.llvm.org/D28952

Files:
  CMakeLists.txt
  cmake/modules/FindZ3.cmake
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Core/Analyses.def
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bool-assignment.c
  test/Analysis/expr-inspection.c
  test/Analysis/ptr-arith.c
  test/Analysis/unsupported-types.c
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -15,6 +15,7 @@
 config.target_triple = "@TARGET_TRIPLE@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
 config.have_zlib = "@HAVE_LIBZ@"
+config.have_z3 = "@CLANG_HAVE_Z3@"
 config.clang_arcmt = @ENABLE_CLANG_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @ENABLE_CLANG_STATIC_ANALYZER@
Index: test/Analysis/unsupported-types.c
===
--- /dev/null
+++ test/Analysis/unsupported-types.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -triple x86_64-unknown-linux -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -triple powerpc64-linux-gnu -verify %s
+
+#define _Complex_I  (__extension__ 1.0iF)
+
+void clang_analyzer_eval(int);
+
+void complex_float(double _Complex x, double _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void complex_int(int _Complex x, int _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void longdouble_float(long double x, long double y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 0.0L && y != 1.0L)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 1.0L); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -255,30 +255,24 @@
 void zero_implies_reversed_equal(int *lhs, int *rhs) {
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
   if ((rhs - lhs) == 0) {
-// FIXME: Should be FALSE.
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-// FIXME: Should be TRUE.
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
+clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
 return;
   }
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-  // FIXME: Should be FALSE.
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
+  clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
 }
 
 void canonical_equal(int *lhs, int *rhs) {
   clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
   if (lhs == rhs) {
-// FIXME: Should be TRUE.
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
 return;
   }
   clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
 
-  // FIXME: Should be FALSE.
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
 }
 
 void compare_element_region_and_base(int *p) {
Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -19,4 +19,4 @@
 
 // CHECK: Expressions:
 // CHECK-NEXT: clang_analyzer_printState : &code{clang_analyzer_printState}
-// CHECK-NEXT: Ranges are empty.
+// CHECK-NEXT: {{(Ranges are empty)|(Constraints:[[:space:]]*$)}}
Index: test/Analysis/bool-assignment.c
===
--- test/Analysis/bool

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I'd like to note that one of the main challenges with this implementation was 
needing to perform type reinference and implicit casting within the constraint 
manager, since the constraints are sometimes implicit. (e.g. `(int) x` instead 
of `x == 0`). It also didn't help that the Sema methods for handling type 
conversion aren't directly usable.

> Can you talk a bit about your motivation for working on this project and what 
> your goals are?

The original goal was to add support for symbolic execution on floating-point 
types, and analyze some real-world programs to look for interesting bugs. 
Unfortunately, it turns out that most floating-point bugs in the past tend to 
originate lower in the compiler toolchain, within the register allocator (e.g. 
non-determinism involving x87 FPU 80-bit registers), intermediate optimization 
passes (see Alive-FP), string conversions involving floating-point types (which 
may be too complex to model easily), or unexpected interactions between various 
1st/3rd-party code when modifying the C99 thread-local floating-point 
environment (from fpenv.h). But the latter isn't really well modeled by 
Clang/LLVM, resulting in false-positives when real-world program use 
floating-point exception handlers to catch underflow/overflow/etc conditions. 
There's also the floating-point rounding mode that can be changed at runtime, 
but again, it's not something that's currently modeled by Clang/LLVM. I have 
seen a set of patches on the mailing list (by Sergey, I believe) that improve 
support for this, but to my knowledge they haven't been merged yet.

> Have you compared the performance when using Z3 vs the current builtin 
> solver? I saw that you mention performance issues on large codebases, but 
> could you give some specific numbers of the tradeoffs between performance, 
> code coverage, and the quality of reported bugs. (One very rough idea to use 
> a stronger solver while still keeping the analyzer performant would be to 
> only use it for false positive refutation.)

On a virtualized i5-2500k system, I get the following performance numbers on 
the testsuite, with a Clang/LLVM BUILD_SHARED_LIBS release build and Z3 
https://github.com/z3prover/z3/commits/1bfd09e16bd9aaeae8a6b2841a2e8da615fdcda4 
(post-4.5.0):
RangeConstraintManager: 13.46 sec
Z3ConstraintManager (plugin): 416.5 sec
Z3ConstraintManager (built-in): 414.6 sec
Z3ConstraintManager (built-in, with below changes): 303.92 sec
Z3ConstraintManager (built-in, with below changes and no simplify): 250.71 sec

In addition to the caveats above, the other more practical problem is that the 
performance is really bad on medium to large-sized codebases. I did some 
testing on gmp, mpfr, libvpx, daala, libsdl, and opus, and for all of these 
projects, with static analysis enabled (via scan-build, and a release mode 
Clang/LLVM), the builds (in parallel) wouldn't finish even after a week on a 
Xeon E5-2620v3 workstation, and there were a lot of false positives that were 
hard to dig through. I did end up finding and reporting one bug in mpfr 
(https://sympa.inria.fr/sympa/arc/mpfr/2017-01/msg00012.html), but it's not 
floating-point related, and I didn't check if it could also be found by vanilla 
Clang/LLVM. With regards to the performance, I believe that this was due to 
multiple factors, and correct me if I'm wrong:

1. The Z3 solver timeout is set to 15 sec, which is probably too large (for an 
example that fails, see https://github.com/Z3Prover/z3/issues/834). But when 
timeouts do occur, they can cause runtime failures in the static analyzer: e.g. 
the assertion to avoid overconstrained states (in ConstraintManager.h), or 
underconstrained states (in BugReporterVisitors.cpp).
2. The symbolic expression complexity parameter (`MaxComp`) is set to 1, 
which is probably also too large. I'm guessing that overly-complex constraints 
are being generated, which can't be meaningfully solved and just bog down the 
analyzer.
3. Within the analyzer, states are removed and iterated one-by-one off of the 
internal worklist, in a single-thread in a single-process. I'm guessing that 
this means the analyzer ends up waiting on Z3 most of the time. This is also 
why I started running analyses in parallel on different projects, because the 
analysis for each project would occupy just one core.
4. The same queries end up being sent to the constraint manager despite no 
meaningful change in the program state, through either `assume()` or 
`getSymVal()`. But for expensive queries, this means that a lot of time/compute 
is just wasted. I think this could be improved by implementing a SMT query 
cache, and potentially by only adding relevant constraints to the solver state. 
I actually have a draft attempt at a simple query cache implementation 
(https://github.com/ddcc/clang/commit/e0236ff8f7b9c16dd29e8529420ab14b4309c3e6),
 but it's very hacky, isn't entirely memory-safe, caused some testsuite 
failures, and I ran o

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 85314.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -69,7 +69,7 @@
   static int stat;
   clang_analyzer_explain(x + 1); // expected-warning-re^\(argument 'x'\) \+ 1$
   clang_analyzer_explain(1 + y); // expected-warning-re^\(argument 'y'\) \+ 1$
-  clang_analyzer_explain(x + y); // expected-warning-re^unknown value$
+  clang_analyzer_explain(x + y); // expected-warning-re^\(argument 'x'\) \+ \(argument 'y'\)$
   clang_analyzer_explain(z); // expected-warning-re^undefined value$
   clang_analyzer_explain(&z); // expected-warning-re^pointer to local variable 'z'$
   clang_analyzer_explain(stat); // expected-warning-re^signed 32-bit integer '0'$
Index: test/Analysis/conditional-path-notes.c
===
--- test/Analysis/conditional-path-notes.c
+++ test/Analysis/conditional-path-notes.c
@@ -77,7 +77,8 @@
 
 void testNonDiagnosableBranchArithmetic(int a, int b) {
   if (a - b) {
-// expected-note@-1 {{Taking true branch}}
+// expected-note@-1 {{Assuming the condition is true}}
+// expected-note@-2 {{Taking true branch}}
 *(volatile int *)0 = 1; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1 {{Dereference of null pointer}}
   }
@@ -1573,12 +1574,75 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line79
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col11
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
@@ -1594,25 +1658,25 @@
 // CHECK-NEXT: start
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHEC

[PATCH] D28954: [analyzer] Add support for symbolic float expressions

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 85315.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D28954

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/FloatingPointMath.cpp
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/diagnostics/macros.cpp
  test/Analysis/float-rules.c
  test/Analysis/float.c
  test/Analysis/inline.cpp
  test/Analysis/operator-calls.cpp

Index: test/Analysis/operator-calls.cpp
===
--- test/Analysis/operator-calls.cpp
+++ test/Analysis/operator-calls.cpp
@@ -81,8 +81,8 @@
   void test(int coin) {
 // Force a cache-out when we try to conjure a temporary region for the operator call.
 // ...then, don't crash.
-clang_analyzer_eval(+(coin ? getSmallOpaque() : getSmallOpaque())); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(+(coin ? getLargeOpaque() : getLargeOpaque())); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(+(coin ? getSmallOpaque() : getSmallOpaque())); // expected-warning{{TRUE}}
+clang_analyzer_eval(+(coin ? getLargeOpaque() : getLargeOpaque())); // expected-warning{{TRUE}}
   }
 }
 
Index: test/Analysis/inline.cpp
===
--- test/Analysis/inline.cpp
+++ test/Analysis/inline.cpp
@@ -285,11 +285,11 @@
   }
 
   void testFloatReference() {
-clang_analyzer_eval(defaultFloatReference(1) == -1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(defaultFloatReference() == -42); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(defaultFloatReference(1) == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(defaultFloatReference() == -42); // expected-warning{{TRUE}}
 
-clang_analyzer_eval(defaultFloatReferenceZero(1) == -1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(defaultFloatReferenceZero() == 0); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(defaultFloatReferenceZero(1) == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(defaultFloatReferenceZero() == 0); // expected-warning{{TRUE}}
   }
 
   char defaultString(const char *s = "abc") {
Index: test/Analysis/float.c
===
--- /dev/null
+++ test/Analysis/float.c
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+// REQUIRES: z3
+
+void clang_analyzer_eval(int);
+
+void float1() {
+  float z1 = 0.0, z2 = -0.0;
+  clang_analyzer_eval(z1 == z2); // expected-warning{{TRUE}}
+}
+
+void float2(float a, float b) {
+  clang_analyzer_eval(a == b); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a != b); // expected-warning{{UNKNOWN}}
+}
+
+void float3(float a, float b) {
+  if (__builtin_isnan(a) || __builtin_isnan(b) || a < b) {
+clang_analyzer_eval(a > b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+return;
+  }
+  clang_analyzer_eval(a >= b); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == b); // expected-warning{{UNKNOWN}}
+}
+
+void float4(float a) {
+  if (__builtin_isnan(a) || a < 0.0f)
+return;
+  clang_analyzer_eval(a >= 0.0Q); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a >= 0.0F); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a >= 0.0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a >= 0); // expected-warning{{TRUE}}
+}
+
+void float5() {
+  double value = 1.0;
+  double pinf = __builtin_inf();
+  double ninf = -__builtin_inf();
+  double nan = __builtin_nan("");
+
+  /* NaN */
+  clang_analyzer_eval(__builtin_isnan(value)); // expected-warning{{FALSE}}
+  clang_analyzer_eval(__builtin_isnan(nan)); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(__builtin_isnan(0.0 / 0.0)); // expected-warning{{TRUE}}
+  clang_analyzer_eval(__builtin_isnan(pinf / ninf)); // expected-warning{{TRUE}}
+  clang_analyzer_eval(__builtin_isnan(pinf / pinf)); // expected-warning{{TRUE}}
+  clang_analyzer_eval(__builtin_isnan(ninf / 

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 85316.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D28955

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/dead-stores.m
  test/Analysis/explain-svals.cpp
  test/Analysis/malloc.c
  test/Analysis/misc-ps-eager-assume.m
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -146,7 +146,7 @@
 void test_isgraph_isprint(int x) {
   char y = x;
   if (isgraph(y))
-clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
+clang_analyzer_eval(isprint(y)); // expected-warning{{TRUE}}
 }
 
 int isdigit(int);
Index: test/Analysis/misc-ps-eager-assume.m
===
--- test/Analysis/misc-ps-eager-assume.m
+++ test/Analysis/misc-ps-eager-assume.m
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks %s -analyzer-eagerly-assume
-// expected-no-diagnostics
 
 // Delta-reduced header stuff (needed for test cases).
 typedef signed char BOOL;
@@ -56,7 +55,7 @@
 void handle_symbolic_cast_in_condition(void) {
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
 
-  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"];
+  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"]; // expected-warning {{Assignment of a non-Boolean value}}
   NSMutableArray* array = needsAnArray ? [[NSMutableArray alloc] init] : 0;
   if(needsAnArray)
 [array release];
Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1656,13 +1656,13 @@
 void testOffsetPassedToStrlen() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
+  size_t length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
 }
 
 void testOffsetPassedToStrlenThenFree() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string);
+  size_t length = strlen(string);
   free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}}
 }
 
@@ -1705,7 +1705,7 @@
 }
 
 char *dupstrNoWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocNoWarn(len + 1);
   strcpy(p, s); // no-warning
   return p;
@@ -1721,7 +1721,7 @@
 }
 
 char *dupstrWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocWarn(len + 1);
   strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
   return p;
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -41,11 +41,11 @@
 
 void test_2(char *ptr, int ext) {
   clang_analyzer_explain((void *) "asdf"); // expected-warning-re^pointer to element of type 'char' with index 0 of string literal "asdf"$
-  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^cast of type 'int' of metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
   clang_analyzer_explain(conjure()); // expected-warning-re^symbol of type 'int' conjured at statement 'conjure\(\)'$
   clang_analyzer_explain(glob); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob'$
   clang_analyzer_explain(glob_ptr); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob_ptr'$
-  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^extent of pointee of argument 'ptr'$
+  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^cast of type 'int' of extent of pointee of argument 'ptr'$
   int *x = new int[ext];
   clang_analyzer_explain(x); // expected-warning-re^pointer to element of type 'int' with index 0 of heap segment that starts at symbol of type 'int \*' conjured at statement 'new int \[ext\]'$
   // Sic! What gets computed is the extent of the element-region.
Index: test/Analysis/dead-stores.m
==

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> - That said, I think there are still significant optimization opportunities. 
> It looks like when performing a query you create a new solver for each set of 
> constraints. My understanding (and perhaps @nlopes can correct me here) is 
> that these days Z3 is quite good at incremental solves so you may be leaving 
> a lot of performance on the table. For example, in `getSymVal()` you create 
> one solver to query for a satisfying assignment and then later create a 
> completely new one to determine whether it is the only satisfying assignment. 
> Since those two queries share all of their conjuncts but one it might be 
> faster to reuse the first solver and add the new assertion for the second. 
> Similarly, since analyzer exploration is a DFS, you could imagine building up 
> the path condition incrementally. Z3 has solver APIs for pushing and popping 
> assertions. (Again, @nlopes may have a better idea of whether this would pay 
> off in practice.)

I agree that the performance is the main problem, and that there are still a 
lot of performance optimizations available. I've separated the Z3 solver and 
model into separate classes, and reuse the solver now in `getSymVal()` and 
`checkNull()`. I looked at using the push/pop approach briefly when I started 
writing the solver, but since the static analyzer itself seems to have an 
internal worklist, I wasn't sure if the state traversal order is deterministic 
and predictable, so I didn't try to reuse a single solver across all program 
states. This is the new timing:

Z3ConstraintManager (built-in, previous changes and solver state reuse): 202.37 
sec

> - It would be good to measure the increased peak memory usage with the Z3 
> constraint solver. The analyzer is often used as part of an IDE and so it 
> needs to be able to run in memory constrained environments (such as laptops). 
> Further, since multiple invocations of clang are often run simultaneously, 
> memory is often a more precious resource than CPU time.

On the following testcases, the memory usage increase is around 18%. Using GNU 
time, measuring the max RSS in kbytes:
edges-new.mm (RangeConstraintManager): 45184
edges-new.mm (Z3ConstraintManager): 53568
malloc-plist.c (RangeConstraintManager): 40264
malloc-plist.c (Z3ConstraintManager): 47888

> - As @a.sidorin noted, there is a significant test and maintenance cost to 
> keeping two constraint managers around. Since the testing matrix would 
> double, anything we can do to limit the need to modify/duplicate tests would 
> be a huge savings. Is it possible to use lit substitution to call the 
> analyzer twice for each run line: once with the range constraint manager and 
> once with z3 (for builds where z3 is requested)? This, combined with 
> automatically adding the #defines that @NoQ suggested would provide a 
> mechanism to share most tests between the constraint managers.

I think this is reasonable, the only remaining question is how to identify when 
lit should call the analyzer twice, e.g. match on patch against 
`test/Analysis`? Also, note that the argument passed to clang (`-Xclang 
-analyzer-constraints=z3` vs. `-analyzer-constraints=z3`) need to change 
depending on whether the actual compiler or just the compiler driver is invoked.

> - Right now the CMake uses find_package and builds with Z3 if it is found to 
> be installed on the building system. I think this is too aggressive. It would 
> be better to have the build explicitly opt in to using Z3. Without this, a 
> user could accidentally build a clang that dynamically links to their local 
> Z3 but then fails to launch with a load error when distributed. Similarly, it 
> would be good to be able to explicitly set the the location of the Z3 to be 
> used at build time for systems with multiple Z3s installed.

Done.


https://reviews.llvm.org/D28952



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 85621.
ddcc added a comment.

Add CMake CLANG_BUILD_Z3 option, add Z3Model and Z3Solver classes, reuse solver 
in checkNull() and getSymVal()


https://reviews.llvm.org/D28952

Files:
  CMakeLists.txt
  cmake/modules/FindZ3.cmake
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Core/Analyses.def
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bool-assignment.c
  test/Analysis/expr-inspection.c
  test/Analysis/ptr-arith.c
  test/Analysis/unsupported-types.c
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -15,6 +15,7 @@
 config.target_triple = "@TARGET_TRIPLE@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
 config.have_zlib = "@HAVE_LIBZ@"
+config.have_z3 = "@CLANG_HAVE_Z3@"
 config.clang_arcmt = @ENABLE_CLANG_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @ENABLE_CLANG_STATIC_ANALYZER@
Index: test/Analysis/unsupported-types.c
===
--- /dev/null
+++ test/Analysis/unsupported-types.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -triple x86_64-unknown-linux -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -triple powerpc64-linux-gnu -verify %s
+
+#define _Complex_I  (__extension__ 1.0iF)
+
+void clang_analyzer_eval(int);
+
+void complex_float(double _Complex x, double _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void complex_int(int _Complex x, int _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void longdouble_float(long double x, long double y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 0.0L && y != 1.0L)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 1.0L); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -255,30 +255,24 @@
 void zero_implies_reversed_equal(int *lhs, int *rhs) {
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
   if ((rhs - lhs) == 0) {
-// FIXME: Should be FALSE.
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-// FIXME: Should be TRUE.
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
+clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
 return;
   }
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-  // FIXME: Should be FALSE.
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
+  clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
 }
 
 void canonical_equal(int *lhs, int *rhs) {
   clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
   if (lhs == rhs) {
-// FIXME: Should be TRUE.
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
 return;
   }
   clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
 
-  // FIXME: Should be FALSE.
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
 }
 
 void compare_element_region_and_base(int *p) {
Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -19,4 +19,4 @@
 
 // CHECK: Expressions:
 // CHECK-NEXT: clang_analyzer_printState : &code{clang_analyzer_printState}
-// CHECK-NEXT: Ranges are empty.
+// CHECK-NEXT: {{(Ranges are empty)|(Constraints:[[:space:]]*$)}}
Index: test/Analysis/bool-assignment.c
===
--- test/Analysis/bool-assignment.c
+++ test/Analysis/bool-as

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> Regarding incremental solving with Z3 (or with most SMT solvers in general), 
> let me just lower the expectations a bit: 

Ok, that is good to know. It seems that the performance benefits of incremental 
solving are unclear, and would be nontrivial to implement (maybe store the 
states in a DAG, then pop back to the lowest common ancestor from the previous 
state, and push down to the new state).

> The improvement here (200s vs. 250s) makes me think that some form of 
> caching/incrementalization could pay off.

I'm optimistic that implementing a SMT query cache would significantly improve 
performance (for both constraint managers), but the current version of that 
patch has some memory-safety problems (since ProgramStateRef is a 
reference-counting pointer), and I currently don't have the time to fix it up.

> We could add a new lit substitution '%clang_cc1_analyze' that expands to 
> '%clang_cc1 -analyze -analyzer-constraints=foo 
> -DANALYZER_HAS_CONSTRAINT_MANAGER_FOO" and change the analyzer tests to use 
> it.

I've updated the current version of this diff to use this approach, though 
there are still the two remaining testcase failures mentioned earlier that I'm 
not sure how to handle. I also had to remove the argument `-fsyntax-only` in 
some testcases, because the argument seems to need to appear before `-analyze` 
(which isn't possible using the substitution-based approach), though it doesn't 
seem to affect the outcome of the testcase. However, I haven't been able to 
figure out how to get llvm-lit to execute the same statement twice with 
different substitutions. From lit.cfg, it seems that I can only define 
additional substitutions, and not execute a test multiple times. I'm not 
familiar with the test infrastructure, do you have any suggestions?


https://reviews.llvm.org/D28952



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D28952#659728, @nlopes wrote:

> Let me give just 2 more Z3-related suggestions:
>
> - instead of re-creating the solver, it might be faster to do Z3_solver_reset
> - "once in a while" it might be helpful to delete everything (all solvers, 
> asts, context) and call Z3_reset_memory.  Z3's small object pool is not very 
> good and keeps growing /ad infinitum/, so cleaning it up once a while is a 
> good thing (improves cache usage and reduces memory consumption).
>
>   BTW, you may want to call Z3_finalize_memory at exit to keep clang 
> valgrind/asan-clean.  (Z3 keeps a lot of internal buffers otherwise)


Thanks, the most recent revision now reuses the solver object, and frees Z3 
memory at exit. I'm skipping the reset memory part for now, because my 
understanding is that a new instance of `clang -cc1` will be spun up for each 
compilation unit anyway, and it'd be tricky to find a suitable point to do that 
while maintaining consistency.

On the testsuite, the runtime is now down to 185 secs, and a valgrind test is 
much cleaner.


https://reviews.llvm.org/D28952



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I added support for a callback field in lit's configuration (see 
https://reviews.llvm.org/D29684), which is used to execute each testcase for 
each supported constraint solver backends at runtime. I believe this resolves 
all remaining issues, except for the remaining two testcase failures:

1. The testcase Analysis/reference.cpp currently fails, because the 
RangeConstraintManager has an internal optimization that assumes references are 
known to be non-zero (see RangeConstraintManager.cpp:422). This optimization is 
probably in the wrong place, and should be moved into the analyzer and out of 
the constraint solver. Unless anyone objects, I plan to modify this testcase so 
that this result is fine for the z3 solver backend.
2. The testcase Analysis/ptr-arith.c currently fails, because handling of 
ptrdiff_t is somewhat tricky. This constraint manager interface maps ptrdiff_t 
to a signed bitvector in Z3, and pointers to unsigned bitvectors in Z3. 
However, since this testcase compares the pointer values both directly and 
indirectly via ptrdiff_t, if a < b is true using a signed comparison, the same 
is not necessarily true using an unsigned comparison. Likewise, unless anyone 
objects, I plan to whitelist this result for the z3 solver backend.


https://reviews.llvm.org/D28952



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

ping


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 105913.
ddcc marked an inline comment as done.
ddcc added a comment.

Drop duplicate code


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
 
 
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col25
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:li

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I tested the following software, both before and after applying this patch, 
using RangeConstraintManager.

Software, Version, Compile Time (before), Bugs Reported (before), Compile Time 
(after), Bugs Reported (after)
openssl, 1.1.0f, 11 min, 126, 12 min, 126
sqlite, 3.18.2, 31 min, 86, 31 min, 82
wireshark, 2.2.7, 105 min, 109, 119 min, 108

For sqlite, the differences of four reported bugs all appear to be false 
positives, but for wireshark, the one difference appears to be a false negative.




Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:363
   // instead of generating an Unknown value and propagate the taint info to it.
-  const unsigned MaxComp = 1; // 10 28X
 

NoQ wrote:
> ddcc wrote:
> > zaks.anna wrote:
> > > Reducing the MaxComp is going to regress taint analysis..
> > > 
> > > > I've updated this revision to account for the recent SVal 
> > > > simplification commit by @NoQ, 
> > > 
> > > Which commit introduced the regression?
> > > 
> > > > but now there is an exponential blowup problem that prevents testcase 
> > > > PR24184.cpp from terminating, 
> > > 
> > > What triggers the regression? Removing the if statement above? Does the 
> > > regression only effect the Z3 "mode" (I am guessing not since you said 
> > > "due to an interaction between Simplifier::VisitNonLocSymbolVal() and 
> > > SValBuilder::makeSymExprValNN()")? 
> > > 
> > > Reducing the MaxComp is going to regress taint analysis..
> > 
> > I think the original intention was to increase `MaxComp`, not decrease it, 
> > but I will restore the original value here.
> > 
> > > What triggers the regression? Removing the if statement above? Does the 
> > > regression only effect the Z3 "mode"
> > 
> > No, the regression isn't specifically due to this code, but with @NoQ 's 
> > patch for `SVal` simplification (rL300178), and this commit extending it to 
> > handle `SymbolCast` and `IntSymExpr`, the cast of `ST *` used in the loop 
> > of case 3 of PR24184.cpp becomes "simplified" (inlined) repeatedly on each 
> > recursive iteration through `Simplifier::VisitNonLocSymbolVal()` -> 
> > `SValBuilder::makeSymExprValNN()`, causing a big slowdown in runtime 
> > performance.
> > 
> > The naive way to prevent it is to drop `MaxComp` (but this isn't 
> > reasonable, since it needs to be absurdly low, e.g. `10`). Alternatively, 
> > simplification for `SymbolCast` can be dropped from this commit (but it 
> > will eliminate some of the other analysis improvements), or, most likely, 
> > introduce another parameter to reduce recursion between these two functions.
> We talked a bit and we're pretty much fine with reduce `MaxComp` to 10 if 
> it's worth it in terms of performance.
Just to clarify, the current change from 100 to 10 in 
`simplifySVal::SimplyVisitNonLocSymbolVal` resolves the problem; the value of 
`MaxComp` here is unchanged.



Comment at: test/Analysis/plist-macros.cpp:2
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix 
-analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix 
-analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config 
path-diagnostics-alternate=ture %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s

NoQ wrote:
> > `path-diagnostics-alternate=ture`
> 
> Ouch. Thanks for fixing this.
> 
> Maybe it'd be great to implement an option that enables validating analyzer 
> config option values, and turn this option on in `%clang_analyze_cc1`.
I agree that'd be useful, but I think that it should be a separate future patch.


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I've also uploaded the results to https://dcddcc.com/csa


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 106084.
ddcc added a comment.

Split plist-macros.cpp, and update analyzer_test.py to support tests that 
require not z3


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL307833: [analyzer] Support generating and reasoning over 
more symbolic constraint types (authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D28953?vs=106084&id=106284#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28953

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  cfe/trunk/test/Analysis/analyzer_test.py
  cfe/trunk/test/Analysis/bitwise-ops.c
  cfe/trunk/test/Analysis/bool-assignment.c
  cfe/trunk/test/Analysis/conditional-path-notes.c
  cfe/trunk/test/Analysis/explain-svals.cpp
  cfe/trunk/test/Analysis/plist-macros-z3.cpp
  cfe/trunk/test/Analysis/plist-macros.cpp
  cfe/trunk/test/Analysis/range_casts.c
  cfe/trunk/test/Analysis/std-c-library-functions.c

Index: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -1034,31 +1034,39 @@
 ProgramStateRef Z3ConstraintManager::assumeSymInclusiveRange(
 ProgramStateRef State, SymbolRef Sym, const llvm::APSInt &From,
 const llvm::APSInt &To, bool InRange) {
+  ASTContext &Ctx = getBasicVals().getContext();
+  // FIXME: This should be a cast from a 1-bit integer type to a boolean type,
+  // but the former is not available in Clang. Instead, extend the APSInt
+  // directly.
+  bool isBoolTy = From.getBitWidth() == 1 && getAPSIntType(From).isNull();
+
   QualType RetTy;
   // The expression may be casted, so we cannot call getZ3DataExpr() directly
   Z3Expr Exp = getZ3Expr(Sym, &RetTy);
-
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType RTy = isBoolTy ? Ctx.BoolTy : getAPSIntType(From);
+  Z3Expr FromExp =
+  isBoolTy ? Z3Expr::fromAPSInt(From.extend(Ctx.getTypeSize(Ctx.BoolTy)))
+   : Z3Expr::fromAPSInt(From);
 
   // Construct single (in)equality
   if (From == To)
 return assumeZ3Expr(State, Sym,
 getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE,
  FromExp, RTy, nullptr));
 
+  assert((getAPSIntType(From) == getAPSIntType(To)) &&
+ "Range values have different types!");
+
+  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
   // Construct two (in)equalities, and a logical and/or
   Z3Expr LHS =
   getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr);
   Z3Expr RHS =
   getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr);
   return assumeZ3Expr(
   State, Sym,
-  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy));
+  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS,
+RetTy->isSignedIntegerOrEnumerationType()));
 }
 
 ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State,
@@ -1406,6 +1414,7 @@
QualType isIntegralOrEnumerationType() &&
   RTy->isIntegralOrEnumerationType()) {
@@ -1468,10 +1477,10 @@
 void Z3ConstraintManager::doIntTypeConversion(T &LHS, QualType isPromotableIntegerType()) {
Index: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -100,7 +100,7 @@
 
   if (T->isNullPtrType())
 return makeZeroVal(T);
-  
+
   if (!SymbolManager::canSymbolicate(T))
 return UnknownVal();
 
@@ -354,17 +354,14 @@
BinaryOperator::Opcode Op,
NonLoc LHS, NonLoc RHS,
QualType ResultTy) {
-  if (!State->isTainted(RHS) && !State->isTainted(LHS))
-return UnknownVal();
-
   const SymExpr *symLHS = LHS.getAsSymExpr();
   co

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Reverted in https://reviews.llvm.org/rL307853


Repository:
  rL LLVM

https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Reverted in https://reviews.llvm.org/rL307853


Repository:
  rL LLVM

https://reviews.llvm.org/D28953



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