vabridgers updated this revision to Diff 358792. vabridgers added a comment.
address comments, add test case suggested by @bjope Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105974/new/ https://reviews.llvm.org/D105974 Files: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp clang/test/Analysis/solver-sym-simplification-ptr-bool.cl Index: clang/test/Analysis/solver-sym-simplification-ptr-bool.cl =================================================================== --- /dev/null +++ clang/test/Analysis/solver-sym-simplification-ptr-bool.cl @@ -0,0 +1,23 @@ +// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown -target-cpu verde -analyze -analyzer-checker=core %s + +// expected-no-diagnostics + +// This test case covers an issue found in the static analyzer +// solver where pointer sizes were assumed. Pointer sizes may vary on other +// architectures. This issue was originally discovered on a downstream, +// custom target, this assert occurs on the custom target and this one +// without the fix, and is fixed with this change. +// +// The assertion appears to be happening as a result of evaluating the +// SymIntExpr (reg_$0<int * p>) != 0U in VisitSymIntExpr located in +// SimpleSValBuilder.cpp. The LHS is evaluated to 32b and the RHS is +// evaluated to 16b. This eventually leads to the assertion in APInt.h. +// +// APInt.h:1151: bool llvm::APInt::operator==(const llvm::APInt &) const: Assertion `BitWidth == RHS.BitWidth && "Comparison requires equal bit widths"' +// +void test(__attribute__((address_space(256))) int * p) { + __attribute__((address_space(256))) int * q = p-1; + if (q) {} + if (q) {} + (void)q; +} Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -712,9 +712,23 @@ // symbols to use, only content metadata. return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR)); - if (const SymbolicRegion *SymR = R->getSymbolicBase()) - return makeNonLoc(SymR->getSymbol(), BO_NE, - BasicVals.getZeroWithPtrWidth(), CastTy); + if (const SymbolicRegion *SymR = R->getSymbolicBase()) { + SymbolRef Sym = SymR->getSymbol(); + QualType Ty = Sym->getType(); + // This change is needed for architectures with varying + // pointer widths. See the amdgcn opencl reproducer with + // this change as an example: solver-sym-simplification-ptr-bool.cl + // FIXME: We could encounter a reference here, + // try returning a concrete 'true' since it might + // be easier on the solver. + // FIXME: Cleanup remainder of `getZeroWithPtrWidth ()` + // and `getIntWithPtrWidth()` functions to prevent future + // confusion + const llvm::APSInt &Zero = Ty->isReferenceType() + ? BasicVals.getZeroWithPtrWidth() + : BasicVals.getZeroWithTypeSize(Ty); + return makeNonLoc(Sym, BO_NE, Zero, CastTy); + } // Non-symbolic memory regions are always true. return makeTruthVal(true, CastTy); }
Index: clang/test/Analysis/solver-sym-simplification-ptr-bool.cl =================================================================== --- /dev/null +++ clang/test/Analysis/solver-sym-simplification-ptr-bool.cl @@ -0,0 +1,23 @@ +// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown -target-cpu verde -analyze -analyzer-checker=core %s + +// expected-no-diagnostics + +// This test case covers an issue found in the static analyzer +// solver where pointer sizes were assumed. Pointer sizes may vary on other +// architectures. This issue was originally discovered on a downstream, +// custom target, this assert occurs on the custom target and this one +// without the fix, and is fixed with this change. +// +// The assertion appears to be happening as a result of evaluating the +// SymIntExpr (reg_$0<int * p>) != 0U in VisitSymIntExpr located in +// SimpleSValBuilder.cpp. The LHS is evaluated to 32b and the RHS is +// evaluated to 16b. This eventually leads to the assertion in APInt.h. +// +// APInt.h:1151: bool llvm::APInt::operator==(const llvm::APInt &) const: Assertion `BitWidth == RHS.BitWidth && "Comparison requires equal bit widths"' +// +void test(__attribute__((address_space(256))) int * p) { + __attribute__((address_space(256))) int * q = p-1; + if (q) {} + if (q) {} + (void)q; +} Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -712,9 +712,23 @@ // symbols to use, only content metadata. return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR)); - if (const SymbolicRegion *SymR = R->getSymbolicBase()) - return makeNonLoc(SymR->getSymbol(), BO_NE, - BasicVals.getZeroWithPtrWidth(), CastTy); + if (const SymbolicRegion *SymR = R->getSymbolicBase()) { + SymbolRef Sym = SymR->getSymbol(); + QualType Ty = Sym->getType(); + // This change is needed for architectures with varying + // pointer widths. See the amdgcn opencl reproducer with + // this change as an example: solver-sym-simplification-ptr-bool.cl + // FIXME: We could encounter a reference here, + // try returning a concrete 'true' since it might + // be easier on the solver. + // FIXME: Cleanup remainder of `getZeroWithPtrWidth ()` + // and `getIntWithPtrWidth()` functions to prevent future + // confusion + const llvm::APSInt &Zero = Ty->isReferenceType() + ? BasicVals.getZeroWithPtrWidth() + : BasicVals.getZeroWithTypeSize(Ty); + return makeNonLoc(Sym, BO_NE, Zero, CastTy); + } // Non-symbolic memory regions are always true. return makeTruthVal(true, CastTy); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits