[PATCH] D54013: [analyzer] MallocChecker: Avoid redundant transitions.
NoQ updated this revision to Diff 172629.
NoQ added a comment.
Re-upload with context. Whoops.
https://reviews.llvm.org/D54013
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2366,20 +2366,23 @@
CheckerContext &C) const
{
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
+ RegionStateTy RS = OldRS;
SmallVector Errors;
for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) {
if (SymReaper.isDead(I->first)) {
if (I->second.isAllocated() || I->second.isAllocatedOfSizeZero())
Errors.push_back(I->first);
// Remove the dead symbol from the map.
RS = F.remove(RS, I->first);
-
}
}
+ if (RS == OldRS)
+return;
+
// Cleanup the Realloc Pairs Map.
ReallocPairsTy RP = state->get();
for (ReallocPairsTy::iterator I = RP.begin(), E = RP.end(); I != E; ++I) {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2366,20 +2366,23 @@
CheckerContext &C) const
{
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
+ RegionStateTy RS = OldRS;
SmallVector Errors;
for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) {
if (SymReaper.isDead(I->first)) {
if (I->second.isAllocated() || I->second.isAllocatedOfSizeZero())
Errors.push_back(I->first);
// Remove the dead symbol from the map.
RS = F.remove(RS, I->first);
-
}
}
+ if (RS == OldRS)
+return;
+
// Cleanup the Realloc Pairs Map.
ReallocPairsTy RP = state->get();
for (ReallocPairsTy::iterator I = RP.begin(), E = RP.end(); I != E; ++I) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54013: [analyzer] MallocChecker: Avoid redundant transitions.
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369 ProgramStateRef state = C.getState(); - RegionStateTy RS = state->get(); + RegionStateTy OldRS = state->get(); RegionStateTy::Factory &F = state->get_context(); george.karpenkov wrote: > Szelethus wrote: > > We are acquiring an object of type `ImmutableMap`, modify it, and put it > > back into `state`? Can't we just modify it through `ProgramState`'s > > interface directly? > state is immutable, I don't think we can put anything into it. > We also can't modify ImmutableMap because, well, it's immutable. Poor choice of words, I admit. What I actually meant is that maybe it would be more straighforward if we didnt create a local varable here. But I'm fine with being in the wrong :) Repository: rC Clang https://reviews.llvm.org/D54013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54013: [analyzer] MallocChecker: Avoid redundant transitions.
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369 ProgramStateRef state = C.getState(); - RegionStateTy RS = state->get(); + RegionStateTy OldRS = state->get(); RegionStateTy::Factory &F = state->get_context(); Szelethus wrote: > We are acquiring an object of type `ImmutableMap`, modify it, and put it back > into `state`? Can't we just modify it through `ProgramState`'s interface > directly? state is immutable, I don't think we can put anything into it. We also can't modify ImmutableMap because, well, it's immutable. Repository: rC Clang https://reviews.llvm.org/D54013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54013: [analyzer] MallocChecker: Avoid redundant transitions.
Szelethus added a comment. Please reupload with full context. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369 ProgramStateRef state = C.getState(); - RegionStateTy RS = state->get(); + RegionStateTy OldRS = state->get(); RegionStateTy::Factory &F = state->get_context(); We are acquiring an object of type `ImmutableMap`, modify it, and put it back into `state`? Can't we just modify it through `ProgramState`'s interface directly? Repository: rC Clang https://reviews.llvm.org/D54013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54013: [analyzer] MallocChecker: Avoid redundant transitions.
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov,
rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, szepet, baloghadamsoftware.
In its `checkDeadSymbols` callback, when no pointers are tracked and the state
is not updated, `MallocChecker` would update the state from "the map of symbols
is unset" to "the map of symbols is set to an empty map", which generates a
redundant exploded node. Now that zombie symbols are fixed in
https://reviews.llvm.org/D18860, we would call `checkDeadSymbols()` with no
dead symbols more often. Avoid the unnecessary transition here.
Repository:
rC Clang
https://reviews.llvm.org/D54013
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2366,9 +2366,10 @@
CheckerContext &C) const
{
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
+ RegionStateTy RS = OldRS;
SmallVector Errors;
for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) {
if (SymReaper.isDead(I->first)) {
@@ -2376,10 +2377,12 @@
Errors.push_back(I->first);
// Remove the dead symbol from the map.
RS = F.remove(RS, I->first);
-
}
}
+ if (RS == OldRS)
+return;
+
// Cleanup the Realloc Pairs Map.
ReallocPairsTy RP = state->get();
for (ReallocPairsTy::iterator I = RP.begin(), E = RP.end(); I != E; ++I) {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2366,9 +2366,10 @@
CheckerContext &C) const
{
ProgramStateRef state = C.getState();
- RegionStateTy RS = state->get();
+ RegionStateTy OldRS = state->get();
RegionStateTy::Factory &F = state->get_context();
+ RegionStateTy RS = OldRS;
SmallVector Errors;
for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) {
if (SymReaper.isDead(I->first)) {
@@ -2376,10 +2377,12 @@
Errors.push_back(I->first);
// Remove the dead symbol from the map.
RS = F.remove(RS, I->first);
-
}
}
+ if (RS == OldRS)
+return;
+
// Cleanup the Realloc Pairs Map.
ReallocPairsTy RP = state->get();
for (ReallocPairsTy::iterator I = RP.begin(), E = RP.end(); I != E; ++I) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
