Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ SVal getDynamicExtentWithOffset(ProgramStateRef State, SVal BufV);
DefinedOrUnknownSVal getDynamicElementCountWithOffset(ProgramStateRef State,
SVal BufV, QualType Ty);

void markAllDynamicExtentLive(ProgramStateRef State, SymbolReaper &SymReaper);

} // namespace ento
} // namespace clang

Expand Down
7 changes: 7 additions & 0 deletions clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,12 @@ ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR,
return State->set<DynamicExtentMap>(MR->StripCasts(), Size);
}

void markAllDynamicExtentLive(ProgramStateRef State, SymbolReaper &SymReaper) {
Copy link
Contributor

@NagyDonat NagyDonat Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps rename this to transferLivenessToDynamicExtent or something similar now that we no longer mark all the dynamic extent values as live?

for (const auto &I : State->get<DynamicExtentMap>())
if (SymbolRef Sym = I.second.getAsSymbol())
if (SymReaper.isLiveRegion(I.first))
SymReaper.markLive(Sym);
}

} // namespace ento
} // namespace clang
5 changes: 5 additions & 0 deletions clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,11 @@ void ExprEngine::removeDead(ExplodedNode *Pred, ExplodedNodeSet &Out,
getCheckerManager().runCheckersForDeadSymbols(CheckedSet, Pred, SymReaper,
DiagnosticStmt, *this, K);

// Extend lifetime of symbols used for dynamic extent while the parent region
// is live. In this way size information about memory allocations is not lost
// if the region remains live.
markAllDynamicExtentLive(CleanedState, SymReaper);

// For each node in CheckedSet, generate CleanedNodes that have the
// environment, the store, and the constraints cleaned up but have the
// user-supplied states as the predecessors.
Expand Down
22 changes: 0 additions & 22 deletions clang/test/Analysis/ArrayBound/verbose-tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -381,30 +381,12 @@ int *symbolicExtent(int arg) {
return 0;
int *mem = (int*)malloc(arg);

// TODO: without the following reference to 'arg', the analyzer would discard
// the range information about (the symbolic value of) 'arg'. This is
// incorrect because while the variable itself is inaccessible, it becomes
// the symbolic extent of 'mem', so we still want to reason about its
// potential values.
(void)arg;
Comment on lines -384 to -389
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there other similar void casts remaining int the test set that could be cleaned up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find more in the "ArrayBound" tests. Other tests may contain similar casts but this is more difficult to find.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is why I was curious to see how many of the casts in other test could be dropped after this change. This is not too simple, but with search and replacing with some regex might make it doable if you apply the replacement per file and add the change to the staging area if all the clang tests still pass. (And log what files didn't work out of the box so you could check them manually)
I'm pretty sure Cursor or even some simplified Bash script could automate this for you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be done of course separately from this patch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the Analysis tests but did not find similar casts (for purpose of keeping constraints of dynamic extent), just one place (that is changed in the last commit).


mem[8] = -2;
// expected-warning@-1 {{Out of bound access to memory after the end of the heap area}}
// expected-note@-2 {{Access of 'int' element in the heap area at index 8}}
return mem;
}

int *symbolicExtentDiscardedRangeInfo(int arg) {
// This is a copy of the case 'symbolicExtent' without the '(void)arg' hack.
// TODO: if the analyzer can detect the out-of-bounds access within this
// testcase, then remove this and the `(void)arg` hack from `symbolicExtent`.
if (arg >= 5)
return 0;
int *mem = (int*)malloc(arg);
mem[8] = -2;
return mem;
}

void symbolicIndex(int arg) {
// expected-note@+2 {{Assuming 'arg' is >= 12}}
// expected-note@+1 {{Taking true branch}}
Expand All @@ -426,9 +408,5 @@ int *nothingIsCertain(int x, int y) {
// {{Access of 'int' element in the heap area at an overflowing index}}
// but apparently the analyzer isn't smart enough to deduce this.

// Keep constraints alive. (Without this, the overeager garbage collection of
// constraints would _also_ prevent the intended behavior in this testcase.)
(void)x;

return mem;
}
Loading