Skip to content

Commit

Permalink
[analyzer] Fix performance of getTaintedSymbolsImpl() (llvm#89606)
Browse files Browse the repository at this point in the history
Previously the function
```
std::vector<SymbolRef> taint::getTaintedSymbolsImpl(ProgramStateRef State,
                                                    const MemRegion *Reg,
                                                    TaintTagType K,
                                                    bool returnFirstOnly)
```
(one of the 4 overloaded variants under this name) was handling element
regions in a highly inefficient manner: it performed the "also examine
the super-region" step twice. (Once in the branch for element regions,
and once in the more general branch for all `SubRegion`s -- note that
`ElementRegion` is a subclass of `SubRegion`.)

As pointer arithmetic produces `ElementRegion`s, it's not too difficult
to get a chain of N nested element regions where this inefficient
recursion would produce 2^N calls.

This commit is essentially NFC, apart from the performance improvements
and the removal of (probably irrelevant) duplicate entries from the
return value of `getTaintedSymbols()` calls.

Fixes llvm#89045

(cherry picked from commit ce763bf)
  • Loading branch information
NagyDonat authored and tstellar committed Apr 25, 2024
1 parent 7699b34 commit 3b4ba72
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions clang/lib/StaticAnalyzer/Checkers/Taint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,21 +216,17 @@ std::vector<SymbolRef> taint::getTaintedSymbolsImpl(ProgramStateRef State,
std::vector<SymbolRef> TaintedSymbols;
if (!Reg)
return TaintedSymbols;
// Element region (array element) is tainted if either the base or the offset
// are tainted.

// Element region (array element) is tainted if the offset is tainted.
if (const ElementRegion *ER = dyn_cast<ElementRegion>(Reg)) {
std::vector<SymbolRef> TaintedIndex =
getTaintedSymbolsImpl(State, ER->getIndex(), K, returnFirstOnly);
llvm::append_range(TaintedSymbols, TaintedIndex);
if (returnFirstOnly && !TaintedSymbols.empty())
return TaintedSymbols; // return early if needed
std::vector<SymbolRef> TaintedSuperRegion =
getTaintedSymbolsImpl(State, ER->getSuperRegion(), K, returnFirstOnly);
llvm::append_range(TaintedSymbols, TaintedSuperRegion);
if (returnFirstOnly && !TaintedSymbols.empty())
return TaintedSymbols; // return early if needed
}

// Symbolic region is tainted if the corresponding symbol is tainted.
if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(Reg)) {
std::vector<SymbolRef> TaintedRegions =
getTaintedSymbolsImpl(State, SR->getSymbol(), K, returnFirstOnly);
Expand All @@ -239,6 +235,8 @@ std::vector<SymbolRef> taint::getTaintedSymbolsImpl(ProgramStateRef State,
return TaintedSymbols; // return early if needed
}

// Any subregion (including Element and Symbolic regions) is tainted if its
// super-region is tainted.
if (const SubRegion *ER = dyn_cast<SubRegion>(Reg)) {
std::vector<SymbolRef> TaintedSubRegions =
getTaintedSymbolsImpl(State, ER->getSuperRegion(), K, returnFirstOnly);
Expand Down Expand Up @@ -318,4 +316,4 @@ std::vector<SymbolRef> taint::getTaintedSymbolsImpl(ProgramStateRef State,
}
}
return TaintedSymbols;
}
}

0 comments on commit 3b4ba72

Please sign in to comment.