[clang] [C23] Fix crash with _BitInt running clang-tidy #65889
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-tidy ChangesThis crash was exposed recently in our randomized testing. _BitInts were not being handled properly during IntegerLiteral visitation. This patch addresses the problem for now. The BitIntType has no getKind() method, so the FoldingSetID is taken from the APInt value representing the _BitInt(), similar to other methods in StmtProfile.cpp. Crash seen (summary form): clang-tidy: /llvm/include/llvm/Support/Casting.h:566: decltype(auto) llvm::cast(const From&) [with To = clang::BuiltinType; From = clang::QualType]: Assertion `isa(Val) && "cast() argument of incompatible type!"' failed ... clang::QualType>(clang::QualType const&) /llvm/include/llvm/Support/Casting.h:566:3 #10 clang::BuiltinType const* clang::Type::castAsclang::BuiltinType() const /tools/clang/include/clang/AST/TypeNodes.inc:86:1 #11 (anonymous namespace)::StmtProfiler::VisitIntegerLiteral( clang::IntegerLiteral const*) /clang/lib/AST/StmtProfile.cpp:1362:64 #12 clang::StmtVisitorBase<llvm::make_const_ptr, (anonymous namespace)::StmtProfiler, void>::Visit(clang::Stmt const*) /clang/include/clang/AST/StmtNodes.inc:1225:1 ... Reviewed By: donat.nagyFull diff: https://github.com/llvm/llvm-project/pull/65889.diff 2 Files Affected:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions-bitint-no-crash.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions-bitint-no-crash.c
new file mode 100644
index 000000000000000..55921205f34875e
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions-bitint-no-crash.c
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s bugprone-inc-dec-in-conditions %t
+
+#define foo(x) \
+ ({ \
+ _BitInt(5) y = x; \
+ 16777215wb ?: ++y; \
+ })
+
+_BitInt(8) v_401_0() { 0 && foo(0); }
+// CHECK-MESSAGES: warning
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 27f71edd6f99b32..0a6bc388bd73e18 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -1333,7 +1333,15 @@ void StmtProfiler::VisitPredefinedExpr(const PredefinedExpr *S) {
void StmtProfiler::VisitIntegerLiteral(const IntegerLiteral *S) {
VisitExpr(S);
S->getValue().Profile(ID);
- ID.AddInteger(S->getType()->castAs()->getKind());
+
+ int FoldingSetID = 0;
+
+ if (S->getType()->isBitIntType())
+ FoldingSetID = S->getValue().getSExtValue();
+ else
+ FoldingSetID = S->getType()->castAs()->getKind();
+
+ ID.AddInteger(FoldingSetID);
}
void StmtProfiler::VisitFixedPointLiteral(const FixedPointLiteral *S) {
|
|
@llvm/pr-subscribers-clang ChangesThis crash was exposed recently in our randomized testing. _BitInts were not being handled properly during IntegerLiteral visitation. This patch addresses the problem for now. The BitIntType has no getKind() method, so the FoldingSetID is taken from the APInt value representing the _BitInt(), similar to other methods in StmtProfile.cpp. Crash seen (summary form): clang-tidy: /llvm/include/llvm/Support/Casting.h:566: decltype(auto) llvm::cast(const From&) [with To = clang::BuiltinType; From = clang::QualType]: Assertion `isa(Val) && "cast() argument of incompatible type!"' failed ... clang::QualType>(clang::QualType const&) /llvm/include/llvm/Support/Casting.h:566:3 #10 clang::BuiltinType const* clang::Type::castAsclang::BuiltinType() const /tools/clang/include/clang/AST/TypeNodes.inc:86:1 #11 (anonymous namespace)::StmtProfiler::VisitIntegerLiteral( clang::IntegerLiteral const*) /clang/lib/AST/StmtProfile.cpp:1362:64 #12 clang::StmtVisitorBase<llvm::make_const_ptr, (anonymous namespace)::StmtProfiler, void>::Visit(clang::Stmt const*) /clang/include/clang/AST/StmtNodes.inc:1225:1 ... Reviewed By: donat.nagyFull diff: https://github.com/llvm/llvm-project/pull/65889.diff 2 Files Affected:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions-bitint-no-crash.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions-bitint-no-crash.c
new file mode 100644
index 000000000000000..55921205f34875e
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions-bitint-no-crash.c
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s bugprone-inc-dec-in-conditions %t
+
+#define foo(x) \
+ ({ \
+ _BitInt(5) y = x; \
+ 16777215wb ?: ++y; \
+ })
+
+_BitInt(8) v_401_0() { 0 && foo(0); }
+// CHECK-MESSAGES: warning
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 27f71edd6f99b32..0a6bc388bd73e18 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -1333,7 +1333,15 @@ void StmtProfiler::VisitPredefinedExpr(const PredefinedExpr *S) {
void StmtProfiler::VisitIntegerLiteral(const IntegerLiteral *S) {
VisitExpr(S);
S->getValue().Profile(ID);
- ID.AddInteger(S->getType()->castAs()->getKind());
+
+ int FoldingSetID = 0;
+
+ if (S->getType()->isBitIntType())
+ FoldingSetID = S->getValue().getSExtValue();
+ else
+ FoldingSetID = S->getType()->castAs()->getKind();
+
+ ID.AddInteger(FoldingSetID);
}
void StmtProfiler::VisitFixedPointLiteral(const FixedPointLiteral *S) {
|
clang-tools-extra/test/clang-tidy/checkers/bugprone/inc-dec-in-conditions-bitint-no-crash.c
Outdated
Show resolved
Hide resolved
vabridgers
force-pushed
the
bitint-integerliteral-fix
branch
from
1311ae7 to
753f8fd
Compare
|
The status above is showing 1 change requested, but I think I've addressed all comments. Please review at your convenience. Thank you. |
vabridgers
force-pushed
the
bitint-integerliteral-fix
branch
from
753f8fd to
73d3a59
Compare
vabridgers
force-pushed
the
bitint-integerliteral-fix
branch
from
73d3a59 to
b7d3ca2
Compare
This crash was exposed recently in our randomized testing. _BitInts were not being handled properly during IntegerLiteral visitation. This patch addresses the problem for now. The BitIntType has no getKind() method, so the FoldingSetID is taken from the APInt value representing the _BitInt(), similar to other methods in StmtProfile.cpp. Seems also the const qualifier was missing on the nonstatic profile method of the BitIntType class. Crash seen (summary form): clang-tidy: <src-root>/llvm/include/llvm/Support/Casting.h:566: decltype(auto) llvm::cast(const From&) [with To = clang::BuiltinType; From = clang::QualType]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed ... llvm#9 <address> decltype(auto) llvm::cast<clang::BuiltinType, clang::QualType>(clang::QualType const&) <src-root>/llvm/include/llvm/Support/Casting.h:566:3 llvm#10 <address> clang::BuiltinType const* clang::Type::castAs<clang::BuiltinType>() const <bin-root>/tools/clang/include/clang/AST/TypeNodes.inc:86:1 llvm#11 <address> (anonymous namespace)::StmtProfiler::VisitIntegerLiteral( clang::IntegerLiteral const*) <src-root>/clang/lib/AST/StmtProfile.cpp:1362:64 llvm#12 <address> clang::StmtVisitorBase<llvm::make_const_ptr, (anonymous namespace)::StmtProfiler, void>::Visit(clang::Stmt const*) <src-root>/clang/include/clang/AST/StmtNodes.inc:1225:1 ... Reviewed By: donat.nagy
vabridgers
force-pushed
the
bitint-integerliteral-fix
branch
from
b7d3ca2 to
79e1ab6
Compare
|
I've updated the review per recent comments, please review at your convenience. Thanks! |
| ID.AddInteger(S->getType()->castAs<BuiltinType>()->getKind()); | ||
|
|
||
| QualType T = S->getType(); | ||
| ID.AddInteger(T->getTypeClass()); |
There was a problem hiding this comment.
This addition of the type class doesn't observe the Canonical attribute, leading to IntegerLiterals with different types but same canonical types mismatching even when Canonical is true.
There was a problem hiding this comment.
Oooh, good catch! Thanks for that! Can you contribute a patch to fix that? (That is, canonicalize T).
There was a problem hiding this comment.
Sure thing. I haven't been able to find a reproducer for upstream clang, so I don't have a test case, but it doesn't seem to break any tests at least. Waiting for it to push to github now.
This crash was exposed recently in our randomized testing. _BitInts were not being handled properly during IntegerLiteral visitation. This patch addresses the problem for now.
The BitIntType has no getKind() method, so the FoldingSetID is taken from the APInt value representing the _BitInt(), similar to other methods in StmtProfile.cpp.
Crash seen (summary form):
clang-tidy: /llvm/include/llvm/Support/Casting.h:566: decltype(auto) llvm::cast(const From&) [with To = clang::BuiltinType; From = clang::QualType]: Assertion `isa(Val) && "cast() argument of incompatible type!"' failed
Reviewed By: donat.nagy