[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 477233.
ASDenysPetrov added a comment.

Completely reworked solution.


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

https://reviews.llvm.org/D103096

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/svalbuilder-casts.cpp
  clang/test/Analysis/symbol-integral-cast.cpp

Index: clang/test/Analysis/symbol-integral-cast.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-integral-cast.cpp
@@ -0,0 +1,395 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -analyzer-config eagerly-assume=false -analyzer-config support-symbolic-integer-casts=true -verify %s
+
+template 
+void clang_analyzer_eval(T);
+void clang_analyzer_warnIfReached();
+template
+void clang_analyzer_value(T x);
+
+typedef short int16_t;
+typedef int int32_t;
+typedef unsigned short uint16_t;
+typedef unsigned int uint32_t;
+
+void test1(int x) {
+  // Even if two lower bytes of `x` equal to zero, it doesn't mean that
+  // the entire `x` is zero. We are not able to know the exact value of x.
+  // It can be one of  65536 possible values like [0, 65536, 131072, ...]
+  // and so on. To avoid huge range sets we still assume `x` in the range
+  // [INT_MIN, INT_MAX].
+  if (!(short)x) {
+if (!x) {
+  clang_analyzer_value((short)x); // expected-warning {{16s:0}}
+  clang_analyzer_value(x); // expected-warning {{32s:0}}
+} else {
+  // FIXME: Shall be simplified to ConcreteInt 16s:0.
+  clang_analyzer_value((short)x); // expected-warning {{16s:{ [0, 0] }}}
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, -1], [1, 2147483647] }}}
+}
+  }
+}
+
+void test2(int x) {
+  // If two lower bytes of `x` equal to zero, and we know x to be 65537,
+  // which is not truncated to short as zero. Thus the branch is infisible.
+  short s = x;
+  if (!s) {
+if (x == 65537) {
+  clang_analyzer_warnIfReached(); // no-warning
+} else {
+  clang_analyzer_value(s); // expected-warning {{16s:0}}
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 65536], [65538, 2147483647] }}}
+}
+  }
+}
+
+void test3(int x, short s) {
+  s = x;
+  if ((short)x > -10 && s < 10) {
+if (x > 0 && x < 10) {
+  // FIXME: If the range of the whole variable was constrained then reason
+  // again about truncated bytes to make the ranges more precise.
+  // Shall be 16s:{ [1, 9] }
+  clang_analyzer_value((short)x); // expected-warning {{16s:{ [-9, 9] }}}
+  clang_analyzer_value(x); // expected-warning {{32s:{ [1, 9] }}}
+}
+  }
+}
+
+void test4(unsigned x) {
+  if ((char)x > 8) {
+clang_analyzer_value((char)x); // expected-warning {{8s:{ [9, 127] }}}
+if (x < 42) {
+  // FIXME: Update lower(less) bytes if higher(more) bytes are updated.
+  // Should be 8s:{ [9, 41] }.
+  clang_analyzer_value((char)x); // expected-warning {{8s:{ [9, 127] }}}
+}
+  }
+}
+
+void test5(unsigned x) {
+  if ((char)x > -10 && (char)x < 10) {
+if ((short)x == 8) {
+  clang_analyzer_value(x); // expected-warning {{32u:{ [0, 4294967295] }}}
+  clang_analyzer_value((short)x); // expected-warning {{16s:{ [8, 8] }}}
+  // FIXME: Update lower(less) bytes if higher(more) bytes are updated.
+  // Should be 8s:{ [8, 8] }.
+  clang_analyzer_value((char)x); // expected-warning {{8s:{ [-9, 9] }}}
+}
+  }
+}
+
+void test6(int x) {
+  // Even if two lower bytes of `x` less than zero, it doesn't mean that `x`
+  // can't be greater than zero. Thence we don't change the native range of
+  // `x` and this branch is feasible.
+  if (x > 0)
+if ((short)x < 0)
+  clang_analyzer_value(x); // expected-warning {{32s:{ [1, 2147483647] }}}
+}
+
+void test7(int x) {
+  // The range of two lower bytes of `x` [1, SHORT_MAX] is enough to cover
+  // all possible values of char [CHAR_MIN, CHAR_MAX]. So the lowest byte
+  // can be lower than zero.
+  if ((short)x > 0) {
+if ((char)x < 0)
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test8(int x) {
+  // Promotion from `signed int` to `signed long long` also reasoning about the
+  // original range, because we know the fact that even after promotion it
+  // remains in the range [INT_MIN, INT_MAX].
+  if ((long long)x < 0)
+clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, -1] }}}
+}
+
+void test9(signed int x) {
+  // Any cast `signed` to `unsigned` produces an unsigned range, which is
+  // [0, UNSIGNED_MAX] and can not be lower than zero.
+  if ((unsigned long long)x < 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+
+  if ((unsigned int)x < 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else

[PATCH] D138319: [analyzer] Prepare structures for integral cast feature introducing

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: NoQ, MaskRay, martong, steakhal.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, StephenFan, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Change structures of storing bindings between Symbols and Equivalent Classes.
Add a Bitwidth characteristic as a common feature of integral Symbol to make 
`(char)(int x)` and `(uchar)(int x)` treated under the same Equivalent Class.

The link **Symbol **- **Class **was direct and now it depends on the 
effective(minimal) Bitwidth of the Symbol.
The link **Class **- **Symbol **stays as previously.

Some test cases are affected by regression for the sake of the next patch which 
will fix it.

This patch does not introduce integral casts but prepares the field for the 
core patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138319

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/svalbuilder-casts.cpp

Index: clang/test/Analysis/svalbuilder-casts.cpp
===
--- clang/test/Analysis/svalbuilder-casts.cpp
+++ clang/test/Analysis/svalbuilder-casts.cpp
@@ -39,17 +39,17 @@
 
   // These are not truncated to short as zero.
   static_assert((short)1 != 0, "");
-  clang_analyzer_eval(x == 1);   // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == 1);   // expected-warning{{UNKNOWN}}
   static_assert((short)-1 != 0, "");
-  clang_analyzer_eval(x == -1);  // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == -1);  // expected-warning{{UNKNOWN}}
   static_assert((short)65537 != 0, "");
-  clang_analyzer_eval(x == 65537);   // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == 65537);   // expected-warning{{UNKNOWN}}
   static_assert((short)-65537 != 0, "");
-  clang_analyzer_eval(x == -65537);  // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == -65537);  // expected-warning{{UNKNOWN}}
   static_assert((short)131073 != 0, "");
-  clang_analyzer_eval(x == 131073);  // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == 131073);  // expected-warning{{UNKNOWN}}
   static_assert((short)-131073 != 0, "");
-  clang_analyzer_eval(x == -131073); // expected-warning{{FALSE}}
+  clang_analyzer_eval(x == -131073); // expected-warning{{UNKNOWN}}
 
   // Check for implicit cast.
   short s = y;
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Basic/JsonSupport.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
@@ -872,17 +873,18 @@
 }
 LLVM_DUMP_METHOD void RangeSet::dump() const { dump(llvm::errs()); }
 
-REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(SymbolSet, SymbolRef)
-
 namespace {
 class EquivalenceClass;
+using BitWidthType = uint32_t;
 } // end anonymous namespace
 
-REGISTER_MAP_WITH_PROGRAMSTATE(ClassMap, SymbolRef, EquivalenceClass)
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(SymbolSet, SymbolRef)
+REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ClassSet, EquivalenceClass)
+REGISTER_MAP_FACTORY_WITH_PROGRAMSTATE(ClassMap, BitWidthType, EquivalenceClass)
+
+REGISTER_MAP_WITH_PROGRAMSTATE(SymClassMap, SymbolRef, ClassMap)
 REGISTER_MAP_WITH_PROGRAMSTATE(ClassMembers, EquivalenceClass, SymbolSet)
 REGISTER_MAP_WITH_PROGRAMSTATE(ConstraintRange, EquivalenceClass, RangeSet)
-
-REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ClassSet, EquivalenceClass)
 REGISTER_MAP_WITH_PROGRAMSTATE(DisequalityMap, EquivalenceClass, ClassSet)
 
 namespace {
@@ -989,11 +991,10 @@
 return getRepresentativeSymbol()->getType();
   }
 
-  EquivalenceClass() = delete;
+  EquivalenceClass() = default;
   EquivalenceClass(const EquivalenceClass &) = default;
-  EquivalenceClass =(const EquivalenceClass &) = delete;
-  EquivalenceClass(EquivalenceClass &&) = default;
-  EquivalenceClass =(EquivalenceClass &&) = delete;
+  EquivalenceClass(SymbolRef Sym) : ID(reinterpret_cast(Sym)) {}
+  EquivalenceClass =(const EquivalenceClass &) = default;
 
   bool operator==(const EquivalenceClass ) const {
 return ID == Other.ID;
@@ -1010,9 +1011,6 @@
   void Profile(llvm::FoldingSetNodeID ) const { Profile(ID, this->ID); }
 
 private:
-  /* implicit */ EquivalenceClass(SymbolRef Sym)
-  : ID(reinterpret_cast(Sym)) {}
-
   /// This function is intended to be used ONLY within the class.
 

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong

> So, the intersection should be empty in the above mentioned ambiguous case, 
> shouldn't' it?

No. My current implementation doesn't contain this check in ConstraintAssignor 
in favor of the solution simplification. It'll be added in the next patches.

But still I think, this solution is not accomplished. Consider next. Say, 
symbol `(char)(int x)` is associated to the range `[42, 42]`, and can be 
simplified to constant `42 of char`. And `(int x)` is associated to the range 
`[43, 43]` Your patch will omit looking into the symbol itself and unwrap it 
starting visiting the operand instead. Thus, it will return the constant 43 for 
`(int x)`.
Moreover, if `(int x)` is 43, it will contradict to 42 (aka infeasible state). 
We also have no decision what to return in this case.

For me, this is really uncertain patch that I'm not 100% sure in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong

> Then we can iterate onto the solution. What we could do is to collect the 
> constants and types on the way of the cast visitation and then apply the same 
> logic that you have in D103096 .

Suppose we have found the way to handle it. But what if we find a contradiction 
while simplifying, like `(short)(int x) = 0` and `(int x) = 1`. So that we 
would already know that it is impossible. What `SVal` should we return in such 
case? Undef? Unknown? Original one?

> But then there is an open question: what should we do if there is another 
> kind of symbol in the chain of SymbolCasts? E.g SymbolCast, UnarySymExpr, 
> SymbolCast.

IMO, it doesn't matter, since we are just waiting for a constant. Doesn't 
matter to what `Sym` it is associated. The main question, as I said, what we 
shall do with two different constants.

Do you have any ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D126481#3818769 , @martong wrote:

> Yeah okay. I get it now.  Thank you for your patience and your time on 
> elaborating the issue.
>
> First, I think we'd need to fabricate a test case that shows us the bug even 
> without applying your patch (D103096 ).
>
> Then we can iterate onto the solution. What we could do is to collect the 
> constants and types on the way of the cast visitation and then apply the same 
> logic that you have in D103096 .

Suppose we have found the way to handle it. But what if we find a contradiction 
while simplifying, like `(short)(int x) = 0` and `(int x) = 1`. So that we 
would already know that it is impossible. What `SVal` should we return in such 
case? Undef? Unknown? Original one?

> But then there is an open question: what should we do if there is another 
> kind of symbol in the chain of SymbolCasts? E.g SymbolCast, UnarySymExpr, 
> SymbolCast.

IMO, it doesn't matter, since we are just waiting for a constant. Doesn't 
matter to what `Sym` it is associated. The main question, as I said, what we 
shall do with two different constants.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong wrote:
I think you did get it. I'm not talking about range but about concrete int. And 
I see that the current behavior needs an improvement or rework.
Consider next code, which I used in my example:

  1. void test(int x) {
  2.   assert((short)x == 0);
  3.   clang_analyzer_eval(x == 1);
  4. }

Your patch does next:

- on the line 3 we know that `(int)(short)(int x) = 0` and `(int x) = 1`
- simplify `(int)(short)(int x)`. Try to get a constant for `(short)(int x)`. 
Result is nothing because it is not presented in the range map. **Continue 
**unwrapping.
- go deeper and simplify `(short)(int x)`.  Try to get a constant for `(int 
x)`. Result is 1. **Stop **visiting.
- return 1.
- intersect the range of the original symbol `(int)(short)(int x)` which is 
**0** and the range which is returned - **1**
- result is an **infeasible** state.

My patch above yours does next:

- on the line 3 we know that `(int)(short)(int x) = 0` and `(int x) = 1`, but 
now we also know that  `(short)(int x) = 0` as an equivalent for 
`(int)(short)(int x)` due to the improvement.
- simplify `(int)(short)(int x)`. Try to get a constant for `(short)(int x)`. 
Result is 0. **Stop **visiting.
- return 0.
- intersect the range of the original symbol `(int)(short)(int x)` which is 
**0** and the range which is returned - **0**
- result is a **feasible** state.

Here what I'm saying. This is not about ranges. This simplification dosn't take 
into account that differents operands of the cast symbol may have different 
asocciated ranges or constants. And what do we have to do we them in this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Denys Petrov via Phabricator via cfe-commits]

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

I've just investigated this patch deeply. I think we have wrong logic here.

Assume we have `(int)(short)(int x)`. `VisitSymbolCast` will try to get the 
constant recursively in the next order:

- `(short)(int x)`
- `(int x)`

And here is my concern. Whether it is correct to get the range for `int x` and 
consider it as a correct simplification of `(int)(short)(int x)`. IMO it can't 
be simplified like that. It should go through the set of conventions and 
intersections.
Working on an integral cast feature I've encountered in the situation when I 
can get the range for `(short)(int x)` in the chain above. And it logically 
matches with the original symbol `(int)(short)(int x)`. After that, your 
solution (this patch) stops and returns this range, missing the range 
associated with `int x`.  That borns the problem:

| **before my patch**  | **after my patch** 
   |
| (int)(short)(int x) = 0  | (int)(short)(int x) = 0
   |
| (short)(int x) = ? / go next | (short)(int x) = 0 / match 
   |
| (int x) = 1 / match  | (int x) = 1 / doesn't reach because of the 
previous match |
| **result**   | **result** 
   |
| 0 and 1 infeasible   | 0 and 0 feasible   
   |
|

I think we have to improve this behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@manas Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D131707#3723403 , @ASDenysPetrov 
wrote:

> @steakhal
> Thank you for the review.
> Could you please do testing for me, since I'm on Windows and have no prepared 
> testing environment as I know you do.
> I'll add an assertion.




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

https://reviews.llvm.org/D131707

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

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 452689.
ASDenysPetrov added a comment.

Add assertion accroding to the remark.


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

https://reviews.llvm.org/D131707

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -529,6 +529,8 @@
 
 /// TypedValueRegion - An abstract class representing regions having a typed 
