-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ClangSA] APInt::getSExtValue() crash in SValBuilder::evalIntegralCast() with _BitInt of size > 128 #61960
Comments
@llvm/issue-subscribers-clang-static-analyzer |
evalIntegralCast 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. <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 ... #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/llvm-project#61960 Reviewed By: donat.nagy
@vabridgers Could you please link the PR here? |
Ah, now I see. It wasn't even on llvm/llvm-project; It was on your fork. |
Hi @steakhal! I agree, my first time using this new workflow. I'm probably making mistakes, but only way to learn :) haha! Here are links to three pull requests I created related to _BitInt impact on tidy and csa. #65887 - for this github issue created by @AdamMagierFOSS #65888 - a tidy issue, for now, just proposing we skip the in magic numbers for _BitInt() #65889 - a tidy issue - BitIntType has no getKind() issue, so this exposed a crash in VisitIntegerLiteral. The "fix" is to detect a _BitInt() and use an ID derived from the _BitInt() value. |
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
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 ... #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: #61960 Reviewed By: donat.nagy
) 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
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
evalIntegralCast was using makeIntVal, and when _BitInt() types were introduced this exposed a crash in evalIntegralCast as a result. This is a reapply of a previous patch that failed post merge on the arm buildbots, because arm cannot handle large BitInts. Pinning the triple for the testcase solves that problem. 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 ... #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: #61960 Reviewed By: donat.nagy
Observed on most recent main branch as of earlier today, commit 9ef7013.
Minimal Reproducer (
test.c
):Command:
Crash Output:
The same crash occurs when using
unsigned _BitInt
, but the assert fires on getZExtValue() instead of getSExtValue().The text was updated successfully, but these errors were encountered: