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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
