[clang][analyzer][NFC] Add a helper for conjuring symbols at call events #137182
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Per suggestion in llvm#128251 (comment), adding a new helper function in `SValBuilder` to conjure a symbol when given a `CallEvent`. Tested manually (with assertions) that the `LocationContext *` obtained from the `CallEvent` are identical to those passed in the original argument.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Fangyi Zhou (fangyi-zhou) ChangesPer suggestion in Tested manually (with assertions) that the Full diff: https://github.com/llvm/llvm-project/pull/137182.diff 8 Files Affected:
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 54430d426a82a..3f3e6bdb9ff3d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -46,10 +46,10 @@ class Stmt;
namespace ento {
+class CallEvent;
class ConditionTruthVal;
class ProgramStateManager;
class StoreRef;
-
class SValBuilder {
virtual void anchor();
@@ -209,6 +209,12 @@ class SValBuilder {
const LocationContext *LCtx,
QualType type,
unsigned visitCount);
+ DefinedOrUnknownSVal conjureSymbolVal(const CallEvent &call, QualType type,
+ unsigned visitCount,
+ const void *symbolTag = nullptr);
+ DefinedOrUnknownSVal conjureSymbolVal(const CallEvent &call,
+ unsigned visitCount,
+ const void *symbolTag = nullptr);
/// Conjure a symbol representing heap allocated memory region.
///
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 39dcaf02dbe25..9d3eda8f7f982 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1514,8 +1514,7 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallEvent &Call,
// If we don't know how much we copied, we can at least
// conjure a return value for later.
if (lastElement.isUnknown())
- lastElement = C.getSValBuilder().conjureSymbolVal(
- nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
+ lastElement = C.getSValBuilder().conjureSymbolVal(Call, C.blockCount());
// The byte after the last byte copied is the return value.
state = state->BindExpr(Call.getOriginExpr(), LCtx, lastElement);
@@ -1665,8 +1664,7 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallEvent &Call,
State = CheckBufferAccess(C, State, Left, Size, AccessKind::read, CK);
if (State) {
// The return value is the comparison result, which we don't know.
- SVal CmpV = Builder.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx,
- C.blockCount());
+ SVal CmpV = Builder.conjureSymbolVal(Call, C.blockCount());
State = State->BindExpr(Call.getOriginExpr(), LCtx, CmpV);
C.addTransition(State);
}
@@ -1769,8 +1767,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C,
// no guarantee the full string length will actually be returned.
// All we know is the return value is the min of the string length
// and the limit. This is better than nothing.
- result = C.getSValBuilder().conjureSymbolVal(
- nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
+ result = C.getSValBuilder().conjureSymbolVal(Call, C.blockCount());
NonLoc resultNL = result.castAs<NonLoc>();
if (strLengthNL) {
@@ -1793,8 +1790,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C,
// If we don't know the length of the string, conjure a return
// value, so it can be used in constraints, at least.
if (result.isUnknown()) {
- result = C.getSValBuilder().conjureSymbolVal(
- nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
+ result = C.getSValBuilder().conjureSymbolVal(Call, C.blockCount());
}
}
@@ -2261,8 +2257,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
// If this is a stpcpy-style copy, but we were unable to check for a buffer
// overflow, we still need a result. Conjure a return value.
if (ReturnEnd && Result.isUnknown()) {
- Result = svalBuilder.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx,
- C.blockCount());
+ Result = svalBuilder.conjureSymbolVal(Call, C.blockCount());
}
}
// Set the return value.
@@ -2361,8 +2356,7 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallEvent &Call,
const StringLiteral *RightStrLiteral =
getCStringLiteral(C, state, Right.Expression, RightVal);
bool canComputeResult = false;
- SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, Call.getOriginExpr(),
- LCtx, C.blockCount());
+ SVal resultVal = svalBuilder.conjureSymbolVal(Call, C.blockCount());
if (LeftStrLiteral && RightStrLiteral) {
StringRef LeftStrRef = LeftStrLiteral->getString();
@@ -2467,16 +2461,13 @@ void CStringChecker::evalStrsep(CheckerContext &C,
// Overwrite the search string pointer. The new value is either an address
// further along in the same string, or NULL if there are no more tokens.
- State =
- State->bindLoc(*SearchStrLoc,
- SVB.conjureSymbolVal(getTag(), Call.getOriginExpr(),
- LCtx, CharPtrTy, C.blockCount()),
- LCtx);
+ State = State->bindLoc(
+ *SearchStrLoc,
+ SVB.conjureSymbolVal(Call, CharPtrTy, C.blockCount(), getTag()), LCtx);
} else {
assert(SearchStrVal.isUnknown());
// Conjure a symbolic value. It's the best we can do.
- Result = SVB.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx,
- C.blockCount());
+ Result = SVB.conjureSymbolVal(Call, C.blockCount());
}
// Set the return value, and finish.
@@ -2519,8 +2510,7 @@ void CStringChecker::evalStdCopyCommon(CheckerContext &C,
SValBuilder &SVB = C.getSValBuilder();
- SVal ResultVal =
- SVB.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
+ SVal ResultVal = SVB.conjureSymbolVal(Call, C.blockCount());
State = State->BindExpr(Call.getOriginExpr(), LCtx, ResultVal);
C.addTransition(State);
diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
index 6076a6bc78973..74eda4eaa599c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
@@ -130,8 +130,7 @@ void ErrnoTesterChecker::evalSetErrnoIfErrorRange(CheckerContext &C,
ProgramStateRef StateFailure = State->BindExpr(
Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(1, true));
- DefinedOrUnknownSVal ErrnoVal = SVB.conjureSymbolVal(
- nullptr, Call.getOriginExpr(), C.getLocationContext(), C.blockCount());
+ DefinedOrUnknownSVal ErrnoVal = SVB.conjureSymbolVal(Call, C.blockCount());
StateFailure = StateFailure->assume(ErrnoVal, true);
assert(StateFailure && "Failed to assume on an initial value.");
StateFailure =
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index cc1ced7358710..85e22daaedeec 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -931,8 +931,7 @@ bool RetainCountChecker::evalCall(const CallEvent &Call,
if (RetVal.isUnknown() ||
(hasTrustedImplementationAnnotation && !ResultTy.isNull())) {
SValBuilder &SVB = C.getSValBuilder();
- RetVal =
- SVB.conjureSymbolVal(nullptr, CE, LCtx, ResultTy, C.blockCount());
+ RetVal = SVB.conjureSymbolVal(Call, C.blockCount());
}
// Bind the value.
diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index 321388ad857f4..2feb05b142a36 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -853,7 +853,7 @@ void SmartPtrModeling::handleBoolConversion(const CallEvent &Call,
const LocationContext *LC = C.getLocationContext();
InnerPointerVal = C.getSValBuilder().conjureSymbolVal(
- CallExpr, LC, InnerPointerType, C.blockCount());
+ Call, InnerPointerType, C.blockCount());
State = State->set<TrackedRegionMap>(ThisRegion, InnerPointerVal);
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 9c0b79ab58618..4b97c51ca39fa 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -584,11 +584,9 @@ class StdLibraryFunctionsChecker
const Summary &Summary,
CheckerContext &C) const override {
SValBuilder &SVB = C.getSValBuilder();
- NonLoc ErrnoSVal =
- SVB.conjureSymbolVal(&Tag, Call.getOriginExpr(),
- C.getLocationContext(), C.getASTContext().IntTy,
- C.blockCount())
- .castAs<NonLoc>();
+ NonLoc ErrnoSVal = SVB.conjureSymbolVal(Call, C.getASTContext().IntTy,
+ C.blockCount(), &Tag)
+ .castAs<NonLoc>();
return errno_modeling::setErrnoForStdFailure(State, C, ErrnoSVal);
}
};
@@ -1482,7 +1480,7 @@ bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call,
const LocationContext *LC = C.getLocationContext();
const auto *CE = cast<CallExpr>(Call.getOriginExpr());
SVal V = C.getSValBuilder().conjureSymbolVal(
- CE, LC, CE->getType().getCanonicalType(), C.blockCount());
+ Call, CE->getType().getCanonicalType(), C.blockCount());
State = State->BindExpr(CE, LC, V);
C.addTransition(State);
diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
index fefe846b6911f..850411dc3354f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
@@ -206,8 +206,8 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall(
const auto *CE = cast<CallExpr>(Call.getOriginExpr());
// Function call will return a pointer to the new symbolic region.
- DefinedOrUnknownSVal RetVal = C.getSValBuilder().conjureSymbolVal(
- CE, LCtx, CE->getType(), C.blockCount());
+ DefinedOrUnknownSVal RetVal =
+ C.getSValBuilder().conjureSymbolVal(Call, CE->getType(), C.blockCount());
State = State->BindExpr(CE, LCtx, RetVal);
const auto *SymRegOfRetVal =
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index eb5054708fece..9e0800ba9e2fa 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -24,6 +24,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -192,18 +193,22 @@ DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const Stmt *stmt,
const LocationContext *LCtx,
QualType type,
unsigned visitCount) {
- if (type->isNullPtrType())
- return makeZeroVal(type);
-
- if (!SymbolManager::canSymbolicate(type))
- return UnknownVal();
-
- SymbolRef sym = SymMgr.conjureSymbol(stmt, LCtx, type, visitCount);
+ return conjureSymbolVal(/*symbolTag=*/nullptr, stmt, LCtx, type, visitCount);
+}
- if (Loc::isLocType(type))
- return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
+DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const CallEvent &call,
+ unsigned visitCount,
+ const void *symbolTag) {
+ return conjureSymbolVal(symbolTag, call.getOriginExpr(),
+ call.getLocationContext(), visitCount);
+}
- return nonloc::SymbolVal(sym);
+DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const CallEvent &call,
+ QualType type,
+ unsigned visitCount,
+ const void *symbolTag) {
+ return conjureSymbolVal(symbolTag, call.getOriginExpr(),
+ call.getLocationContext(), type, visitCount);
}
DefinedSVal SValBuilder::getConjuredHeapSymbolVal(const Expr *E,
|
steakhal
reviewed
Apr 24, 2025
fangyi-zhou
commented
Apr 24, 2025
| @@ -853,7 +853,7 @@ void SmartPtrModeling::handleBoolConversion(const CallEvent &Call, | |||
|
|
|||
| const LocationContext *LC = C.getLocationContext(); | |||
| InnerPointerVal = C.getSValBuilder().conjureSymbolVal( | |||
| CallExpr, LC, InnerPointerType, C.blockCount()); | |||
| Call, InnerPointerType, C.blockCount()); | |||
There was a problem hiding this comment.
For example, this inner pointer type is sometimes different from the call result type.
| C.getLocationContext(), C.getASTContext().IntTy, | ||
| C.blockCount()) | ||
| .castAs<NonLoc>(); | ||
| NonLoc ErrnoSVal = SVB.conjureSymbolVal(Call, C.getASTContext().IntTy, |
There was a problem hiding this comment.
This int type is different from the call result type at times too.
|
Could you please merge this pull request first, now that the other one got reverted? |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/16638 Here is the relevant piece of the build log for the reference |
|
On first look the buildbot failure seems to be unrelated to this change |
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…nts (llvm#137182) Per suggestion in llvm#128251 (comment), adding a new helper function in `SValBuilder` to conjure a symbol when given a `CallEvent`. Tested manually (with assertions) that the `LocationContext *` obtained from the `CallEvent` are identical to those passed in the original argument.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…nts (llvm#137182) Per suggestion in llvm#128251 (comment), adding a new helper function in `SValBuilder` to conjure a symbol when given a `CallEvent`. Tested manually (with assertions) that the `LocationContext *` obtained from the `CallEvent` are identical to those passed in the original argument.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…nts (llvm#137182) Per suggestion in llvm#128251 (comment), adding a new helper function in `SValBuilder` to conjure a symbol when given a `CallEvent`. Tested manually (with assertions) that the `LocationContext *` obtained from the `CallEvent` are identical to those passed in the original argument.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
…nts (llvm#137182) Per suggestion in llvm#128251 (comment), adding a new helper function in `SValBuilder` to conjure a symbol when given a `CallEvent`. Tested manually (with assertions) that the `LocationContext *` obtained from the `CallEvent` are identical to those passed in the original argument.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Per suggestion in
#128251 (comment), adding a new helper function in
SValBuilderto conjure a symbol when given aCallEvent.Tested manually (with assertions) that the
LocationContext *obtained from theCallEventare identical to those passed in the original argument.