value.
 class TypedValueRegion : public TypedRegion {
+  mutable QualType CachedLocationType;
+
   void anchor() override;
 
 protected:
@@ -540,12 +542,15 @@
   virtual QualType getValueType() const = 0;
 
   QualType getLocationType() const override {
-// FIXME: We can possibly optimize this later to cache this value.
-QualType T = getValueType();
-ASTContext  = getContext();
-if (T->getAs())
-  return ctx.getObjCObjectPointerType(T);
-return ctx.getPointerType(getValueType());
+if (CachedLocationType.isNull()) {
+  QualType T = getValueType();
+  ASTContext  = getContext();
+  CachedLocationType = T->isObjCObjectType() ? 
C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+  assert(!CachedLocationType.isNull() &&
+ "Cached value supposed to be non-null.");
+}
+return CachedLocationType;
   }
 
   QualType getDesugaredValueType(ASTContext ) const {


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -529,6 +529,8 @@
 
 /// TypedValueRegion - An abstract class representing regions having a typed value.
 class TypedValueRegion : public TypedRegion {
+  mutable QualType CachedLocationType;
+
   void anchor() override;
 
 protected:
@@ -540,12 +542,15 @@
   virtual QualType getValueType() const = 0;
 
   QualType getLocationType() const override {
-// FIXME: We can possibly optimize this later to cache this value.
-QualType T = getValueType();
-ASTContext  = getContext();
-if (T->getAs())
-  return ctx.getObjCObjectPointerType(T);
-return ctx.getPointerType(getValueType());
+if (CachedLocationType.isNull()) {
+  QualType T = getValueType();
+  ASTContext  = getContext();
+  CachedLocationType = T->isObjCObjectType() ? C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+  assert(!CachedLocationType.isNull() &&
+ "Cached value supposed to be non-null.");
+}
+return CachedLocationType;
   }
 
   QualType getDesugaredValueType(ASTContext ) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal
Thank you for the review.
Could you please do testing for me, since I'm on Windows and have no prepared 
testing environment as I know you are.
I'll add an assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131707

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

[PATCH] D131707: [analyzer]{NFC} Cache the result of getLocationType in TypedValueRegion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: steakhal, martong, NoQ, xazax.hun, isuckatcs.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Fix FIXME. Produce `QualType` once in `getLocationType` at the first call. Then 
cache the result and return it for the next calls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131707

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -529,6 +529,8 @@
 
 /// TypedValueRegion - An abstract class representing regions having a typed 
value.
 class TypedValueRegion : public TypedRegion {
+  mutable QualType CachedLocationType;
+
   void anchor() override;
 
 protected:
@@ -540,12 +542,13 @@
   virtual QualType getValueType() const = 0;
 
   QualType getLocationType() const override {
-// FIXME: We can possibly optimize this later to cache this value.
-QualType T = getValueType();
-ASTContext  = getContext();
-if (T->getAs())
-  return ctx.getObjCObjectPointerType(T);
-return ctx.getPointerType(getValueType());
+if (CachedLocationType.isNull()) {
+  QualType T = getValueType();
+  ASTContext  = getContext();
+  CachedLocationType = T->isObjCObjectType() ? 
C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+}
+return CachedLocationType;
   }
 
   QualType getDesugaredValueType(ASTContext ) const {


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -529,6 +529,8 @@
 
 /// TypedValueRegion - An abstract class representing regions having a typed value.
 class TypedValueRegion : public TypedRegion {
+  mutable QualType CachedLocationType;
+
   void anchor() override;
 
 protected:
@@ -540,12 +542,13 @@
   virtual QualType getValueType() const = 0;
 
   QualType getLocationType() const override {
-// FIXME: We can possibly optimize this later to cache this value.
-QualType T = getValueType();
-ASTContext  = getContext();
-if (T->getAs())
-  return ctx.getObjCObjectPointerType(T);
-return ctx.getPointerType(getValueType());
+if (CachedLocationType.isNull()) {
+  QualType T = getValueType();
+  ASTContext  = getContext();
+  CachedLocationType = T->isObjCObjectType() ? C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+}
+return CachedLocationType;
   }
 
   QualType getDesugaredValueType(ASTContext ) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131514: [analyzer] [NFC] Add more test cases for equality tracking

[Denys Petrov 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 rG7e2b995e0cce: [analyzer] [NFC] Add more test cases for 
equality tracking (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131514

Files:
  clang/test/Analysis/equality_tracking.c

Index: clang/test/Analysis/equality_tracking.c
===
--- clang/test/Analysis/equality_tracking.c
+++ clang/test/Analysis/equality_tracking.c
@@ -8,6 +8,7 @@
 #define CHAR_MAX (char)(UCHAR_MAX & (UCHAR_MAX >> 1))
 #define CHAR_MIN (char)(UCHAR_MAX & ~(UCHAR_MAX >> 1))
 
+void clang_analyzer_value(int);
 void clang_analyzer_eval(int);
 void clang_analyzer_warnIfReached(void);
 
@@ -233,3 +234,139 @@
 clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
   }
 }
+
+void deletePointBefore(int x, int tmp) {
+  if(tmp == 0)
+if(x != tmp)
+ clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, -1], [1, 2147483647] }}}
+}
+
+void deletePointAfter(int x, int tmp) {
+  if(x != tmp)
+if(tmp == 2147483647)
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 2147483646] }}}
+}
+
+void deleteTwoPoints(int x, int tmp1, int tmp2) {
+  if(x != tmp1) {
+if (tmp1 == 42 && tmp2 == 87) {
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 41], [43, 2147483647] }}}
+  if(x != tmp2)
+clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 41], [43, 86], [88, 2147483647] }}}
+}
+  }
+}
+
+void deleteAllPoints(unsigned char x, unsigned char *arr) {
+
+#define cond(n) \
+arr[n##0] == n##0 && \
+arr[n##1] == n##1 && \
+arr[n##2] == n##2 && \
+arr[n##3] == n##3 && \
+arr[n##4] == n##4 && \
+arr[n##5] == n##5 && \
+arr[n##6] == n##6 && \
+arr[n##7] == n##7 && \
+arr[n##8] == n##8 && \
+arr[n##9] == n##9 && \
+
+#define condX(n) \
+arr[n##0] != x && \
+arr[n##1] != x && \
+arr[n##2] != x && \
+arr[n##3] != x && \
+arr[n##4] != x && \
+arr[n##5] != x && \
+arr[n##6] != x && \
+arr[n##7] != x && \
+arr[n##8] != x && \
+arr[n##9] != x && \
+
+  clang_analyzer_value(x); // expected-warning {{{ [0, 255] }}}
+  if (
+cond()  // 0  .. 9
+cond(1) // 10 .. 19
+cond(2) // 20 .. 29
+cond(3) // 30 .. 39
+cond(4) // 40 .. 49
+cond(5) // 50 .. 59
+cond(6) // 60 .. 69
+cond(7) // 70 .. 79
+cond(8) // 80 .. 89
+cond(9) // 90 .. 99
+cond(10) // 100 .. 209
+cond(11) // 110 .. 219
+cond(12) // 120 .. 229
+cond(13) // 130 .. 239
+cond(14) // 140 .. 249
+cond(15) // 150 .. 259
+cond(16) // 160 .. 269
+cond(17) // 170 .. 279
+cond(18) // 180 .. 289
+cond(19) // 190 .. 199
+cond(20) // 200 .. 209
+cond(21) // 210 .. 219
+cond(22) // 220 .. 229
+cond(23) // 230 .. 239
+cond(24) // 240 .. 249
+arr[250] == 250 &&
+arr[251] == 251 &&
+arr[252] == 252 &&
+arr[253] == 253 &&
+arr[254] == 254 &&
+arr[255] == 255
+) {
+if (
+  condX()  // 0  .. 9
+  condX(1) // 10 .. 19
+  condX(2) // 20 .. 29
+  condX(3) // 30 .. 39
+  condX(4) // 40 .. 49
+  condX(5) // 50 .. 59
+  condX(6) // 60 .. 69
+  condX(7) // 70 .. 79
+  condX(8) // 80 .. 89
+  condX(9) // 90 .. 99
+  condX(10) // 100 .. 209
+  condX(11) // 110 .. 219
+  condX(12) // 120 .. 229
+  condX(13) // 130 .. 239
+  condX(14) // 140 .. 249
+  condX(15) // 150 .. 259
+  condX(16) // 160 .. 269
+  condX(17) // 170 .. 279
+  condX(18) // 180 .. 289
+  condX(19) // 190 .. 199
+  condX(20) // 200 .. 209
+  condX(21) // 210 .. 219
+  condX(22) // 220 .. 229
+  condX(23) // 230 .. 239
+  arr[240] != x &&
+  arr[241] != x &&
+  arr[242] != x &&
+  arr[243] != x &&
+  arr[244] != x &&
+  arr[245] != x &&
+  arr[246] != x &&
+  arr[247] != x &&
+  arr[248] != x &&
+  arr[249] != x
+  ) {
+  clang_analyzer_value(x); // expected-warning {{{ [250, 255] }}}
+  if (
+  arr[250] != x &&
+  arr[251] != x &&
+  //skip arr[252]
+  arr[253] != x &&
+  arr[254] != x &&
+  arr[255] != x
+  ) {
+clang_analyzer_value(x); // expected-warning {{32s:252}}
+if (arr[252] != x) {
+  clang_analyzer_warnIfReached(); // unreachable
+}
+  }
+}
+  }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131514: [analyzer] [NFC] Add more test cases for equality tracking

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 451527.
ASDenysPetrov added a comment.

C-standard does not support templates. Replaced function template with the 
function.


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

https://reviews.llvm.org/D131514

Files:
  clang/test/Analysis/equality_tracking.c

Index: clang/test/Analysis/equality_tracking.c
===
--- clang/test/Analysis/equality_tracking.c
+++ clang/test/Analysis/equality_tracking.c
@@ -8,6 +8,7 @@
 #define CHAR_MAX (char)(UCHAR_MAX & (UCHAR_MAX >> 1))
 #define CHAR_MIN (char)(UCHAR_MAX & ~(UCHAR_MAX >> 1))
 
+void clang_analyzer_value(int);
 void clang_analyzer_eval(int);
 void clang_analyzer_warnIfReached(void);
 
@@ -233,3 +234,139 @@
 clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
   }
 }
+
+void deletePointBefore(int x, int tmp) {
+  if(tmp == 0)
+if(x != tmp)
+ clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, -1], [1, 2147483647] }}}
+}
+
+void deletePointAfter(int x, int tmp) {
+  if(x != tmp)
+if(tmp == 2147483647)
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 2147483646] }}}
+}
+
+void deleteTwoPoints(int x, int tmp1, int tmp2) {
+  if(x != tmp1) {
+if (tmp1 == 42 && tmp2 == 87) {
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 41], [43, 2147483647] }}}
+  if(x != tmp2)
+clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 41], [43, 86], [88, 2147483647] }}}
+}
+  }
+}
+
+void deleteAllPoints(unsigned char x, unsigned char *arr) {
+
+#define cond(n) \
+arr[n##0] == n##0 && \
+arr[n##1] == n##1 && \
+arr[n##2] == n##2 && \
+arr[n##3] == n##3 && \
+arr[n##4] == n##4 && \
+arr[n##5] == n##5 && \
+arr[n##6] == n##6 && \
+arr[n##7] == n##7 && \
+arr[n##8] == n##8 && \
+arr[n##9] == n##9 && \
+
+#define condX(n) \
+arr[n##0] != x && \
+arr[n##1] != x && \
+arr[n##2] != x && \
+arr[n##3] != x && \
+arr[n##4] != x && \
+arr[n##5] != x && \
+arr[n##6] != x && \
+arr[n##7] != x && \
+arr[n##8] != x && \
+arr[n##9] != x && \
+
+  clang_analyzer_value(x); // expected-warning {{{ [0, 255] }}}
+  if (
+cond()  // 0  .. 9
+cond(1) // 10 .. 19
+cond(2) // 20 .. 29
+cond(3) // 30 .. 39
+cond(4) // 40 .. 49
+cond(5) // 50 .. 59
+cond(6) // 60 .. 69
+cond(7) // 70 .. 79
+cond(8) // 80 .. 89
+cond(9) // 90 .. 99
+cond(10) // 100 .. 209
+cond(11) // 110 .. 219
+cond(12) // 120 .. 229
+cond(13) // 130 .. 239
+cond(14) // 140 .. 249
+cond(15) // 150 .. 259
+cond(16) // 160 .. 269
+cond(17) // 170 .. 279
+cond(18) // 180 .. 289
+cond(19) // 190 .. 199
+cond(20) // 200 .. 209
+cond(21) // 210 .. 219
+cond(22) // 220 .. 229
+cond(23) // 230 .. 239
+cond(24) // 240 .. 249
+arr[250] == 250 &&
+arr[251] == 251 &&
+arr[252] == 252 &&
+arr[253] == 253 &&
+arr[254] == 254 &&
+arr[255] == 255
+) {
+if (
+  condX()  // 0  .. 9
+  condX(1) // 10 .. 19
+  condX(2) // 20 .. 29
+  condX(3) // 30 .. 39
+  condX(4) // 40 .. 49
+  condX(5) // 50 .. 59
+  condX(6) // 60 .. 69
+  condX(7) // 70 .. 79
+  condX(8) // 80 .. 89
+  condX(9) // 90 .. 99
+  condX(10) // 100 .. 209
+  condX(11) // 110 .. 219
+  condX(12) // 120 .. 229
+  condX(13) // 130 .. 239
+  condX(14) // 140 .. 249
+  condX(15) // 150 .. 259
+  condX(16) // 160 .. 269
+  condX(17) // 170 .. 279
+  condX(18) // 180 .. 289
+  condX(19) // 190 .. 199
+  condX(20) // 200 .. 209
+  condX(21) // 210 .. 219
+  condX(22) // 220 .. 229
+  condX(23) // 230 .. 239
+  arr[240] != x &&
+  arr[241] != x &&
+  arr[242] != x &&
+  arr[243] != x &&
+  arr[244] != x &&
+  arr[245] != x &&
+  arr[246] != x &&
+  arr[247] != x &&
+  arr[248] != x &&
+  arr[249] != x
+  ) {
+  clang_analyzer_value(x); // expected-warning {{{ [250, 255] }}}
+  if (
+  arr[250] != x &&
+  arr[251] != x &&
+  //skip arr[252]
+  arr[253] != x &&
+  arr[254] != x &&
+  arr[255] != x
+  ) {
+clang_analyzer_value(x); // expected-warning {{32s:252}}
+if (arr[252] != x) {
+  clang_analyzer_warnIfReached(); // unreachable
+}
+  }
+}
+  }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131514: [analyzer] {NFC} Add more test cases for equality tracking

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: martong, steakhal, NoQ.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Cover `ConstraintAssignor::assign(EquivalenceClass, RangeSet)` function with 
more regression tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131514

Files:
  clang/test/Analysis/equality_tracking.c

Index: clang/test/Analysis/equality_tracking.c
===
--- clang/test/Analysis/equality_tracking.c
+++ clang/test/Analysis/equality_tracking.c
@@ -8,6 +8,8 @@
 #define CHAR_MAX (char)(UCHAR_MAX & (UCHAR_MAX >> 1))
 #define CHAR_MIN (char)(UCHAR_MAX & ~(UCHAR_MAX >> 1))
 
+template
+void clang_analyzer_value(T x);
 void clang_analyzer_eval(int);
 void clang_analyzer_warnIfReached(void);
 
@@ -233,3 +235,139 @@
 clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
   }
 }
+
+void deletePointBefore(int x, int tmp) {
+  if(tmp == 0)
+if(x != tmp)
+ clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, -1], [1, 2147483647] }}}
+}
+
+void deletePointAfter(int x, int tmp) {
+  if(x != tmp)
+if(tmp == 2147483647)
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 2147483646] }}}
+}
+
+void deleteTwoPoints(int x, int tmp1, int tmp2) {
+  if(x != tmp1) {
+if (tmp1 == 42 && tmp2 == 87) {
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 41], [43, 2147483647] }}}
+  if(x != tmp2)
+clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 41], [43, 86], [88, 2147483647] }}}
+}
+  }
+}
+
+void deleteAllPoints(unsigned char x, unsigned char *arr) {
+
+#define cond(n) \
+arr[n##0] == n##0 && \
+arr[n##1] == n##1 && \
+arr[n##2] == n##2 && \
+arr[n##3] == n##3 && \
+arr[n##4] == n##4 && \
+arr[n##5] == n##5 && \
+arr[n##6] == n##6 && \
+arr[n##7] == n##7 && \
+arr[n##8] == n##8 && \
+arr[n##9] == n##9 && \
+
+#define condX(n) \
+arr[n##0] != x && \
+arr[n##1] != x && \
+arr[n##2] != x && \
+arr[n##3] != x && \
+arr[n##4] != x && \
+arr[n##5] != x && \
+arr[n##6] != x && \
+arr[n##7] != x && \
+arr[n##8] != x && \
+arr[n##9] != x && \
+
+  clang_analyzer_value(x); // expected-warning {{8u:{ [0, 255] }}}
+  if (
+cond()  // 0  .. 9
+cond(1) // 10 .. 19
+cond(2) // 20 .. 29
+cond(3) // 30 .. 39
+cond(4) // 40 .. 49
+cond(5) // 50 .. 59
+cond(6) // 60 .. 69
+cond(7) // 70 .. 79
+cond(8) // 80 .. 89
+cond(9) // 90 .. 99
+cond(10) // 100 .. 209
+cond(11) // 110 .. 219
+cond(12) // 120 .. 229
+cond(13) // 130 .. 239
+cond(14) // 140 .. 249
+cond(15) // 150 .. 259
+cond(16) // 160 .. 269
+cond(17) // 170 .. 279
+cond(18) // 180 .. 289
+cond(19) // 190 .. 199
+cond(20) // 200 .. 209
+cond(21) // 210 .. 219
+cond(22) // 220 .. 229
+cond(23) // 230 .. 239
+cond(24) // 240 .. 249
+arr[250] == 250 &&
+arr[251] == 251 &&
+arr[252] == 252 &&
+arr[253] == 253 &&
+arr[254] == 254 &&
+arr[255] == 255
+) {
+if (
+  condX()  // 0  .. 9
+  condX(1) // 10 .. 19
+  condX(2) // 20 .. 29
+  condX(3) // 30 .. 39
+  condX(4) // 40 .. 49
+  condX(5) // 50 .. 59
+  condX(6) // 60 .. 69
+  condX(7) // 70 .. 79
+  condX(8) // 80 .. 89
+  condX(9) // 90 .. 99
+  condX(10) // 100 .. 209
+  condX(11) // 110 .. 219
+  condX(12) // 120 .. 229
+  condX(13) // 130 .. 239
+  condX(14) // 140 .. 249
+  condX(15) // 150 .. 259
+  condX(16) // 160 .. 269
+  condX(17) // 170 .. 279
+  condX(18) // 180 .. 289
+  condX(19) // 190 .. 199
+  condX(20) // 200 .. 209
+  condX(21) // 210 .. 219
+  condX(22) // 220 .. 229
+  condX(23) // 230 .. 239
+  arr[240] != x &&
+  arr[241] != x &&
+  arr[242] != x &&
+  arr[243] != x &&
+  arr[244] != x &&
+  arr[245] != x &&
+  arr[246] != x &&
+  arr[247] != x &&
+  arr[248] != x &&
+  arr[249] != x
+  ) {
+  clang_analyzer_value(x); // expected-warning {{8u:{ [250, 255] }}}
+  if (
+  arr[250] != x &&
+  arr[251] != x &&
+  //skip arr[252]
+  arr[253] != x &&
+  arr[254] != x &&
+  arr[255] != x
+  ) {
+clang_analyzer_value(x); // expected-warning {{8u:252}}
+if (arr[252] != x) {
+  clang_analyzer_warnIfReached(); // unreachable
+}
+  }
+}
+  }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

The //motivation //for this revision is lost due to the logical mistake in the 
later. However, this revision could be used for other improvements in the 
future. I will not merge it until new //motivation //has become.


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

https://reviews.llvm.org/D130372

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

[PATCH] D131006: [analyzer] Use DisequalityMap while inferring constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong 
This solution has an essential logical mistake. It relies on the erroneous 
assumption that
if `x > 0 && x != y` then `y` is in range `[MIN, 0]` but that's **not** true.
`x` and `y` could be `1` and `2` respectively.

My idea will only work with concrete ints but not ranges and it is already 
implemented in `RangeConstraintManager` in the snippet you mentioned 
(`ConstraintAssignor::assign`).

I'm sorry for the lost time.

P.S. The best I can do is just to add more tests for this. I will requalificate 
the revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131006

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

[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D130372#3699036 , @martong wrote:

> LGTM! Nice work!

Thank you. I prefer to load this along with the motivation part D131006 
.


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

https://reviews.llvm.org/D130372

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

[PATCH] D131006: [analyzer] Use DisequalityMap while inferring constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D131006#3699017 , @martong wrote:

> Awesome!

Thank you!




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1512-1516
+if (IsFirst) {
+  IsFirst = false;
+  RS = *RSPtr;
+} else
+  RS = RangeFactory.unite(RS, *RSPtr);

martong wrote:
> `unite` should be working with an empty set as well, shouldn't it?
You are right.

It's interesting that for some reason I was sure that `unite` returns //empty// 
if any given set is //empty// (mixed up with `intersect`). And this is at a 
time when I'm the author of `unite` :-P



Comment at: clang/test/Analysis/range-inferring-from-disequality-map.cpp:50-51
+if(x != tmp1 && x != tmp2)
+  // TODO: This condition should be infeasible.
+  //   Thus, the branch should be unreachable.
+  clang_analyzer_value(x); // expected-warning {{{ empty }}}

martong wrote:
> Why can't we return an empty set from `getInvertedRangeFromDisequalityMap` in 
> this case? `intersect` should handle the rest then.
Currently, we can't.  At least, in this patch.
We produce infeasible branches (aka `nullptr` `ProgramStateRef`) when use 
`ConstraintAssignor::assign`.
But here we use `SymbolicRangeInferrer` which only produces a RangeSet. In 
other words, we don't store this range anywhere but infer it every time.

Basically, this is a work for the next patches. They should remove the TODOs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131006

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

[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:1130
+  // Check inverting single range.
+  this->checkInvert({{MIN, MAX}}, {});
+  this->checkInvert({{MIN, MID}}, {{MID + 1, MAX}});

martong wrote:
> ASDenysPetrov wrote:
> > martong wrote:
> > > I'd expect that inversion on finite sets is an invert-able function thus
> > > ```
> > > this->checkInvert({}, {MIN, MAX});
> > > ```
> > > would make sense instead of assertion.
> > > 
> > > Besides, it would make sense to test the composition of two invert 
> > > operations to identity. 
> > I'm afraid the function would be overheaded and less usable. Why do we have 
> > an assertion here? I thought about this. This is kinda a tradeoff.
> > 
> > What range would be a correct range from inversion of `[]`? `[-128, 127]`? 
> > `[0, 65535]`?
> > In order to make `[MIN, MAX]` from empty range `[]` we should know the 
> > exact `APSIntType`. We extract the type from the given range 
> > `What.getAPSIntType();`.
> > 
> > But though, let's consider we have `QualType` as a second parameter. We 
> > have two options about what type to use.
> > 1. Always use the second parameter. The function would not only invert but 
> > do what `castTo` actually do. Such behavior would violates a single 
> > responsibility principle and duplicate the functionality.
> > 2. Always use the type from the given range and for the case of empty range 
> > from the second parameter. The function contract would be more complex and 
> > obscure.
> > 
> > So, I decided not to introduce a new parameter and shift responsibility to 
> > the user. Empty range is an edge case when we usually produce infeasible 
> > state or use `infer(QualType)`. This may pretty fit with what user is going 
> > to do first before using `invert`, so it shouldn't affect the convinience 
> > of function usage too much.
> Okay, I see your point.
> 
> So, considering `RangeSet::Factory::invert(RangeSet What)` we have only one 
> parameter and that is the RangeSet to be inverted. And that set might be 
> empty and in that case we are left without any type information, thus there 
> is no way to return [MIN, MAX]. Please correct me if I am wrong.
Exactly. That's what I'm talking about..


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

https://reviews.llvm.org/D130372

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

[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong

> Now I'm working on the next patch and you'll see a motivation soon.

Here is the motivation but it still causes some tests failure D131006 
.


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

https://reviews.llvm.org/D130372

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

[PATCH] D131006: [analyzer] Use DisequalityMap while inferring constraints

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: martong, steakhal, NoQ.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Infer range using associated unequal symbols from DisequalityMap.
Example:

  if(x == 42)
if(x != y)
  y; // [-2147483648, 41]U[43, 2147483647]

NOTE: Currently, this revision causes test failure due to assertion in related 
to `IteratorModeling.cpp` in `relateSymbols` on line
`assert(isa(CompSym) && "Symbol comparison must be a 
`SymIntExpr`");`. It needs to be fixed in some way before loading. The revision 
is exposed to show the motivation for D130372 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131006

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/range-inferring-from-disequality-map.cpp

Index: clang/test/Analysis/range-inferring-from-disequality-map.cpp
===
--- /dev/null
+++ clang/test/Analysis/range-inferring-from-disequality-map.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
+
+template
+void clang_analyzer_value(T x);
+
+void test1(int x, int tmp) {
+  if(tmp != 0)
+if(x != tmp)
+ clang_analyzer_value(x); // expected-warning {{32s:{ [0, 0] }}}
+  // TODO:  TODO: Keep x range correct even if associated disequalities are
+  // already dead.
+  (void)tmp; // Keep alive.
+}
+
+void test2(int x, int tmp) {
+  if(x != tmp)
+if(tmp < 0)
+  clang_analyzer_value(x); // expected-warning {{32s:{ [0, 2147483647] }}}
+  // TODO:  TODO: Keep x range correct even if associated disequalities are
+  // already dead.
+  (void)tmp; // Keep alive.
+}
+
+void test3(int x, int tmp) {
+  if(x != tmp)
+if(tmp > 42 && tmp < 87)
+  clang_analyzer_value(x); // expected-warning {{32s:{ [-2147483648, 42], [87, 2147483647] }}}
+  // TODO:  TODO: Keep x range correct even if associated disequalities are
+  // already dead.
+  (void)tmp; // Keep alive.
+}
+
+void test4(int x, int tmp1, int tmp2) {
+  if(x != tmp1) {
+if (tmp1 < 0 && tmp2 > 0) {
+  clang_analyzer_value(x); // expected-warning {{32s:{ [0, 2147483647] }}}
+  if(x != tmp2)
+clang_analyzer_value(x); // expected-warning {{32s:{ [0, 0] }}}
+}
+  }
+  // TODO:  TODO: Keep x range correct even if associated disequalities are
+  // already dead.
+  (void)tmp1; // Keep alive.
+  (void)tmp2; // Keep alive.
+}
+
+void test5(int x, int tmp1, int tmp2) {
+  if (tmp1 < 42 && tmp2 >= 42) 
+if(x != tmp1 && x != tmp2)
+  // TODO: This condition should be infeasible.
+  //   Thus, the branch should be unreachable.
+  clang_analyzer_value(x); // expected-warning {{{ empty }}}
+  // TODO:  TODO: Keep x range correct even if associated disequalities are
+  // already dead.
+  (void)tmp1; // Keep alive.
+  (void)tmp2; // Keep alive.
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1327,6 +1327,8 @@
  // Of course, we should take the constraint directly
  // associated with this symbol into consideration.
  getConstraint(State, Sym),
+ // Use inverted ranges from DisequalityMap.
+ getInvertedRangeFromDisequalityMap(State, Sym),
  // Apart from the Sym itself, we can infer quite a lot if
  // we look into subexpressions of Sym.
  Visit(Sym));
@@ -1490,6 +1492,39 @@
 return RangeFactory.deletePoint(Domain, IntType.getZeroValue());
   }
 
+  Optional getInvertedRangeFromDisequalityMap(ProgramStateRef State,
+SymbolRef Sym) {
+QualType T = Sym->getType();
+// We only support integral types.
+if (!T->isIntegralOrEnumerationType())
+  return llvm::None;
+
+EquivalenceClass EC = EquivalenceClass::find(State, Sym);
+const ClassSet *CS = State->get(EC);
+
+if (!CS)
+  return llvm::None;
+
+bool IsFirst = true;
+RangeSet RS = RangeFactory.getEmptySet();
+for (EquivalenceClass EC : *CS) {
+  if (const RangeSet *RSPtr = getConstraint(State, EC)) {
+if (IsFirst) {
+  IsFirst = false;
+  RS = *RSPtr;
+} else
+  RS = RangeFactory.unite(RS, *RSPtr);
+  }
+}
+
+if (IsFirst)
+  return llvm::None;
+
+RS = RangeFactory.castTo(RS, T);
+RS = RangeFactory.invert(RS);
+return RS;
+  }
+
   template 
  

[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 448161.
ASDenysPetrov added a comment.

Make changes according to the remarks.

- Add complexity to function description.
- Rewrite the loop making it deterministic.
- Add back invert operation to the tests.


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

https://reviews.llvm.org/D130372

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -275,6 +275,20 @@
 static constexpr APSIntType ToTy = APSIntTy;
 this->checkCastToImpl(from(What, FromTy), ToTy, from(Expected, ToTy));
   }
+
+  void checkInvertImpl(RangeSet What, RangeSet Expected) {
+RangeSet Result = F.invert(What);
+EXPECT_EQ(Result, Expected) << "while inverting " << toString(What);
+// Do invert back, except empty range because it is not permitted.
+if (Result.isEmpty())
+  return;
+RangeSet BackResult = F.invert(Result);
+EXPECT_EQ(BackResult, What) << "while inverting " << toString(Result);
+  }
+
+  void checkInvert(RawRangeSet What, RawRangeSet Expected) {
+this->checkInvertImpl(from(What), from(Expected));
+  }
 };
 
 using IntTypes = ::testing::Types;
+  constexpr auto MIN = TV::MIN;
+  constexpr auto MAX = TV::MAX;
+  constexpr auto MID = TV::MID;
+  constexpr auto A = TV::A;
+  constexpr auto B = TV::B;
+  constexpr auto C = TV::C;
+  constexpr auto D = TV::D;
+
+  // Empty set is forbidden by the function contract.
+  // It produces an assertion.
+  /*this->checkInvert({}, ASSERTION);*/
+
+  // Check inverting single point.
+  this->checkInvert({{MIN, MIN}}, {{MIN + 1, MAX}});
+  this->checkInvert({{MAX, MAX}}, {{MIN, MAX - 1}});
+  this->checkInvert({{MID, MID}}, {{MIN, MID - 1}, {MID + 1, MAX}});
+  this->checkInvert({{A, A}}, {{MIN, A - 1}, {A + 1, MAX}});
+
+  // Check inverting two points.
+  this->checkInvert({{MIN, MIN}, {MAX, MAX}}, {{MIN + 1, MAX - 1}});
+  this->checkInvert({{MID, MID}, {MAX, MAX}},
+{{MIN, MID - 1}, {MID + 1, MAX - 1}});
+  this->checkInvert({{MIN, MIN}, {D, D}}, {{MIN + 1, D - 1}, {D + 1, MAX}});
+  this->checkInvert({{B, B}, {C, C}},
+{{MIN, B - 1}, {B + 1, C - 1}, {C + 1, MAX}});
+
+  // Check inverting single range.
+  this->checkInvert({{MIN, MAX}}, {});
+  this->checkInvert({{MIN, MID}}, {{MID + 1, MAX}});
+  this->checkInvert({{MID, MAX}}, {{MIN, MID - 1}});
+  this->checkInvert({{A, D}}, {{MIN, A - 1}, {D + 1, MAX}});
+
+  // Check adding two ranges.
+  this->checkInvert({{MIN, MID - 1}, {MID + 1, MAX}}, {{MID, MID}});
+  this->checkInvert({{MIN, B}, {C, D}}, {{B + 1, C - 1}, {D + 1, MAX}});
+  this->checkInvert({{MIN + 1, A}, {B, MAX}}, {{MIN, MIN}, {A + 1, B - 1}});
+  this->checkInvert({{A, B}, {C, MAX - 1}},
+{{MIN, A - 1}, {B + 1, C - 1}, {MAX, MAX}});
+
+  // Check adding three ranges.
+  this->checkInvert({{MIN, A}, {B, C}, {D, MAX}},
+{{A + 1, B - 1}, {C + 1, D - 1}});
+  this->checkInvert({{A, B - 1}, {B + 1, C - 1}, {C + 1, MAX}},
+{{MIN, A - 1}, {B, B}, {C, C}});
+  this->checkInvert({{MIN, B - 1}, {B + 1, C - 1}, {C + 1, D}},
+{{B, B}, {C, C}, {D + 1, MAX}});
+  this->checkInvert({{A, B - 1}, {C, C}, {D, D}},
+{{MIN, A - 1}, {B, C - 1}, {C + 1, D - 1}, {D + 1, MAX}});
+}
+
 } // namespace
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -611,12 +611,12 @@
   if (What.isEmpty())
 return getEmptySet();
 
-  const llvm::APSInt SampleValue = What.getMinValue();
-  const llvm::APSInt  = ValueFactory.getMinValue(SampleValue);
-  const llvm::APSInt  = ValueFactory.getMaxValue(SampleValue);
+  const APSIntType Ty = What.getAPSIntType();
+  const llvm::APSInt  = ValueFactory.getMinValue(Ty);
+  const llvm::APSInt  = ValueFactory.getMaxValue(Ty);
 
   ContainerType Result;
-  Result.reserve(What.size() + (SampleValue == MIN));
+  Result.reserve(What.size() + (What.getMinValue() == MIN));
 
   // Handle a special case for MIN value.
   const_iterator It = What.begin();
@@ -708,6 +708,46 @@
   return castTo(What, ValueFactory.getAPSIntType(T));
 }
 
+RangeSet RangeSet::Factory::invert(RangeSet What) {
+  assert(!What.isEmpty());
+
+  // Prepare some constants.
+  const APSIntType Ty = What.getAPSIntType();
+  const llvm::APSInt  = ValueFactory.getMinValue(Ty);
+  const llvm::APSInt  = ValueFactory.getMaxValue(Ty);
+  const bool NoMin = What.getMinValue() != MIN;
+  const bool NoMax 

[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov marked 3 inline comments as done.
ASDenysPetrov added a comment.

Now I'm working on the next patch and you'll see a motivation soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130372

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

[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov marked 6 inline comments as done.
ASDenysPetrov added a comment.

@martong Thanks for review.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:728-729
+  ContainerType Result;
+  Result.reserve(What.size() + 1 - bool(What.getMinValue() != MIN) -
+ bool(What.getMaxValue() != MAX));
+

martong wrote:
> There might be a flaw here. Should this be `==` instead of `!=` ?
> Consider e.g. when `What` has two elements `[[MIN, -1], [1, MAX]]` then the 
> inverse should be `[0,0]` which has size `1`.
Yes. Definetely. It remained from my previous version of calculation.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:281
+RangeSet Result = F.invert(What);
+EXPECT_EQ(Result, Expected) << "while inverting " << toString(What);
+  }

martong wrote:
> I think, it would make sense to test the composition of two invert operations 
> to identity. 
> ```
> EXPECT_EQ(What, F.invert(F.invert(What));
> ```
> This, of course requires that empty set is a possible input.
Great idea. I will do it, except `[]`.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:1130
+  // Check inverting single range.
+  this->checkInvert({{MIN, MAX}}, {});
+  this->checkInvert({{MIN, MID}}, {{MID + 1, MAX}});

martong wrote:
> I'd expect that inversion on finite sets is an invert-able function thus
> ```
> this->checkInvert({}, {MIN, MAX});
> ```
> would make sense instead of assertion.
> 
> Besides, it would make sense to test the composition of two invert operations 
> to identity. 
I'm afraid the function would be overheaded and less usable. Why do we have an 
assertion here? I thought about this. This is kinda a tradeoff.

What range would be a correct range from inversion of `[]`? `[-128, 127]`? `[0, 
65535]`?
In order to make `[MIN, MAX]` from empty range `[]` we should know the exact 
`APSIntType`. We extract the type from the given range `What.getAPSIntType();`.

But though, let's consider we have `QualType` as a second parameter. We have 
two options about what type to use.
1. Always use the second parameter. The function would not only invert but do 
what `castTo` actually do. Such behavior would violates a single responsibility 
principle and duplicate the functionality.
2. Always use the type from the given range and for the case of empty range 
from the second parameter. The function contract would be more complex and 
obscure.

So, I decided not to introduce a new parameter and shift responsibility to the 
user. Empty range is an edge case when we usually produce infeasible state or 
use `infer(QualType)`. This may pretty fit with what user is going to do first 
before using `invert`, so it shouldn't affect the convinience of function usage 
too much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130372

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

[PATCH] D130372: [analyzer] Add a new factory function RangeSet::Factory::invert

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: martong, NoQ, steakhal, balazske.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Add a new factory function `RangeSet::Factory::invert`. It performs an 
inversion operation on the given set and produces a new one as a result. The 
inversion of the set is a set containing all the values except those which are 
in the original one.

NOTE: User shall guarantee that the given set is not empty.

Example:
original: `int8[0, 42]` inverse: `int8[-128, -1]U[43, 127]`;
original: `int8[-128, 127]` inverse: `int8[]`;
original: `[]` inverse: `raise an assertion`.

The motivation is to extend the set of operations that can be performed on 
range sets. Specifically, this function is needed for the next patch in the 
stack.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130372

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -275,6 +275,15 @@
 static constexpr APSIntType ToTy = APSIntTy;
 this->checkCastToImpl(from(What, FromTy), ToTy, from(Expected, ToTy));
   }
+
+  void checkInvertImpl(RangeSet What, RangeSet Expected) {
+RangeSet Result = F.invert(What);
+EXPECT_EQ(Result, Expected) << "while inverting " << toString(What);
+  }
+
+  void checkInvert(RawRangeSet What, RawRangeSet Expected) {
+this->checkInvertImpl(from(What), from(Expected));
+  }
 };
 
 using IntTypes = ::testing::Types;
+  constexpr auto MIN = TV::MIN;
+  constexpr auto MAX = TV::MAX;
+  constexpr auto MID = TV::MID;
+  constexpr auto A = TV::A;
+  constexpr auto B = TV::B;
+  constexpr auto C = TV::C;
+  constexpr auto D = TV::D;
+
+  // Empty set is forbidden by the function contract.
+  // It produces an assertion.
+  /*this->checkInvert({}, ASSERTION);*/
+
+  // Check inverting single point.
+  this->checkInvert({{MIN, MIN}}, {{MIN + 1, MAX}});
+  this->checkInvert({{MAX, MAX}}, {{MIN, MAX - 1}});
+  this->checkInvert({{MID, MID}}, {{MIN, MID - 1}, {MID + 1, MAX}});
+  this->checkInvert({{A, A}}, {{MIN, A - 1}, {A + 1, MAX}});
+
+  // Check inverting two points.
+  this->checkInvert({{MIN, MIN}, {MAX, MAX}}, {{MIN + 1, MAX - 1}});
+  this->checkInvert({{MID, MID}, {MAX, MAX}},
+{{MIN, MID - 1}, {MID + 1, MAX - 1}});
+  this->checkInvert({{MIN, MIN}, {D, D}}, {{MIN + 1, D - 1}, {D + 1, MAX}});
+  this->checkInvert({{B, B}, {C, C}},
+{{MIN, B - 1}, {B + 1, C - 1}, {C + 1, MAX}});
+
+  // Check inverting single range.
+  this->checkInvert({{MIN, MAX}}, {});
+  this->checkInvert({{MIN, MID}}, {{MID + 1, MAX}});
+  this->checkInvert({{MID, MAX}}, {{MIN, MID - 1}});
+  this->checkInvert({{A, D}}, {{MIN, A - 1}, {D + 1, MAX}});
+
+  // Check adding two ranges.
+  this->checkInvert({{MIN, MID - 1}, {MID + 1, MAX}}, {{MID, MID}});
+  this->checkInvert({{MIN, B}, {C, D}}, {{B + 1, C - 1}, {D + 1, MAX}});
+  this->checkInvert({{MIN + 1, A}, {B, MAX}}, {{MIN, MIN}, {A + 1, B - 1}});
+  this->checkInvert({{A, B}, {C, MAX - 1}},
+{{MIN, A - 1}, {B + 1, C - 1}, {MAX, MAX}});
+
+  // Check adding three ranges.
+  this->checkInvert({{MIN, A}, {B, C}, {D, MAX}},
+{{A + 1, B - 1}, {C + 1, D - 1}});
+  this->checkInvert({{A, B - 1}, {B + 1, C - 1}, {C + 1, MAX}},
+{{MIN, A - 1}, {B, B}, {C, C}});
+  this->checkInvert({{MIN, B - 1}, {B + 1, C - 1}, {C + 1, D}},
+{{B, B}, {C, C}, {D + 1, MAX}});
+  this->checkInvert({{A, B - 1}, {C, C}, {D, D}},
+{{MIN, A - 1}, {B, C - 1}, {C + 1, D - 1}, {D + 1, MAX}});
+}
+
 } // namespace
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -611,12 +611,12 @@
   if (What.isEmpty())
 return getEmptySet();
 
-  const llvm::APSInt SampleValue = What.getMinValue();
-  const llvm::APSInt  = ValueFactory.getMinValue(SampleValue);
-  const llvm::APSInt  = ValueFactory.getMaxValue(SampleValue);
+  const APSIntType Ty = What.getAPSIntType();
+  const llvm::APSInt  = ValueFactory.getMinValue(Ty);
+  const llvm::APSInt  = ValueFactory.getMaxValue(Ty);
 
   ContainerType Result;
-  Result.reserve(What.size() + (SampleValue == MIN));
+  Result.reserve(What.size() + 

[PATCH] D130029: [analyzer][NFC] Use `SValVisitor` instead of explicit helper functions

[Denys Petrov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa364987368a1: [analyzer][NFC] Use `SValVisitor` instead of 
explicit helper functions (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130029

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -19,15 +19,16 @@
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/Type.h"
-#include "clang/Basic/LLVM.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
@@ -617,517 +618,478 @@
 }
 
 //===--===//
-// Cast methods.
-// `evalCast` is the main method
-// `evalCastKind` and `evalCastSubKind` are helpers
+// Cast method.
+// `evalCast` and its helper `EvalCastVisitor`
 //===--===//
 
-/// Cast a given SVal to another SVal using given QualType's.
-/// \param V -- SVal that should be casted.
-/// \param CastTy -- QualType that V should be casted according to.
-/// \param OriginalTy -- QualType which is associated to V. It provides
-/// additional information about what type the cast performs from.
-/// \returns the most appropriate casted SVal.
-/// Note: Many cases don't use an exact OriginalTy. It can be extracted
-/// from SVal or the cast can performs unconditionaly. Always pass OriginalTy!
-/// It can be crucial in certain cases and generates different results.
-/// FIXME: If `OriginalTy.isNull()` is true, then cast performs based on CastTy
-/// only. This behavior is uncertain and should be improved.
-SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) {
-  if (CastTy.isNull())
-return V;
-
-  CastTy = Context.getCanonicalType(CastTy);
+namespace {
+class EvalCastVisitor : public SValVisitor {
+private:
+  SValBuilder 
+  ASTContext 
+  QualType CastTy, OriginalTy;
 
-  const bool IsUnknownOriginalType = OriginalTy.isNull();
-  if (!IsUnknownOriginalType) {
-OriginalTy = Context.getCanonicalType(OriginalTy);
+public:
+  EvalCastVisitor(SValBuilder , QualType CastTy, QualType OriginalTy)
+  : VB(VB), Context(VB.getContext()), CastTy(CastTy),
+OriginalTy(OriginalTy) {}
 
-if (CastTy == OriginalTy)
+  SVal Visit(SVal V) {
+if (CastTy.isNull())
   return V;
 
-// FIXME: Move this check to the most appropriate
-// evalCastKind/evalCastSubKind function. For const casts, casts to void,
-// just propagate the value.
-if (!CastTy->isVariableArrayType() && !OriginalTy->isVariableArrayType())
-  if (shouldBeModeledWithNoOp(Context, Context.getPointerType(CastTy),
-  Context.getPointerType(OriginalTy)))
-return V;
-  }
-
-  // Cast SVal according to kinds.
-  switch (V.getBaseKind()) {
-  case SVal::UndefinedValKind:
-return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  case SVal::UnknownValKind:
-return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  case SVal::LocKind:
-return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  case SVal::NonLocKind:
-return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  }
-
-  llvm_unreachable("Unknown SVal kind");
-}
-
-SVal SValBuilder::evalCastKind(UndefinedVal V, QualType CastTy,
-   QualType OriginalTy) {
-  return V;
-}
-
-SVal SValBuilder::evalCastKind(UnknownVal V, QualType CastTy,
-   QualType OriginalTy) {
-  return V;
-}
-
-SVal SValBuilder::evalCastKind(Loc V, QualType CastTy, QualType OriginalTy) {
-  switch (V.getSubKind()) {
-  case loc::ConcreteIntKind:
-return evalCastSubKind(V.castAs(), CastTy, OriginalTy);
-  case loc::GotoLabelKind:
-return evalCastSubKind(V.castAs(), CastTy, OriginalTy);
-  case 

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D112621#3660256 , @manas wrote:

> Is there any other way to formulate the expression so that it constructs `LHS 
> = [1, 2] U [8, 9]` and doesn't bifurcate?

Try this `u1 > 0 && u1 < 10 && u1 != 3 && u1 != 4 && u1 != 5 && u1 != 6 && u1 
!= 7 && u2 > 4 && u2 < 7`. This is a bit verbose but it will avoid bifurcating.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1624-1626
+  if (LHS.isEmpty() || RHS.isEmpty()) {
+return RangeFactory.getEmptySet();
+  }

Syntax: We usually don't use braces for single-line statements.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1630-1631
+
+  RangeSet CastedLHS = RangeFactory.castTo(LHS, ResultType);
+  RangeSet CastedRHS = RangeFactory.castTo(RHS, ResultType);
+

And again. This is incorrect to cast your `un/signeds` to `bool`.
First, `castTo` currently does not support this operation correctly AFAIR (My 
fault. I'll add the support later). And thus, I've no idea why your tests pass.
Second, you will get from e.g. `u32[1,9]`  -  `bool[1,1]` and from `i32[42, 
45]` - `bool[1,1]`. Then `bool[1,1]` would be equal to `bool[1,1]`, but it 
shouldn't in terms of `u/i32`.

Here you should emulate the behavior of C++ abstract machine, hence cast both 
to the biggest type or unsigned one.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1633-1636
+  if (CastedLHS == RangeFactory.getEmptySet() ||
+  CastedRHS == RangeFactory.getEmptySet()) {
+return infer(T);
+  }

`castTo` won't produce you empty RangeSets unless the originals were already 
empty, but we checked for this previously.  So, you don't need this check.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1415-1416
+
+  Range CoarseLHS = fillGaps(LHS);
+  Range CoarseRHS = fillGaps(RHS);
+

manas wrote:
> ASDenysPetrov wrote:
> > Avoid filling the gaps, because it's completely possible to compare all the 
> > ranges.
> > E.g. //LHS //`[1,2]U[8,9]`  and  //RHS //`[5,6]` are not equal.
> > And you don't need to fiil the gap in LHS, because it will lead you to a 
> > wrong answer, like `[1,9] != [5,6]` => **UNKNOWN** instead of **TRUE**.
> If we don't fill gaps, then we will be making this method O(n) instead of 
> O(1) as of now. As I see it, filling RHS (and not filling LHS), it can 
> iterate over all ranges in LHS, and check each range's intersection with 
> CoarseRHS.
> 
> Before, I do this, I just wanted to point out the unreachability of concrete 
> cases to this method. I have tried to explain this below.
Here I'd rather get a correct result and wait a bit then contra-versa.



Comment at: clang/test/Analysis/constant-folding.c:289
+
+  // Check when RHS is in between two Ranges in LHS
+  if (((u1 >= 1 && u1 <= 2) || (u1 >= 8 && u1 <= 9)) &&

manas wrote:
> @ASDenysPetrov I have used your example, but I realized that the path 
> bifurcates and sizeof `LHS` RangeSet is always 1 inside 
> `VisitBinaryOperator`.
Just do as I advised in the main comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D130029: [analyzer][NFC] Use `SValVisitor` instead of explicit helper functions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D90157#2518118 , @steakhal wrote:

> Why don't you use the `SValVisitor` instead?

@steakhal Finally fulfilled you suggestion :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130029

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

[PATCH] D130029: [analyzer][NFC] Use `SValVisitor` instead of explicit helper functions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: martong, NoQ, steakhal.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Get rid of explicit function splitting in favor of specifically designed 
Visitor.
Move logic from a family of `evalCastKind` and `evalCastSubKind` helper 
functions to `SValVisitor`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130029

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -19,15 +19,16 @@
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/Type.h"
-#include "clang/Basic/LLVM.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
@@ -617,517 +618,478 @@
 }
 
 //===--===//
-// Cast methods.
-// `evalCast` is the main method
-// `evalCastKind` and `evalCastSubKind` are helpers
+// Cast method.
+// `evalCast` and its helper `EvalCastVisitor`
 //===--===//
 
-/// Cast a given SVal to another SVal using given QualType's.
-/// \param V -- SVal that should be casted.
-/// \param CastTy -- QualType that V should be casted according to.
-/// \param OriginalTy -- QualType which is associated to V. It provides
-/// additional information about what type the cast performs from.
-/// \returns the most appropriate casted SVal.
-/// Note: Many cases don't use an exact OriginalTy. It can be extracted
-/// from SVal or the cast can performs unconditionaly. Always pass OriginalTy!
-/// It can be crucial in certain cases and generates different results.
-/// FIXME: If `OriginalTy.isNull()` is true, then cast performs based on CastTy
-/// only. This behavior is uncertain and should be improved.
-SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) {
-  if (CastTy.isNull())
-return V;
-
-  CastTy = Context.getCanonicalType(CastTy);
+namespace {
+class EvalCastVisitor : public SValVisitor {
+private:
+  SValBuilder 
+  ASTContext 
+  QualType CastTy, OriginalTy;
 
-  const bool IsUnknownOriginalType = OriginalTy.isNull();
-  if (!IsUnknownOriginalType) {
-OriginalTy = Context.getCanonicalType(OriginalTy);
+public:
+  EvalCastVisitor(SValBuilder , QualType CastTy, QualType OriginalTy)
+  : VB(VB), Context(VB.getContext()), CastTy(CastTy),
+OriginalTy(OriginalTy) {}
 
-if (CastTy == OriginalTy)
+  SVal Visit(SVal V) {
+if (CastTy.isNull())
   return V;
 
-// FIXME: Move this check to the most appropriate
-// evalCastKind/evalCastSubKind function. For const casts, casts to void,
-// just propagate the value.
-if (!CastTy->isVariableArrayType() && !OriginalTy->isVariableArrayType())
-  if (shouldBeModeledWithNoOp(Context, Context.getPointerType(CastTy),
-  Context.getPointerType(OriginalTy)))
-return V;
-  }
-
-  // Cast SVal according to kinds.
-  switch (V.getBaseKind()) {
-  case SVal::UndefinedValKind:
-return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  case SVal::UnknownValKind:
-return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  case SVal::LocKind:
-return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  case SVal::NonLocKind:
-return evalCastKind(V.castAs(), CastTy, OriginalTy);
-  }
-
-  llvm_unreachable("Unknown SVal kind");
-}
-
-SVal SValBuilder::evalCastKind(UndefinedVal V, QualType CastTy,
-   QualType OriginalTy) {
-  return V;
-}
-
-SVal SValBuilder::evalCastKind(UnknownVal V, QualType CastTy,
-   QualType OriginalTy) {
-  return V;
-}
-
-SVal 

[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov closed this revision.
ASDenysPetrov added a comment.

Closed with bc08c3cb7f8e797fee14e96eedd3dc358608ada3 





Comment at: clang/test/Analysis/print-ranges.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config eagerly-assume=false -verify %s
+// REQUIRES: no-z3

martong wrote:
> Don't forget to pin the target/triple.
Why does this specific test need it?



Comment at: clang/test/Analysis/print-ranges.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config eagerly-assume=false -verify %s
+

NoQ wrote:
> ASDenysPetrov wrote:
> > NoQ wrote:
> > > I suspect this test will crash when clang is built with Z3, because the 
> > > Z3 constraint manager doesn't implement your new function yet.
> > Agree. Is it enough `REQUIRES: no-z3` or to add `#ifdef ANALYZER_CM_Z3`?
> That should be good. My personal tradition in such cases is to double-check 
> that this doesn't disable the test entirely.
I added `REQUIRES: z3` it prints `Unsupported`, then added `REQUIRES: no-z3` 
and it passed.


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

https://reviews.llvm.org/D129498

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@manas

I'm sorry but it seems like I brought you a new work :-) I've just loaded these 
two patches (D129678  and D129498 
) and now you have to rebase your changes. 
But there is a good news as well. You will be able to use 
`clang_analyzer_value` function for your tests if needed.

I appreciate your efforts but I should tell that I'm currently working on the 
thing that should resolve the issue you are trying to solve D103096 
.

> The coverage showing unreachability of `VisitBinaryOperator` for 
> concrete integer cases.

Maybe it's better to remove that unreachable part but leave the tests for 
concrete ints just to verify that all the cases are covered.

Also I expect to see test case for `short-ushort`, `char-uchar` pairs, because 
if it would turn out that the `int-uint` is only pair that we handle the patch 
would be disappointing, unfortunately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

[Denys Petrov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG82f76c04774f: [analyzer][NFC] Tidy up handler-functions in 
SymbolicRangeInferrer (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129678

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1213,13 +1213,21 @@
   }
 
   RangeSet VisitSymExpr(SymbolRef Sym) {
-// If we got to this function, the actual type of the symbolic
+if (Optional RS = getRangeForNegatedSym(Sym))
+  return *RS;
+// If we've reached this line, the actual type of the symbolic
 // expression is not supported for advanced inference.
 // In this case, we simply backoff to the default "let's simply
 // infer the range from the expression's type".
 return infer(Sym->getType());
   }
 
+  RangeSet VisitUnarySymExpr(const UnarySymExpr *USE) {
+if (Optional RS = getRangeForNegatedUnarySym(USE))
+  return *RS;
+return infer(USE->getType());
+  }
+
   RangeSet VisitSymIntExpr(const SymIntExpr *Sym) {
 return VisitBinaryOperator(Sym);
   }
@@ -1228,14 +1236,25 @@
 return VisitBinaryOperator(Sym);
   }
 
-  RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
+  RangeSet VisitSymSymExpr(const SymSymExpr *SSE) {
 return intersect(
 RangeFactory,
+// If Sym is a difference of symbols A - B, then maybe we have range
+// set stored for B - A.
+//
+// If we have range set stored for both A - B and B - A then
+// calculate the effective range set by intersecting the range set
+// for A - B and the negated range set of B - A.
+getRangeForNegatedSymSym(SSE),
+// If Sym is a comparison expression (except <=>),
+// find any other comparisons with the same operands.
+// See function description.
+getRangeForComparisonSymbol(SSE),
 // If Sym is (dis)equality, we might have some information
 // on that in our equality classes data structure.
-getRangeForEqualities(Sym),
+getRangeForEqualities(SSE),
 // And we should always check what we can get from the operands.
-VisitBinaryOperator(Sym));
+VisitBinaryOperator(SSE));
   }
 
 private:
@@ -1264,25 +1283,13 @@
   }
 
   RangeSet infer(SymbolRef Sym) {
-return intersect(
-RangeFactory,
-// Of course, we should take the constraint directly associated with
-// this symbol into consideration.
-getConstraint(State, Sym),
-// If Sym is a difference of symbols A - B, then maybe we have range
-// set stored for B - A.
-//
-// If we have range set stored for both A - B and B - A then
-// calculate the effective range set by intersecting the range set
-// for A - B and the negated range set of B - A.
-getRangeForNegatedSub(Sym),
-// If Sym is a comparison expression (except <=>),
-// find any other comparisons with the same operands.
-// See function description.
-getRangeForComparisonSymbol(Sym),
-// Apart from the Sym itself, we can infer quite a lot if we look
-// into subexpressions of Sym.
-Visit(Sym));
+return intersect(RangeFactory,
+ // Of course, we should take the constraint directly
+ // associated with this symbol into consideration.
+ getConstraint(State, Sym),
+ // Apart from the Sym itself, we can infer quite a lot if
+ // we look into subexpressions of Sym.
+ Visit(Sym));
   }
 
   RangeSet infer(EquivalenceClass Class) {
@@ -1443,38 +1450,53 @@
 return RangeFactory.deletePoint(Domain, IntType.getZeroValue());
   }
 
-  Optional getRangeForNegatedSub(SymbolRef Sym) {
+  template 
+  Optional getRangeForNegatedExpr(ProduceNegatedSymFunc F,
+QualType T) {
 // Do not negate if the type cannot be meaningfully negated.
-if (!Sym->getType()->isUnsignedIntegerOrEnumerationType() &&
-!Sym->getType()->isSignedIntegerOrEnumerationType())
+if (!T->isUnsignedIntegerOrEnumerationType() &&
+!T->isSignedIntegerOrEnumerationType())
   return llvm::None;
 
-const RangeSet *NegatedRange = nullptr;
-SymbolManager  = State->getSymbolManager();
-if (const auto *USE = dyn_cast(Sym)) {
-  if (USE->getOpcode() == UO_Minus) {
-// Just get the operand when we negate a symbol that is already negated.
-// -(-a) == a
-NegatedRange = getConstraint(State, USE->getOperand());
-  

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 444752.
ASDenysPetrov added a comment.

Fixed a typo that caused `constraint_manager_negate.c` and `unary-sym-expr.c` 
tests crashes.


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

https://reviews.llvm.org/D129678

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1213,13 +1213,21 @@
   }
 
   RangeSet VisitSymExpr(SymbolRef Sym) {
-// If we got to this function, the actual type of the symbolic
+if (Optional RS = getRangeForNegatedSym(Sym))
+  return *RS;
+// If we've reached this line, the actual type of the symbolic
 // expression is not supported for advanced inference.
 // In this case, we simply backoff to the default "let's simply
 // infer the range from the expression's type".
 return infer(Sym->getType());
   }
 
+  RangeSet VisitUnarySymExpr(const UnarySymExpr *USE) {
+if (Optional RS = getRangeForNegatedUnarySym(USE))
+  return *RS;
+return infer(USE->getType());
+  }
+
   RangeSet VisitSymIntExpr(const SymIntExpr *Sym) {
 return VisitBinaryOperator(Sym);
   }
@@ -1228,14 +1236,25 @@
 return VisitBinaryOperator(Sym);
   }
 
-  RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
+  RangeSet VisitSymSymExpr(const SymSymExpr *SSE) {
 return intersect(
 RangeFactory,
+// If Sym is a difference of symbols A - B, then maybe we have range
+// set stored for B - A.
+//
+// If we have range set stored for both A - B and B - A then
+// calculate the effective range set by intersecting the range set
+// for A - B and the negated range set of B - A.
+getRangeForNegatedSymSym(SSE),
+// If Sym is a comparison expression (except <=>),
+// find any other comparisons with the same operands.
+// See function description.
+getRangeForComparisonSymbol(SSE),
 // If Sym is (dis)equality, we might have some information
 // on that in our equality classes data structure.
-getRangeForEqualities(Sym),
+getRangeForEqualities(SSE),
 // And we should always check what we can get from the operands.
-VisitBinaryOperator(Sym));
+VisitBinaryOperator(SSE));
   }
 
 private:
@@ -1264,25 +1283,13 @@
   }
 
   RangeSet infer(SymbolRef Sym) {
-return intersect(
-RangeFactory,
-// Of course, we should take the constraint directly associated with
-// this symbol into consideration.
-getConstraint(State, Sym),
-// If Sym is a difference of symbols A - B, then maybe we have range
-// set stored for B - A.
-//
-// If we have range set stored for both A - B and B - A then
-// calculate the effective range set by intersecting the range set
-// for A - B and the negated range set of B - A.
-getRangeForNegatedSub(Sym),
-// If Sym is a comparison expression (except <=>),
-// find any other comparisons with the same operands.
-// See function description.
-getRangeForComparisonSymbol(Sym),
-// Apart from the Sym itself, we can infer quite a lot if we look
-// into subexpressions of Sym.
-Visit(Sym));
+return intersect(RangeFactory,
+ // Of course, we should take the constraint directly
+ // associated with this symbol into consideration.
+ getConstraint(State, Sym),
+ // Apart from the Sym itself, we can infer quite a lot if
+ // we look into subexpressions of Sym.
+ Visit(Sym));
   }
 
   RangeSet infer(EquivalenceClass Class) {
@@ -1443,38 +1450,53 @@
 return RangeFactory.deletePoint(Domain, IntType.getZeroValue());
   }
 
-  Optional getRangeForNegatedSub(SymbolRef Sym) {
+  template 
+  Optional getRangeForNegatedExpr(ProduceNegatedSymFunc F,
+QualType T) {
 // Do not negate if the type cannot be meaningfully negated.
-if (!Sym->getType()->isUnsignedIntegerOrEnumerationType() &&
-!Sym->getType()->isSignedIntegerOrEnumerationType())
+if (!T->isUnsignedIntegerOrEnumerationType() &&
+!T->isSignedIntegerOrEnumerationType())
   return llvm::None;
 
-const RangeSet *NegatedRange = nullptr;
-SymbolManager  = State->getSymbolManager();
-if (const auto *USE = dyn_cast(Sym)) {
-  if (USE->getOpcode() == UO_Minus) {
-// Just get the operand when we negate a symbol that is already negated.
-// -(-a) == a
-NegatedRange = getConstraint(State, USE->getOperand());
-  }
-} else if (const SymSymExpr *SSE = dyn_cast(Sym)) {
-

[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 444689.
ASDenysPetrov added a comment.

Stick to name `clang_analyzer_value`.
Change function name from `printRange` to `printValue`.
Make description more precise in the documentation.


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

https://reviews.llvm.org/D129498

Files:
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SVals.cpp
  clang/test/Analysis/print-ranges.cpp

Index: clang/test/Analysis/print-ranges.cpp
===
--- /dev/null
+++ clang/test/Analysis/print-ranges.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+// UNSUPPORTED z3
+
+template 
+void clang_analyzer_value(T x);
+void clang_analyzer_value();
+template 
+void clang_analyzer_value(T1 x, T2 y);
+
+void test1(char x) {
+  clang_analyzer_value(x); // expected-warning{{8s:{ [-128, 127] }}}
+  if (x > 42)
+clang_analyzer_value(x); // expected-warning{{8s:{ [43, 127] }}}
+  if (x == 42)
+clang_analyzer_value(x); // expected-warning{{8s:42}}
+}
+
+void test2(short x) {
+  clang_analyzer_value(x); // expected-warning{{16s:{ [-32768, 32767] }}}
+  if (x < 4200)
+clang_analyzer_value(x); // expected-warning{{16s:{ [-32768, 4199] }}}
+  if (x == 4200)
+clang_analyzer_value(x); // expected-warning{{16s:4200}}
+}
+
+void test3(unsigned long long x) {
+  clang_analyzer_value(x); // expected-warning{{64u:{ [0, 18446744073709551615] }}}
+  if (x != 4200)
+clang_analyzer_value(x); // expected-warning{{64u:{ [0, 4199], [4201, 18446744073709551615] }}}
+  if (x == 18446744073709551615ull)
+clang_analyzer_value(x); // expected-warning{{64u:18446744073709551615}}
+}
+
+struct S {};
+void test4(S s) {
+  clang_analyzer_value(s); // expected-warning{{n/a}}
+}
+
+void test5() {
+  clang_analyzer_value(); // expected-warning{{Missing argument}}
+}
+
+void test6(int x, int y) {
+  if (x == 42 && y == 24)
+// Ignore 'y'.
+clang_analyzer_value(x, y); // expected-warning{{32s:42}}
+}
Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -109,6 +109,14 @@
   return getAsLocSymbol(IncludeBaseRegions);
 }
 
+const llvm::APSInt *SVal::getAsInteger() const {
+  if (auto CI = getAs())
+return >getValue();
+  if (auto CI = getAs())
+return >getValue();
+  return nullptr;
+}
+
 const MemRegion *SVal::getAsRegion() const {
   if (Optional X = getAs())
 return X->getRegion();
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1801,6 +1801,8 @@
 
   void printJson(raw_ostream , ProgramStateRef State, const char *NL = "\n",
  unsigned int Space = 0, bool IsDot = false) const override;
+  void printValue(raw_ostream , ProgramStateRef State,
+  SymbolRef Sym) override;
   void printConstraints(raw_ostream , ProgramStateRef State,
 const char *NL = "\n", unsigned int Space = 0,
 bool IsDot = false) const;
@@ -3154,6 +3156,13 @@
   printDisequalities(Out, State, NL, Space, IsDot);
 }
 
+void RangeConstraintManager::printValue(raw_ostream , ProgramStateRef State,
+SymbolRef Sym) {
+  const RangeSet RS = getRange(State, Sym);
+  Out << RS.getBitWidth() << (RS.isUnsigned() ? "u:" : "s:");
+  RS.dump(Out);
+}
+
 static std::string toString(const SymbolRef ) {
   std::string S;
   llvm::raw_string_ostream O(S);
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -40,6 +40,7 @@
   void analyzerNumTimesReached(const CallExpr *CE, CheckerContext ) const;
   void analyzerCrash(const CallExpr *CE, CheckerContext ) const;
   void analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext ) const;
+  void analyzerValue(const CallExpr *CE, CheckerContext ) const;
   void analyzerDumpSValType(const CallExpr *CE, CheckerContext ) const;
   void analyzerDump(const CallExpr *CE, CheckerContext ) const;
   void analyzerExplain(const CallExpr *CE, CheckerContext ) const;
@@ -60,6 +61,7 @@
   Optional ExprVal = None) const;
   

[PATCH] D129678: Tidy up handler-functions in SymbolicRangeInferrer

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: martong, steakhal, NoQ.
ASDenysPetrov added a project: clang.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Sorted some handler-functions into more appropriate visitor functions of the 
SymbolicRangeInferrer.

- Spread `getRangeForNegatedSub` body over several visitor functions: 
`VisitSymExpr`, `VisitSymIntExpr`, `VisitSymSymExpr`.
- Moved `getRangeForComparisonSymbol` from `infer` to `VisitSymSymExpr`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129678

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1213,13 +1213,21 @@
   }
 
   RangeSet VisitSymExpr(SymbolRef Sym) {
-// If we got to this function, the actual type of the symbolic
+if (Optional RS = getRangeForNegatedSym(Sym))
+  return *RS;
+// If we've reached this line, the actual type of the symbolic
 // expression is not supported for advanced inference.
 // In this case, we simply backoff to the default "let's simply
 // infer the range from the expression's type".
 return infer(Sym->getType());
   }
 
+  RangeSet VisitSymIntExpr(const UnarySymExpr *USE) {
+if (Optional RS = getRangeForNegatedUnarySym(USE))
+  return *RS;
+return infer(USE->getType());
+  }
+
   RangeSet VisitSymIntExpr(const SymIntExpr *Sym) {
 return VisitBinaryOperator(Sym);
   }
@@ -1228,14 +1236,25 @@
 return VisitBinaryOperator(Sym);
   }
 
-  RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
+  RangeSet VisitSymSymExpr(const SymSymExpr *SSE) {
 return intersect(
 RangeFactory,
+// If Sym is a difference of symbols A - B, then maybe we have range
+// set stored for B - A.
+//
+// If we have range set stored for both A - B and B - A then
+// calculate the effective range set by intersecting the range set
+// for A - B and the negated range set of B - A.
+getRangeForNegatedSymSym(SSE),
+// If Sym is a comparison expression (except <=>),
+// find any other comparisons with the same operands.
+// See function description.
+getRangeForComparisonSymbol(SSE),
 // If Sym is (dis)equality, we might have some information
 // on that in our equality classes data structure.
-getRangeForEqualities(Sym),
+getRangeForEqualities(SSE),
 // And we should always check what we can get from the operands.
-VisitBinaryOperator(Sym));
+VisitBinaryOperator(SSE));
   }
 
 private:
@@ -1264,25 +1283,13 @@
   }
 
   RangeSet infer(SymbolRef Sym) {
-return intersect(
-RangeFactory,
-// Of course, we should take the constraint directly associated with
-// this symbol into consideration.
-getConstraint(State, Sym),
-// If Sym is a difference of symbols A - B, then maybe we have range
-// set stored for B - A.
-//
-// If we have range set stored for both A - B and B - A then
-// calculate the effective range set by intersecting the range set
-// for A - B and the negated range set of B - A.
-getRangeForNegatedSub(Sym),
-// If Sym is a comparison expression (except <=>),
-// find any other comparisons with the same operands.
-// See function description.
-getRangeForComparisonSymbol(Sym),
-// Apart from the Sym itself, we can infer quite a lot if we look
-// into subexpressions of Sym.
-Visit(Sym));
+return intersect(RangeFactory,
+ // Of course, we should take the constraint directly
+ // associated with this symbol into consideration.
+ getConstraint(State, Sym),
+ // Apart from the Sym itself, we can infer quite a lot if
+ // we look into subexpressions of Sym.
+ Visit(Sym));
   }
 
   RangeSet infer(EquivalenceClass Class) {
@@ -1443,38 +1450,53 @@
 return RangeFactory.deletePoint(Domain, IntType.getZeroValue());
   }
 
-  Optional getRangeForNegatedSub(SymbolRef Sym) {
+  template 
+  Optional getRangeForNegatedExpr(ProduceNegatedSymFunc F,
+QualType T) {
 // Do not negate if the type cannot be meaningfully negated.
-if (!Sym->getType()->isUnsignedIntegerOrEnumerationType() &&
-!Sym->getType()->isSignedIntegerOrEnumerationType())
+if (!T->isUnsignedIntegerOrEnumerationType() &&
+!T->isSignedIntegerOrEnumerationType())
   return llvm::None;
 
-const RangeSet *NegatedRange = 

[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 444336.
ASDenysPetrov added a comment.

Constrained the test by adding llvm-lit `REQUIRES` command.
Documented a new function at 
https://clang.llvm.org/docs/analyzer/developer-docs/DebugChecks.html


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

https://reviews.llvm.org/D129498

Files:
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SVals.cpp
  clang/test/Analysis/print-ranges.cpp

Index: clang/test/Analysis/print-ranges.cpp
===
--- /dev/null
+++ clang/test/Analysis/print-ranges.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+// REQUIRES: no-z3
+
+template 
+void clang_analyzer_value(T x);
+void clang_analyzer_value();
+template 
+void clang_analyzer_value(T1 x, T2 y);
+
+void test1(char x) {
+  clang_analyzer_value(x); // expected-warning{{8s:{ [-128, 127] }}}
+  if (x > 42)
+clang_analyzer_value(x); // expected-warning{{8s:{ [43, 127] }}}
+  if (x == 42)
+clang_analyzer_value(x); // expected-warning{{8s:42}}
+}
+
+void test2(short x) {
+  clang_analyzer_value(x); // expected-warning{{16s:{ [-32768, 32767] }}}
+  if (x < 4200)
+clang_analyzer_value(x); // expected-warning{{16s:{ [-32768, 4199] }}}
+  if (x == 4200)
+clang_analyzer_value(x); // expected-warning{{16s:4200}}
+}
+
+void test3(unsigned long long x) {
+  clang_analyzer_value(x); // expected-warning{{64u:{ [0, 18446744073709551615] }}}
+  if (x != 4200)
+clang_analyzer_value(x); // expected-warning{{64u:{ [0, 4199], [4201, 18446744073709551615] }}}
+  if (x == 18446744073709551615ull)
+clang_analyzer_value(x); // expected-warning{{64u:18446744073709551615}}
+}
+
+struct S {};
+void test4(S s) {
+  clang_analyzer_value(s); // expected-warning{{n/a}}
+}
+
+void test5() {
+  clang_analyzer_value(); // expected-warning{{Missing argument}}
+}
+
+void test6(int x, int y) {
+  if (x == 42 && y == 24)
+// Ignore 'y'.
+clang_analyzer_value(x, y); // expected-warning{{32s:42}}
+}
Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -109,6 +109,14 @@
   return getAsLocSymbol(IncludeBaseRegions);
 }
 
+const llvm::APSInt *SVal::getAsInteger() const {
+  if (auto CI = getAs())
+return >getValue();
+  if (auto CI = getAs())
+return >getValue();
+  return nullptr;
+}
+
 const MemRegion *SVal::getAsRegion() const {
   if (Optional X = getAs())
 return X->getRegion();
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1801,6 +1801,8 @@
 
   void printJson(raw_ostream , ProgramStateRef State, const char *NL = "\n",
  unsigned int Space = 0, bool IsDot = false) const override;
+  void printRange(raw_ostream , ProgramStateRef State,
+  SymbolRef Sym) override;
   void printConstraints(raw_ostream , ProgramStateRef State,
 const char *NL = "\n", unsigned int Space = 0,
 bool IsDot = false) const;
@@ -3154,6 +3156,13 @@
   printDisequalities(Out, State, NL, Space, IsDot);
 }
 
+void RangeConstraintManager::printRange(raw_ostream , ProgramStateRef State,
+SymbolRef Sym) {
+  const RangeSet RS = getRange(State, Sym);
+  Out << RS.getBitWidth() << (RS.isUnsigned() ? "u:" : "s:");
+  RS.dump(Out);
+}
+
 static std::string toString(const SymbolRef ) {
   std::string S;
   llvm::raw_string_ostream O(S);
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -40,6 +40,7 @@
   void analyzerNumTimesReached(const CallExpr *CE, CheckerContext ) const;
   void analyzerCrash(const CallExpr *CE, CheckerContext ) const;
   void analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext ) const;
+  void analyzerValue(const CallExpr *CE, CheckerContext ) const;
   void analyzerDumpSValType(const CallExpr *CE, CheckerContext ) const;
   void analyzerDump(const CallExpr *CE, CheckerContext ) const;
   void analyzerExplain(const CallExpr *CE, CheckerContext ) const;
@@ -60,6 +61,7 @@
   Optional ExprVal = None) 

[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D129498#3644222 , @NoQ wrote:

> Looks great!
>
> Maybe `clang_analyzer_range()` instead?

This was its first name. I refused. First, because it emits concrete integers 
as well and moreover we can extend it for arrays or strings e.g. Second, the 
ranges is just an implementation detail.

> Please add documentation to 
> https://clang.llvm.org/docs/analyzer/developer-docs/DebugChecks.html

I'll do.




Comment at: clang/test/Analysis/print-ranges.cpp:1
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config eagerly-assume=false -verify %s
+

NoQ wrote:
> I suspect this test will crash when clang is built with Z3, because the Z3 
> constraint manager doesn't implement your new function yet.
Agree. Is it enough `REQUIRES: no-z3` or to add `#ifdef ANALYZER_CM_Z3`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129498

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

[PATCH] D104647: [analyzer] Support SVal::getType for pointer-to-member values

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.
Herald added a project: All.

@vsavchenko Is this alive?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104647

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@manas ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: martong, steakhal, NoQ, Charusso.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Introduce a new function `clang_analyzer_value`. It emits a report that in turn 
prints a `RangeSet` or `APSInt` associated with `SVal`. If there is no 
associated value, prints `n/a`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129498

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SVals.cpp
  clang/test/Analysis/print-ranges.cpp

Index: clang/test/Analysis/print-ranges.cpp
===
--- /dev/null
+++ clang/test/Analysis/print-ranges.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+
+template 
+void clang_analyzer_value(T x);
+void clang_analyzer_value();
+template 
+void clang_analyzer_value(T1 x, T2 y);
+
+void test1(char x) {
+  clang_analyzer_value(x); // expected-warning{{8s:{ [-128, 127] }}}
+  if (x > 42)
+clang_analyzer_value(x); // expected-warning{{8s:{ [43, 127] }}}
+  if (x == 42)
+clang_analyzer_value(x); // expected-warning{{8s:42}}
+}
+
+void test2(short x) {
+  clang_analyzer_value(x); // expected-warning{{16s:{ [-32768, 32767] }}}
+  if (x < 4200)
+clang_analyzer_value(x); // expected-warning{{16s:{ [-32768, 4199] }}}
+  if (x == 4200)
+clang_analyzer_value(x); // expected-warning{{16s:4200}}
+}
+
+void test3(unsigned long long x) {
+  clang_analyzer_value(x); // expected-warning{{64u:{ [0, 18446744073709551615] }}}
+  if (x != 4200)
+clang_analyzer_value(x); // expected-warning{{64u:{ [0, 4199], [4201, 18446744073709551615] }}}
+  if (x == 18446744073709551615ull)
+clang_analyzer_value(x); // expected-warning{{64u:18446744073709551615}}
+}
+
+struct S {};
+void test4(S s) {
+  clang_analyzer_value(s); // expected-warning{{n/a}}
+}
+
+void test5() {
+  clang_analyzer_value(); // expected-warning{{Missing argument}}
+}
+
+void test6(int x, int y) {
+  if (x == 42 && y == 24)
+// Ignore 'y'.
+clang_analyzer_value(x, y); // expected-warning{{32s:42}}
+}
Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -109,6 +109,14 @@
   return getAsLocSymbol(IncludeBaseRegions);
 }
 
+const llvm::APSInt *SVal::getAsInteger() const {
+  if (auto CI = getAs())
+return >getValue();
+  if (auto CI = getAs())
+return >getValue();
+  return nullptr;
+}
+
 const MemRegion *SVal::getAsRegion() const {
   if (Optional X = getAs())
 return X->getRegion();
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1801,6 +1801,8 @@
 
   void printJson(raw_ostream , ProgramStateRef State, const char *NL = "\n",
  unsigned int Space = 0, bool IsDot = false) const override;
+  void printRange(raw_ostream , ProgramStateRef State,
+  SymbolRef Sym) override;
   void printConstraints(raw_ostream , ProgramStateRef State,
 const char *NL = "\n", unsigned int Space = 0,
 bool IsDot = false) const;
@@ -3154,6 +3156,13 @@
   printDisequalities(Out, State, NL, Space, IsDot);
 }
 
+void RangeConstraintManager::printRange(raw_ostream , ProgramStateRef State,
+SymbolRef Sym) {
+  const RangeSet RS = getRange(State, Sym);
+  Out << RS.getBitWidth() << (RS.isUnsigned() ? "u:" : "s:");
+  RS.dump(Out);
+}
+
 static std::string toString(const SymbolRef ) {
   std::string S;
   llvm::raw_string_ostream O(S);
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -40,6 +40,7 @@
   void analyzerNumTimesReached(const CallExpr *CE, CheckerContext ) const;
   void analyzerCrash(const CallExpr *CE, CheckerContext ) const;
   void analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext ) const;
+  void analyzerValue(const CallExpr *CE, CheckerContext ) const;
   void analyzerDumpSValType(const 

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov marked 10 inline comments as done.
ASDenysPetrov added a comment.

@martong Thank you for your patience. I started moving to a bit another 
direction that we can improving it iteratively.
Just spoiling, in my latest solution a single symbol will be associated to many 
classes. Here are also tricky cases:

---

Consider equalities

  a32 == (int8)b32
  b32 == c32

Class-Symbol map is:

  class1 { a32 , (int8)b32 }
  class2 { b32 , c32 }

Symbol-Class map is:

  a32 { 32 : class1 }
  b32 {  8 : class1, 32 : class2  }
  c32 { 32 : class2 }

If we know:

  (int8)c32 == -1

then what is:

  (int8)a32 - ?

Should we traverse like `a ->  32 -> class1 -> (int8)b32 -> b32 ->  class2 -> 
c32 -> (int8)c32 ` ?

---

The `x8 == y32` we can treat as a range of `int8` ( {-128, 127} or  {0, 255} ).

---

For `(int8)x32 == (int16)x32` we can eliminate one of the symbols in the class 
a s a redundant one.

---

If `x32 == 0` then we can simplify(merge) such classes `(int16)x32 == y` and 
`(int8)x32 == z` into a single class. I beleive there are more cases.


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

https://reviews.llvm.org/D103096

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

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

[Denys Petrov via Phabricator via cfe-commits]

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

@aganea Thank you for fixing this.
`TestValues` structure impies to hold a set of values which can do some kind of 
convertions including truncations. This is what tests are about. That's true, 
it may happen that some test cases don't need some values. You can carry them 
out to some `TestValues2` structure and use them instead. That also would work. 
So it's up to you. I'm OK with both solutions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129152

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov marked 4 inline comments as done.
ASDenysPetrov added a comment.

@martong Just FYI. I've been working on reworking this solution to using 
`EquivalenceClasses` for several weeks. It turned out that this is an extremely 
hard task to acomplish. There a lot of cast cases like: `(int8)x==y, 
(uint16)a==(int64)b, (uint8)y == b`, Merging and inferring all of this without 
going beyond the complexity O(n) is really tricky.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1289-1291
+  auto It = llvm::find_if(*CM, [MinBitWidth](CastMap::value_type ) {
+return Item.first >= MinBitWidth;
+  });

martong wrote:
> There might be a problem here because the iteration of the map is 
> non-deterministic. We should probably have a copy that is sorted, or the 
> container should be sorted (sorted immutable list maybe?).
> 
> Your tests below passed probably because the cast chains are too small. Could 
> you please have a test, where the chain is really long (20 maybe) and 
> shuffled.
> (My thanks for @steakhal for this additional comment.)
I've checked. `ImmutableSet` gave me a sorted order. But I agree that it could 
be just a coincidence. I'll try to add more tests.


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

https://reviews.llvm.org/D103096

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

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Denys Petrov via Phabricator via cfe-commits]

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

In D126481#3545350 , @martong wrote:

> This part of the SValBuilder is responsible for **constant folding**. We need 
> this constant folding, so the engine can work with less symbols, this way it 
> can be more efficient. Whenever a symbol is constrained with a constant then 
> we substitute the symbol with the corresponding integer. If a symbol is 
> constrained with a range, then the symbol is kept and we fall-back to use the 
> range based constraint manager, which is not that efficient. This patch is 
> the natural extension of the existing constant folding machinery with the 
> support of `SymbolCast` symbols.

Now I see. Thanks. I also wouldn't mind if you add this explanation to the 
summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong As you said, my solution D103096  
suppose to pass these and more other tests cases. So how it will help in 
combination with my solution D103096 ? 
Although, your patch is really simple but it's more like a plug then a real 
`SymbolCast ` support, isn't it? I don't quite understand the motivation.




Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1354
+return I->second;
+  const SymExpr *OpSym = S->getOperand();
+  SVal OpVal = getConstOrVisit(OpSym);

Should this imply to use the root symbol and not the second nested one?
E.g. from `(int)(short)(x)` do you want `(short)(x)` or `(x)`?
`getOperand` gives you `(short)(x)` in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D103096#3502955 , @martong wrote:

> Ping

Thank you, folk, for taking you time. I'll surely make corresponding changes 
according ещ your suggestions and notify you then. Sorry, @martong, for the 
late response. I'm pretty loaded recent times.


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

https://reviews.llvm.org/D103096

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 425248.
ASDenysPetrov marked 4 inline comments as done.
ASDenysPetrov edited the summary of this revision.
ASDenysPetrov added a comment.

@martong thank you for the idea. I've tried to implement it. Could you look at 
the patch once again, please?


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

https://reviews.llvm.org/D103096

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
  clang/test/Analysis/symbol-integral-cast.cpp

Index: clang/test/Analysis/symbol-integral-cast.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-integral-cast.cpp
@@ -0,0 +1,374 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -analyzer-config eagerly-assume=false -analyzer-config support-symbolic-integer-casts=true -verify %s
+
+template 
+void clang_analyzer_eval(T);
+void clang_analyzer_warnIfReached();
+
+typedef short int16_t;
+typedef int int32_t;
+typedef unsigned short uint16_t;
+typedef unsigned int uint32_t;
+
+void test1(int x) {
+  // Even if two lower bytes of `x` equal to zero, it doesn't mean that
+  // the entire `x` is zero. We are not able to know the exact value of x.
+  // It can be one of  65536 possible values like [0, 65536, 131072, ...]
+  // and so on. To avoid huge range sets we still assume `x` in the range
+  // [INT_MIN, INT_MAX].
+  if (!(short)x) {
+if (!x)
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test2(int x) {
+  // If two lower bytes of `x` equal to zero, and we know x to be 65537,
+  // which is not truncated to short as zero. Thus the branch is infisible.
+  short s = x;
+  if (!s) {
+if (x == 65537)
+  clang_analyzer_warnIfReached(); // no-warning
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test3(int x, short s) {
+  s = x;
+  if ((short)x > -10 && s < 10) {
+if (x > 0 && x < 10) {
+  // If the range of the whole variable was constrained then reason again
+  // about truncated bytes to make the ranges more precise.
+  clang_analyzer_eval((short)x <= 0); // expected-warning {{FALSE}}
+}
+  }
+}
+
+void test4(unsigned x) {
+  if ((char)x > 8) {
+// Constraint the range of the lowest byte of `x` to [9, CHAR_MAX].
+// The original range of `x` still remains [0, UINT_MAX].
+clang_analyzer_eval((char)x < 42); // expected-warning {{UNKNOWN}}
+if (x < 42) {
+  // Constraint the original range to [0, 41] and update (re-constraint)
+  // the range of the lowest byte of 'x' to [9, 41].
+  clang_analyzer_eval((char)x < 42); // expected-warning {{TRUE}}
+}
+  }
+}
+
+void test5(unsigned x) {
+  if ((char)x > -10 && (char)x < 10) {
+if ((short)x == 8) {
+  // If the range of higher bytes(short) was constrained then reason again
+  // about smaller truncated ranges(char) to make it more precise.
+  clang_analyzer_eval((char)x == 8);  // expected-warning {{TRUE}}
+  clang_analyzer_eval((short)x == 8); // expected-warning {{TRUE}}
+  // We still assume full version of `x` in the range [INT_MIN, INT_MAX].
+  clang_analyzer_eval(x == 8); // expected-warning {{UNKNOWN}}
+}
+  }
+}
+
+void test6(int x) {
+  // Even if two lower bytes of `x` less than zero, it doesn't mean that `x`
+  // can't be greater than zero. Thence we don't change the native range of
+  // `x` and this branch is feasible.
+  if (x > 0)
+if ((short)x < 0)
+  clang_analyzer_eval(x > 0); // expected-warning {{TRUE}}
+}
+
+void test7(int x) {
+  // The range of two lower bytes of `x` [1, SHORT_MAX] is enough to cover
+  // all possible values of char [CHAR_MIN, CHAR_MAX]. So the lowest byte
+  // can be lower than zero.
+  if ((short)x > 0) {
+if ((char)x < 0)
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test8(int x) {
+  // Promotion from `signed int` to `signed long long` also reasoning about the
+  // original range, because we know the fact that even after promotion it
+  // remains in the range [INT_MIN, INT_MAX].
+  if ((long long)x < 0)
+clang_analyzer_eval(x < 0); // expected-warning {{TRUE}}
+}
+
+void test9(signed int x) {
+  // Any cast `signed` to `unsigned` produces an unsigned range, which is
+  // [0, UNSIGNED_MAX] and can not be lower than zero.
+  if ((unsigned long long)x < 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+
+  if 

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov marked 10 inline comments as done.
ASDenysPetrov added a comment.

Thank you for the review @martong! Your work is not less harder then mine. I'll 
rework and update the revision ASAP.




Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:421-426
+  // Unwrap symbolic expression to skip argument casts on function call.
+  // This is useful when there is no way for overloading function in C
+  // but we need to pass different types of arguments and
+  // implicit cast occures.
+  Sym = Sym->ignoreCasts();
+

martong wrote:
> Does it really matter? I mean, why do we need this change?
I investigated. This changes is not obligatory now. I'll remove it.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2227-2251
+  llvm::SmallSet SimplifiedClasses;
+  // Iterate over all equivalence classes and try to simplify them.
+  ClassMembersTy Members = State->get();
+  for (std::pair ClassToSymbolSet : Members) {
+EquivalenceClass Class = ClassToSymbolSet.first;
+State = EquivalenceClass::simplify(Builder, RangeFactory, State, Class);
+if (!State)

martong wrote:
> I think this hunk should remain in `assignSymExprToConst`. Why did you move 
> it?
I'll remove. It's unrelated one.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2280-2290
+/// Return a symbol which is the best canidate to save it in the constraint
+/// map. We should correct symbol because in case of truncation cast we can
+/// only reason about truncated bytes but not the whole value. E.g. (char)(int
+/// x), we can store constraints for the first lower byte but we still don't
+/// know the root value. Also in case of promotion or converion we should
+/// store the root value instead of cast symbol, because we can always get
+/// a correct range using `castTo` metho. And we are not intrested in any

martong wrote:
> I think, we should definitely store the constraints as they appear in the 
> analyzed code. This way we mix the infer logic into the constraint setting, 
> which is bad.
> I mean, we should simply store the constraint directly for the symbol as it 
> is. And then only in `VisitSymbolCast` should we infer the proper value from 
> the stored constraint (if we can).
> 
> (Of course, if we have related symbols (casts of the original symbol) then 
> their constraints must be updated.)
I see what you mean. I thought about this. Here what I've faced with.
# Let's say you meet `(wchar_t)x > 0`, which you store like a pair {(wchar_t)x, 
[1,32767]}.
# Then you meet `(short)x < 0`, where you have to answer whether it's `true` or 
`false`.
# So would be your next step? Brute forcing all the constraint pairs to find 
any `x`-related symbol? Obviously, O(n) complexity for every single integral 
symbol is inefficient.
What I propose is to "canonize" arbitrary types to a general form where this 
form could be a part of key along with `x` and we could get a constraint with a 
classic map complexity. So that:
# You meet `(wchar_t)x > 0`, which you convert `wchar_t` to `int16` and store 
like a pair {(int16)x, [1,32767]}.
# Then you meet `(short)x < 0`, where you convert `short` to `int16` and get a 
constraint.
That's why I've introduced `NominalTypeList`.
But now I admited your concern about arbitrary size of integers and will 
redesign my solution.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2346-2350
+/// FIXME: Update bigger casts. We only can reason about ranges of smaller
+/// types, because it would be too complicated to update, say, the entire
+/// `int` range if you only have knowledge that its lowest byte has been
+/// changed. So we don't touch bigger casts and they may be potentially
+/// invalid. For future, for:

martong wrote:
> Instead of a noop we should be more conservative in this case. We should 
> invalidate (remove) the constraints of all the symbols that have more bits 
> than the currently set symbol. However, we might be clever in cases of 
> special values (e.g `0` or in case of the `true` rangeSet {[MIN, -1], [1, 
> MAX]}).
No, it's incorrect. Consider next:
```
int x;
if(x > 100 || x < 10) 
  return;
// x (100'000, 1000'000) 
if((int8)x != 42) 
  return;
// x (100'000, 1000'000) && (int8)x (42, 42) 
```
We can't just remove or invalidate `x (100'000, 1000'000)` because this range 
will still stay //true//.
Strictly speaking `x` range should be updated with values 100394, 102442, 
108586, ...,, 960554 and any other value within the range which has its lowest 
byte equals to 42.
We can't just update the `RangeSet` with such a big amount of values due to 
performance issues. So we just assume it as less accurate.


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

https://reviews.llvm.org/D103096

___
cfe-commits mailing 

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 423813.
ASDenysPetrov added a comment.

Loaded with comment updateds according to the remarks.
@martong thank you for your time for the review!


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

https://reviews.llvm.org/D103094

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -40,12 +40,18 @@
   const Range ) {
   return OS << toString(R);
 }
+LLVM_ATTRIBUTE_UNUSED static std::ostream <<(std::ostream ,
+  APSIntType Ty) {
+  return OS << (Ty.isUnsigned() ? "u" : "s") << Ty.getBitWidth();
+}
 
 } // namespace ento
 } // namespace clang
 
 namespace {
 
+template  constexpr bool is_signed_v = std::is_signed::value;
+
 template  struct TestValues {
   static constexpr T MIN = std::numeric_limits::min();
   static constexpr T MAX = std::numeric_limits::max();
@@ -53,7 +59,7 @@
   // which unary minus does not affect on,
   // e.g. int8/int32(0), uint8(128), uint32(2147483648).
   static constexpr T MID =
-  std::is_signed::value ? 0 : ~(static_cast(-1) / static_cast(2));
+  is_signed_v ? 0 : ~(static_cast(-1) / static_cast(2));
   static constexpr T A = MID - (MAX - MID) / 3 * 2;
   static constexpr T B = MID - (MAX - MID) / 3;
   static constexpr T C = -B;
@@ -61,8 +67,40 @@
 
   static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
 "Values shall be in an ascending order");
+  // Clear bits in low bytes by the given amount.
+  template 
+  static constexpr T ClearLowBytes =
+  static_cast(static_cast(Value)
+ << ((Bytes >= CHAR_BIT) ? 0 : Bytes) * CHAR_BIT);
+
+  template 
+  static constexpr T TruncZeroOf = ClearLowBytes;
+
+  // Random number with active bits in every byte. 0x'
+  static constexpr T XAAA = static_cast(
+  0b10101010'10101010'10101010'10101010'10101010'10101010'10101010'10101010);
+  template 
+  static constexpr T XAAATruncZeroOf = TruncZeroOf; // 0x'AB00
+
+  // Random number with active bits in every byte. 0x'
+  static constexpr T X555 = static_cast(
+  0b01010101'01010101'01010101'01010101'01010101'01010101'01010101'01010101);
+  template 
+  static constexpr T X555TruncZeroOf = TruncZeroOf; // 0x'5600
+
+  // Numbers for ranges with the same bits in the lowest byte.
+  // 0x'AA2A
+  static constexpr T FromA = ClearLowBytes + 42;
+  static constexpr T ToA = FromA + 2; // 0x'AA2C
+  // 0x'552A
+  static constexpr T FromB = ClearLowBytes + 42;
+  static constexpr T ToB = FromB + 2; // 0x'552C
 };
 
+template 
+static constexpr APSIntType APSIntTy =
+APSIntType(sizeof(T) * CHAR_BIT, !is_signed_v);
+
 template  class RangeSetTest : public testing::Test {
 public:
   // Init block
@@ -74,21 +112,24 @@
   // End init block
 
   using Self = RangeSetTest;
-  using RawRange = std::pair;
-  using RawRangeSet = std::initializer_list;
-
-  const llvm::APSInt (BaseType X) {
-static llvm::APSInt Base{sizeof(BaseType) * CHAR_BIT,
- std::is_unsigned::value};
-Base = X;
-return BVF.getValue(Base);
+  template  using RawRangeT = std::pair;
+  template 
+  using RawRangeSetT = std::initializer_list>;
+  using RawRange = RawRangeT;
+  using RawRangeSet = RawRangeSetT;
+
+  template  const llvm::APSInt (T X) {
+static llvm::APSInt Int = APSIntTy.getZeroValue();
+Int = X;
+return BVF.getValue(Int);
   }
 
-  Range from(const RawRange ) {
+  template  Range from(const RawRangeT ) {
 return Range(from(Init.first), from(Init.second));
   }
 
-  RangeSet from(const RawRangeSet ) {
+  template 
+  RangeSet from(RawRangeSetT Init, APSIntType Ty = APSIntTy) {
 RangeSet RangeSet = F.getEmptySet();
 for (const auto  : Init) {
   RangeSet = F.add(RangeSet, from(Raw));
@@ -211,9 +252,20 @@
RawRangeSet RawExpected) {
 wrap(::checkDeleteImpl, Point, RawFrom, RawExpected);
   }
-};
 
-} // namespace
+  void checkCastToImpl(RangeSet What, APSIntType Ty, RangeSet Expected) {
+RangeSet Result = F.castTo(What, Ty);
+EXPECT_EQ(Result, Expected)
+<< "while casting " << toString(What) << " to " << Ty;
+  }
+
+  template 
+  void checkCastTo(RawRangeSetT What, RawRangeSetT Expected) {
+static constexpr APSIntType FromTy = APSIntTy;
+static constexpr APSIntType ToTy = APSIntTy;
+this->checkCastToImpl(from(What, FromTy), ToTy, from(Expected, ToTy));
+  }
+};
 
 using IntTypes = 

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

[Denys Petrov 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 rGe37726beb22a: [analyzer] Implemented 
RangeSet::Factory::castTo function to perform promotions… (authored by 
ASDenysPetrov).

Changed prior to commit:
  https://reviews.llvm.org/D103094?vs=423080=423708#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103094

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -40,12 +40,18 @@
   const Range ) {
   return OS << toString(R);
 }
+LLVM_ATTRIBUTE_UNUSED static std::ostream <<(std::ostream ,
+  APSIntType Ty) {
+  return OS << (Ty.isUnsigned() ? "u" : "s") << Ty.getBitWidth();
+}
 
 } // namespace ento
 } // namespace clang
 
 namespace {
 
+template  constexpr bool is_signed_v = std::is_signed::value;
+
 template  struct TestValues {
   static constexpr T MIN = std::numeric_limits::min();
   static constexpr T MAX = std::numeric_limits::max();
@@ -53,7 +59,7 @@
   // which unary minus does not affect on,
   // e.g. int8/int32(0), uint8(128), uint32(2147483648).
   static constexpr T MID =
-  std::is_signed::value ? 0 : ~(static_cast(-1) / static_cast(2));
+  is_signed_v ? 0 : ~(static_cast(-1) / static_cast(2));
   static constexpr T A = MID - (MAX - MID) / 3 * 2;
   static constexpr T B = MID - (MAX - MID) / 3;
   static constexpr T C = -B;
@@ -61,8 +67,40 @@
 
   static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
 "Values shall be in an ascending order");
+  // Clear bits in low bytes by the given amount.
+  template 
+  static constexpr T ClearLowBytes =
+  static_cast(static_cast(Value)
+ << ((Bytes >= CHAR_BIT) ? 0 : Bytes) * CHAR_BIT);
+
+  template 
+  static constexpr T TruncZeroOf = ClearLowBytes;
+
+  // Random number with active bits in every byte. 0x'
+  static constexpr T XAAA = static_cast(
+  0b10101010'10101010'10101010'10101010'10101010'10101010'10101010'10101010);
+  template 
+  static constexpr T XAAATruncZeroOf = TruncZeroOf; // 0x'AB00
+
+  // Random number with active bits in every byte. 0x'
+  static constexpr T X555 = static_cast(
+  0b01010101'01010101'01010101'01010101'01010101'01010101'01010101'01010101);
+  template 
+  static constexpr T X555TruncZeroOf = TruncZeroOf; // 0x'5600
+
+  // Numbers for ranges with the same bits in the lowest byte.
+  // 0x'AA2A
+  static constexpr T FromA = ClearLowBytes + 42;
+  static constexpr T ToA = FromA + 2; // 0x'AA2C
+  // 0x'552A
+  static constexpr T FromB = ClearLowBytes + 42;
+  static constexpr T ToB = FromB + 2; // 0x'552C
 };
 
+template 
+static constexpr APSIntType APSIntTy =
+APSIntType(sizeof(T) * CHAR_BIT, !is_signed_v);
+
 template  class RangeSetTest : public testing::Test {
 public:
   // Init block
@@ -74,21 +112,24 @@
   // End init block
 
   using Self = RangeSetTest;
-  using RawRange = std::pair;
-  using RawRangeSet = std::initializer_list;
-
-  const llvm::APSInt (BaseType X) {
-static llvm::APSInt Base{sizeof(BaseType) * CHAR_BIT,
- std::is_unsigned::value};
-Base = X;
-return BVF.getValue(Base);
+  template  using RawRangeT = std::pair;
+  template 
+  using RawRangeSetT = std::initializer_list>;
+  using RawRange = RawRangeT;
+  using RawRangeSet = RawRangeSetT;
+
+  template  const llvm::APSInt (T X) {
+static llvm::APSInt Int = APSIntTy.getZeroValue();
+Int = X;
+return BVF.getValue(Int);
   }
 
-  Range from(const RawRange ) {
+  template  Range from(const RawRangeT ) {
 return Range(from(Init.first), from(Init.second));
   }
 
-  RangeSet from(const RawRangeSet ) {
+  template 
+  RangeSet from(RawRangeSetT Init, APSIntType Ty = APSIntTy) {
 RangeSet RangeSet = F.getEmptySet();
 for (const auto  : Init) {
   RangeSet = F.add(RangeSet, from(Raw));
@@ -211,9 +252,20 @@
RawRangeSet RawExpected) {
 wrap(::checkDeleteImpl, Point, RawFrom, RawExpected);
   }
-};
 
-} // namespace
+  void checkCastToImpl(RangeSet What, APSIntType Ty, RangeSet Expected) {
+RangeSet Result = F.castTo(What, Ty);
+EXPECT_EQ(Result, Expected)
+<< "while casting " << toString(What) << " to " << Ty;
+  }
+
+  template 
+  void checkCastTo(RawRangeSetT What, RawRangeSetT Expected) {
+

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 423080.
ASDenysPetrov added a comment.

Updated according to suggestions.
@martong thank you for the review.


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

https://reviews.llvm.org/D103094

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -40,12 +40,18 @@
   const Range ) {
   return OS << toString(R);
 }
+LLVM_ATTRIBUTE_UNUSED static std::ostream <<(std::ostream ,
+  APSIntType Ty) {
+  return OS << (Ty.isUnsigned() ? "u" : "s") << Ty.getBitWidth();
+}
 
 } // namespace ento
 } // namespace clang
 
 namespace {
 
+template  constexpr bool is_signed_v = std::is_signed::value;
+
 template  struct TestValues {
   static constexpr T MIN = std::numeric_limits::min();
   static constexpr T MAX = std::numeric_limits::max();
@@ -53,7 +59,7 @@
   // which unary minus does not affect on,
   // e.g. int8/int32(0), uint8(128), uint32(2147483648).
   static constexpr T MID =
-  std::is_signed::value ? 0 : ~(static_cast(-1) / static_cast(2));
+  is_signed_v ? 0 : ~(static_cast(-1) / static_cast(2));
   static constexpr T A = MID - (MAX - MID) / 3 * 2;
   static constexpr T B = MID - (MAX - MID) / 3;
   static constexpr T C = -B;
@@ -61,8 +67,40 @@
 
   static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
 "Values shall be in an ascending order");
+  // Clear bits in low bytes by the given amount.
+  template 
+  static constexpr T ClearLowBytes =
+  static_cast(static_cast(Value)
+ << ((Bytes >= CHAR_BIT) ? 0 : Bytes) * CHAR_BIT);
+
+  template 
+  static constexpr T TruncZeroOf = ClearLowBytes;
+
+  // Random number with active bits in every byte. 0x'
+  static constexpr T XAAA = static_cast(
+  0b10101010'10101010'10101010'10101010'10101010'10101010'10101010'10101010);
+  template 
+  static constexpr T XAAATruncZeroOf = TruncZeroOf; // 0x'AB00
+
+  // Random number with active bits in every byte. 0x'
+  static constexpr T X555 = static_cast(
+  0b01010101'01010101'01010101'01010101'01010101'01010101'01010101'01010101);
+  template 
+  static constexpr T X555TruncZeroOf = TruncZeroOf; // 0x'5600
+
+  // Numbers for ranges with the same bits in the lowest byte.
+  // 0x'AA2A
+  static constexpr T FromA = ClearLowBytes + 42;
+  static constexpr T ToA = FromA + 2; // 0x'AA2C
+  // 0x'552A
+  static constexpr T FromB = ClearLowBytes + 42;
+  static constexpr T ToB = FromB + 2; // 0x'552C
 };
 
+template 
+static constexpr APSIntType APSIntTy =
+APSIntType(sizeof(T) * CHAR_BIT, !is_signed_v);
+
 template  class RangeSetTest : public testing::Test {
 public:
   // Init block
@@ -74,21 +112,24 @@
   // End init block
 
   using Self = RangeSetTest;
-  using RawRange = std::pair;
-  using RawRangeSet = std::initializer_list;
-
-  const llvm::APSInt (BaseType X) {
-static llvm::APSInt Base{sizeof(BaseType) * CHAR_BIT,
- std::is_unsigned::value};
-Base = X;
-return BVF.getValue(Base);
+  template  using RawRangeT = std::pair;
+  template 
+  using RawRangeSetT = std::initializer_list>;
+  using RawRange = RawRangeT;
+  using RawRangeSet = RawRangeSetT;
+
+  template  const llvm::APSInt (T X) {
+static llvm::APSInt Int = APSIntTy.getZeroValue();
+Int = X;
+return BVF.getValue(Int);
   }
 
-  Range from(const RawRange ) {
+  template  Range from(const RawRangeT ) {
 return Range(from(Init.first), from(Init.second));
   }
 
-  RangeSet from(const RawRangeSet ) {
+  template 
+  RangeSet from(RawRangeSetT Init, APSIntType Ty = APSIntTy) {
 RangeSet RangeSet = F.getEmptySet();
 for (const auto  : Init) {
   RangeSet = F.add(RangeSet, from(Raw));
@@ -211,9 +252,20 @@
RawRangeSet RawExpected) {
 wrap(::checkDeleteImpl, Point, RawFrom, RawExpected);
   }
-};
 
-} // namespace
+  void checkCastToImpl(RangeSet What, APSIntType Ty, RangeSet Expected) {
+RangeSet Result = F.castTo(What, Ty);
+EXPECT_EQ(Result, Expected)
+<< "while casting " << toString(What) << " to " << Ty;
+  }
+
+  template 
+  void checkCastTo(RawRangeSetT What, RawRangeSetT Expected) {
+static constexpr APSIntType FromTy = APSIntTy;
+static constexpr APSIntType ToTy = APSIntTy;
+this->checkCastToImpl(from(What, FromTy), ToTy, from(Expected, ToTy));
+  }
+};
 
 using IntTypes = ::testing::Types;
@@ -594,3 +646,425 @@
  

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:358
+  assert(!isEmpty());
+  return begin()->From().isUnsigned();
+}

martong wrote:
> Probably it is unrelated to this patch, but
> Could it happen that `(++begin())->From().isUnsigned()` gives a different 
> signedness? Or we had a separate assertion when we  added the second `Range`? 
> The same question applies to the below two functions as well. Seems like in 
> `unite` we don't have such validity check...
>Or we had a separate assertion when we added the second Range?
I didn't find any assertion while adding.
Moreover, I'm not sure if there can happen `APSInts` of different types in a 
single `Range`. We only have `assert(From < To)` in a ctor.
>Seems like in `unite` we don't have such validity check...
Not only `unite` doesn't have such, but all the rest don't as well. We rely on 
a fact that the caller garantees the validity of RangeSet by using 
`AdjustmentType`.
But, yes, it's worth to make some checks.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:687
+
+  if (IsConversion && (!IsPromotion || !What.isUnsigned()))
+return makePersistent(convertTo(What, Ty));

martong wrote:
> Could you please explain why do we need the `|| !What.isUnsigned()` part of 
> the condition?
> 
> This would make much more sense to me:
> ```
> if (IsConversion && !IsPromotion)
> return makePersistent(convertTo(What, Ty));
> ```
Look. Here we handle 2 cases:

  - `IsConversion && !IsPromotion`. In this case we handle changing a sign with 
same bitwidth: char -> uchar, uint -> int. Here we convert //negatives //to 
//positives //and //positives which is out of range// to //negatives//. We use 
`convertTo` function for that.
 
  - `IsConversion && IsPromotion && !What.isUnsigned()`. In this case we handle 
changing a sign from signeds to unsigneds with higher bitwidth: char -> uint, 
int-> uint64. The point is that we also need convert //negatives //to 
//positives //and use `convertTo` function as well. For example, we don't need 
such a convertion when converting //unsigned //to //signed with higher 
bitwidth//, because all the values of //unsigned //is valid for the such 
//signed//.
 






Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:690
+
+  // Promotion from unsigneds to signeds/unsigneds left.
+

martong wrote:
> martong wrote:
> > I think it would be more consistent with the other operations (truncate and 
> > convert) if we had this logic in its own separate function: `promoteTo`. 
> > And this function should be called only if `isPromotion` is set (?)
> This comment is confusing, since we can get here also if
> `isConversion` is false and
> `isPromotion` is false 
Nothing confusing is here.
We have 7 main cases:
NOOP: **u -> u, s -> s**
Conversion: **u -> s, s -> u**
Truncation: **U-> u, S -> s**
Promotion: `u->U`, `s -> S`
Truncation + Conversion: **U -> s, S -> u**
Promotion + Conversion: **s -> U**, `u -> S`

As you can see, we've handled all the **bolds** above this line .
So only promotions from //unsigneds// left. That means, `isPromotion` never 
should be `false` here. We could add an assertion here if you want.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:690-711
+  // Promotion from unsigneds to signeds/unsigneds left.
+
+  using llvm::APInt;
+  using llvm::APSInt;
+  ContainerType Result;
+  // We definitely know the size of the result set.
+  Result.reserve(What.size());

ASDenysPetrov wrote:
> martong wrote:
> > martong wrote:
> > > I think it would be more consistent with the other operations (truncate 
> > > and convert) if we had this logic in its own separate function: 
> > > `promoteTo`. And this function should be called only if `isPromotion` is 
> > > set (?)
> > This comment is confusing, since we can get here also if
> > `isConversion` is false and
> > `isPromotion` is false 
> Nothing confusing is here.
> We have 7 main cases:
> NOOP: **u -> u, s -> s**
> Conversion: **u -> s, s -> u**
> Truncation: **U-> u, S -> s**
> Promotion: `u->U`, `s -> S`
> Truncation + Conversion: **U -> s, S -> u**
> Promotion + Conversion: **s -> U**, `u -> S`
> 
> As you can see, we've handled all the **bolds** above this line .
> So only promotions from //unsigneds// left. That means, `isPromotion` never 
> should be `false` here. We could add an assertion here if you want.
That makes sense. I'll do.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:792
+  //unite -> uchar(-2, 1)
+  auto CastRange = [Ty,  = ValueFactory](const Range ) -> Bounds {
+// Get bounds of the given range.

martong wrote:
> Why not `-> Range`?
Because `Range` requires `from < to` in its contract. But I need to store the 
values after conversions. You can 

[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov accepted this revision.
ASDenysPetrov added a comment.

Thanks! LGTM.




Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:87-90
+  std::string CheckerFullName = StringRef(getCheckerFullName(, "-")).lower();
+  return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) +
+  CheckerFullName)
   .str();

IMO it's a bit messy here.


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

https://reviews.llvm.org/D121387

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

[PATCH] D122243: [analyzer][NFC] Introduce the checker package separator character

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov accepted this revision.
ASDenysPetrov added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122243

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

[PATCH] D107636: [analyzer][solver] Compute adjustment for unsupported symbols as well

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.
Herald added a project: All.

I checked the tests file on the latest sources. It passes even without your 
changes. Maybe this patch is already outdated.




Comment at: clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp:82-86
   if (IsExpectedEqual) {
 return assumeSymNE(State, CanonicalEquality, Zero, Zero);
   }
 
   return assumeSymEQ(State, CanonicalEquality, Zero, Zero);

Do we need the same changes here as below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107636

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Here are my remarks.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1415-1416
+
+  Range CoarseLHS = fillGaps(LHS);
+  Range CoarseRHS = fillGaps(RHS);
+

Avoid filling the gaps, because it's completely possible to compare all the 
ranges.
E.g. //LHS //`[1,2]U[8,9]`  and  //RHS //`[5,6]` are not equal.
And you don't need to fiil the gap in LHS, because it will lead you to a wrong 
answer, like `[1,9] != [5,6]` => **UNKNOWN** instead of **TRUE**.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1420-1421
+
+  auto ConvertedCoarseLHS = convert(CoarseLHS, ResultType);
+  auto ConvertedCoarseRHS = convert(CoarseRHS, ResultType);
+

The `ResultType` of `BO_NE` is `bool`. You don't need to convert your **integer 
**ranges to **boolean**. Otherwise, you'll lose the information to compare. 



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1430-1433
+  if ((ConvertedCoarseLHS->To() < ConvertedCoarseRHS->From()) ||
+  (ConvertedCoarseLHS->From() > ConvertedCoarseRHS->To())) {
+return getTrueRange(T);
+  }

You can simply check an intersection (`RangeSet::Factory::intersect`) between 
the ranges.
If the result range is //empty// then return TRUE.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1437-1441
+  if (ConvertedCoarseLHS->getConcreteValue() &&
+  ConvertedCoarseLHS->getConcreteValue() ==
+  ConvertedCoarseRHS->getConcreteValue()) {
+return getFalseRange(T);
+  }

This is OK but check `ConvertedCoarseRHS->getConcreteValue()` for `nullptr` 
before getting the value.




Comment at: clang/test/Analysis/constant-folding.c:339
+  }
+}

I'd like to see the cases with concrete integers as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.
Herald added a project: All.

Ping. If there is somebody interested in this? :)


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

https://reviews.llvm.org/D103096

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

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.
Herald added a project: All.

Ping.


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

https://reviews.llvm.org/D103094

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

[PATCH] D108032: [analyzer] Retrieve a character from CompoundLiteralExpr as an initializer for constant arrays.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov abandoned this revision.
ASDenysPetrov added a comment.

Paused for a while.


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

https://reviews.llvm.org/D108032

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

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1088
 }
+
+SVal clang::ento::SValBuilder::simplifySymbolCast(nonloc::SymbolVal V,

ASDenysPetrov wrote:
> Notify the user about the contract.
Mistake. `CastTy`, not `SafeTy`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117229

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

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1088
 }
+
+SVal clang::ento::SValBuilder::simplifySymbolCast(nonloc::SymbolVal V,

Notify the user about the contract.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1095-1097
+auto SV = makeSymbolVal(SE);
+assert(SV == V);
+return SV;

Explain please, why do you need to create a new `SymbolVal` here, but not just 
returning `V` as long as you know that the type is the same?
And `assert` also seems weird for me. If there's smth obscure that you're 
trying to handle, please add some explaination comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117229

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

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1022
 
+SVal clang::ento::SValBuilder::simplifySymbolCast(SymbolRef SE,
+  QualType CastTy) {

And it'd be great if you bring the original comment here, as the logic is here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117229

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

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Thank you @martong.
I'll load it ASAP. It's great to see symcasts is closer.

Shame on me :) I'm mixed up in my own patches. I forgot that I separated them 
consciously for this purpose.


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

https://reviews.llvm.org/D105340

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

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D105340#3232671 , @NoQ wrote:

> This looks great with the option flag. Landing this patch will enable more 
> people to test the new mode and produce feedback on whether the constraint 
> solver keeps working well enough in presence of the new symbols.

Many thanks for your approval, @NoQ! The upset thing is that to get this loaded 
we also should close this parent revision D103094 
 :-(


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

https://reviews.llvm.org/D105340

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

[PATCH] D117035: [analyzer] Added more tests for scalars, enums and records for StrictAliasingChecker

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: steakhal, xazax.hun, martong, NoQ.
Herald added subscribers: manas, jeroen.dobbelaere, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
ASDenysPetrov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Added tests to check aliasing:

- scalars through records;
- enums through records;
- records through scalars;
- records through enums;
- records through records;
- through multiple levels of aliasing
- in different syntax contexts.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117035

Files:
  clang/test/Analysis/Checkers/StrictAliasingChecker/enum-ptr-to-record.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/enum-var-to-record.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/multi-casts.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/record-ptr-to-enum.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/record-ptr-to-record.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/record-ptr-to-scalar.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/record-var-to-enum.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/record-var-to-record.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/record-var-to-scalar.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/records.h
  clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-ptr-to-record.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-var-to-record.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/various-indirection.cpp

Index: clang/test/Analysis/Checkers/StrictAliasingChecker/various-indirection.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/various-indirection.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -std=c++20 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+// NOTE: -relaxed-aliasing flag disables StrictAliasing checker.
+
+#include "enums.h"
+#include "records.h"
+
+#define CAST(var, type) (reinterpret_cast(var))
+#define DEREF(var, type) *CAST(var, type)
+#define SUBSCRIPT(var, type) \
+  CAST(var, type)\
+  [0]
+#define DEREF_ADD(var, type) *(CAST(var, type) + 0)
+
+#define CHECK_DEREF(var, type)   \
+  { auto y = DEREF(var, type); } \
+  { DEREF(var, type) = type{}; }
+
+#define CHECK_SUBSCRIPT(var, type)   \
+  { auto y = SUBSCRIPT(var, type); } \
+  { SUBSCRIPT(var, type) = type{}; }
+
+#define CHECK_DEREF_ADD(var, type)   \
+  { auto y = DEREF_ADD(var, type); } \
+  { DEREF_ADD(var, type) = type{}; }
+
+#define CHECK_MEMBER_ARROW(var, type) \
+  { auto y = CAST(var, type)->x; }\
+  { CAST(var, type)->x = decltype(CAST(var, type)->x){}; }
+
+#define CHECK_MEMBER_DOT(var, type)  \
+  { auto y = (*CAST(var, type)).x; } \
+  { (*CAST(var, type)).x = decltype((*CAST(var, type)).x){}; }
+
+#define CHECK_SCALAR_NO_WARN(type) \
+  {\
+type x{};  \
+CHECK_DEREF(, type)  \
+CHECK_SUBSCRIPT(, type)  \
+CHECK_DEREF_ADD(, type)  \
+  }
+
+#define CHECK_RECORD_NO_WARN(type) \
+  {\
+CHECK_SCALAR_NO_WARN(type) \
+type x{};  \
+CHECK_MEMBER_ARROW(, type)   \
+CHECK_MEMBER_DOT(, type) \
+  }
+
+#define CHECK_NOOP_NO_WARN(type1, type2) \
+  {  \
+type1 x{};   \
+DEREF(, type2);\
+SUBSCRIPT(, type2);\
+DEREF_ADD(, type2);\
+  }
+
+void test_store_load() {
+  CHECK_SCALAR_NO_WARN(bool)  // no-warning
+  CHECK_SCALAR_NO_WARN(char)  // no-warning
+  CHECK_SCALAR_NO_WARN(int)   // no-warning
+  CHECK_SCALAR_NO_WARN(double)// no-warning
+  CHECK_SCALAR_NO_WARN(ClassEnum) // no-warning
+  CHECK_SCALAR_NO_WARN(Enum)  // no-warning
+
+  CHECK_RECORD_NO_WARN(StructInt)   // no-warning
+  CHECK_RECORD_NO_WARN(Base)// no-warning
+  CHECK_RECORD_NO_WARN(MostDerived) // no-warning
+  CHECK_RECORD_NO_WARN(UnionUchar)  // no-warning
+}
+
+// TODO: All should not warn.
+void test_noop() {
+  CHECK_NOOP_NO_WARN(bool, MostDerived)  // expected-warning 3{{Undefined behavior}}
+  CHECK_NOOP_NO_WARN(char, int)  // expected-warning 3{{Undefined behavior}}
+  CHECK_NOOP_NO_WARN(double, ClassEnum)  // expected-warning 3{{Undefined behavior}}
+  CHECK_NOOP_NO_WARN(ClassEnum, StructInt)   // expected-warning 3{{Undefined behavior}}
+  CHECK_NOOP_NO_WARN(Enum, UnionUchar)   // expected-warning 3{{Undefined behavior}}
+  CHECK_NOOP_NO_WARN(StructInt, std::byte)   // no-warning
+  CHECK_NOOP_NO_WARN(AnotherBase, float) // expected-warning 3{{Undefined behavior}}
+  

[PATCH] D117033: [analyzer] Added more tests for scalars and enums for StrictAliasingChecker

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: NoQ, martong, steakhal, xazax.hun.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, jeroen.dobbelaere, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Added tests to check aliasing:

- scalars through enums;
- enums through enums;
- enums through scalars.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117033

Files:
  clang/test/Analysis/Checkers/StrictAliasingChecker/enam-ptr-to-scalar.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/enam-var-to-scalar.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/enum-ptr-to-enum.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/enum-var-to-enum.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/enums.h
  clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-ptr-to-enum.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-var-to-enum.cpp

Index: clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-var-to-enum.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-var-to-enum.cpp
@@ -0,0 +1,179 @@
+// RUN: %clang_cc1 -std=c++20 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+// NOTE: -relaxed-aliasing flag disables StrictAliasing checker.
+
+#include "enums.h"
+
+#define CHECK(var, type) \
+  { auto y = *(type *) }
+
+#define CHECKS \
+  CHECK(x, std::OtherByte) \
+  CHECK(x, ClassEnum)  \
+  CHECK(x, ClassEnumChar)  \
+  CHECK(x, ClassEnumUChar) \
+  CHECK(x, ClassEnumInt)   \
+  CHECK(x, Enum)   \
+  CHECK(x, EnumChar)   \
+  CHECK(x, EnumUChar)  \
+  CHECK(x, EnumInt)\
+  CHECK(x, decltype(E_Dummy))
+
+void bool_test() {
+  bool x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void char_test() {
+  char x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void short_test() {
+  short x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void int_test() {
+  int x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void long_test() {
+  long x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void long_long_test() {
+  long long x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void schar_test() {
+  signed char x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void uchar_test() {
+  unsigned char x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void ushort_test() {
+  unsigned short x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void uint_test() {
+  unsigned int x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void ulong_test() {
+  unsigned long x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void ulong_long_test() {
+  unsigned long long x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void float_test() {
+  float x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void double_test() {
+  double x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void long_double_test() {
+  long double x = 42;
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void bool_test(bool x) {
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void char_test(char x) {
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void short_test(short x) {
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void int_test(int x) {
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void long_test(long x) {
+  CHECK(x, std::byte) // no-warning
+  CHECKS  // expected-warning 10{{Undefined behavior}}
+}
+
+void long_long_test(long long x) {
+  CHECK(x, 

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 398986.
ASDenysPetrov added a comment.

Improved the checker. Reworked tests.


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

https://reviews.llvm.org/D114718

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/cv.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-ptr-to-scalar.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-var-to-scalar.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/typedef.cpp

Index: clang/test/Analysis/Checkers/StrictAliasingChecker/typedef.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/typedef.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++20 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+// NOTE: -relaxed-aliasing flag disables StrictAliasing checker.
+
+typedef int MyInt;
+typedef MyInt MyAnotherInt;
+typedef char MyChar;
+typedef short MyShort;
+typedef MyChar *MyCharPtr;
+typedef MyShort *MyShortPtr;
+typedef MyInt *MyIntPtr;
+
+MyAnotherInt x = 42;
+
+void typedef_test() {
+  MyChar y1 = *(MyCharPtr)   // no-warning
+  MyShort y2 = *(MyShortPtr) // expected-warning{{Undefined behavior}}
+  MyInt y3 = *(MyIntPtr) // no-warning
+}
Index: clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-var-to-scalar.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/scalar-var-to-scalar.cpp
@@ -0,0 +1,650 @@
+// RUN: %clang_cc1 -std=c++20 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+// NOTE: -relaxed-aliasing flag disables StrictAliasing checker.
+
+#define CHECK(var, type) \
+  { auto y = *(type *) }
+
+void bool_test() {
+  bool x = true;
+  CHECK(x, bool) // no-warning
+
+  CHECK(x, char)  // no-warning
+  CHECK(x, short) // expected-warning {{Undefined behavior}}
+  CHECK(x, int)   // expected-warning {{Undefined behavior}}
+  CHECK(x, long)  // expected-warning {{Undefined behavior}}
+  CHECK(x, long long) // expected-warning {{Undefined behavior}}
+
+  CHECK(x, signed char)// expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned char)  // no-warning
+  CHECK(x, unsigned short) // expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned int)   // expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned long)  // expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned long long) // expected-warning {{Undefined behavior}}
+
+  CHECK(x, float)   // expected-warning {{Undefined behavior}}
+  CHECK(x, double)  // expected-warning {{Undefined behavior}}
+  CHECK(x, long double) // expected-warning {{Undefined behavior}}
+}
+
+void char_test() {
+  char x = 42;
+  CHECK(x, bool) // expected-warning {{Undefined behavior}}
+
+  CHECK(x, char)  // no-warning
+  CHECK(x, short) // expected-warning {{Undefined behavior}}
+  CHECK(x, int)   // expected-warning {{Undefined behavior}}
+  CHECK(x, long)  // expected-warning {{Undefined behavior}}
+  CHECK(x, long long) // expected-warning {{Undefined behavior}}
+
+  CHECK(x, signed char)// expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned char)  // no-warning
+  CHECK(x, unsigned short) // expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned int)   // expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned long)  // expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned long long) // expected-warning {{Undefined behavior}}
+
+  CHECK(x, float)   // expected-warning {{Undefined behavior}}
+  CHECK(x, double)  // expected-warning {{Undefined behavior}}
+  CHECK(x, long double) // expected-warning {{Undefined behavior}}
+}
+
+void short_test() {
+  short x = 42;
+  CHECK(x, bool) // expected-warning {{Undefined behavior}}
+
+  CHECK(x, char)  // no-warning
+  CHECK(x, short) // no-warning
+  CHECK(x, int)   // expected-warning {{Undefined behavior}}
+  CHECK(x, long)  // expected-warning {{Undefined behavior}}
+  CHECK(x, long long) // expected-warning {{Undefined behavior}}
+
+  CHECK(x, signed char)// expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned char)  // no-warning
+  CHECK(x, unsigned short) // no-warning
+  CHECK(x, unsigned int)   // expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned long)  // expected-warning {{Undefined behavior}}
+  CHECK(x, unsigned long long) // expected-warning {{Undefined behavior}}
+
+  CHECK(x, float)   // expected-warning {{Undefined behavior}}
+  CHECK(x, double)  // expected-warning 

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:420
+  auto FromTy = symRHS->getType();
+  auto ToTy = RLocAsInt->getType(this->Context);
+  symRHS = this->getSymbolManager().getCastSymbol(symRHS, FromTy, ToTy);

Please take into account that `SVal::getType` may return NULL `QualType`. See 
function's description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115932

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

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:415-423
+  // FIXME This should be done in getAsSymbol. But then getAsSymbol should be
+  // the member function of SValBuilder (?)
+  if (symRHS)
+if (auto RLocAsInt = RHS.getAs()) {
+  auto FromTy = symRHS->getType();
+  auto ToTy = RLocAsInt->getType(this->Context);
+  symRHS = this->getSymbolManager().getCastSymbol(symRHS, FromTy, ToTy);

IMO this is not the right place for producing `SymbolCast` because we can meet 
`(type)x`-like semantics in a different contexts and expressions.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:419-420
+if (auto RLocAsInt = RHS.getAs()) {
+  auto FromTy = symRHS->getType();
+  auto ToTy = RLocAsInt->getType(this->Context);
+  symRHS = this->getSymbolManager().getCastSymbol(symRHS, FromTy, ToTy);

According to https://llvm.org/docs/CodingStandards.html#id28



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:471-478
+  if (Optional RV = rhs.getAs()) {
+// Support pointer arithmetic where the addend is on the left
+// and the pointer on the right.
 
-if (IsCommutative(op)) {
-  // Swap operands.
-  return evalBinOpLN(state, op, *RV, lhs.castAs(), type);
-}
+assert(op == BO_Add);
 
+// Commute the operands.

This can be carried out to the separate revert revision as it is not related to 
this one directly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115932

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

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Starting producing `SymbolCast` you should be careful. Please pay attention on 
my revision D105340  before doing this. I 
don't want you walk through the thing I fought.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115932

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

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Just a ping. I'd like to have this patch stack loaded somewhen :)


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

https://reviews.llvm.org/D105340

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal Thanks! I appreciate your comprehensive comment! I'll take everything 
you suggested into account.




Comment at: 
clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp:2
+// RUN: %clang_cc1 -std=c++20 -analyze -analyzer-config eagerly-assume=false 
-analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+// NOTE: -relaxed-aliasing flag disables StrictAliasing checker.
+

steakhal wrote:
> I think `fno-strict-aliasing` should achieve this, by which the driver should 
> transform that flag into the `-relaxed-aliasing` flag consumed by the backend.
Yes, but here in the RUN line you should use a backend flag `-relaxed-aliasing` 
instead of `-fno-strict-aliasing`. `-fno-strict-aliasing` won't work here. I've 
tried.
P.S. Note that the checker works relying on the presentence of 
`-relaxed-aliasing`, so here the absense of `-relaxed-aliasing` is en 
equivalent of `-fstrict-aliasing`.


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

https://reviews.llvm.org/D114718

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

[PATCH] D115883: [analyzer][NFC] Change return value of StoreManager::attemptDownCast function from SVal to Optional

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov closed this revision.
ASDenysPetrov added a comment.

Closed with rGda8bd972a33ad642d6237b5b95b31a2a20f46a35 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115883

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

[PATCH] D115883: [analyzer][NFC] Change return value of StoreManager::attemptDownCast function from SVal to Optional

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@xazax.hun 
Many thanks for the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115883

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

[PATCH] D115883: [analyzer][NFC] Change return value of StoreManager::attemptDownCast function from SVal to Optional

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: xazax.hun, steakhal, NoQ, martong, 
baloghadamsoftware.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Refactor return value of `StoreManager::attemptDownCast` function by removing 
the last parameter `bool ` and replace the return value `SVal` with 
`Optional`.  Make the function consistent with the family of 
`evalDerivedToBase` by renaming it to `evalBaseToDerived`. Aligned the code on 
the call side with these changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115883

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp

Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -314,10 +314,7 @@
   return nullptr;
 }
 
-SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
-   bool ) {
-  Failed = false;
-
+Optional StoreManager::evalBaseToDerived(SVal Base, QualType TargetType) {
   const MemRegion *MR = Base.getAsRegion();
   if (!MR)
 return UnknownVal();
@@ -392,7 +389,9 @@
   }
 
   // We failed if the region we ended up with has perfect type info.
-  Failed = isa(MR);
+  if (isa(MR))
+return None;
+
   return UnknownVal();
 }
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -439,14 +439,15 @@
 if (CastE->isGLValue())
   resultType = getContext().getPointerType(resultType);
 
-bool Failed = false;
-
-// Check if the value being cast evaluates to 0.
-if (val.isZeroConstant())
-  Failed = true;
-// Else, evaluate the cast.
-else
-  val = getStoreManager().attemptDownCast(val, T, Failed);
+bool Failed = true;
+
+// Check if the value being cast does not evaluates to 0.
+if (!val.isZeroConstant())
+  if (Optional V =
+  StateMgr.getStoreManager().evalBaseToDerived(val, T)) {
+val = *V;
+Failed = false;
+  }
 
 if (Failed) {
   if (T->isReferenceType()) {
@@ -478,14 +479,13 @@
 if (CastE->isGLValue())
   resultType = getContext().getPointerType(resultType);
 
-bool Failed = false;
-
 if (!val.isConstant()) {
-  val = getStoreManager().attemptDownCast(val, T, Failed);
+  Optional V = getStoreManager().evalBaseToDerived(val, T);
+  val = V ? *V : UnknownVal();
 }
 
 // Failed to cast or the result is unknown, fall back to conservative.
-if (Failed || val.isUnknown()) {
+if (val.isUnknown()) {
   val =
 svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
  currBldrCtx->blockCount());
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -762,9 +762,9 @@
   QualType Ty = Ctx.getPointerType(Ctx.getRecordType(Class));
 
   // FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
-  bool Failed;
-  ThisVal = StateMgr.getStoreManager().attemptDownCast(ThisVal, Ty, Failed);
-  if (Failed) {
+  Optional V =
+  StateMgr.getStoreManager().evalBaseToDerived(ThisVal, Ty);
+  if (!V.hasValue()) {
 // We might have suffered some sort of placement new earlier, so
 // we're constructing in a completely unexpected storage.
 // Fall back to a generic pointer cast for this-value.
@@ -772,7 +772,8 @@
 const CXXRecordDecl *StaticClass = StaticMD->getParent();
 QualType StaticTy = Ctx.getPointerType(Ctx.getRecordType(StaticClass));
 ThisVal = SVB.evalCast(ThisVal, Ty, StaticTy);
-  }
+  } else
+ThisVal = *V;
 }
 
 if (!ThisVal.isUnknown())
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -172,9 +172,9 @@
   ///dynamic_cast.
   ///  - We don't know (base is a symbolic region and we don't have
   ///enough info to determine if the cast will succeed at run time).
-  /// The function returns an SVal 

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong
Thanks for clarification.

> TLDR, it is recommended to use the arcanist.

I'm not able to use arcanist. It doesn't work on Windows (at least I've tryed 
several ways to set up it).

BTW. I found a revision from which the 
test(//symbol-simplification-nonloc-loc.cpp//) file starts to assert. It's your 
one D113754 . You can checkout a revision 
before and test the file. I think we should investigate it deeper to understand 
the real cause.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong wrote:

> Denis, you can see in the `Revision Contents` that Diff 3 has the baseline 
> commit `63a6348`. When I check out `63a6348` then the newly added test file 
> triggers the assertion about `BO_Add`.

Yes is see it:
F21029827: image.png 
I don't get this feature. Is it a parent or something? Please explain how to 
interpret this table. And how can I use it myself and when?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 394290.
ASDenysPetrov added a comment.

Changed handler `check::` functions. Reworked. Covered more cases.

Several cases left (marked as FIXME in the test file). For the glance some of 
them we can't handle because of (possibly) wrong symbolic modeling.

WIP.


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

https://reviews.llvm.org/D114718

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp

Index: clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
@@ -0,0 +1,811 @@
+// RUN: %clang_cc1 -std=c++20 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+// NOTE: -relaxed-aliasing flag disables StrictAliasing checker.
+
+template 
+void clang_analyzer_dump(T x);
+void clang_analyzer_eval(int);
+
+namespace std {
+enum class byte : unsigned char {};
+enum class OtherByte : unsigned char {};
+}; // namespace std
+enum class EnumInt : int {};
+
+union UnionUchar {
+  unsigned char x;
+  char y;
+};
+union UnionInt {
+  int x;
+};
+class Class {};
+class ClassInt {
+public:
+  int x;
+};
+class Base {
+public:
+  int x;
+};
+struct AnotherBase {
+  int y;
+};
+class Derived : public AnotherBase, public Base {
+  int z;
+};
+class MostDerived : public Derived {
+  int w;
+};
+
+using AliasedStdByte = std::byte;
+using AliasedChar = char;
+using AliasedSChar = const signed char;
+using AliasedInt = int;
+using AliasedUInt = const unsigned int;
+using AliasedULong = unsigned long;
+using AliasedBase = Base;
+using AliasedAnotherBase = AnotherBase;
+
+namespace ns1 {
+
+void int_casts() {
+  using MyInt = int;
+  MyInt x = {};
+  // int to records
+  {
+auto *ptr = (Class *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (ClassInt *)
+auto y = ptr[0]; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (UnionUchar *)
+ptr->x = 42; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (UnionInt *)
+ptr->x = 42; // expected-warning{{Undefined behavior}}
+  }
+  // int to records
+  {
+auto *ptr = (std::byte *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (std::OtherByte *)
+auto y = *ptr; // expected-warning{{Undefined behavior. Attempting to access the stored value of type}}
+  }
+  {
+auto *ptr = (EnumInt *)
+auto y = ptr[0]; // expected-warning{{Undefined behavior}}
+  }
+  // int to scalars
+  {
+auto *ptr = (const char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (unsigned char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed char *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (short *)
+auto y = ptr[0]; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned short *)
+ptr[0] = 42; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (int *)
+ptr[0] = 42; // no-warning
+  }
+  {
+auto *ptr = (const volatile unsigned int *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed int *)
+*ptr = 24; // no-warning
+  }
+  {
+auto *ptr = (long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long long *)
+auto y = ptr[0]; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (float *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (double *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long double *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  // int to aliases
+  {
+auto *ptr = (AliasedStdByte *)
+auto y = ptr[0]; // no-warning
+  }
+  {
+auto *ptr = (AliasedChar *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (AliasedSChar *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (AliasedULong *)
+auto y = ptr[0]; // expected-warning{{Undefined behavior}}
+  }
+  {

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

> @steakhal
> I don't get this one. I've provided a bunch of tests, even annotated with 
> `no-crash` comments where we crashed prior to this change.

I wasn't able to catch any crashes with your tests file 
//(symbol-simplification-nonloc-loc.cpp)// on the baseline before your patch 
(D115149#3180005 ). So I ask you to 
provide the concrete example you've caught which promted you to do this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Denys Petrov via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a399bf4b3aa: [analyzer] Implemented 
RangeSet::Factory::unite function to handle… (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99797

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -35,12 +35,34 @@
   const RangeSet ) {
   return OS << toString(Set);
 }
+// We need it here for better fail diagnostics from gtest.
+LLVM_ATTRIBUTE_UNUSED static std::ostream <<(std::ostream ,
+  const Range ) {
+  return OS << toString(R);
+}
 
 } // namespace ento
 } // namespace clang
 
 namespace {
 
+template  struct TestValues {
+  static constexpr T MIN = std::numeric_limits::min();
+  static constexpr T MAX = std::numeric_limits::max();
+  // MID is a value in the middle of the range
+  // which unary minus does not affect on,
+  // e.g. int8/int32(0), uint8(128), uint32(2147483648).
+  static constexpr T MID =
+  std::is_signed::value ? 0 : ~(static_cast(-1) / static_cast(2));
+  static constexpr T A = MID - (MAX - MID) / 3 * 2;
+  static constexpr T B = MID - (MAX - MID) / 3;
+  static constexpr T C = -B;
+  static constexpr T D = -A;
+
+  static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
+"Values shall be in an ascending order");
+};
+
 template  class RangeSetTest : public testing::Test {
 public:
   // Init block
@@ -55,24 +77,11 @@
   using RawRange = std::pair;
   using RawRangeSet = std::initializer_list;
 
-  static constexpr BaseType getMin() {
-return std::numeric_limits::min();
-  }
-  static constexpr BaseType getMax() {
-return std::numeric_limits::max();
-  }
-  static constexpr BaseType getMid() {
-return isSigned() ? 0 : ~(fromInt(-1) / fromInt(2));
-  }
-
-  static constexpr bool isSigned() { return std::is_signed::value; }
-  static constexpr BaseType fromInt(int X) { return static_cast(X); }
-
-  static llvm::APSInt Base;
   const llvm::APSInt (BaseType X) {
-llvm::APSInt Dummy = Base;
-Dummy = X;
-return BVF.getValue(Dummy);
+static llvm::APSInt Base{sizeof(BaseType) * CHAR_BIT,
+ std::is_unsigned::value};
+Base = X;
+return BVF.getValue(Base);
   }
 
   Range from(const RawRange ) {
@@ -160,7 +169,7 @@
 
   void checkAdd(RawRangeSet RawLHS, RawRangeSet RawRHS,
 RawRangeSet RawExpected) {
-wrap(::checkAddImpl, RawRHS, RawLHS, RawExpected);
+wrap(::checkAddImpl, RawLHS, RawRHS, RawExpected);
   }
 
   void checkAdd(RawRangeSet RawLHS, BaseType RawRHS, RawRangeSet RawExpected) {
@@ -168,6 +177,29 @@
  RawExpected);
   }
 
+  template 
+  void checkUniteImpl(RangeSet LHS, RHSType RHS, RangeSet Expected) {
+RangeSet Result = F.unite(LHS, RHS);
+EXPECT_EQ(Result, Expected)
+<< "while uniting " << toString(LHS) << " and " << toString(RHS);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, RawRange RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS, RawExpected);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, RawRangeSet RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS, RawExpected);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, BaseType RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS,
+ RawExpected);
+  }
+
   void checkDeleteImpl(const llvm::APSInt , RangeSet From,
RangeSet Expected) {
 RangeSet Result = F.deletePoint(From, Point);
@@ -183,29 +215,19 @@
 
 } // namespace
 
-template 
-llvm::APSInt RangeSetTest::Base{sizeof(BaseType) * 8, !isSigned()};
-
 using IntTypes = ::testing::Types;
 TYPED_TEST_SUITE(RangeSetTest, IntTypes, );
 
 TYPED_TEST(RangeSetTest, RangeSetNegateTest) {
-  // Use next values of the range {MIN, A, B, MID, C, D, MAX}.
-
-  constexpr TypeParam MIN = TestFixture::getMin();
-  constexpr TypeParam MAX = TestFixture::getMax();
-  // MID is a value in the middle of the range
-  // which unary minus does not affect on,
-  // e.g. int8/int32(0), uint8(128), uint32(2147483648).
-  constexpr TypeParam MID = TestFixture::getMid();
-  constexpr TypeParam A = MID - TestFixture::fromInt(42 + 42);
-  constexpr TypeParam B = MID - 

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 393509.
ASDenysPetrov added a comment.

Fixed code formatting in the unit test file according to remarks. Ready to load.


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

https://reviews.llvm.org/D99797

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -35,12 +35,34 @@
   const RangeSet ) {
   return OS << toString(Set);
 }
+// We need it here for better fail diagnostics from gtest.
+LLVM_ATTRIBUTE_UNUSED static std::ostream <<(std::ostream ,
+  const Range ) {
+  return OS << toString(R);
+}
 
 } // namespace ento
 } // namespace clang
 
 namespace {
 
+template  struct TestValues {
+  static constexpr T MIN = std::numeric_limits::min();
+  static constexpr T MAX = std::numeric_limits::max();
+  // MID is a value in the middle of the range
+  // which unary minus does not affect on,
+  // e.g. int8/int32(0), uint8(128), uint32(2147483648).
+  static constexpr T MID =
+  std::is_signed::value ? 0 : ~(static_cast(-1) / static_cast(2));
+  static constexpr T A = MID - (MAX - MID) / 3 * 2;
+  static constexpr T B = MID - (MAX - MID) / 3;
+  static constexpr T C = -B;
+  static constexpr T D = -A;
+
+  static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
+"Values shall be in an ascending order");
+};
+
 template  class RangeSetTest : public testing::Test {
 public:
   // Init block
@@ -55,24 +77,11 @@
   using RawRange = std::pair;
   using RawRangeSet = std::initializer_list;
 
-  static constexpr BaseType getMin() {
-return std::numeric_limits::min();
-  }
-  static constexpr BaseType getMax() {
-return std::numeric_limits::max();
-  }
-  static constexpr BaseType getMid() {
-return isSigned() ? 0 : ~(fromInt(-1) / fromInt(2));
-  }
-
-  static constexpr bool isSigned() { return std::is_signed::value; }
-  static constexpr BaseType fromInt(int X) { return static_cast(X); }
-
-  static llvm::APSInt Base;
   const llvm::APSInt (BaseType X) {
-llvm::APSInt Dummy = Base;
-Dummy = X;
-return BVF.getValue(Dummy);
+static llvm::APSInt Base{sizeof(BaseType) * CHAR_BIT,
+ std::is_unsigned::value};
+Base = X;
+return BVF.getValue(Base);
   }
 
   Range from(const RawRange ) {
@@ -160,7 +169,7 @@
 
   void checkAdd(RawRangeSet RawLHS, RawRangeSet RawRHS,
 RawRangeSet RawExpected) {
-wrap(::checkAddImpl, RawRHS, RawLHS, RawExpected);
+wrap(::checkAddImpl, RawLHS, RawRHS, RawExpected);
   }
 
   void checkAdd(RawRangeSet RawLHS, BaseType RawRHS, RawRangeSet RawExpected) {
@@ -168,6 +177,29 @@
  RawExpected);
   }
 
+  template 
+  void checkUniteImpl(RangeSet LHS, RHSType RHS, RangeSet Expected) {
+RangeSet Result = F.unite(LHS, RHS);
+EXPECT_EQ(Result, Expected)
+<< "while uniting " << toString(LHS) << " and " << toString(RHS);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, RawRange RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS, RawExpected);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, RawRangeSet RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS, RawExpected);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, BaseType RawRHS,
+  RawRangeSet RawExpected) {
+wrap(::checkUniteImpl, RawLHS, RawRHS,
+ RawExpected);
+  }
+
   void checkDeleteImpl(const llvm::APSInt , RangeSet From,
RangeSet Expected) {
 RangeSet Result = F.deletePoint(From, Point);
@@ -183,29 +215,19 @@
 
 } // namespace
 
-template 
-llvm::APSInt RangeSetTest::Base{sizeof(BaseType) * 8, !isSigned()};
-
 using IntTypes = ::testing::Types;
 TYPED_TEST_SUITE(RangeSetTest, IntTypes, );
 
 TYPED_TEST(RangeSetTest, RangeSetNegateTest) {
-  // Use next values of the range {MIN, A, B, MID, C, D, MAX}.
-
-  constexpr TypeParam MIN = TestFixture::getMin();
-  constexpr TypeParam MAX = TestFixture::getMax();
-  // MID is a value in the middle of the range
-  // which unary minus does not affect on,
-  // e.g. int8/int32(0), uint8(128), uint32(2147483648).
-  constexpr TypeParam MID = TestFixture::getMid();
-  constexpr TypeParam A = MID - TestFixture::fromInt(42 + 42);
-  constexpr TypeParam B = MID - TestFixture::fromInt(42);
-  constexpr TypeParam C = -B;
-  constexpr TypeParam D = -A;
-
-  static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
-"Values shall be in an ascending order");
+  using 

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:113
+  QualType getOriginalType(CheckerContext , SVal V, QualType T) const {
+assert(V.getAs() && "Location shall be a Loc.");
+V = C.getState()->getSVal(V.castAs(), T);

ASDenysPetrov wrote:
> NoQ wrote:
> > I suspect it might also be `UnknownVal` (?) It usually makes sense to 
> > protect against such scenarios with an early return.
> I'll try to add some tests to model this.
'ExprEngine::evalLocation' has an early check for //unknown//, so `Location` is 
never be //unknown//.


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

https://reviews.llvm.org/D114718

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

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:463
+  if (const Optional RV = rhs.getAs()) {
+const auto IsCommutative = [](BinaryOperatorKind Op) {
+  return Op == BO_Mul || Op == BO_Add || Op == BO_And || Op == BO_Xor ||

steakhal wrote:
> ASDenysPetrov wrote:
> > IMO it should be in a family of `BinaryOperator::is..` methods.
> It isn't as easy as it might seem at first glance. I've considered doing it 
> that way.
> In our context, we don't really deal with floating-point numbers, but in the 
> generic implementation, we should have the type in addition to the OpKind. We 
> don't need that complexity, so I sticked to inlining them here.
I see your concerns about floats but the operators don't stop to be commutative 
themselves. Yes, it may not work with some types but it will still be 
commutative. Leave the context for a user outside the function. It may bother 
you only a few operators from the set and you just check whether they belong to 
the set or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal
 @NoQ wrote:

> I'm confident that there's a way to get it right, simply because the program 
> under analysis is type-correct. If it's the simplifier, let's fix the 
> simplifier.

I agree with your main thought. But I also believe there definitely are thing 
to improve everywhere. And this one could wait until we find the root cause and 
correct it somewhere before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

It seems like your test file passes even before your patch. I've just checked 
it. My last pull from the baseline was on Nov 15.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 392821.
ASDenysPetrov added a comment.

Improved dynamic type recognition. Provided `-fstrict-aliasing` compiler flag 
dependency.
Still has issues with some cases (see it at the bottom of the test file). WIP.


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

https://reviews.llvm.org/D114718

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp

Index: clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
@@ -0,0 +1,524 @@
+// RUN: %clang_cc1 -std=c++20 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+// NOTE: -relaxed-aliasing flag disables StrictAliasing checker.
+
+template 
+void clang_analyzer_dump(T x);
+void clang_analyzer_eval(int);
+
+namespace std {
+enum class byte : unsigned char {};
+enum class OtherByte : unsigned char {};
+}; // namespace std
+enum class EnumInt : int {};
+
+union UnionUchar {
+  unsigned char x;
+  char y;
+};
+union UnionInt {
+  int x;
+};
+class Class {};
+class ClassInt {
+public:
+  int x;
+};
+class Base {
+public:
+  int x;
+};
+struct AnotherBase {
+  int y;
+};
+class Derived : public AnotherBase, public Base {
+  int z;
+};
+class MostDerived : public Derived {
+  int w;
+};
+
+using AliasedStdByte = std::byte;
+using AliasedChar = char;
+using AliasedSChar = const signed char;
+using AliasedInt = int;
+using AliasedUInt = const unsigned int;
+using AliasedULong = unsigned long;
+using AliasedBase = Base;
+using AliasedAnotherBase = AnotherBase;
+
+namespace ns1 {
+
+void int_casts() {
+  using MyInt = int;
+  MyInt x = {};
+  // int to records
+  {
+auto *ptr = (Class *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (ClassInt *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (UnionUchar *)
+ptr->x = 42; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (UnionInt *)
+ptr->x = 42; // expected-warning{{Undefined behavior}}
+  }
+  // int to records
+  {
+auto *ptr = (std::byte *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (std::OtherByte *)
+auto y = *ptr; // expected-warning{{Undefined behavior. Attempting to access the stored value of type}}
+  }
+  {
+auto *ptr = (EnumInt *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  // int to scalars
+  {
+auto *ptr = (const char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (unsigned char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed char *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (int *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (const volatile unsigned int *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed int *)
+*ptr = 24; // no-warning
+  }
+  {
+auto *ptr = (long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (float *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (double *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long double *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  // int to aliases
+  {
+auto *ptr = (AliasedStdByte *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (AliasedChar *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (AliasedSChar *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (AliasedULong *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (AliasedInt *)
+auto y = *ptr; // 

[PATCH] D114006: [analyzer][NFC] Enable access to CodeGenOptions from analyzer's instances

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 392666.
ASDenysPetrov added a comment.

Fixed unit test.


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

https://reviews.llvm.org/D114006

Files:
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
  clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
  clang/unittests/StaticAnalyzer/Reusables.h

Index: clang/unittests/StaticAnalyzer/Reusables.h
===
--- clang/unittests/StaticAnalyzer/Reusables.h
+++ clang/unittests/StaticAnalyzer/Reusables.h
@@ -57,12 +57,12 @@
 
 public:
   ExprEngineConsumer(CompilerInstance )
-  : C(C),
-ChkMgr(C.getASTContext(), *C.getAnalyzerOpts(), C.getPreprocessor()),
+  : C(C), ChkMgr(C.getASTContext(), *C.getAnalyzerOpts(),
+ C.getCodeGenOpts(), C.getPreprocessor()),
 CTU(C), Consumers(),
 AMgr(C.getASTContext(), C.getPreprocessor(), Consumers,
  CreateRegionStoreManager, CreateRangeConstraintManager, ,
- *C.getAnalyzerOpts()),
+ *C.getAnalyzerOpts(), C.getCodeGenOpts()),
 VisitedCallees(), FS(),
 Eng(CTU, AMgr, , , ExprEngine::Inline_Regular) {}
 };
Index: clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
@@ -18,11 +18,12 @@
 namespace ento {
 
 CheckerManager::CheckerManager(
-ASTContext , AnalyzerOptions , const Preprocessor ,
+ASTContext , AnalyzerOptions ,
+const CodeGenOptions , const Preprocessor ,
 ArrayRef plugins,
 ArrayRef> checkerRegistrationFns)
-: Context(), LangOpts(Context.getLangOpts()), AOptions(AOptions),
-  PP(), Diags(Context.getDiagnostics()),
+: Context(), LangOpts(Context.getLangOpts()), CodeGenOpts(CGOpts),
+  AOptions(AOptions), PP(), Diags(Context.getDiagnostics()),
   RegistryData(std::make_unique()) {
   CheckerRegistry Registry(*RegistryData, plugins, Context.getDiagnostics(),
AOptions, checkerRegistrationFns);
@@ -33,9 +34,10 @@
 
 CheckerManager::CheckerManager(AnalyzerOptions ,
const LangOptions ,
+   const CodeGenOptions ,
DiagnosticsEngine ,
ArrayRef plugins)
-: LangOpts(LangOpts), AOptions(AOptions), Diags(Diags),
+: LangOpts(LangOpts), CodeGenOpts(CGOpts), AOptions(AOptions), Diags(Diags),
   RegistryData(std::make_unique()) {
   CheckerRegistry Registry(*RegistryData, plugins, Diags, AOptions, {});
   Registry.initializeRegistry(*this);
Index: clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
@@ -25,35 +25,29 @@
 using namespace clang;
 using namespace ento;
 
+static CheckerManager CreateCheckerManager(CompilerInstance ) {
+  return {*CI.getAnalyzerOpts(), CI.getLangOpts(), CI.getCodeGenOpts(),
+  CI.getDiagnostics(), CI.getFrontendOpts().Plugins};
+}
+
 void ento::printCheckerHelp(raw_ostream , CompilerInstance ) {
   out << "OVERVIEW: Clang Static Analyzer Checkers List\n\n";
   out << "USAGE: -analyzer-checker \n\n";
 
-  auto CheckerMgr = std::make_unique(
-  *CI.getAnalyzerOpts(), CI.getLangOpts(), CI.getDiagnostics(),
-  CI.getFrontendOpts().Plugins);
-
-  CheckerMgr->getCheckerRegistryData().printCheckerWithDescList(
+  CreateCheckerManager(CI).getCheckerRegistryData().printCheckerWithDescList(
   *CI.getAnalyzerOpts(), out);
 }
 
 void ento::printEnabledCheckerList(raw_ostream , CompilerInstance ) {
   out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
 
-  auto CheckerMgr = std::make_unique(
-  *CI.getAnalyzerOpts(), CI.getLangOpts(), CI.getDiagnostics(),
-  CI.getFrontendOpts().Plugins);
-
-  CheckerMgr->getCheckerRegistryData().printEnabledCheckerList(out);
+  CreateCheckerManager(CI).getCheckerRegistryData().printEnabledCheckerList(
+  out);
 }
 
 void ento::printCheckerConfigList(raw_ostream , CompilerInstance ) {
 
-  auto CheckerMgr = std::make_unique(
-  *CI.getAnalyzerOpts(), CI.getLangOpts(), CI.getDiagnostics(),
-  CI.getFrontendOpts().Plugins);
-
-  CheckerMgr->getCheckerRegistryData().printCheckerOptionList(
+  CreateCheckerManager(CI).getCheckerRegistryData().printCheckerOptionList(
   *CI.getAnalyzerOpts(), out);
 }
 
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

[PATCH] D114006: [analyzer][NFC] Enable access to CodeGenOptions from analyzer's instances

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 392557.
ASDenysPetrov edited the summary of this revision.
ASDenysPetrov added a comment.

Passed a `CodeGenOptions` reference to `CheckerManager` as well. (Adjusted to 
D114718 )


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

https://reviews.llvm.org/D114006

Files:
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
  clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
  clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
  clang/unittests/StaticAnalyzer/Reusables.h

Index: clang/unittests/StaticAnalyzer/Reusables.h
===
--- clang/unittests/StaticAnalyzer/Reusables.h
+++ clang/unittests/StaticAnalyzer/Reusables.h
@@ -62,7 +62,7 @@
 CTU(C), Consumers(),
 AMgr(C.getASTContext(), C.getPreprocessor(), Consumers,
  CreateRegionStoreManager, CreateRangeConstraintManager, ,
- *C.getAnalyzerOpts()),
+ *C.getAnalyzerOpts(), C.getCodeGenOpts()),
 VisitedCallees(), FS(),
 Eng(CTU, AMgr, , , ExprEngine::Inline_Regular) {}
 };
Index: clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
@@ -18,11 +18,12 @@
 namespace ento {
 
 CheckerManager::CheckerManager(
-ASTContext , AnalyzerOptions , const Preprocessor ,
+ASTContext , AnalyzerOptions ,
+const CodeGenOptions , const Preprocessor ,
 ArrayRef plugins,
 ArrayRef> checkerRegistrationFns)
-: Context(), LangOpts(Context.getLangOpts()), AOptions(AOptions),
-  PP(), Diags(Context.getDiagnostics()),
+: Context(), LangOpts(Context.getLangOpts()), CodeGenOpts(CGOpts),
+  AOptions(AOptions), PP(), Diags(Context.getDiagnostics()),
   RegistryData(std::make_unique()) {
   CheckerRegistry Registry(*RegistryData, plugins, Context.getDiagnostics(),
AOptions, checkerRegistrationFns);
@@ -33,9 +34,10 @@
 
 CheckerManager::CheckerManager(AnalyzerOptions ,
const LangOptions ,
+   const CodeGenOptions ,
DiagnosticsEngine ,
ArrayRef plugins)
-: LangOpts(LangOpts), AOptions(AOptions), Diags(Diags),
+: LangOpts(LangOpts), CodeGenOpts(CGOpts), AOptions(AOptions), Diags(Diags),
   RegistryData(std::make_unique()) {
   CheckerRegistry Registry(*RegistryData, plugins, Diags, AOptions, {});
   Registry.initializeRegistry(*this);
Index: clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
@@ -25,35 +25,29 @@
 using namespace clang;
 using namespace ento;
 
+static CheckerManager CreateCheckerManager(CompilerInstance ) {
+  return {*CI.getAnalyzerOpts(), CI.getLangOpts(), CI.getCodeGenOpts(),
+  CI.getDiagnostics(), CI.getFrontendOpts().Plugins};
+}
+
 void ento::printCheckerHelp(raw_ostream , CompilerInstance ) {
   out << "OVERVIEW: Clang Static Analyzer Checkers List\n\n";
   out << "USAGE: -analyzer-checker \n\n";
 
-  auto CheckerMgr = std::make_unique(
-  *CI.getAnalyzerOpts(), CI.getLangOpts(), CI.getDiagnostics(),
-  CI.getFrontendOpts().Plugins);
-
-  CheckerMgr->getCheckerRegistryData().printCheckerWithDescList(
+  CreateCheckerManager(CI).getCheckerRegistryData().printCheckerWithDescList(
   *CI.getAnalyzerOpts(), out);
 }
 
 void ento::printEnabledCheckerList(raw_ostream , CompilerInstance ) {
   out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
 
-  auto CheckerMgr = std::make_unique(
-  *CI.getAnalyzerOpts(), CI.getLangOpts(), CI.getDiagnostics(),
-  CI.getFrontendOpts().Plugins);
-
-  CheckerMgr->getCheckerRegistryData().printEnabledCheckerList(out);
+  CreateCheckerManager(CI).getCheckerRegistryData().printEnabledCheckerList(
+  out);
 }
 
 void ento::printCheckerConfigList(raw_ostream , CompilerInstance ) {
 
-  auto CheckerMgr = std::make_unique(
-  *CI.getAnalyzerOpts(), CI.getLangOpts(), CI.getDiagnostics(),
-  CI.getFrontendOpts().Plugins);
-
-  CheckerMgr->getCheckerRegistryData().printCheckerOptionList(
+  CreateCheckerManager(CI).getCheckerRegistryData().printCheckerOptionList(
   *CI.getAnalyzerOpts(), out);
 }
 
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:463
+  if (const Optional RV = rhs.getAs()) {
+const auto IsCommutative = [](BinaryOperatorKind Op) {
+  return Op == BO_Mul || Op == BO_Add || Op == BO_And || Op == BO_Xor ||

IMO it should be in a family of `BinaryOperator::is..` methods.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@martong

> Nice, assiduous work!

Many thanks for your time! Your work is not less assiduous!

> (LHS, RHS) swaps

it doesn't really matter, as the operation is comutative, but, yes, it does 
matter to depict the particular test case. I'll revise twise LHS-RHS's.

> I am going to continue with the next patch in the stack. I recognize that the 
> handling of casts is very important and problems related to not handling them 
> occurs even more frequently as other parts of the engine evolve (e.g. 
> https://reviews.llvm.org/D113753#3167134)

Aha. I saw you patch. I'll join to review it.




Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:441
+  //___/__/_\__\___   ___/___\___
+  this->checkUnite({{MID, MID}}, {A, D}, {{A, D}});
+  this->checkUnite({{B, C}}, {A, D}, {{A, D}});

martong wrote:
> 
There are three overloads of `checkUnite` which correspond to three overloads 
of `RangeSet::unite`. I made it consistent to other check-functions (`add` 
e.g.).



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:449
+  //___/___\___   ___/___\___
+  this->checkUnite({{MIN, MIN}}, MIN, {{MIN, MIN}});
+  this->checkUnite({{A, B}}, {A, B}, {{A, B}});

martong wrote:
> I think, either we should use `{{X, X}}` or `X` everywhere, but not mixed. 
This tests different oveloaded versions of `RangeSet::unite`.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:528-529
+  //_/__\_/__\_/\_/\_/\_   _/__\_/__\_/\_/\_/\_
+  this->checkUnite({{MIN, A}, {A + 2, B}}, {{MID, C}, {C + 2, D - 2}, {D, 
MAX}},
+   {{MIN, A}, {A + 2, B}, {MID, C}, {C + 2, D - 2}, {D, MAX}});
+  this->checkUnite({{MIN, MIN}, {A, A}}, {{B, B}, {C, C}, {MAX, MAX}},

martong wrote:
> I think we could better format these more complex cases.
clang-fromat acts on its own. But I agree, it looks the way better. I'll 
consider wrapping it into `// clang-format on/off` directives.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:590-592
+  this->checkUnite({{A, B - 1}, {B + 1, C - 1}, {C + 2, D}, {MAX - 1, MAX}},
+   {{MIN, MIN}, {B, MID}, {MID + 1, C}, {C + 4, D - 1}},
+   {{MIN, MIN}, {A, C}, {C + 2, D}, {MAX - 1, MAX}});

martong wrote:
> martong wrote:
> > 
> What do you think about this format? The result can be easily verified this 
> way I think, but a bit ugly ...
I think ASCII art does this job. Let code look as code :)


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

https://reviews.llvm.org/D99797

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@xazax.hun  Updated the summary for better undersanding differencies between 
the Standards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114718

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:48-49
+  //
+  // NOTE: User must provide canonical and unqualified QualType's for the
+  // correct result.
+  static bool canAccess(QualType From, QualType To, ASTContext ) {

NoQ wrote:
> `assert()` it? Or maybe canonicalize defensively so that not to duplicate 
> code in the caller, given that there's really only one correct way to do that?
>Or maybe canonicalize defensively so that not to duplicate code in the caller,
We need canonical types in the bug report as well, so we have to get them on 
the caller side.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:58
+  : From(From), To(To), Ctx(Ctx) {}
+  bool canAccessImpl() {
+return isSame() || isCharOrByte() || isOppositeSign();

xazax.hun wrote:
> I'd love to see some detailed descriptions of the strict aliasing rule, what 
> parts are we checking for and what parts we don't. E.g. it would be nice to 
> document the differences between C and C++ aliasing rules. I do remember 
> something about prefixes, i.e.: it would be well defined to access something 
> with the dynamic type `struct { int x, y; };` and read `struct{ int x; };` 
> from it. Does that not apply to C++?
>I'd love to see some detailed descriptions of the strict aliasing rule, 
I've added a description in the header.
>it would be nice to document the differences between C and C++ aliasing rules.
I need some time to gather those differences and will add them to the 
description as well.
> it would be well defined to access something with the dynamic type struct { 
> int x, y; }; and read struct{ int x; }; from it. Does that not apply to C++?
AFAIK it's UB if you do any access to the object which lifetime is not started 
in C++.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:80
+class StrictAliasingChecker : public Checker {
+  mutable std::unique_ptr BT;
+

NoQ wrote:
> The modern solution is
> ```lang=c++
> BugType BT{this, "...", "..."};
> ```
Got it.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:113
+  QualType getOriginalType(CheckerContext , SVal V, QualType T) const {
+assert(V.getAs() && "Location shall be a Loc.");
+V = C.getState()->getSVal(V.castAs(), T);

NoQ wrote:
> I suspect it might also be `UnknownVal` (?) It usually makes sense to protect 
> against such scenarios with an early return.
I'll try to add some tests to model this.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:141-145
+auto ER = dyn_cast(R);
+while (ER) {
+  R = ER->getSuperRegion();
+  ER = dyn_cast(R);
+}

NoQ wrote:
> You're basically running into https://bugs.llvm.org/show_bug.cgi?id=43364 
> here. The element region is not necessarily a consequence of a pointer cast; 
> it may also be a legitimate array element access, and there's no way to 
> figure out what it really was. So I suspect that you may fail the following 
> test case:
> ```lang=c++
> void foo() {
>   int x[10];
>   int *p = [1];
>   *p = 1; // int incompatible with int[10]
> }
> ```
> 
> Separately from that, ultimatetly you'll most likely have to handle //regions 
> with symbolic base// separately. These regions are special because they are 
> built when the information about the original type is not really unavailable. 
> For now you're dodging this problem by only supporting `VarRegion`s with 
> element sub-regions. But in the general case you may encounter code like this
> ```lang=c++
> void foo(void *p) {
>   int *x = p;
>   float *y = x;
>   *y = 1.0;
> }
> ```
> which may be valid if the original pointer `p` points to a `float`. But this 
> can be delayed until later.
Oh, unfortunately I've stumbled upon these representation issues:
```
int arr[2][8]
auto p1 = (int(*)[8])arr[0]; // {arr,0 S64b,int[8]}
auto p2 = (int(*)[8])arr;// {arr,0 S64b,int[8]}
auto p3 = (int(*)[7])arr[0]; // {arr,0 S64b,int[7]}
auto x = *p1; // OK
auto y = *p2; // UB
auto z = *p3; // UB
```
I think we need some sort of a new region `CastRegion` that we can store the 
**provenance/origin** and **cast** type separately (in a canonical form), like 
I did for integers in  D103096. I can investigate it soon. And `ElementRegion` 
would be in charge of arrays only.
Or
We could modificate `ElementRegion` to be always in a canonical form which we 
could easily differentiate.



Comment at: clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp:157
+
+ExplodedNode *Node = C.generateNonFatalErrorNode();
+if (!BT)

NoQ wrote:
> I suggest a fatal error node. Undefined behavior already occured, there's 
> nothing interesting in the follow-up.
> I suggest a fatal error node. 
That's a big discussion what would be 

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@rsmith, @aaron.ballman I kindly invite you to join the review process 
especially in a part of the Standard interpretation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114718

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

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: rsmith, martong, NoQ, vsavchenko, steakhal, 
aaron.ballman, xazax.hun, Szelethus.
ASDenysPetrov added a project: clang.
Herald added subscribers: manas, jeroen.dobbelaere, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, mgorny.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

`StrictAliasingChecker` implements checks on violation of the next paragraph of 
the Standard which is known as **Strict Aliasing Rule**. It operates on 
variable loads and stores. The checker compares the original type of the value 
with the type of the pointer with which the value is aliased.
The paragraph:

> C++20 7.2.1 p11 [basic.lval]:
>  If a program attempts to access the stored value of an object through a 
> glvalue whose type is not similar to one of the following types the behavior 
> is undefined:
>
> - the dynamic type of the object,
> - a type that is the signed or unsigned type corresponding to the dynamic 
> type of the object, or
> - a char, unsigned char, or std​::​byte type.

Example:

  int x = 42;
  auto c = *((char*)); // The original type is `int`. The aliased type is 
`char`. OK. 
  auto f = *((float*)); // The original type is `int`. The aliased type is 
`float`. UB. 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114718

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/StrictAliasingChecker.cpp
  clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp

Index: clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/StrictAliasingChecker/strict-aliasing.cpp
@@ -0,0 +1,160 @@
+// RUN: %clang_cc1 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=debug.ExprInspection,alpha.core.StrictAliasing -verify %s
+
+template 
+void clang_analyzer_dump(T x);
+void clang_analyzer_eval(int);
+
+namespace std {
+enum class byte : unsigned char {};
+enum class otherByte : unsigned char {};
+}; // namespace std
+enum class intEnum : int {};
+
+class Class {};
+class ClassInt {
+  int x;
+};
+
+using AliasedStdByte = std::byte;
+using AliasedChar = char;
+using AliasedSChar = signed char;
+using AliasedInt = int;
+using AliasedUInt = unsigned int;
+using AliasedULong = unsigned long;
+
+namespace ns1 {
+
+void var_cast() {
+  using MyInt = int;
+  MyInt x = {};
+  {
+auto *ptr = (std::byte *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (std::otherByte *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (intEnum *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (Class *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (ClassInt *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (unsigned char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (const char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (const unsigned char *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed char *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed short *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (int *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (unsigned int *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (signed int *)
+auto y = *ptr; // no-warning
+  }
+  {
+auto *ptr = (long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (unsigned long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (signed long long *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (float *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (double *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto *ptr = (long double *)
+auto y = *ptr; // expected-warning{{Undefined behavior}}
+  }
+  {
+auto 

[PATCH] D114441: [analyzer][NFC] Refactor AnalysisConsumer::getModeForDecl()

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:599-603
+  const SourceLocation Loc = [](Decl *D) -> SourceLocation {
+const Stmt *Body = D->getBody();
+SourceLocation SL = Body ? Body->getBeginLoc() : D->getLocation();
+return SM.getExpansionLoc(SL);
+  }(D);

Why don't just leave this snippet as it was but add `const SourceLocation Loc` 
instead of re-assigning `SL`? Your construction made me stumbled for a while. 
As this patch is meant to bring readability, I wouldn't like to see such a 
code-trick. I see your intention to hide `SL` but IMO let compiler do this for 
us. See suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114441

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

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov abandoned this revision.
ASDenysPetrov added a comment.

Temporary suspended this revision in favor of making a new checker 
//StrictAliasingChecker//, which would define an access to values through 
unpermited types as Unefined Behavior according to certain statements of the 
current Standard.


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

https://reviews.llvm.org/D110927

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

[PATCH] D114006: [analyzer][NFC] Enable access to CodeGenOptions from analyzer's instances

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D114006#3142313 , @martong wrote:

> LGTM!

Thanks, guys!
I'll load it as soon as D110927  also be 
ready for load(I'm working on it), as this patch is just a preparatory one.


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

https://reviews.llvm.org/D114006

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 388552.
ASDenysPetrov added a comment.

Rebased.


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

https://reviews.llvm.org/D103096

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
  clang/test/Analysis/symbol-integral-cast.cpp

Index: clang/test/Analysis/symbol-integral-cast.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-integral-cast.cpp
@@ -0,0 +1,374 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -analyzer-config eagerly-assume=false -analyzer-config support-symbolic-integer-casts=true -verify %s
+
+template 
+void clang_analyzer_eval(T);
+void clang_analyzer_warnIfReached();
+
+typedef short int16_t;
+typedef int int32_t;
+typedef unsigned short uint16_t;
+typedef unsigned int uint32_t;
+
+void test1(int x) {
+  // Even if two lower bytes of `x` equal to zero, it doesn't mean that
+  // the entire `x` is zero. We are not able to know the exact value of x.
+  // It can be one of  65536 possible values like [0, 65536, 131072, ...]
+  // and so on. To avoid huge range sets we still assume `x` in the range
+  // [INT_MIN, INT_MAX].
+  if (!(short)x) {
+if (!x)
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test2(int x) {
+  // If two lower bytes of `x` equal to zero, and we know x to be 65537,
+  // which is not truncated to short as zero. Thus the branch is infisible.
+  short s = x;
+  if (!s) {
+if (x == 65537)
+  clang_analyzer_warnIfReached(); // no-warning
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test3(int x, short s) {
+  s = x;
+  if ((short)x > -10 && s < 10) {
+if (x > 0 && x < 10) {
+  // If the range of the whole variable was constrained then reason again
+  // about truncated bytes to make the ranges more precise.
+  clang_analyzer_eval((short)x <= 0); // expected-warning {{FALSE}}
+}
+  }
+}
+
+void test4(unsigned x) {
+  if ((char)x > 8) {
+// Constraint the range of the lowest byte of `x` to [9, CHAR_MAX].
+// The original range of `x` still remains [0, UINT_MAX].
+clang_analyzer_eval((char)x < 42); // expected-warning {{UNKNOWN}}
+if (x < 42) {
+  // Constraint the original range to [0, 42] and update (re-constraint)
+  // the range of the lowest byte of 'x' to [9, 42].
+  clang_analyzer_eval((char)x < 42); // expected-warning {{TRUE}}
+}
+  }
+}
+
+void test5(unsigned x) {
+  if ((char)x > -10 && (char)x < 10) {
+if ((short)x == 8) {
+  // If the range of higher bytes(short) was constrained then reason again
+  // about smaller truncated ranges(char) to make it more precise.
+  clang_analyzer_eval((char)x == 8);  // expected-warning {{TRUE}}
+  clang_analyzer_eval((short)x == 8); // expected-warning {{TRUE}}
+  // We still assume full version of `x` in the range [INT_MIN, INT_MAX].
+  clang_analyzer_eval(x == 8); // expected-warning {{UNKNOWN}}
+}
+  }
+}
+
+void test6(int x) {
+  // Even if two lower bytes of `x` less than zero, it doesn't mean that `x`
+  // can't be greater than zero. Thence we don't change the native range of
+  // `x` and this branch is feasible.
+  if (x > 0)
+if ((short)x < 0)
+  clang_analyzer_eval(x > 0); // expected-warning {{TRUE}}
+}
+
+void test7(int x) {
+  // The range of two lower bytes of `x` [1, SHORT_MAX] is enough to cover
+  // all possible values of char [CHAR_MIN, CHAR_MAX]. So the lowest byte
+  // can be lower than zero.
+  if ((short)x > 0) {
+if ((char)x < 0)
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test8(int x) {
+  // Promotion from `signed int` to `signed long long` also reasoning about the
+  // original range, because we know the fact that even after promotion it
+  // remains in the range [INT_MIN, INT_MAX].
+  if ((long long)x < 0)
+clang_analyzer_eval(x < 0); // expected-warning {{TRUE}}
+}
+
+void test9(signed int x) {
+  // Any cast `signed` to `unsigned` produces an unsigned range, which is
+  // [0, UNSIGNED_MAX] and can not be lower than zero.
+  if ((unsigned long long)x < 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+
+  if ((unsigned int)x < 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning 

[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 388512.
ASDenysPetrov added a comment.

Fixed missed part during rebasing in the unit test.


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

https://reviews.llvm.org/D103094

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -40,12 +40,18 @@
   const Range ) {
   return OS << toString(R);
 }
+LLVM_ATTRIBUTE_UNUSED static std::ostream <<(std::ostream ,
+  APSIntType Ty) {
+  return OS << (Ty.isUnsigned() ? "u" : "s") << Ty.getBitWidth();
+}
 
 } // namespace ento
 } // namespace clang
 
 namespace {
 
+template  constexpr bool is_signed_v = std::is_signed::value;
+
 template  struct TestValues {
   static constexpr T MIN = std::numeric_limits::min();
   static constexpr T MAX = std::numeric_limits::max();
@@ -53,7 +59,7 @@
   // which unary minus does not affect on,
   // e.g. int8/int32(0), uint8(128), uint32(2147483648).
   static constexpr T MID =
-  std::is_signed::value ? 0 : ~(static_cast(-1) / static_cast(2));
+  is_signed_v ? 0 : ~(static_cast(-1) / static_cast(2));
   static constexpr T A = MID - (MAX - MID) / 3 * 2;
   static constexpr T B = MID - (MAX - MID) / 3;
   static constexpr T C = -B;
@@ -61,8 +67,40 @@
 
   static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
 "Values shall be in an ascending order");
+  // Clear bits in low bytes by the given amount.
+  template 
+  static constexpr T ClearLowBytes =
+  static_cast(static_cast(Value)
+ << ((Bytes >= CHAR_BIT) ? 0 : Bytes) * CHAR_BIT);
+
+  template 
+  static constexpr T TruncZeroOf = ClearLowBytes;
+
+  // Random number with active bits in every byte. 0x'
+  static constexpr T XAAA = static_cast(
+  0b10101010'10101010'10101010'10101010'10101010'10101010'10101010'10101010);
+  template 
+  static constexpr T XAAATruncZeroOf = TruncZeroOf; // 0x'AB00
+
+  // Random number with active bits in every byte. 0x'
+  static constexpr T X555 = static_cast(
+  0b01010101'01010101'01010101'01010101'01010101'01010101'01010101'01010101);
+  template 
+  static constexpr T X555TruncZeroOf = TruncZeroOf; // 0x'5600
+
+  // Numbers for ranges with the same bits in the lowest byte.
+  // 0x'AA2A
+  static constexpr T FromA = ClearLowBytes + 42;
+  static constexpr T ToA = FromA + 2; // 0x'AA2C
+  // 0x'552A
+  static constexpr T FromB = ClearLowBytes + 42;
+  static constexpr T ToB = FromB + 2; // 0x'552C
 };
 
+template 
+static constexpr APSIntType APSIntTy = APSIntType(sizeof(T) * CHAR_BIT,
+  !is_signed_v);
+
 template  class RangeSetTest : public testing::Test {
 public:
   // Init block
@@ -74,21 +112,24 @@
   // End init block
 
   using Self = RangeSetTest;
-  using RawRange = std::pair;
-  using RawRangeSet = std::initializer_list;
-
-  const llvm::APSInt (BaseType X) {
-static llvm::APSInt Base{sizeof(BaseType) * CHAR_BIT,
- std::is_unsigned::value};
-Base = X;
-return BVF.getValue(Base);
+  template  using RawRangeT = std::pair;
+  template 
+  using RawRangeSetT = std::initializer_list>;
+  using RawRange = RawRangeT;
+  using RawRangeSet = RawRangeSetT;
+
+  template  const llvm::APSInt (T X) {
+static llvm::APSInt Int = APSIntTy.getZeroValue();
+Int = X;
+return BVF.getValue(Int);
   }
 
-  Range from(const RawRange ) {
+  template  Range from(const RawRangeT ) {
 return Range(from(Init.first), from(Init.second));
   }
 
-  RangeSet from(const RawRangeSet ) {
+  template 
+  RangeSet from(RawRangeSetT Init, APSIntType Ty = APSIntTy) {
 RangeSet RangeSet = F.getEmptySet();
 for (const auto  : Init) {
   RangeSet = F.add(RangeSet, from(Raw));
@@ -211,9 +252,20 @@
RawRangeSet RawExpected) {
 wrap(::checkDeleteImpl, Point, RawFrom, RawExpected);
   }
-};
 
-} // namespace
+  void checkCastToImpl(RangeSet What, APSIntType Ty, RangeSet Expected) {
+RangeSet Result = F.castTo(What, Ty);
+EXPECT_EQ(Result, Expected)
+<< "while casting " << toString(What) << " to " << Ty;
+  }
+
+  template 
+  void checkCastTo(RawRangeSetT What, RawRangeSetT Expected) {
+static constexpr APSIntType FromTy = APSIntTy;
+static constexpr APSIntType ToTy = APSIntTy;
+this->checkCastToImpl(from(What, FromTy), ToTy, from(Expected, ToTy));
+  }
+};
 
 using IntTypes =