[Analyzer] Produce SymbolCast for pointer to integer cast

[martong]

Summary

Produce SymbolCast for a pointer-to-integer cast if the
support-symbolic-integer-casts flag is enabled.

[ASDenysPetrov]

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

1022

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

[martong]

• Move the comment hunk

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

1022

Ok, I've moved it.

[ASDenysPetrov]

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

1088

Notify the user about the contract.

1095–1097

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.

[ASDenysPetrov]

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

1088

Mistake. CastTy, not SafeTy

[martong]

Thanks for the review Denys, and sorry for the long delay with the update. I hope that this patch is going to complement nicely the rest of the cast patches.

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

1088

Okay, I've added an assertion for this check.

1095–1097

You're right, we can just simply return V, I've updated like so. (I just wanted to make sure that the makeSymbolVal is consistent, but that has nothing to do with this patch.)

[martong]

• add new assert for canonical type
• remove superflous assert about makeSymbolVal

[martong]

Ping

[martong]

• Rebase to llvm/main

[steakhal]

Looks correct to me.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h

110–113

You don't need to have gap between these two. They belong in the same overload set.

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

776–785

clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp

25–28

Is this the only obstacle to eradicate LocAsInteger?

clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp

2–7

I think you need to pin the target. You assume a specific pointer bitwidth in the test.

24

