From 95ac7cf69dbd683f70b123446115fe3893844526 Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Thu, 8 May 2025 17:48:41 -0700 Subject: [PATCH 1/5] [StaticAnalyzer] Handle __builtin_bit_cast Previously, CSA did not handle __builtin_bit_cast correctly. It evaluated the LvalueToRvalue conversion for the casting expression, but did not actually convert the value of the expression to be of the destination type. This commit fixes the problem. rdar://149987320 --- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 26 ++++++++++++++++++- clang/test/Analysis/builtin_bitcast.cpp | 14 ++++++++-- clang/test/Analysis/exercise-ps.c | 2 +- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 3d0a69a515ab8..f2f640f459776 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -287,10 +287,34 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, if (CastE->getCastKind() == CK_LValueToRValue || CastE->getCastKind() == CK_LValueToRValueBitCast) { + ExplodedNodeSet dstEvalLoad; + for (ExplodedNode *subExprNode : dstPreStmt) { ProgramStateRef state = subExprNode->getState(); const LocationContext *LCtx = subExprNode->getLocationContext(); - evalLoad(Dst, CastE, CastE, subExprNode, state, state->getSVal(Ex, LCtx)); + evalLoad(dstEvalLoad, CastE, CastE, subExprNode, state, + state->getSVal(Ex, LCtx)); + } + if (CastE->getCastKind() == CK_LValueToRValue) { + Dst.insert(dstEvalLoad); + return; + } + assert(CastE->getCastKind() == CK_LValueToRValueBitCast && + "unexpected cast kind"); + // Need to simulate the actual cast operation: + StmtNodeBuilder Bldr(dstEvalLoad, Dst, *currBldrCtx); + + for (ExplodedNode *Node : dstEvalLoad) { + ProgramStateRef state = Node->getState(); + const LocationContext *LCtx = Node->getLocationContext(); + // getAsRegion should always be successful since Ex is an lvalue: + SVal OrigV = state->getSVal(state->getSVal(Ex, LCtx).getAsRegion()); + SVal CastedV = + svalBuilder.evalCast(svalBuilder.simplifySVal(state, OrigV), + CastE->getType(), Ex->getType()); + + state = state->BindExpr(CastE, LCtx, CastedV); + Bldr.generateNode(CastE, Node, state); } return; } diff --git a/clang/test/Analysis/builtin_bitcast.cpp b/clang/test/Analysis/builtin_bitcast.cpp index 5a0d9e7189b8e..9309ca7785a92 100644 --- a/clang/test/Analysis/builtin_bitcast.cpp +++ b/clang/test/Analysis/builtin_bitcast.cpp @@ -39,7 +39,7 @@ struct A { } }; void gh_69922(size_t p) { - // expected-warning-re@+1 {{(reg_${{[0-9]+}}) & 1U}} + // expected-warning@+1 {{Unknown}} clang_analyzer_dump(__builtin_bit_cast(A*, p & 1)); __builtin_bit_cast(A*, p & 1)->set(2); // no-crash @@ -49,5 +49,15 @@ void gh_69922(size_t p) { // store to the member variable `n`. clang_analyzer_dump(__builtin_bit_cast(A*, p & 1)->n); // Ideally, this should print "2". - // expected-warning-re@-1 {{(reg_${{[0-9]+}}) & 1U}} + // expected-warning@-1 {{Unknown}} +} + +namespace { + typedef unsigned long uintptr_t; + + bool previously_crash(const void *& ptr) { + clang_analyzer_dump(__builtin_bit_cast(void*, static_cast(-1))); + // expected-warning-re@-1 {{{{[0-9]+}} (Loc)}} + return ptr == __builtin_bit_cast(void*, static_cast(-1)); + } } diff --git a/clang/test/Analysis/exercise-ps.c b/clang/test/Analysis/exercise-ps.c index 50643d5b04687..21d97a364e190 100644 --- a/clang/test/Analysis/exercise-ps.c +++ b/clang/test/Analysis/exercise-ps.c @@ -41,7 +41,7 @@ void f4(char *array) { _Static_assert(sizeof(int) == 4, "Wrong triple for the test"); - clang_analyzer_dump_int(__builtin_bit_cast(int, b)); // expected-warning {{lazyCompoundVal}} + clang_analyzer_dump_int(__builtin_bit_cast(int, b)); // expected-warning {{Unknown}} clang_analyzer_dump_int(array[__builtin_bit_cast(int, b)]); // expected-warning {{Unknown}} array[__builtin_bit_cast(int, b)] = 0x10; // no crash From 1d20c183135e20fad03a7598d5771540db44f0b2 Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Thu, 8 May 2025 18:31:21 -0700 Subject: [PATCH 2/5] For __builtin_bit_cast, we just need to evalLocation for the second argument instead of evalLoad. --- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index f2f640f459776..e84fda8950e64 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -285,23 +285,27 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, ExplodedNodeSet dstPreStmt; getCheckerManager().runCheckersForPreStmt(dstPreStmt, Pred, CastE, *this); - if (CastE->getCastKind() == CK_LValueToRValue || - CastE->getCastKind() == CK_LValueToRValueBitCast) { + if (CastE->getCastKind() == CK_LValueToRValue) { + for (ExplodedNode *subExprNode : dstPreStmt) { + ProgramStateRef state = subExprNode->getState(); + const LocationContext *LCtx = subExprNode->getLocationContext(); + evalLoad(Dst, CastE, CastE, subExprNode, state, state->getSVal(Ex, LCtx)); + } + return; + } + if (CastE->getCastKind() == CK_LValueToRValueBitCast) { + // Handle `__builtin_bit_cast`: ExplodedNodeSet dstEvalLoad; + // Simulate the lvalue-to-rvalue conversion on `Ex`: for (ExplodedNode *subExprNode : dstPreStmt) { ProgramStateRef state = subExprNode->getState(); const LocationContext *LCtx = subExprNode->getLocationContext(); - evalLoad(dstEvalLoad, CastE, CastE, subExprNode, state, - state->getSVal(Ex, LCtx)); - } - if (CastE->getCastKind() == CK_LValueToRValue) { - Dst.insert(dstEvalLoad); - return; + evalLocation(dstEvalLoad, CastE, Ex, subExprNode, state, + state->getSVal(Ex, LCtx), true); } - assert(CastE->getCastKind() == CK_LValueToRValueBitCast && - "unexpected cast kind"); - // Need to simulate the actual cast operation: + // Simulate the operation that actually casts the original value to a new + // value of the destination type : StmtNodeBuilder Bldr(dstEvalLoad, Dst, *currBldrCtx); for (ExplodedNode *Node : dstEvalLoad) { From 24a6aadba7a85d2dee449f870998c81a83acde27 Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Fri, 9 May 2025 12:06:30 -0700 Subject: [PATCH 3/5] Change Variable Names to follow the UpperCamelCase standard --- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index e84fda8950e64..172d0a66daf04 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -282,43 +282,43 @@ ProgramStateRef ExprEngine::handleLValueBitCast( void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, ExplodedNode *Pred, ExplodedNodeSet &Dst) { - ExplodedNodeSet dstPreStmt; - getCheckerManager().runCheckersForPreStmt(dstPreStmt, Pred, CastE, *this); + ExplodedNodeSet DstPreStmt; + getCheckerManager().runCheckersForPreStmt(DstPreStmt, Pred, CastE, *this); if (CastE->getCastKind() == CK_LValueToRValue) { - for (ExplodedNode *subExprNode : dstPreStmt) { - ProgramStateRef state = subExprNode->getState(); - const LocationContext *LCtx = subExprNode->getLocationContext(); - evalLoad(Dst, CastE, CastE, subExprNode, state, state->getSVal(Ex, LCtx)); + for (ExplodedNode *Node : DstPreStmt) { + ProgramStateRef State = Node->getState(); + const LocationContext *LCtx = Node->getLocationContext(); + evalLoad(Dst, CastE, CastE, Node, State, State->getSVal(Ex, LCtx)); } return; } if (CastE->getCastKind() == CK_LValueToRValueBitCast) { // Handle `__builtin_bit_cast`: - ExplodedNodeSet dstEvalLoad; + ExplodedNodeSet DstEvalLoc; // Simulate the lvalue-to-rvalue conversion on `Ex`: - for (ExplodedNode *subExprNode : dstPreStmt) { - ProgramStateRef state = subExprNode->getState(); - const LocationContext *LCtx = subExprNode->getLocationContext(); - evalLocation(dstEvalLoad, CastE, Ex, subExprNode, state, - state->getSVal(Ex, LCtx), true); + for (ExplodedNode *Node : DstPreStmt) { + ProgramStateRef State = Node->getState(); + const LocationContext *LCtx = Node->getLocationContext(); + evalLocation(DstEvalLoc, CastE, Ex, Node, State, State->getSVal(Ex, LCtx), + true); } // Simulate the operation that actually casts the original value to a new // value of the destination type : - StmtNodeBuilder Bldr(dstEvalLoad, Dst, *currBldrCtx); + StmtNodeBuilder Bldr(DstEvalLoc, Dst, *currBldrCtx); - for (ExplodedNode *Node : dstEvalLoad) { - ProgramStateRef state = Node->getState(); + for (ExplodedNode *Node : DstEvalLoc) { + ProgramStateRef State = Node->getState(); const LocationContext *LCtx = Node->getLocationContext(); // getAsRegion should always be successful since Ex is an lvalue: - SVal OrigV = state->getSVal(state->getSVal(Ex, LCtx).getAsRegion()); + SVal OrigV = State->getSVal(State->getSVal(Ex, LCtx).getAsRegion()); SVal CastedV = - svalBuilder.evalCast(svalBuilder.simplifySVal(state, OrigV), + svalBuilder.evalCast(svalBuilder.simplifySVal(State, OrigV), CastE->getType(), Ex->getType()); - state = state->BindExpr(CastE, LCtx, CastedV); - Bldr.generateNode(CastE, Node, state); + State = State->BindExpr(CastE, LCtx, CastedV); + Bldr.generateNode(CastE, Node, State); } return; } @@ -330,8 +330,8 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, if (const ExplicitCastExpr *ExCast=dyn_cast_or_null(CastE)) T = ExCast->getTypeAsWritten(); - StmtNodeBuilder Bldr(dstPreStmt, Dst, *currBldrCtx); - for (ExplodedNode *Pred : dstPreStmt) { + StmtNodeBuilder Bldr(DstPreStmt, Dst, *currBldrCtx); + for (ExplodedNode *Pred : DstPreStmt) { ProgramStateRef state = Pred->getState(); const LocationContext *LCtx = Pred->getLocationContext(); From a6506b7713c93a3a8d5f28848b13e699c00e96df Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Fri, 9 May 2025 13:19:22 -0700 Subject: [PATCH 4/5] Add handling for Loc::ConcreteInt --- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 17 +++++++++++------ clang/test/Analysis/builtin_bitcast.cpp | 7 ++++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 172d0a66daf04..460cd17ee8a00 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -311,12 +311,17 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, for (ExplodedNode *Node : DstEvalLoc) { ProgramStateRef State = Node->getState(); const LocationContext *LCtx = Node->getLocationContext(); - // getAsRegion should always be successful since Ex is an lvalue: - SVal OrigV = State->getSVal(State->getSVal(Ex, LCtx).getAsRegion()); - SVal CastedV = - svalBuilder.evalCast(svalBuilder.simplifySVal(State, OrigV), - CastE->getType(), Ex->getType()); - + // Although `Ex` is an lvalue, it could have `Loc::ConcreteInt` kind + // (e.g., `(int *)123456`). In such cases, there is no MemRegion + // available and we can't get the value to be casted. + const MemRegion *MR = State->getSVal(Ex, LCtx).getAsRegion(); + SVal CastedV = UnknownVal(); + + if (MR) { + SVal OrigV = State->getSVal(MR); + CastedV = svalBuilder.evalCast(svalBuilder.simplifySVal(State, OrigV), + CastE->getType(), Ex->getType()); + } State = State->BindExpr(CastE, LCtx, CastedV); Bldr.generateNode(CastE, Node, State); } diff --git a/clang/test/Analysis/builtin_bitcast.cpp b/clang/test/Analysis/builtin_bitcast.cpp index 9309ca7785a92..de5367e2c4ee0 100644 --- a/clang/test/Analysis/builtin_bitcast.cpp +++ b/clang/test/Analysis/builtin_bitcast.cpp @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -verify %s \ -// RUN: -analyzer-checker=core,debug.ExprInspection +// RUN: -analyzer-checker=debug.ExprInspection -analyzer-disable-checker=core template void clang_analyzer_dump(T); using size_t = decltype(sizeof(int)); @@ -60,4 +60,9 @@ namespace { // expected-warning-re@-1 {{{{[0-9]+}} (Loc)}} return ptr == __builtin_bit_cast(void*, static_cast(-1)); } + + void check_loc_concreteInt() { + clang_analyzer_dump(__builtin_bit_cast(unsigned, *(reinterpret_cast(0xdeadbeef)))); + // expected-warning@-1 {{Unknown}} + } } From afdbece8285221cd36c53244f96b7a800b501c38 Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Mon, 12 May 2025 12:29:02 -0700 Subject: [PATCH 5/5] address comments --- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 3 +-- clang/test/Analysis/builtin_bitcast.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 460cd17ee8a00..886c51908b010 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -314,10 +314,9 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, // Although `Ex` is an lvalue, it could have `Loc::ConcreteInt` kind // (e.g., `(int *)123456`). In such cases, there is no MemRegion // available and we can't get the value to be casted. - const MemRegion *MR = State->getSVal(Ex, LCtx).getAsRegion(); SVal CastedV = UnknownVal(); - if (MR) { + if (const MemRegion *MR = State->getSVal(Ex, LCtx).getAsRegion()) { SVal OrigV = State->getSVal(MR); CastedV = svalBuilder.evalCast(svalBuilder.simplifySVal(State, OrigV), CastE->getType(), Ex->getType()); diff --git a/clang/test/Analysis/builtin_bitcast.cpp b/clang/test/Analysis/builtin_bitcast.cpp index de5367e2c4ee0..2ba32ec6d23d2 100644 --- a/clang/test/Analysis/builtin_bitcast.cpp +++ b/clang/test/Analysis/builtin_bitcast.cpp @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -verify %s \ -// RUN: -analyzer-checker=debug.ExprInspection -analyzer-disable-checker=core +// RUN: -analyzer-checker=core,debug.ExprInspection -analyzer-disable-checker=core.FixedAddressDereference template void clang_analyzer_dump(T); using size_t = decltype(sizeof(int));