[analyzer] Fix crash analyzing _BitInt() in evalIntegralCast#65887
[analyzer] Fix crash analyzing _BitInt() in evalIntegralCast#65887vabridgers merged 1 commit intollvm:mainfrom
Conversation
|
@llvm/pr-subscribers-clang ChangesevalIntegralCast is using APInt method to get the value of _BitInt() values after _BitInt() changes were introduced. Some of those methods assume values are less than or equal to 64-bits, which is not true for _BitInt() types. This change simply side steps that issue if the _BitInt() type is greater than 64 bits. This was caught with our internal randomized testing. /llvm/include/llvm/ADT/APInt.h:1510: ... /llvm/include/llvm/ADT/APInt.h:1510:5 llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>, clang::ento::SVal, clang::QualType, clang::QualType) /clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:607:24 clang::Expr const*, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) /clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:413:61 ... Fixes: #61960 Reviewed By: donat.nagyFull diff: https://github.com/llvm/llvm-project/pull/65887.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 4fe828bdf7681fc..c9765e3a653e30a 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -598,6 +598,12 @@ SVal SValBuilder::evalIntegralCast(ProgramStateRef state, SVal val,
APSIntType ToType(getContext().getTypeSize(castTy),
castTy->isUnsignedIntegerType());
llvm::APSInt ToTypeMax = ToType.getMaxValue();
+ // With the introduction of _BitInt(), integral types can be
+ // > 64 bits. So check for this and skip the size checks
+ // falling back to making a non loc return type.
+ if (ToTypeMax.getSignificantBits() > 64) {
+ return makeNonLoc(se, originalTy, castTy);
+ }
NonLoc ToTypeMaxVal =
makeIntVal(ToTypeMax.isUnsigned() ? ToTypeMax.getZExtValue()
: ToTypeMax.getSExtValue(),
diff --git a/clang/test/Analysis/bitint-no-crash.c b/clang/test/Analysis/bitint-no-crash.c
new file mode 100644
index 000000000000000..6fa041974a3c981
--- /dev/null
+++ b/clang/test/Analysis/bitint-no-crash.c
@@ -0,0 +1,11 @@
+ // RUN: %clang_analyze_cc1 -analyzer-checker=core \
+ // RUN: -analyzer-checker=debug.ExprInspection \
+ // RUN: -verify %s
+
+// Don't crash when using _BitInt()
+// expected-no-diagnostics
+_BitInt(256) a;
+_BitInt(129) b;
+void c() {
+ b = a;
+}
|
|
I guess this is one sideeffect of |
NagyDonat
left a comment
There was a problem hiding this comment.
I agree that it would be useful to systematically check the use of the APSInt -> uint64_t conversions, because it's likely that there are other ones that can lead to crashes.
82992c1 to
ae0b0a0
Compare
|
The status above shows 1 change requested, but I believe I've resolved the requested changes. Please review at your convenience. Thank you. |
NagyDonat
left a comment
There was a problem hiding this comment.
Don't forget to update the commit message!
steakhal
left a comment
There was a problem hiding this comment.
What are the semantics of assigning a wider biting to a smaller one?
BTW LGTM.
evalIntegralCast was using makeIntVal, and when _BitInt() types were introduced this exposed a crash in evalIntegralCast as a result. Improve evalIntegralCast to use makeIntVal more efficiently to avoid the crash exposed by use of _BitInt. This was caught with our internal randomized testing. <src-root>/llvm/include/llvm/ADT/APInt.h:1510: int64_t llvm::APInt::getSExtValue() const: Assertion `getSignificantBits() <= 64 && "Too many bits for int64_t"' failed.a ... llvm#9 <address> llvm::APInt::getSExtValue() const <src-root>/llvm/include/llvm/ADT/APInt.h:1510:5 llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>, clang::ento::SVal, clang::QualType, clang::QualType) <src-root>/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:607:24 clang::Expr const*, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) <src-root>/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:413:61 ... Fixes: llvm#61960 Reviewed By: donat.nagy
ae0b0a0 to
ea63aae
Compare
|
I reverted this patch since buildbots have been complaining for almost an hour. |
) evalIntegralCast was using makeIntVal, and when _BitInt() types were introduced this exposed a crash in evalIntegralCast as a result. Improve evalIntegralCast to use makeIntVal more efficiently to avoid the crash exposed by use of _BitInt. This was caught with our internal randomized testing. <src-root>/llvm/include/llvm/ADT/APInt.h:1510: int64_t llvm::APInt::getSExtValue() const: Assertion `getSignificantBits() <= 64 && "Too many bits for int64_t"' failed.a ... llvm#9 <address> llvm::APInt::getSExtValue() const <src-root>/llvm/include/llvm/ADT/APInt.h:1510:5 llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>, clang::ento::SVal, clang::QualType, clang::QualType) <src-root>/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:607:24 clang::Expr const*, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) <src-root>/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:413:61 ... Fixes: llvm#61960 Reviewed By: donat.nagy
…lvm#65887)" This reverts commit 4898c33. Lots of buildbots are failing, probably because lots of targets not supporting large _BitInt types.
evalIntegralCast was using makeIntVal, and when _BitInt() types were
introduced this exposed a crash in evalIntegralCast as a result.
Improve evalIntegralCast to use makeIntVal more efficiently to avoid the
crash exposed by use of _BitInt.
This was caught with our internal randomized testing.
/llvm/include/llvm/ADT/APInt.h:1510:
int64_t llvm::APInt::getSExtValue() const: Assertion
`getSignificantBits() <= 64 && "Too many bits for int64_t"' failed.a
...
llvm::APInt::getSExtValue() const#9
/llvm/include/llvm/ADT/APInt.h:1510:5
llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>,
clang::ento::SVal, clang::QualType, clang::QualType)
/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:607:24
clang::Expr const*, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&)
/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:413:61
...
Fixes: #61960
Reviewed By: donat.nagy