martong created this revision. martong added a reviewer: vsavchenko. Herald added subscribers: manas, steakhal, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a reviewer: Szelethus. martong requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
https://bugs.llvm.org/show_bug.cgi?id=51109 When we merged two classes, `*this` became an obsolete representation of the new `State`. This is b/c the member relations had changed during the previous merge of another member of the same class in a way that `*this` had no longer any members. (`mergeImpl` might keep the member relations to `Other` and could dissolve `*this`.) Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D106285 Files: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp clang/test/Analysis/solver-sym-simplification-no-crash2.c Index: clang/test/Analysis/solver-sym-simplification-no-crash2.c =================================================================== --- /dev/null +++ clang/test/Analysis/solver-sym-simplification-no-crash2.c @@ -0,0 +1,23 @@ +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -verify + +// Here, we test that symbol simplification in the solver does not produce any +// crashes. +// https://bugs.llvm.org/show_bug.cgi?id=51109 + +// expected-no-diagnostics + +int a, b, c, d; +void f() { + a = -1; + d = b * a; + a = d / c; + if (a < 7 / b) + return; + if (d *a / c < 7 / b) + return; + if (b == 1 && c == -1) + return; +} Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp +++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp @@ -588,9 +588,10 @@ areEqual(ProgramStateRef State, SymbolRef First, SymbolRef Second); /// Iterate over all symbols and try to simplify them. - LLVM_NODISCARD ProgramStateRef simplify(SValBuilder &SVB, - RangeSet::Factory &F, - ProgramStateRef State); + LLVM_NODISCARD static inline ProgramStateRef simplify(SValBuilder &SVB, + RangeSet::Factory &F, + ProgramStateRef State, + EquivalenceClass Class); void dumpToStream(ProgramStateRef State, raw_ostream &os) const; LLVM_DUMP_METHOD void dump(ProgramStateRef State) const { @@ -1684,7 +1685,7 @@ ClassMembersTy Members = State->get<ClassMembers>(); for (std::pair<EquivalenceClass, SymbolSet> ClassToSymbolSet : Members) { EquivalenceClass Class = ClassToSymbolSet.first; - State = Class.simplify(Builder, RangeFactory, State); + State = EquivalenceClass::simplify(Builder, RangeFactory, State, Class); if (!State) return false; SimplifiedClasses.insert(Class); @@ -1698,7 +1699,7 @@ EquivalenceClass Class = ClassConstraint.first; if (SimplifiedClasses.count(Class)) // Already simplified. continue; - State = Class.simplify(Builder, RangeFactory, State); + State = EquivalenceClass::simplify(Builder, RangeFactory, State, Class); if (!State) return false; } @@ -2078,18 +2079,20 @@ // class to this class. This way, we simplify not just the symbols but the // classes as well: we strive to keep the number of the classes to be the // absolute minimum. -LLVM_NODISCARD ProgramStateRef EquivalenceClass::simplify( - SValBuilder &SVB, RangeSet::Factory &F, ProgramStateRef State) { - SymbolSet ClassMembers = getClassMembers(State); +LLVM_NODISCARD ProgramStateRef +EquivalenceClass::simplify(SValBuilder &SVB, RangeSet::Factory &F, + ProgramStateRef State, EquivalenceClass Class) { + SymbolSet ClassMembers = Class.getClassMembers(State); for (const SymbolRef &MemberSym : ClassMembers) { SymbolRef SimplifiedMemberSym = ento::simplify(State, MemberSym); if (SimplifiedMemberSym && MemberSym != SimplifiedMemberSym) { - EquivalenceClass ClassOfSimplifiedSym = - EquivalenceClass::find(State, SimplifiedMemberSym); // The simplified symbol should be the member of the original Class, // however, it might be in another existing class at the moment. We // have to merge these classes. - State = merge(F, State, ClassOfSimplifiedSym); + // Once we merged two classes, `*this` may became an obsolete + // representation of the new `State` b/c the member relations might have + // changed in a way that `*this` has no longer any members. + State = merge(F, State, MemberSym, SimplifiedMemberSym); if (!State) return nullptr; }
Index: clang/test/Analysis/solver-sym-simplification-no-crash2.c =================================================================== --- /dev/null +++ clang/test/Analysis/solver-sym-simplification-no-crash2.c @@ -0,0 +1,23 @@ +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -verify + +// Here, we test that symbol simplification in the solver does not produce any +// crashes. +// https://bugs.llvm.org/show_bug.cgi?id=51109 + +// expected-no-diagnostics + +int a, b, c, d; +void f() { + a = -1; + d = b * a; + a = d / c; + if (a < 7 / b) + return; + if (d *a / c < 7 / b) + return; + if (b == 1 && c == -1) + return; +} Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp +++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp @@ -588,9 +588,10 @@ areEqual(ProgramStateRef State, SymbolRef First, SymbolRef Second); /// Iterate over all symbols and try to simplify them. - LLVM_NODISCARD ProgramStateRef simplify(SValBuilder &SVB, - RangeSet::Factory &F, - ProgramStateRef State); + LLVM_NODISCARD static inline ProgramStateRef simplify(SValBuilder &SVB, + RangeSet::Factory &F, + ProgramStateRef State, + EquivalenceClass Class); void dumpToStream(ProgramStateRef State, raw_ostream &os) const; LLVM_DUMP_METHOD void dump(ProgramStateRef State) const { @@ -1684,7 +1685,7 @@ ClassMembersTy Members = State->get<ClassMembers>(); for (std::pair<EquivalenceClass, SymbolSet> ClassToSymbolSet : Members) { EquivalenceClass Class = ClassToSymbolSet.first; - State = Class.simplify(Builder, RangeFactory, State); + State = EquivalenceClass::simplify(Builder, RangeFactory, State, Class); if (!State) return false; SimplifiedClasses.insert(Class); @@ -1698,7 +1699,7 @@ EquivalenceClass Class = ClassConstraint.first; if (SimplifiedClasses.count(Class)) // Already simplified. continue; - State = Class.simplify(Builder, RangeFactory, State); + State = EquivalenceClass::simplify(Builder, RangeFactory, State, Class); if (!State) return false; } @@ -2078,18 +2079,20 @@ // class to this class. This way, we simplify not just the symbols but the // classes as well: we strive to keep the number of the classes to be the // absolute minimum. -LLVM_NODISCARD ProgramStateRef EquivalenceClass::simplify( - SValBuilder &SVB, RangeSet::Factory &F, ProgramStateRef State) { - SymbolSet ClassMembers = getClassMembers(State); +LLVM_NODISCARD ProgramStateRef +EquivalenceClass::simplify(SValBuilder &SVB, RangeSet::Factory &F, + ProgramStateRef State, EquivalenceClass Class) { + SymbolSet ClassMembers = Class.getClassMembers(State); for (const SymbolRef &MemberSym : ClassMembers) { SymbolRef SimplifiedMemberSym = ento::simplify(State, MemberSym); if (SimplifiedMemberSym && MemberSym != SimplifiedMemberSym) { - EquivalenceClass ClassOfSimplifiedSym = - EquivalenceClass::find(State, SimplifiedMemberSym); // The simplified symbol should be the member of the original Class, // however, it might be in another existing class at the moment. We // have to merge these classes. - State = merge(F, State, ClassOfSimplifiedSym); + // Once we merged two classes, `*this` may became an obsolete + // representation of the new `State` b/c the member relations might have + // changed in a way that `*this` has no longer any members. + State = merge(F, State, MemberSym, SimplifiedMemberSym); if (!State) return nullptr; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
