NoQ added a comment.

Hi, thanks!, i think this is correct. As in, `LocAsInteger` was clearly a 
mistake to begin with, but this change shouldn't make it worse.

You should be able to get away with not supporting comparisons between regions 
without symbolic base [as integers] and concrete integers because they usually 
yield fairly dumb values anyway. Eg., it's //impossible// for an address of a 
variable to be equal to a null pointer, and whether a variable's address is 
currently equal to `0xbadf00d` is //undefined// anyway - it will cause problems 
when we try to symbolicate our undefined values, but for now we prefer to 
interrupt the analysis whenever they actually end up being used (which causes 
the problem to disappear because symbolication only matters when we use the 
symbol more than once).



================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:574
           llvm::APSInt i = rhs.castAs<nonloc::ConcreteInt>().getValue();
-          BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
+          // FIXME: Handle non-default address spaces for non-symbolic regions.
+          if (SymbolRef lSym = lhs.getAsLocSymbol(true))
----------------
Pls be more specific in this comment that we're making this extra check in 
order to support non-default address spaces. Eg., "If the region has a symbolic 
base, pay attention to the type; it might be coming from a non-default address 
space. For non-symbolic regions it doesn't matter that much because such 
comparisons would most likely evaluate to concrete false anyway. FIXME: We 
might still need to handle the non-comparison case."


================
Comment at: test/Analysis/ptr-cmp-const-trunc.cl:10
+  if ((uint64_t)p <= 1UL << 32)
+    bar(p);
+}
----------------
Pls add a `// no-warning` here, so that it was clear what the test is about.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58665



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

Reply via email to