Why is this `UNKNOWN? I would expect TRUE here as well.

[steakhal]

clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp

24

Hmm, I meant it for line 39 :D

clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h

protected:
   /// - (int)(int x) -> int x
   ///
   /// \param V -- SymbolVal, which pressumably contains SymbolCast or any symbol
   /// that is applicable for cast operation.
   /// \param CastTy -- QualType, which `V` shall be cast to.
   /// \return SVal with simplified cast expression.
   /// \note: Currently only support integral casts.
   SVal simplifySymbolCast(nonloc::SymbolVal V, QualType CastTy);
 
+  SVal simplifySymbolCast(SymbolRef SE, QualType CastTy);
+
 public:
   SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
               ProgramStateManager &stateMgr);
 
   virtual ~SValBuilder() = default;
 
   SVal evalCast(SVal V, QualType CastTy, QualType OriginalTy);
 
   // Handles casts of type CK_IntegralCast.

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

if (ArrayTy) {
       // We will always decay to a pointer.
       QualType ElemTy = ArrayTy->getElementType();
       Val = StateMgr.ArrayToPointer(V, ElemTy);
       // FIXME: Keep these here for now in case we decide soon that we
       // need the original decayed type.
       //    QualType elemTy = cast<ArrayType>(originalTy)->getElementType();
       //    QualType pointerTy = C.getPointerType(elemTy);
     }
+
+    AnalyzerOptions &Opts =
+        StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
+    if (Opts.ShouldSupportSymbolicIntegerCasts) {
+      const MemRegion *R = V.getRegion();
+      if (R && !OriginalTy.isNull())
+        if (const auto *SR = dyn_cast<SymbolicRegion>(R))
+          return simplifySymbolCast(SR->getSymbol(), CastTy);
+    }
+
     const unsigned BitWidth = Context.getIntWidth(CastTy);
     return makeLocAsInteger(Val.castAs<Loc>(), BitWidth);
   }
 
   // Pointer to pointer.
   if (Loc::isLocType(CastTy)) {
 
     if (IsUnknownOriginalType) {

 }
 
 SVal SValBuilder::evalCastSubKind(nonloc::PointerToMember V, QualType CastTy,
                                   QualType OriginalTy) {
   // Member pointer to whatever.
   return V;
 }
 
-SVal clang::ento::SValBuilder::simplifySymbolCast(nonloc::SymbolVal V,
+SVal clang::ento::SValBuilder::simplifySymbolCast(SymbolRef SE,
                                                   QualType CastTy) {
+  QualType T = Context.getCanonicalType(SE->getType());
+  if (!isa<SymbolCast>(SE))
+    return makeNonLoc(SE, T, CastTy);
+
   // We use seven conditions to recognize a simplification case.
   // For the clarity let `CastTy` be `C`, SE->getType() - `T`, root type - `R`,
   // prefix `u` for unsigned, `s` for signed, no prefix - any sign:
   // E.g. (char)(short)(uint x)
   //      ( sC )( sT  )( uR  x)
   //
   // C === R (the same type)
   //  (char)(char x) -> (char x)
SVal clang::ento::SValBuilder::simplifySymbolCast(SymbolRef SE,
   // C > sT == sR
   //  (int)(char)(char x) -> (int)(char x)
   //  (uint)(short)(short x) -> (uint)(short x)
   // C > uT == uR
   //  (int)(uchar)(uchar x) -> (int)(uchar x)
   //  (uint)(ushort)(ushort x) -> (uint)(ushort x)
   //  (llong)(ulong)(uint x) -> (llong)(uint x) (sizeof(ulong) == sizeof(uint))
 
-  SymbolRef SE = V.getSymbol();
-  QualType T = Context.getCanonicalType(SE->getType());
-
-  if (T == CastTy)
-    return V;
-
-  if (!isa<SymbolCast>(SE))
-    return makeNonLoc(SE, T, CastTy);
-
   SymbolRef RootSym = cast<SymbolCast>(SE)->getOperand();
   QualType RT = RootSym->getType().getCanonicalType();
 
   BasicValueFactory &BVF = getBasicValueFactory();
   APSIntType CTy = BVF.getAPSIntType(CastTy);
   APSIntType TTy = BVF.getAPSIntType(T);
 
   const auto WC = CTy.getBitWidth();
SVal clang::ento::SValBuilder::simplifySymbolCast(SymbolRef SE,
   const bool UT = TTy.isUnsigned();
   const bool UR = RTy.isUnsigned();
 
   if (((WT > WR) && (UR || !UT)) || ((WT == WR) && (UT == UR)))
     return makeNonLoc(RootSym, RT, CastTy);
 
   return makeNonLoc(SE, T, CastTy);
 }
+
+SVal clang::ento::SValBuilder::simplifySymbolCast(nonloc::SymbolVal V,
+                                                  QualType CastTy) {
+  SymbolRef SE = V.getSymbol();
+  QualType T = Context.getCanonicalType(SE->getType());
+
+  if (T == CastTy) {
+    auto SV = makeSymbolVal(SE);
+    assert(SV == V);
+    return SV;
+  }
+  return simplifySymbolCast(SE, CastTy);
+}

clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp

+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config support-symbolic-integer-casts=true \
+// RUN:   -triple x86_64-pc-linux-gnu \
+// RUN:   -verify
+
+template <class T>
+void clang_analyzer_dump(T);
+
+void test_memory_region_to_integer_cast(int *p) {
+  long p_int = (long)p;
+  clang_analyzer_dump(p);     // expected-warning{{&SymRegion{reg_$0<int * p>}}}
+  clang_analyzer_dump(p_int); // expected-warning{{(long) (reg_$0<int * p>)}}
+}
+
+void test_label_to_integer_cast(bool coin) {
+  int x = 0;
+p: ++x;
+  if (coin)
+    goto p;
+
+  // FIXME, below we should produce a SymbolCast instead of the LocAsInteger.
+  // However, for that we'd need to be able to store a loc::GotoLabel SVal
+  // inside the SymbolCast. But currently we can put only another SymExpr
+  // inside a SymbolCast.
+
+  // Use of GNU address-of-label extension.
+  clang_analyzer_dump((long)&&p); // expected-warning{{&&p [as 64 bit integer]}}
+}

clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp

+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config support-symbolic-integer-casts=true \
+// RUN:   -verify
+
+template <class T>
+void clang_analyzer_dump(T);
+
+void clang_analyzer_eval(bool);
+
+void test_memory_region_to_integer_cast_and_symbol_simplification(int *p) {
+  long p_as_integer = (long)p;
+  if (42 - p_as_integer < 42)
+    return;
+  // 42 - p_as_integer >= 42
+  // p_as_integer <= 0
+
+  if (p)
+    return;
+  clang_analyzer_eval(p == 0);            // expected-warning{{TRUE}}
+  clang_analyzer_eval(p_as_integer == 0); // expected-warning{{UNKNOWN}}
+
+  if (p_as_integer)
+    return;
+  clang_analyzer_eval(p == 0);            // expected-warning{{TRUE}}
+  clang_analyzer_eval(p_as_integer == 0); // expected-warning{{TRUE}}
+
+  (void)p_as_integer;
+  (void)p;
+}