From 2c6af8f304929bf301dab1f6360e67795e93ac5d Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 14 Sep 2020 15:26:28 -0400 Subject: [PATCH 01/15] Initial fix for #255 --- clang/include/clang/CConv/RewriteUtils.h | 13 +++++--- clang/lib/CConv/ConstraintVariables.cpp | 3 +- clang/lib/CConv/DeclRewriter.cpp | 6 +--- clang/lib/CConv/PersistentSourceLoc.cpp | 4 +-- clang/lib/CConv/ProgramInfo.cpp | 9 ++++-- clang/lib/CConv/RewriteUtils.cpp | 37 ++++++++++++++--------- clang/test/CheckedCRewriter/definedType.c | 28 +++++++++++++++++ clang/test/CheckedCRewriter/macroConcat.c | 21 +++++++++++++ 8 files changed, 91 insertions(+), 30 deletions(-) create mode 100644 clang/test/CheckedCRewriter/macroConcat.c diff --git a/clang/include/clang/CConv/RewriteUtils.h b/clang/include/clang/CConv/RewriteUtils.h index 2327f3b1a097..a1ee8aa90f0b 100644 --- a/clang/include/clang/CConv/RewriteUtils.h +++ b/clang/include/clang/CConv/RewriteUtils.h @@ -28,7 +28,7 @@ class DeclReplacement { std::string getReplacement() const { return Replacement; } - virtual SourceRange getSourceRange() const { + virtual SourceRange getSourceRange(SourceManager &SM) const { return getDecl()->getSourceRange(); } @@ -93,10 +93,13 @@ class FunctionDeclReplacement : && (RewriteReturn || RewriteParams)); } - SourceRange getSourceRange() const override { - FunctionTypeLoc TypeLoc = - getBaseTypeLoc(Decl->getTypeSourceInfo()->getTypeLoc()) - .getAs(); + SourceRange getSourceRange(SourceManager &SM) const override { + TypeSourceInfo *TSInfo = Decl->getTypeSourceInfo(); + if (!TSInfo) + return SourceRange(Decl->getBeginLoc(), + getFunctionDeclarationEnd(Decl, SM)); + FunctionTypeLoc TypeLoc = getBaseTypeLoc(TSInfo->getTypeLoc()) + .getAs(); assert("FunctionDecl doesn't have function type?" && !TypeLoc.isNull()); diff --git a/clang/lib/CConv/ConstraintVariables.cpp b/clang/lib/CConv/ConstraintVariables.cpp index a8d8cd5968ee..ff599dffc262 100644 --- a/clang/lib/CConv/ConstraintVariables.cpp +++ b/clang/lib/CConv/ConstraintVariables.cpp @@ -375,7 +375,8 @@ PointerVariableConstraint::PointerVariableConstraint(const QualType &QT, // inside a macro. Not sure how to handle this, so fall back to tyToStr. if (BaseType.empty()) FoundMatchingType = false; - } + } else + FoundMatchingType = false; } } // Fall back to rebuilding the base type based on type passed to constructor diff --git a/clang/lib/CConv/DeclRewriter.cpp b/clang/lib/CConv/DeclRewriter.cpp index 7085c01d5584..518e1c1ab491 100644 --- a/clang/lib/CConv/DeclRewriter.cpp +++ b/clang/lib/CConv/DeclRewriter.cpp @@ -136,10 +136,6 @@ void DeclRewriter::rewrite(RSet &ToRewrite) { errs() << "with " << N->getReplacement() << "\n"; } - // Get a FullSourceLoc for the start location and add it to the - // list of file ID's we've touched. - SourceRange SR = N->getDecl()->getSourceRange(); - // Exact rewriting procedure depends on declaration type if (auto *PVR = dyn_cast(N)) { assert(N->getStatement() == nullptr); @@ -337,7 +333,7 @@ void DeclRewriter::rewriteFunctionDecl(FunctionDeclReplacement *N) { // Additionally, a source range can be (mis) identified as // spanning multiple files. We don't know how to re-write that, // so don't. - SourceRange SR = N->getSourceRange(); + SourceRange SR = N->getSourceRange(A.getSourceManager()); if (canRewrite(R, SR)) { R.ReplaceText(SR, N->getReplacement()); } else { diff --git a/clang/lib/CConv/PersistentSourceLoc.cpp b/clang/lib/CConv/PersistentSourceLoc.cpp index 6f8be40d53c2..1fe629913548 100644 --- a/clang/lib/CConv/PersistentSourceLoc.cpp +++ b/clang/lib/CConv/PersistentSourceLoc.cpp @@ -23,9 +23,9 @@ PersistentSourceLoc::mkPSL(const Decl *D, ASTContext &C) { SourceLocation SL = D->getLocation(); if (const FunctionDecl *FD = dyn_cast(D)) - SL = C.getSourceManager().getSpellingLoc(FD->getLocation()); + SL = C.getSourceManager().getExpansionLoc(FD->getLocation()); else if (const ParmVarDecl *PV = dyn_cast(D)) - SL = C.getSourceManager().getSpellingLoc(PV->getLocation()); + SL = C.getSourceManager().getExpansionLoc(PV->getLocation()); else if (const VarDecl *V = dyn_cast(D)) SL = C.getSourceManager().getExpansionLoc(V->getLocation()); diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index e83b1aa4cf36..234b1fd793db 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -530,10 +530,13 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, PersistentSourceLoc PLoc = PersistentSourceLoc::mkPSL(D, *astContext); assert(PLoc.valid()); - // We only add a PVConstraint or an FVConstraint if the set at - // Variables[PLoc] does not contain one already. TODO: Explain why would this happen + // We only add a PVConstraint if the set at Variables[PLoc] does not contain + // one already. Two variables can have the same source locations when they are + // declared inside the same macro expansion. Functions are exempt from this + // check because they need to be added to the Extern/Static function maps + // regardless if they are inside a macro expansion. CVarSet &S = Variables[PLoc]; - if (S.size()) return; + if (S.size() && !isa(D)) return; if (FunctionDecl *FD = dyn_cast(D)) { // Function Decls have FVConstraints. diff --git a/clang/lib/CConv/RewriteUtils.cpp b/clang/lib/CConv/RewriteUtils.cpp index 735a06949bac..16eb1f252712 100644 --- a/clang/lib/CConv/RewriteUtils.cpp +++ b/clang/lib/CConv/RewriteUtils.cpp @@ -31,7 +31,7 @@ SourceLocation DComp::getDeclBegin(DeclReplacement *D) const { } SourceRange DComp::getReplacementSourceRange(DeclReplacement *D) const { - SourceRange Range = D->getSourceRange(); + SourceRange Range = D->getSourceRange(SM); // Also take into account whether or not there is a multi-statement // decl, because the generated ranges will overlap. @@ -227,19 +227,28 @@ class TypeExprRewriter CVarSet CVSingleton = Info.getPersistentConstraintVars(E, Context); if (CVSingleton.empty()) return; - ConstraintVariable *CV = getOnly(CVSingleton); - - // Only rewrite if the type has changed. - if (CV->anyChanges(Info.getConstraints().getVariables())){ - // The constraint variable is able to tell us what the new type string - // should be. - std::string - NewType = CV->mkString(Info.getConstraints().getVariables(), false); - - // Replace the original type with this new one - if (canRewrite(Writer, Range)) - Writer.ReplaceText(Range, NewType); - } + + // Macros wil sometimes cause a single expression to have multiple + // constraint variables. These should have been constrained to wild, so + // there shouldn't be any rewriting required. + EnvironmentMap &Vars = Info.getConstraints().getVariables(); + assert(CVSingleton.size() == 1 || llvm::none_of(CVSingleton, + [&Vars](ConstraintVariable *CV) { + return CV->anyChanges(Vars); + })); + + for (auto *CV : CVSingleton) + // Only rewrite if the type has changed. + if (CV->anyChanges(Info.getConstraints().getVariables())){ + // The constraint variable is able to tell us what the new type string + // should be. + std::string + NewType = CV->mkString(Info.getConstraints().getVariables(), false); + + // Replace the original type with this new one + if (canRewrite(Writer, Range)) + Writer.ReplaceText(Range, NewType); + } } }; diff --git a/clang/test/CheckedCRewriter/definedType.c b/clang/test/CheckedCRewriter/definedType.c index c14ed3cb7539..15c8eaffb93f 100644 --- a/clang/test/CheckedCRewriter/definedType.c +++ b/clang/test/CheckedCRewriter/definedType.c @@ -110,3 +110,31 @@ baz (*(*lua_test3)); typedef int *StkId; void lua_test4(StkId *x) {} // CHECK: void lua_test4(_Ptr<_Ptr> x) _Checked {} + +// Things declared inside macros should be WILD unless we start doing something extremely clever + +#define declare_function(x) int *foo##x(int *a) {return a; } int *bar##x(int *a) { return a; } + +declare_function(1) + +#define declare_var(x) int * x##1; int ** x##2; int *** x##3; + +declare_var(y) + +void test() { + int *x = 0; + int *y = 0; + int *a = foo1(x); + int *b = bar1(y); + int *c = y1; + int **d = y2; + int ***e = y3; +} +// CHECK: void test() { +// CHECK: int *x = 0; +// CHECK: int *y = 0; +// CHECK: int *a = foo1(x); +// CHECK: int *b = bar1(y); +// CHECK: int *c = y1; +// CHECK: int **d = y2; +// CHECK: int ***e = y3; diff --git a/clang/test/CheckedCRewriter/macroConcat.c b/clang/test/CheckedCRewriter/macroConcat.c new file mode 100644 index 000000000000..94c08d70ea58 --- /dev/null +++ b/clang/test/CheckedCRewriter/macroConcat.c @@ -0,0 +1,21 @@ +// RUN: cconv-standalone -addcr -alltypes %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s +// RUN: cconv-standalone -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s +// RUN: cconv-standalone -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - +// RUN: cconv-standalone -alltypes -output-postfix=checked %s +// RUN: cconv-standalone -alltypes %S/macroConcat.checked.c -- | count 0 +// RUN: rm %S/macroConcat.checked.c + +#define c(g) void FOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO##g () +c(BAR0); c(BAR1); c(BAR2); c(BAR3); c(BAR4); c(BAR5); c(BAR6); c(BAR7); +c(BAR8); c(BAR9); c(BAR10); c(BAR11); c(BAR12); c(BAR13); c(BAR14); c(BAR15); +c(BAR16); c(BAR17); c(BAR18); c(BAR19); c(BAR20); c(BAR21); c(BAR22); c(BAR23); +c(BAR24); c(BAR25); c(BAR26); c(BAR27); c(BAR28); c(BAR29); c(BAR30); c(BAR31); +c(BAR32); c(BAR33); c(BAR34); c(BAR35); c(BAR36); c(BAR37); c(BAR38); + +#define u(d) void add##d(g) {} +// CHECK: #define u(d) void add##d(g) {} +u(2); +// CHECK: u(2); + +int *a = 0; +// CHECK: _Ptr a = 0; From 2ab4b2393667117b0f0b688175f415fbcac2b7fb Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 14 Sep 2020 16:41:19 -0400 Subject: [PATCH 02/15] Add more const qualifiers --- .../include/clang/CConv/ConstraintVariables.h | 36 ++++++++-------- clang/include/clang/CConv/Constraints.h | 4 +- clang/include/clang/CConv/ProgramInfo.h | 2 +- clang/lib/CConv/AVarBoundsInfo.cpp | 4 +- .../CConv/ArrayBoundsInferenceConsumer.cpp | 16 +++---- clang/lib/CConv/CastPlacement.cpp | 2 +- clang/lib/CConv/ConstraintVariables.cpp | 43 ++++++++++--------- clang/lib/CConv/ProgramInfo.cpp | 4 +- 8 files changed, 58 insertions(+), 53 deletions(-) diff --git a/clang/include/clang/CConv/ConstraintVariables.h b/clang/include/clang/CConv/ConstraintVariables.h index 2cd69220727f..7721855625f3 100644 --- a/clang/include/clang/CConv/ConstraintVariables.h +++ b/clang/include/clang/CConv/ConstraintVariables.h @@ -88,7 +88,7 @@ class ConstraintVariable { // the name of the variable, false for just the type. // The 'forIType' parameter is true when the generated string is expected // to be used inside an itype - virtual std::string mkString(EnvironmentMap &E, + virtual std::string mkString(const EnvironmentMap &E, bool emitName=true, bool forItype=false, bool emitPointee=false) const = 0; @@ -123,20 +123,20 @@ class ConstraintVariable { // have a binding in E other than top. E should be the EnvironmentMap that // results from running unification on the set of constraints and the // environment. - bool isChecked(EnvironmentMap &E) const; + bool isChecked(const EnvironmentMap &E) const; // Returns true if this constraint variable has a different checked type after // running unification. Note that if the constraint variable had a checked // type in the input program, it will have the same checked type after solving // so, the type will not have changed. To test if the type is checked, use // isChecked instead. - virtual bool anyChanges(EnvironmentMap &E) const = 0; + virtual bool anyChanges(const EnvironmentMap &E) const = 0; // Here, AIdx is the pointer level which needs to be checked. // By default, we check for all pointer levels (or VarAtoms) - virtual bool hasWild(EnvironmentMap &E, int AIdx = -1) const = 0; - virtual bool hasArr(EnvironmentMap &E, int AIdx = -1) const = 0; - virtual bool hasNtArr(EnvironmentMap &E, int AIdx = -1) const = 0; + virtual bool hasWild(const EnvironmentMap &E, int AIdx = -1) const = 0; + virtual bool hasArr(const EnvironmentMap &E, int AIdx = -1) const = 0; + virtual bool hasNtArr(const EnvironmentMap &E, int AIdx = -1) const = 0; // Force use of equality constraints in function calls for this CV virtual void equateArgumentConstraints(ProgramInfo &I) = 0; @@ -247,7 +247,7 @@ class PointerVariableConstraint : public ConstraintVariable { // the values used as arguments. std::set argumentConstraints; // Get solution for the atom of a pointer. - const ConstAtom *getSolution(const Atom *A, EnvironmentMap &E) const; + const ConstAtom *getSolution(const Atom *A, const EnvironmentMap &E) const; PointerVariableConstraint(PointerVariableConstraint *Ot, Constraints &CS); PointerVariableConstraint *Parent; @@ -341,7 +341,7 @@ class PointerVariableConstraint : public ConstraintVariable { return S->getKind() == PointerVariable; } - std::string mkString(EnvironmentMap &E, bool EmitName =true, + std::string mkString(const EnvironmentMap &E, bool EmitName =true, bool ForItype = false, bool EmitPointee = false) const override; @@ -355,11 +355,11 @@ class PointerVariableConstraint : public ConstraintVariable { void constrainToWild(Constraints &CS, const std::string &Rsn, PersistentSourceLoc *PL) const override; void constrainOuterTo(Constraints &CS, ConstAtom *C, bool doLB = false); - bool anyChanges(EnvironmentMap &E) const override; - bool anyArgumentIsWild(EnvironmentMap &E); - bool hasWild(EnvironmentMap &E, int AIdx = -1) const override; - bool hasArr(EnvironmentMap &E, int AIdx = -1) const override; - bool hasNtArr(EnvironmentMap &E, int AIdx = -1) const override; + bool anyChanges(const EnvironmentMap &E) const override; + bool anyArgumentIsWild(const EnvironmentMap &E); + bool hasWild(const EnvironmentMap &E, int AIdx = -1) const override; + bool hasArr(const EnvironmentMap &E, int AIdx = -1) const override; + bool hasNtArr(const EnvironmentMap &E, int AIdx = -1) const override; void equateArgumentConstraints(ProgramInfo &I) override; @@ -461,7 +461,7 @@ class FunctionVariableConstraint : public ConstraintVariable { bool solutionEqualTo(Constraints &CS, const ConstraintVariable *CV) const override; - std::string mkString(EnvironmentMap &E, bool EmitName =true, + std::string mkString(const EnvironmentMap &E, bool EmitName =true, bool ForItype = false, bool EmitPointee = false) const override; void print(llvm::raw_ostream &O) const override; @@ -471,10 +471,10 @@ class FunctionVariableConstraint : public ConstraintVariable { void constrainToWild(Constraints &CS, const std::string &Rsn) const override; void constrainToWild(Constraints &CS, const std::string &Rsn, PersistentSourceLoc *PL) const override; - bool anyChanges(EnvironmentMap &E) const override; - bool hasWild(EnvironmentMap &E, int AIdx = -1) const override; - bool hasArr(EnvironmentMap &E, int AIdx = -1) const override; - bool hasNtArr(EnvironmentMap &E, int AIdx = -1) const override; + bool anyChanges(const EnvironmentMap &E) const override; + bool hasWild(const EnvironmentMap &E, int AIdx = -1) const override; + bool hasArr(const EnvironmentMap &E, int AIdx = -1) const override; + bool hasNtArr(const EnvironmentMap &E, int AIdx = -1) const override; void equateArgumentConstraints(ProgramInfo &P) override; diff --git a/clang/include/clang/CConv/Constraints.h b/clang/include/clang/CConv/Constraints.h index 68dc10e43030..85fc4d1acc3f 100644 --- a/clang/include/clang/CConv/Constraints.h +++ b/clang/include/clang/CConv/Constraints.h @@ -529,6 +529,7 @@ class ConstraintsEnv { public: ConstraintsEnv() : consFreeKey(0), useChecked(true) { environment.clear(); } EnvironmentMap &getVariables() { return environment; } + const EnvironmentMap &getVariables() const { return environment; } void dump() const; void print(llvm::raw_ostream &) const; void dump_json(llvm::raw_ostream &) const; @@ -574,7 +575,8 @@ class Constraints { // a client wants to examine the environment is untenable. ConstraintSet &getConstraints() { return constraints; } EnvironmentMap &getVariables() { return environment.getVariables(); } - + const EnvironmentMap &getVariables() const { return environment.getVariables(); } + EnvironmentMap &getitypeVarMap() { return itypeConstraintVars; } void editConstraintHook(Constraint *C); diff --git a/clang/include/clang/CConv/ProgramInfo.h b/clang/include/clang/CConv/ProgramInfo.h index 8987f3647182..573cb994dfb0 100644 --- a/clang/include/clang/CConv/ProgramInfo.h +++ b/clang/include/clang/CConv/ProgramInfo.h @@ -86,7 +86,7 @@ class ProgramInfo : public ProgramVariableAdder { // constraints where appropriate. bool link(); - VariableMap &getVarMap() { return Variables; } + const VariableMap &getVarMap() const { return Variables; } Constraints &getConstraints() { return CS; } AVarBoundsInfo &getABoundsInfo() { return ArrBInfo; } diff --git a/clang/lib/CConv/AVarBoundsInfo.cpp b/clang/lib/CConv/AVarBoundsInfo.cpp index 4db314844bc3..4dd50a26e466 100644 --- a/clang/lib/CConv/AVarBoundsInfo.cpp +++ b/clang/lib/CConv/AVarBoundsInfo.cpp @@ -950,11 +950,11 @@ void AVarBoundsInfo::computerArrPointers(ProgramInfo *PI, auto &BkeyToPSL = DeclVarMap.right(); if (BkeyToPSL.find(Bkey) != BkeyToPSL.end()) { auto &PSL = BkeyToPSL.at(Bkey); - if (hasArray(PI->getVarMap()[PSL], CS)) { + if (hasArray(PI->getVarMap().at(PSL), CS)) { ArrPointers.insert(Bkey); } // Does this array belongs to a valid program variable? - if (isInSrcArray(PI->getVarMap()[PSL], CS)) { + if (isInSrcArray(PI->getVarMap().at(PSL), CS)) { InProgramArrPtrBoundsKeys.insert(Bkey); } continue; diff --git a/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp b/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp index 0fb8e2af731d..7e6e5db16bef 100644 --- a/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp +++ b/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp @@ -101,19 +101,19 @@ static bool hasLengthKeyword(std::string VarName) { } // Check if the provided constraint variable is an array and it needs bounds. -static bool needArrayBounds(ConstraintVariable *CV, - EnvironmentMap &E) { +static bool needArrayBounds(const ConstraintVariable *CV, + const EnvironmentMap &E) { if (CV->hasArr(E, 0)) { - PVConstraint *PV = dyn_cast(CV); + const PVConstraint *PV = dyn_cast(CV); return !PV || PV->isTopCvarUnsizedArr(); } return false; } -static bool needNTArrayBounds(ConstraintVariable *CV, - EnvironmentMap &E) { +static bool needNTArrayBounds(const ConstraintVariable *CV, + const EnvironmentMap &E) { if (CV->hasNtArr(E, 0)) { - PVConstraint *PV = dyn_cast(CV); + const PVConstraint *PV = dyn_cast(CV); return !PV || PV->isTopCvarUnsizedArr(); } return false; @@ -122,7 +122,7 @@ static bool needNTArrayBounds(ConstraintVariable *CV, static bool needArrayBounds(Expr *E, ProgramInfo &Info, ASTContext *C) { ConstraintResolver CR(Info, C); CVarSet ConsVar = CR.getExprConstraintVars(E); - auto &EnvMap = Info.getConstraints().getVariables(); + const auto &EnvMap = Info.getConstraints().getVariables(); for (auto CurrCVar : ConsVar) { if (needArrayBounds(CurrCVar, EnvMap) || needNTArrayBounds(CurrCVar, EnvMap)) return true; @@ -134,7 +134,7 @@ static bool needArrayBounds(Expr *E, ProgramInfo &Info, ASTContext *C) { static bool needArrayBounds(Decl *D, ProgramInfo &Info, ASTContext *C, bool IsNtArr) { CVarSet ConsVar = Info.getVariable(D, C); - auto &E = Info.getConstraints().getVariables(); + const auto &E = Info.getConstraints().getVariables(); for (auto CurrCVar : ConsVar) { if ((!IsNtArr && needArrayBounds(CurrCVar, E)) || (IsNtArr && needNTArrayBounds(CurrCVar, E))) diff --git a/clang/lib/CConv/CastPlacement.cpp b/clang/lib/CConv/CastPlacement.cpp index 91f5ef66e7eb..d09afe23a2f8 100644 --- a/clang/lib/CConv/CastPlacement.cpp +++ b/clang/lib/CConv/CastPlacement.cpp @@ -70,7 +70,7 @@ bool CastPlacementVisitor::VisitCallExpr(CallExpr *CE) { // by src variable is assigned to dst. bool CastPlacementVisitor::needCasting(ConstraintVariable *Src, ConstraintVariable *Dst) { - auto &E = Info.getConstraints().getVariables(); + const auto &E = Info.getConstraints().getVariables(); // Check if the src is a checked type. if (Src->isChecked(E)) { // If Dst has an itype, Src must have exactly the same checked type. If this diff --git a/clang/lib/CConv/ConstraintVariables.cpp b/clang/lib/CConv/ConstraintVariables.cpp index ff599dffc262..89ccba4e9d22 100644 --- a/clang/lib/CConv/ConstraintVariables.cpp +++ b/clang/lib/CConv/ConstraintVariables.cpp @@ -32,7 +32,7 @@ std::string ConstraintVariable::getRewritableOriginalTy() const { } return OrigTyString; } -bool ConstraintVariable::isChecked(EnvironmentMap &E) const { +bool ConstraintVariable::isChecked(const EnvironmentMap &E) const { return getIsOriginallyChecked() || anyChanges(E); } @@ -546,7 +546,7 @@ void PointerVariableConstraint::addArrayAnnotations( // variables and potentially nested function pointer declaration. Produces a // string that can be replaced in the source code. std::string -PointerVariableConstraint::mkString(EnvironmentMap &E, +PointerVariableConstraint::mkString(const EnvironmentMap &E, bool EmitName, bool ForItype, bool EmitPointee) const { @@ -587,7 +587,7 @@ PointerVariableConstraint::mkString(EnvironmentMap &E, VarAtom *VA = dyn_cast(V); assert(VA != nullptr && "Constraint variable can " "be either constant or VarAtom."); - C = E[VA].first; + C = E.at(VA).first; } assert(C != nullptr); @@ -891,19 +891,22 @@ void FunctionVariableConstraint::constrainToWild V->constrainToWild(CS, Rsn, PL); } -bool FunctionVariableConstraint::anyChanges(EnvironmentMap &E) const { +bool FunctionVariableConstraint::anyChanges(const EnvironmentMap &E) const { return ReturnVar->anyChanges(E); } -bool FunctionVariableConstraint::hasWild(EnvironmentMap &E, int AIdx) const { +bool FunctionVariableConstraint::hasWild(const EnvironmentMap &E, + int AIdx) const { return ReturnVar->hasWild(E, AIdx); } -bool FunctionVariableConstraint::hasArr(EnvironmentMap &E, int AIdx) const { +bool FunctionVariableConstraint::hasArr(const EnvironmentMap &E, + int AIdx) const { return ReturnVar->hasArr(E, AIdx); } -bool FunctionVariableConstraint::hasNtArr(EnvironmentMap &E, int AIdx) const { +bool FunctionVariableConstraint::hasNtArr(const EnvironmentMap &E, + int AIdx) const { return ReturnVar->hasNtArr(E, AIdx); } @@ -1024,7 +1027,7 @@ void PointerVariableConstraint::constrainOuterTo(Constraints &CS, ConstAtom *C, } } -bool PointerVariableConstraint::anyArgumentIsWild(EnvironmentMap &E) { +bool PointerVariableConstraint::anyArgumentIsWild(const EnvironmentMap &E) { for (auto *ArgVal : argumentConstraints) { if (!ArgVal->isChecked(E)) { return true; @@ -1033,7 +1036,7 @@ bool PointerVariableConstraint::anyArgumentIsWild(EnvironmentMap &E) { return false; } -bool PointerVariableConstraint::anyChanges(EnvironmentMap &E) const { +bool PointerVariableConstraint::anyChanges(const EnvironmentMap &E) const { // If a pointer variable was checked in the input program, it will have the // same checked type in the output, so it cannot have changed. if (OriginallyChecked) @@ -1061,21 +1064,21 @@ ConstraintVariable *PointerVariableConstraint::getCopy(Constraints &CS) { } const ConstAtom * -PointerVariableConstraint::getSolution(const Atom *A, EnvironmentMap &E) const { +PointerVariableConstraint::getSolution(const Atom *A, + const EnvironmentMap &E) const { const ConstAtom *CS = nullptr; if (const ConstAtom *CA = dyn_cast(A)) { CS = CA; } else if (const VarAtom *VA = dyn_cast(A)) { - // If this is a VarAtom?, we need ot fetch from solution - // i.e., environment. - CS = E[const_cast(VA)].first; + // If this is a VarAtom?, we need to fetch from solution i.e., environment. + CS = E.at(const_cast(VA)).first; } assert(CS != nullptr && "Atom should be either const or var"); return CS; } -bool PointerVariableConstraint::hasWild(EnvironmentMap &E, int AIdx) const -{ +bool PointerVariableConstraint::hasWild(const EnvironmentMap &E, + int AIdx) const { int VarIdx = 0; for (const auto &C : vars) { const ConstAtom *CS = getSolution(C, E); @@ -1092,8 +1095,8 @@ bool PointerVariableConstraint::hasWild(EnvironmentMap &E, int AIdx) const return false; } -bool PointerVariableConstraint::hasArr(EnvironmentMap &E, int AIdx) const -{ +bool PointerVariableConstraint::hasArr(const EnvironmentMap &E, + int AIdx) const { int VarIdx = 0; for (const auto &C : vars) { const ConstAtom *CS = getSolution(C, E); @@ -1110,8 +1113,8 @@ bool PointerVariableConstraint::hasArr(EnvironmentMap &E, int AIdx) const return false; } -bool PointerVariableConstraint::hasNtArr(EnvironmentMap &E, int AIdx) const -{ +bool PointerVariableConstraint::hasNtArr(const EnvironmentMap &E, + int AIdx) const { int VarIdx = 0; for (const auto &C : vars) { const ConstAtom *CS = getSolution(C, E); @@ -1237,7 +1240,7 @@ bool FunctionVariableConstraint:: } std::string -FunctionVariableConstraint::mkString(EnvironmentMap &E, +FunctionVariableConstraint::mkString(const EnvironmentMap &E, bool EmitName, bool ForItype, bool EmitPointee) const { std::string Ret = ""; diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index 234b1fd793db..afb91cd89491 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -182,7 +182,7 @@ void ProgramInfo::print_stats(const std::set &F, raw_ostream &O, O << "Sound handling of var args functions:" << HandleVARARGS << "\n"; } std::map> FilesToVars; - EnvironmentMap Env = CS.getVariables(); + const EnvironmentMap &Env = CS.getVariables(); CVarSet InSrcCVars; unsigned int totC, totP, totNt, totA, totWi; totC = totP = totNt = totA = totWi = 0; @@ -738,7 +738,7 @@ bool ProgramInfo::computeInterimConstraintState // Get all the valid vars of interest i.e., all the Vars that are present // in one of the files being compiled. CAtoms ValidVarsVec; - for (auto &I : Variables) { + for (const auto &I : Variables) { std::string FileName = I.first.getFileName(); if (FilePaths.count(FileName)) { for (auto &C : I.second) { From a6e90db27d51fa1012427f6a0c685c766f550845 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 14 Sep 2020 17:26:08 -0400 Subject: [PATCH 03/15] Move PersistentConstraint methods into ProgramInfo --- .../include/clang/CConv/ConstraintResolver.h | 4 +- clang/include/clang/CConv/ProgramInfo.h | 6 ++- clang/lib/CConv/ConstraintResolver.cpp | 54 +++---------------- clang/lib/CConv/ProgramInfo.cpp | 39 +++++++++++--- clang/lib/CConv/RewriteUtils.cpp | 4 +- 5 files changed, 47 insertions(+), 60 deletions(-) diff --git a/clang/include/clang/CConv/ConstraintResolver.h b/clang/include/clang/CConv/ConstraintResolver.h index f5ac5ef9c2b9..6e77579deb4b 100644 --- a/clang/include/clang/CConv/ConstraintResolver.h +++ b/clang/include/clang/CConv/ConstraintResolver.h @@ -69,9 +69,7 @@ class ConstraintResolver { CVarSet getAllSubExprConstraintVars(std::vector &Exprs); CVarSet getBaseVarPVConstraint(DeclRefExpr *Decl); - bool hasPersistentConstraints(clang::Expr *E); - CVarSet getPersistentConstraints(clang::Expr *E); - void storePersistentConstraints(clang::Expr *E, CVarSet &Vars); + PVConstraint *getRewritablePVConstraint(Expr *E); }; diff --git a/clang/include/clang/CConv/ProgramInfo.h b/clang/include/clang/CConv/ProgramInfo.h index 573cb994dfb0..897a11f039d9 100644 --- a/clang/include/clang/CConv/ProgramInfo.h +++ b/clang/include/clang/CConv/ProgramInfo.h @@ -68,7 +68,11 @@ class ProgramInfo : public ProgramVariableAdder { // should all be empty. void exitCompilationUnit(); - CVarSet &getPersistentConstraintVars(Expr *E, ASTContext *AstContext); + bool hasPersistentConstraints(clang::Expr *E, ASTContext *C) const; + const CVarSet &getPersistentConstraints(clang::Expr *E, ASTContext *C) const; + void storePersistentConstraints(clang::Expr *E, const CVarSet &Vars, + ASTContext *C); + // Get constraint variable for the provided Decl CVarSet getVariable(clang::Decl *D, clang::ASTContext *C); diff --git a/clang/lib/CConv/ConstraintResolver.cpp b/clang/lib/CConv/ConstraintResolver.cpp index c7b689dc766f..7c55d5f5a2f5 100644 --- a/clang/lib/CConv/ConstraintResolver.cpp +++ b/clang/lib/CConv/ConstraintResolver.cpp @@ -176,8 +176,8 @@ CVarSet ConstraintResolver::getInvalidCastPVCons(Expr *E) { // As getInvalidCastPVCons could be called from non-persistent expressions // we need to explicitly store the generated PVConstraints into persistent // constraints. - if (hasPersistentConstraints(E)) - return getPersistentConstraints(E); + if (Info.hasPersistentConstraints(E, Context)) + return Info.getPersistentConstraints(E, Context); DstType = E->getType(); SrcType = E->getType(); @@ -194,7 +194,7 @@ CVarSet ConstraintResolver::getInvalidCastPVCons(Expr *E) { "Cast from " + SrcType.getAsString() + " to " + DstType.getAsString(); P->constrainToWild(Info.getConstraints(), Rsn, &PL); Ret = {P}; - storePersistentConstraints(E, Ret); + Info.storePersistentConstraints(E, Ret, Context); return Ret; } @@ -256,9 +256,8 @@ CVarSet // Apart from the above expressions constraints for all the other // expressions can be cached. // First, check if the expression has constraints that are cached? - if (hasPersistentConstraints(E)) { - return getPersistentConstraints(E); - } + if (Info.hasPersistentConstraints(E, Context)) + return Info.getPersistentConstraints(E, Context); CVarSet Ret = EmptyCSet; if (ExplicitCastExpr *ECE = dyn_cast(E)) { @@ -597,54 +596,13 @@ CVarSet llvm::errs() << "\n"; } } - storePersistentConstraints(E, Ret); + Info.storePersistentConstraints(E, Ret, Context); return Ret; } } return EmptyCSet; } -bool ConstraintResolver::hasPersistentConstraints(clang::Expr *E) { - auto PSL = PersistentSourceLoc::mkPSL(E, *Context); - // Has constraints only if the PSL is valid. - if (PSL.valid()) { - CVarSet &Persist = Info.getPersistentConstraintVars(E, Context); - return !Persist.empty(); - } - return false; -} - -// Get the set of constraint variables for an expression that will persist -// between the constraint generation and rewriting pass. If the expression -// already has a set of persistent constraints, this set is returned. Otherwise, -// the set provided in the arguments is stored persistent and returned. This is -// required for correct cast insertion. -CVarSet - ConstraintResolver::getPersistentConstraints(clang::Expr *E) { - assert (hasPersistentConstraints(E) && - "Persistent constraints not present."); - CVarSet &Persist = Info.getPersistentConstraintVars(E, Context); - return Persist; -} - - -void ConstraintResolver::storePersistentConstraints(clang::Expr *E, - CVarSet &Vars) { - // Store only if the PSL is valid. - auto PSL = PersistentSourceLoc::mkPSL(E, *Context); - // The check Rewrite::isRewritable is needed here to ensure that the - // expression is not inside a macro. If the expression is in a macro, then it - // is possible for there to be multiple expressions that map to the same PSL. - // This could make it look like the constraint variables for an expression - // have been computed and cached when the expression has not in fact been - // visited before. To avoid this, the expression is not cached and instead is - // recomputed each time it's needed. - if (PSL.valid() && Rewriter::isRewritable(E->getBeginLoc())){ - CVarSet &Persist = Info.getPersistentConstraintVars(E, Context); - Persist.insert(Vars.begin(), Vars.end()); - } -} - // Collect constraint variables for Exprs int a set CVarSet ConstraintResolver::getAllSubExprConstraintVars( std::vector &Exprs) { diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index afb91cd89491..c659c493ce72 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -182,7 +182,6 @@ void ProgramInfo::print_stats(const std::set &F, raw_ostream &O, O << "Sound handling of var args functions:" << HandleVARARGS << "\n"; } std::map> FilesToVars; - const EnvironmentMap &Env = CS.getVariables(); CVarSet InSrcCVars; unsigned int totC, totP, totNt, totA, totWi; totC = totP = totNt = totA = totWi = 0; @@ -591,13 +590,39 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, constrainWildIfMacro(S, D->getLocation()); } -CVarSet - &ProgramInfo::getPersistentConstraintVars(Expr *E, - clang::ASTContext *AstContext) { - PersistentSourceLoc PLoc = PersistentSourceLoc::mkPSL(E, *AstContext); - assert(PLoc.valid()); +bool ProgramInfo::hasPersistentConstraints(Expr *E, ASTContext *C) const { + auto PSL = PersistentSourceLoc::mkPSL(E, *C); + // Has constraints only if the PSL is valid. + return PSL.valid() && Variables.find(PSL) != Variables.end() + && !Variables.at(PSL).empty(); +} + +// Get the set of constraint variables for an expression that will persist +// between the constraint generation and rewriting pass. If the expression +// already has a set of persistent constraints, this set is returned. Otherwise, +// the set provided in the arguments is stored persistent and returned. This is +// required for correct cast insertion. +const CVarSet &ProgramInfo::getPersistentConstraints(Expr *E, + ASTContext *C) const { + assert (hasPersistentConstraints(E, C) && + "Persistent constraints not present."); + PersistentSourceLoc PLoc = PersistentSourceLoc::mkPSL(E, *C); + return Variables.at(PLoc); +} - return Variables[PLoc]; +void ProgramInfo::storePersistentConstraints(Expr *E, const CVarSet &Vars, + ASTContext *C) { + // Store only if the PSL is valid. + auto PSL = PersistentSourceLoc::mkPSL(E, *C); + // The check Rewrite::isRewritable is needed here to ensure that the + // expression is not inside a macro. If the expression is in a macro, then it + // is possible for there to be multiple expressions that map to the same PSL. + // This could make it look like the constraint variables for an expression + // have been computed and cached when the expression has not in fact been + // visited before. To avoid this, the expression is not cached and instead is + // recomputed each time it's needed. + if (PSL.valid() && Rewriter::isRewritable(E->getBeginLoc())) + Variables[PSL].insert(Vars.begin(), Vars.end()); } // The Rewriter won't let us re-write things that are in macros. So, we diff --git a/clang/lib/CConv/RewriteUtils.cpp b/clang/lib/CConv/RewriteUtils.cpp index 16eb1f252712..47ba4c56dc1b 100644 --- a/clang/lib/CConv/RewriteUtils.cpp +++ b/clang/lib/CConv/RewriteUtils.cpp @@ -224,7 +224,9 @@ class TypeExprRewriter Rewriter &Writer; void rewriteType(Expr *E, SourceRange &Range) { - CVarSet CVSingleton = Info.getPersistentConstraintVars(E, Context); + if (!Info.hasPersistentConstraints(E, Context)) + return; + const CVarSet &CVSingleton = Info.getPersistentConstraints(E, Context); if (CVSingleton.empty()) return; From e68792d9a31e747802c1a50be9c126165e4c8047 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 14 Sep 2020 17:48:29 -0400 Subject: [PATCH 04/15] Store persistent expression constraints in new map These were previously stored the Variables map that holds declaration constraint variables. --- clang/include/clang/CConv/ProgramInfo.h | 11 ++++++++--- clang/lib/CConv/ProgramInfo.cpp | 8 ++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/CConv/ProgramInfo.h b/clang/include/clang/CConv/ProgramInfo.h index 897a11f039d9..68b9850b2c80 100644 --- a/clang/include/clang/CConv/ProgramInfo.h +++ b/clang/include/clang/CConv/ProgramInfo.h @@ -116,10 +116,15 @@ class ProgramInfo : public ProgramVariableAdder { void constrainWildIfMacro(CVarSet &S, SourceLocation Location); private: - // List of all constraint variables, indexed by their location in the source. - // This information persists across invocations of the constraint analysis - // from compilation unit to compilation unit. + // List of constraint variables for declarations, indexed by their location in + // the source. This information persists across invocations of the constraint + // analysis from compilation unit to compilation unit. VariableMap Variables; + + // Map with the same purpose as the Variables map, this stores constraint vars + // non-declaration expressions. + VariableMap ExprConstraintVars; + // Constraint system. Constraints CS; // Is the ProgramInfo persisted? Only tested in asserts. Starts at true. diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index c659c493ce72..1ce8b9988695 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -593,8 +593,8 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, bool ProgramInfo::hasPersistentConstraints(Expr *E, ASTContext *C) const { auto PSL = PersistentSourceLoc::mkPSL(E, *C); // Has constraints only if the PSL is valid. - return PSL.valid() && Variables.find(PSL) != Variables.end() - && !Variables.at(PSL).empty(); + return PSL.valid() && ExprConstraintVars.find(PSL) != ExprConstraintVars.end() + && !ExprConstraintVars.at(PSL).empty(); } // Get the set of constraint variables for an expression that will persist @@ -607,7 +607,7 @@ const CVarSet &ProgramInfo::getPersistentConstraints(Expr *E, assert (hasPersistentConstraints(E, C) && "Persistent constraints not present."); PersistentSourceLoc PLoc = PersistentSourceLoc::mkPSL(E, *C); - return Variables.at(PLoc); + return ExprConstraintVars.at(PLoc); } void ProgramInfo::storePersistentConstraints(Expr *E, const CVarSet &Vars, @@ -622,7 +622,7 @@ void ProgramInfo::storePersistentConstraints(Expr *E, const CVarSet &Vars, // visited before. To avoid this, the expression is not cached and instead is // recomputed each time it's needed. if (PSL.valid() && Rewriter::isRewritable(E->getBeginLoc())) - Variables[PSL].insert(Vars.begin(), Vars.end()); + ExprConstraintVars[PSL].insert(Vars.begin(), Vars.end()); } // The Rewriter won't let us re-write things that are in macros. So, we From b6bb11ff4523258a32cead439664e294e1aaea34 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 15 Sep 2020 13:35:43 -0400 Subject: [PATCH 05/15] Variables map now associates declaration with a single constraint --- clang/include/clang/CConv/AVarBoundsInfo.h | 8 +- clang/include/clang/CConv/CheckedRegions.h | 4 +- .../include/clang/CConv/ConstraintResolver.h | 5 +- clang/include/clang/CConv/ProgramInfo.h | 7 +- clang/include/clang/CConv/Utils.h | 3 +- clang/lib/CConv/AVarBoundsInfo.cpp | 42 ++--- .../CConv/ArrayBoundsInferenceConsumer.cpp | 8 +- clang/lib/CConv/CheckedRegions.cpp | 50 +++--- clang/lib/CConv/ConstraintBuilder.cpp | 37 ++-- clang/lib/CConv/ConstraintResolver.cpp | 34 ++-- clang/lib/CConv/DeclRewriter.cpp | 15 +- clang/lib/CConv/ProgramInfo.cpp | 163 ++++++++---------- clang/lib/CConv/StructInit.cpp | 14 +- 13 files changed, 177 insertions(+), 213 deletions(-) diff --git a/clang/include/clang/CConv/AVarBoundsInfo.h b/clang/include/clang/CConv/AVarBoundsInfo.h index 865b6481676d..ddb586920351 100644 --- a/clang/include/clang/CConv/AVarBoundsInfo.h +++ b/clang/include/clang/CConv/AVarBoundsInfo.h @@ -174,11 +174,11 @@ class AVarBoundsInfo { clang::Expr *R, ASTContext *C, ConstraintResolver *CR); - bool handleAssignment(clang::Expr *L, CVarSet &LCVars, - clang::Expr *R, CVarSet &RCVars, + bool handleAssignment(clang::Expr *L, const CVarSet &LCVars, + clang::Expr *R, const CVarSet &RCVars, ASTContext *C, ConstraintResolver *CR); - bool handleAssignment(clang::Decl *L, CVarSet &LCVars, - clang::Expr *R, CVarSet &RCVars, + bool handleAssignment(clang::Decl *L, ConstraintVariable *LCVar, + clang::Expr *R, const CVarSet &RCVars, ASTContext *C, ConstraintResolver *CR); // Handle context sensitive assignment. bool handleContextSensitiveAssignment(CallExpr *CE, clang::Decl *L, diff --git a/clang/include/clang/CConv/CheckedRegions.h b/clang/include/clang/CConv/CheckedRegions.h index c4b39ae24347..84736da14907 100644 --- a/clang/include/clang/CConv/CheckedRegions.h +++ b/clang/include/clang/CConv/CheckedRegions.h @@ -84,8 +84,8 @@ class CheckedRegionFinder : public clang::RecursiveASTVisitor &Seen); bool isUncheckedStruct(clang::QualType Qt, std::set &Seen); - bool isWild(const std::set&); - bool isWild(const std::set*); + bool isWild(const ConstraintVariable*); + bool isWild(const FVConstraint*); clang::ASTContext* Context; clang::Rewriter& Writer; diff --git a/clang/include/clang/CConv/ConstraintResolver.h b/clang/include/clang/CConv/ConstraintResolver.h index 6e77579deb4b..5280f342f700 100644 --- a/clang/include/clang/CConv/ConstraintResolver.h +++ b/clang/include/clang/CConv/ConstraintResolver.h @@ -26,8 +26,7 @@ class ConstraintResolver { virtual ~ConstraintResolver(); - void constraintAllCVarsToWild(CVarSet &CSet, - std::string rsn, + void constraintAllCVarsToWild(const CVarSet &CSet, const std::string &Rsn, Expr *AtExpr = nullptr); // Returns a set of ConstraintVariables which represent the result of @@ -48,7 +47,7 @@ class ConstraintResolver { bool containsValidCons(const CVarSet &CVs); bool isValidCons(ConstraintVariable *CV); // Try to get the bounds key from the constraint variable set. - bool resolveBoundsKey(CVarSet &CVs, BoundsKey &BK); + bool resolveBoundsKey(const CVarSet &CVs, BoundsKey &BK); bool resolveBoundsKey(ConstraintVariable *CV, BoundsKey &BK); static bool canFunctionBeSkipped(const std::string &FN); diff --git a/clang/include/clang/CConv/ProgramInfo.h b/clang/include/clang/CConv/ProgramInfo.h index 68b9850b2c80..089e1379f3d4 100644 --- a/clang/include/clang/CConv/ProgramInfo.h +++ b/clang/include/clang/CConv/ProgramInfo.h @@ -74,7 +74,7 @@ class ProgramInfo : public ProgramVariableAdder { ASTContext *C); // Get constraint variable for the provided Decl - CVarSet getVariable(clang::Decl *D, clang::ASTContext *C); + ConstraintVariable *getVariable(clang::Decl *D, clang::ASTContext *C); // Retrieve a function's constraints by decl, or by name; nullptr if not found FVConstraint *getFuncConstraint (FunctionDecl *D, ASTContext *C) const; @@ -82,6 +82,7 @@ class ProgramInfo : public ProgramVariableAdder { FVConstraint *getStaticFuncConstraint(std::string FuncName, std::string FileName) const; + // Check if the given function is an extern function. bool isAnExternFunction(const std::string &FName); @@ -113,7 +114,7 @@ class ProgramInfo : public ProgramVariableAdder { const CallTypeParamBindingsT &getTypeParamBindings(CallExpr *CE, ASTContext *C) const; - void constrainWildIfMacro(CVarSet &S, SourceLocation Location); + void constrainWildIfMacro(ConstraintVariable *CV, SourceLocation Location); private: // List of constraint variables for declarations, indexed by their location in @@ -123,7 +124,7 @@ class ProgramInfo : public ProgramVariableAdder { // Map with the same purpose as the Variables map, this stores constraint vars // non-declaration expressions. - VariableMap ExprConstraintVars; + std::map ExprConstraintVars; // Constraint system. Constraints CS; diff --git a/clang/include/clang/CConv/Utils.h b/clang/include/clang/CConv/Utils.h index bb428620d26b..afa0dd1bd9a9 100644 --- a/clang/include/clang/CConv/Utils.h +++ b/clang/include/clang/CConv/Utils.h @@ -26,8 +26,7 @@ class ConstraintVariable; class ProgramInfo; // Maps a Decl to the set of constraint variables for that Decl. -typedef std::map> VariableMap; +typedef std::map VariableMap; // Maps a Decl to the DeclStmt that defines the Decl. typedef std::map VariableDecltoStmtMap; diff --git a/clang/lib/CConv/AVarBoundsInfo.cpp b/clang/lib/CConv/AVarBoundsInfo.cpp index 4dd50a26e466..fca82c76bc55 100644 --- a/clang/lib/CConv/AVarBoundsInfo.cpp +++ b/clang/lib/CConv/AVarBoundsInfo.cpp @@ -325,8 +325,8 @@ bool AVarBoundsInfo::addAssignment(clang::DeclRefExpr *L, return addAssignment(L->getDecl(), R->getDecl()); } -bool AVarBoundsInfo::handleAssignment(clang::Expr *L, CVarSet &LCVars, - clang::Expr *R, CVarSet &RCVars, +bool AVarBoundsInfo::handleAssignment(clang::Expr *L, const CVarSet &LCVars, + clang::Expr *R, const CVarSet &RCVars, ASTContext *C, ConstraintResolver *CR) { BoundsKey LKey, RKey; if ((CR->resolveBoundsKey(LCVars, LKey) || @@ -338,8 +338,8 @@ bool AVarBoundsInfo::handleAssignment(clang::Expr *L, CVarSet &LCVars, return false; } -bool AVarBoundsInfo::handleAssignment(clang::Decl *L, CVarSet &LCVars, - clang::Expr *R, CVarSet &RCVars, +bool AVarBoundsInfo::handleAssignment(clang::Decl *L, ConstraintVariable *LCVars, + clang::Expr *R, const CVarSet &RCVars, ASTContext *C, ConstraintResolver *CR) { BoundsKey LKey, RKey; if ((CR->resolveBoundsKey(LCVars, LKey) || @@ -538,27 +538,23 @@ void AVarBoundsInfo::insertProgramVar(BoundsKey NK, ProgramVar *PV) { PVarInfo[NK] = PV; } -bool hasArray(const CVarSet &CSet, Constraints &CS) { +bool hasArray(ConstraintVariable *CK, Constraints &CS) { auto &E = CS.getVariables(); - for (auto *CK : CSet) { - if (PVConstraint *PV = dyn_cast(CK)) { - if ((PV->hasArr(E, 0) || PV->hasNtArr(E, 0)) && - PV->isTopCvarUnsizedArr()) { - return true; - } + if (PVConstraint *PV = dyn_cast(CK)) { + if ((PV->hasArr(E, 0) || PV->hasNtArr(E, 0)) && + PV->isTopCvarUnsizedArr()) { + return true; } } return false; } -bool isInSrcArray(const CVarSet &CSet, Constraints &CS) { +bool isInSrcArray(ConstraintVariable *CK, Constraints &CS) { auto &E = CS.getVariables(); - for (auto *CK : CSet) { - if (PVConstraint *PV = dyn_cast(CK)) { - if ((PV->hasArr(E, 0) || PV->hasNtArr(E, 0)) && - PV->isTopCvarUnsizedArr() && PV->isForValidDecl()) { - return true; - } + if (PVConstraint *PV = dyn_cast(CK)) { + if ((PV->hasArr(E, 0) || PV->hasNtArr(E, 0)) && + PV->isTopCvarUnsizedArr() && PV->isForValidDecl()) { + return true; } } return false; @@ -975,11 +971,11 @@ void AVarBoundsInfo::computerArrPointers(ProgramInfo *PI, FV = PI->getExtFuncDefnConstraint(FuncName); } - if (hasArray({FV->getParamVar(ParmNum)}, CS)) { + if (hasArray(FV->getParamVar(ParmNum), CS)) { ArrPointers.insert(Bkey); } // Does this array belongs to a valid program variable? - if (isInSrcArray({FV->getParamVar(ParmNum)}, CS)) { + if (isInSrcArray(FV->getParamVar(ParmNum), CS)) { InProgramArrPtrBoundsKeys.insert(Bkey); } @@ -1003,11 +999,11 @@ void AVarBoundsInfo::computerArrPointers(ProgramInfo *PI, FV = getOnly(Tmp); } - if (hasArray({FV->getReturnVar()}, CS)) { + if (hasArray(FV->getReturnVar(), CS)) { ArrPointers.insert(Bkey); } // Does this array belongs to a valid program variable? - if (isInSrcArray({FV->getReturnVar()}, CS)) { + if (isInSrcArray(FV->getReturnVar(), CS)) { InProgramArrPtrBoundsKeys.insert(Bkey); } continue; @@ -1139,7 +1135,7 @@ ContextSensitiveBoundsKeyVisitor::~ContextSensitiveBoundsKeyVisitor() { bool ContextSensitiveBoundsKeyVisitor::VisitCallExpr(CallExpr *CE) { if (FunctionDecl *FD = dyn_cast_or_null(CE->getCalleeDecl())) { // Contextualize the function at this call-site. - for (auto *CFV : Info.getVariable(FD, Context)) { + if (ConstraintVariable *CFV = Info.getVariable(FD, Context)) { Info.getABoundsInfo().contextualizeCVar(CE, {CFV}); } } diff --git a/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp b/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp index 7e6e5db16bef..f64d5aff8710 100644 --- a/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp +++ b/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp @@ -133,9 +133,8 @@ static bool needArrayBounds(Expr *E, ProgramInfo &Info, ASTContext *C) { static bool needArrayBounds(Decl *D, ProgramInfo &Info, ASTContext *C, bool IsNtArr) { - CVarSet ConsVar = Info.getVariable(D, C); const auto &E = Info.getConstraints().getVariables(); - for (auto CurrCVar : ConsVar) { + if (ConstraintVariable *CurrCVar = Info.getVariable(D, C)) { if ((!IsNtArr && needArrayBounds(CurrCVar, E)) || (IsNtArr && needNTArrayBounds(CurrCVar, E))) return true; @@ -179,10 +178,9 @@ bool tryGetBoundsKeyVar(Expr *E, BoundsKey &BK, ProgramInfo &Info, bool tryGetBoundsKeyVar(Decl *D, BoundsKey &BK, ProgramInfo &Info, ASTContext *Context) { ConstraintResolver CR(Info, Context); - CVarSet CVs = Info.getVariable(D, Context); + ConstraintVariable *CV = Info.getVariable(D, Context); auto &ABInfo = Info.getABoundsInfo(); - return CR.resolveBoundsKey(CVs, BK) || - ABInfo.tryGetVariable(D, BK); + return (CV && CR.resolveBoundsKey(CV, BK)) || ABInfo.tryGetVariable(D, BK); } // Check if the provided expression is a call to one of the known diff --git a/clang/lib/CConv/CheckedRegions.cpp b/clang/lib/CConv/CheckedRegions.cpp index a5ca8aea16f0..3500066a2470 100644 --- a/clang/lib/CConv/CheckedRegions.cpp +++ b/clang/lib/CConv/CheckedRegions.cpp @@ -188,8 +188,8 @@ bool CheckedRegionFinder::VisitCallExpr(CallExpr *C) { || containsUncheckedPtr(type) || (std::any_of(FD->param_begin(), FD->param_end(), [this] (Decl *param) { - auto CVSet = Info.getVariable(param, Context); - return isWild(CVSet ); + auto CV = Info.getVariable(param, Context); + return CV && isWild(CV); })); } handleChildren(C->children()); @@ -200,15 +200,15 @@ bool CheckedRegionFinder::VisitCallExpr(CallExpr *C) { } bool CheckedRegionFinder::VisitVarDecl(VarDecl *VD) { - auto CVSet = Info.getVariable(VD, Context); - Wild = isWild(CVSet) || containsUncheckedPtr(VD->getType()); + auto CV = Info.getVariable(VD, Context); + Wild = (CV && isWild(CV)) || containsUncheckedPtr(VD->getType()); return true; } bool CheckedRegionFinder::VisitParmVarDecl(ParmVarDecl *PVD) { // Check if the variable is WILD. - auto CVSet = Info.getVariable(PVD, Context); - Wild |= isWild(CVSet) | containsUncheckedPtr(PVD->getType()); + auto CV = Info.getVariable(PVD, Context); + Wild |= (CV && isWild(CV)) || containsUncheckedPtr(PVD->getType()); return true; } @@ -216,12 +216,9 @@ bool CheckedRegionFinder::VisitMemberExpr(MemberExpr *E){ ValueDecl *VD = E->getMemberDecl(); if (VD) { // Check if the variable is WILD. - std::set CVSet = Info.getVariable(VD, Context); - for (auto Cv : CVSet) { - if (Cv->hasWild(Info.getConstraints().getVariables())) { - Wild = true; - } - } + ConstraintVariable *Cv = Info.getVariable(VD, Context); + if (Cv && Cv->hasWild(Info.getConstraints().getVariables())) + Wild = true; // Check if the variable contains unchecked types. Wild |= containsUncheckedPtr(VD->getType()); } @@ -231,15 +228,15 @@ bool CheckedRegionFinder::VisitMemberExpr(MemberExpr *E){ bool CheckedRegionFinder::VisitDeclRefExpr(DeclRefExpr* DR) { auto T = DR->getType(); auto D = DR->getDecl(); - auto CVSet = Info.getVariable(D, Context); - bool IW = isWild(CVSet ) || containsUncheckedPtr(T); + auto CV = Info.getVariable(D, Context); + bool IW = (CV && isWild(CV)) || containsUncheckedPtr(T); if (auto FD = dyn_cast(D)) { auto *FV = Info.getFuncConstraint(FD, Context); IW |= FV->hasWild(Info.getConstraints().getVariables()); for (const auto& param: FD->parameters()) { - auto CVSet = Info.getVariable(param, Context); - IW |= isWild(CVSet); + auto CV = Info.getVariable(param, Context); + IW |= (CV && isWild(CV)); } } @@ -298,18 +295,15 @@ bool CheckedRegionFinder::isInStatementPosition(CallExpr *C) { } } -bool CheckedRegionFinder::isWild(const std::set &S) { - for (auto Cv : S) - if (Cv->hasWild(Info.getConstraints().getVariables())) - return true; - +bool CheckedRegionFinder::isWild(const ConstraintVariable* Cv) { + if (Cv->hasWild(Info.getConstraints().getVariables())) + return true; return false; } -bool CheckedRegionFinder::isWild(const std::set *S) { - for (auto Fv : *S) - if (Fv->hasWild(Info.getConstraints().getVariables())) - return true; +bool CheckedRegionFinder::isWild(const FVConstraint* Fv) { + if (Fv->hasWild(Info.getConstraints().getVariables())) + return true; return false; } @@ -367,10 +361,8 @@ bool CheckedRegionFinder::isUncheckedStruct(QualType Qt, std::set & for (auto const &Fld : D->fields()) { auto Ftype = Fld->getType(); Unsafe |= containsUncheckedPtrAcc(Ftype, Seen); - std::set CVSet = - Info.getVariable(Fld, Context); - for (auto Cv : CVSet) - Unsafe |= Cv->hasWild(Info.getConstraints().getVariables()); + ConstraintVariable *Cv = Info.getVariable(Fld, Context); + Unsafe |= (Cv && Cv->hasWild(Info.getConstraints().getVariables())); } return Unsafe; } diff --git a/clang/lib/CConv/ConstraintBuilder.cpp b/clang/lib/CConv/ConstraintBuilder.cpp index c9f48a29c883..e21f4c1b4fff 100644 --- a/clang/lib/CConv/ConstraintBuilder.cpp +++ b/clang/lib/CConv/ConstraintBuilder.cpp @@ -58,9 +58,10 @@ void processRecordDecl(RecordDecl *Declaration, ProgramInfo &Info, // mark field wild if the above is true and the field is a pointer if ((FieldTy->isPointerType() || FieldTy->isArrayType()) && (FieldInUnionOrSysHeader || IsInLineStruct)) { - CVarSet C = Info.getVariable(F, Context); - std::string Rsn = "External struct field or union encountered"; - CB.constraintAllCVarsToWild(C, Rsn, nullptr); + if (ConstraintVariable *CV = Info.getVariable(F, Context)) { + std::string Rsn = "External struct field or union encountered"; + CB.constraintAllCVarsToWild({CV}, Rsn, nullptr); + } } } } @@ -95,8 +96,9 @@ class FunctionVisitor : public RecursiveASTVisitor { (VD->getType()->isPointerType() || VD->getType()->isArrayType())) { if (lastRecordLocation == VD->getBeginLoc().getRawEncoding()) { - CVarSet C = Info.getVariable(VD, Context); - CB.constraintAllCVarsToWild(C, "Inline struct encountered.", nullptr); + if (ConstraintVariable *CV = Info.getVariable(VD, Context)) + CB.constraintAllCVarsToWild({CV}, "Inline struct encountered.", + nullptr); } } } @@ -176,11 +178,13 @@ class FunctionVisitor : public RecursiveASTVisitor { } } else if (FunctionDecl *FD = dyn_cast(D)) { FuncName = FD->getNameAsString(); - FVCons = Info.getVariable(FD, Context); TFD = FD; + if (ConstraintVariable *CV = Info.getVariable(FD, Context)) + FVCons.insert(CV); } else if (DeclaratorDecl *DD = dyn_cast(D)) { - FVCons = Info.getVariable(DD, Context); FuncName = DD->getNameAsString(); + if (ConstraintVariable *CV = Info.getVariable(DD, Context)) + FVCons.insert(CV); } // Collect type parameters for this function call that are @@ -284,7 +288,7 @@ class FunctionVisitor : public RecursiveASTVisitor { // Get function variable constraint of the body PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(S, *Context); - CVarSet Fun = + ConstraintVariable *CV = Info.getVariable(Function, Context); // Constrain the value returned (if present) against the return value @@ -294,13 +298,11 @@ class FunctionVisitor : public RecursiveASTVisitor { CVarSet RconsVar = CB.getExprConstraintVars(RetExpr); // Constrain the return type of the function // to the type of the return expression. - for (const auto &F : Fun) { - if (FVConstraint *FV = dyn_cast(F)) { - // This is to ensure that the return type of the function is same - // as the type of return expression. - constrainConsVarGeq(FV->getReturnVar(), RconsVar, Info.getConstraints(), - &PL, Same_to_Same, false, &Info); - } + if (FVConstraint *FV = dyn_cast_or_null(CV)) { + // This is to ensure that the return type of the function is same + // as the type of return expression. + constrainConsVarGeq(FV->getReturnVar(), RconsVar, Info.getConstraints(), + &PL, Same_to_Same, false, &Info); } return true; } @@ -428,8 +430,9 @@ class ConstraintGenVisitor : public RecursiveASTVisitor { unsigned int BeginLoc = G->getBeginLoc().getRawEncoding(); unsigned int EndLoc = G->getEndLoc().getRawEncoding(); if (lastRecordLocation >= BeginLoc && lastRecordLocation <= EndLoc) { - CVarSet C = Info.getVariable(G, Context); - CB.constraintAllCVarsToWild(C, "Inline struct encountered.", nullptr); + if (ConstraintVariable *CV = Info.getVariable(G, Context)) + CB.constraintAllCVarsToWild({CV}, "Inline struct encountered.", + nullptr); } } diff --git a/clang/lib/CConv/ConstraintResolver.cpp b/clang/lib/CConv/ConstraintResolver.cpp index 7c55d5f5a2f5..3fb11b073d72 100644 --- a/clang/lib/CConv/ConstraintResolver.cpp +++ b/clang/lib/CConv/ConstraintResolver.cpp @@ -19,8 +19,9 @@ using namespace clang; ConstraintResolver::~ConstraintResolver() { } // Force all ConstraintVariables in this set to be WILD -void ConstraintResolver::constraintAllCVarsToWild( - CVarSet &CSet, std::string rsn, Expr *AtExpr) { +void ConstraintResolver::constraintAllCVarsToWild(const CVarSet &CSet, + const std::string &rsn, + Expr *AtExpr) { PersistentSourceLoc Psl; PersistentSourceLoc *PslP = nullptr; if (AtExpr != nullptr) { @@ -244,10 +245,14 @@ CVarSet return CVs; // variable (x) } else if (DeclRefExpr *DRE = dyn_cast(E)) { - return Info.getVariable(DRE->getDecl(), Context); + ConstraintVariable *CV = Info.getVariable(DRE->getDecl(), Context); + assert("Declaration without constraint variable?" && CV); + return {CV}; // x.f } else if (MemberExpr *ME = dyn_cast(E)) { - return Info.getVariable(ME->getMemberDecl(), Context); + ConstraintVariable *CV = Info.getVariable(ME->getMemberDecl(), Context); + assert("Declaration without constraint variable?" && CV); + return {CV}; // Checked-C temporary } else if (CHKCBindTemporaryExpr *CE = dyn_cast(E)) { return getExprConstraintVars(CE->getSubExpr()); @@ -466,8 +471,8 @@ CVarSet /* Normal function call */ } else { - CVarSet TmpCSet = Info.getVariable(FD, Context); - ConstraintVariable *J = getOnly(TmpCSet); + ConstraintVariable *J = Info.getVariable(FD, Context); + assert(J && "Function without constraint variable."); /* Direct function call */ if (FVConstraint *FVC = dyn_cast(J)) ReturnCVs.insert(FVC->getReturnVar()); @@ -644,13 +649,13 @@ void ConstraintResolver::constrainLocalAssign(Stmt *TSt, DeclaratorDecl *D, PLPtr = &PL; } // Get the in-context local constraints. - CVarSet V = Info.getVariable(D, Context); + ConstraintVariable *V = Info.getVariable(D, Context); auto RHSCons = getExprConstraintVars(RHS); - constrainConsVarGeq(V, RHSCons, Info.getConstraints(), PLPtr, CAction, false, - &Info); - - if (AllTypes && !containsValidCons(V) && + if (V) + constrainConsVarGeq(V, RHSCons, Info.getConstraints(), PLPtr, CAction, + false, &Info); + if (AllTypes && !(V && isValidCons(V)) && !containsValidCons(RHSCons)) { auto &ABI = Info.getABoundsInfo(); ABI.handleAssignment(D, V, RHS, RHSCons, Context, this); @@ -690,8 +695,7 @@ PVConstraint *ConstraintResolver::getRewritablePVConstraint(Expr *E) { PVConstraint *P = new PVConstraint(E->getType(), nullptr, E->getStmtClassName(), Info, *Context, nullptr); - CVarSet Tmp = {P}; - Info.constrainWildIfMacro(Tmp, E->getExprLoc()); + Info.constrainWildIfMacro(P, E->getExprLoc()); return P; } @@ -708,7 +712,7 @@ bool ConstraintResolver::isValidCons(ConstraintVariable *CV) { return false; } -bool ConstraintResolver::resolveBoundsKey(CVarSet &CVs, BoundsKey &BK) { +bool ConstraintResolver::resolveBoundsKey(const CVarSet &CVs, BoundsKey &BK) { if (CVs.size() == 1) { auto *OCons = getOnly(CVs); return resolveBoundsKey(OCons, BK); @@ -718,7 +722,7 @@ bool ConstraintResolver::resolveBoundsKey(CVarSet &CVs, BoundsKey &BK) { bool ConstraintResolver::resolveBoundsKey(ConstraintVariable *CV, BoundsKey &BK) { - if (PVConstraint *PV = dyn_cast(CV)) + if (PVConstraint *PV = dyn_cast_or_null(CV)) if (PV->hasBoundsKey()) { BK = PV->getBoundsKey(); return true; diff --git a/clang/lib/CConv/DeclRewriter.cpp b/clang/lib/CConv/DeclRewriter.cpp index 518e1c1ab491..1c0674e97266 100644 --- a/clang/lib/CConv/DeclRewriter.cpp +++ b/clang/lib/CConv/DeclRewriter.cpp @@ -57,8 +57,6 @@ void DeclRewriter::rewriteDecls(ASTContext &Context, ProgramInfo &Info, // Add declarations from this map into the rewriting set for (const auto &V : Info.getVarMap()) { - PersistentSourceLoc PLoc = V.first; - CVarSet Vars = V.second; // PLoc specifies the location of the variable whose type it is to // re-write, but not where the actual type storage is. To get that, we // need to turn PLoc into a Decl and then get the SourceRange for the @@ -66,16 +64,11 @@ void DeclRewriter::rewriteDecls(ASTContext &Context, ProgramInfo &Info, // of the type specifier, since we want where the text is printed before // the variable name, not the typedef or #define that creates the // name of the type. + PersistentSourceLoc PLoc = V.first; if (Decl *D = std::get<1>(PSLMap[PLoc])) { - // We might have one Decl for multiple Vars, however, one will be a - // PointerVar so we'll use that. - PVConstraint *PV = nullptr; - FVConstraint *FV = nullptr; - for (const auto &V : Vars) - if (PVConstraint *T = dyn_cast(V)) - PV = T; - else if (FVConstraint *T = dyn_cast(V)) - FV = T; + ConstraintVariable *CV = V.second; + PVConstraint *PV = dyn_cast(CV); + FVConstraint *FV = dyn_cast(CV); if (PV && PV->anyChanges(Info.getConstraints().getVariables()) && !PV->isPartOfFunctionPrototype()) { diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index 1ce8b9988695..f63d89e1b1a7 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -91,15 +91,10 @@ void ProgramInfo::print(raw_ostream &O) const { O << "Constraint Variables\n"; for ( const auto &I : Variables ) { PersistentSourceLoc L = I.first; - const CVarSet &S = I.second; L.print(O); - O << "=>"; - for (const auto &J : S) { - O << "[ "; - J->print(O); - O << " ]"; - } - O << "\n"; + O << "=>[ "; + I.second->print(O); + O << " ]\n"; } O << "External Function Definitions\n"; @@ -119,22 +114,12 @@ void ProgramInfo::dump_json(llvm::raw_ostream &O) const { O << ",\n"; } PersistentSourceLoc L = I.first; - const CVarSet &S = I.second; O << "{\"line\":\""; L.print(O); - O << "\","; - O << "\"Variables\":["; - bool AddComma1 = false; - for (const auto &J : S) { - if (AddComma1) { - O << ","; - } - J->dump_json(O); - AddComma1 = true; - } - O << "]"; - O << "}"; + O << "\",\"Variables\":["; + I.second->dump_json(O); + O << "]}"; AddComma = true; } O << "]"; @@ -200,31 +185,30 @@ void ProgramInfo::print_stats(const std::set &F, raw_ostream &O, if (J != FilesToVars.end()) std::tie(varC, pC, ntAC, aC, wC) = J->second; - for (auto &C : I.second) { - if (C->isForValidDecl()) { - InSrcCVars.insert(C); - CAtoms FoundVars = getVarsFromConstraint(C); - - varC += FoundVars.size(); - for (const auto &N : FoundVars) { - ConstAtom *CA = CS.getAssignment(N); - switch (CA->getKind()) { - case Atom::A_Arr: - aC += 1; - break; - case Atom::A_NTArr: - ntAC += 1; - break; - case Atom::A_Ptr: - pC += 1; - break; - case Atom::A_Wild: - wC += 1; - break; - case Atom::A_Var: - case Atom::A_Const: - llvm_unreachable("bad constant in environment map"); - } + ConstraintVariable *C = I.second; + if (C->isForValidDecl()) { + InSrcCVars.insert(C); + CAtoms FoundVars = getVarsFromConstraint(C); + + varC += FoundVars.size(); + for (const auto &N : FoundVars) { + ConstAtom *CA = CS.getAssignment(N); + switch (CA->getKind()) { + case Atom::A_Arr: + aC += 1; + break; + case Atom::A_NTArr: + ntAC += 1; + break; + case Atom::A_Ptr: + pC += 1; + break; + case Atom::A_Wild: + wC += 1; + break; + case Atom::A_Var: + case Atom::A_Const: + llvm_unreachable("bad constant in environment map"); } } } @@ -512,11 +496,9 @@ void ProgramInfo::specialCaseVarIntros(ValueDecl *D, ASTContext *Context) { PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(D, *Context); if (!D->getType()->isVoidType()) Rsn = "Variable type is va_list."; - for (const auto &I : getVariable(D, Context)) { - if (PVConstraint *PVC = dyn_cast(I)) { - PVC->constrainToWild(CS, Rsn, &PL); - } - } + ConstraintVariable *CV = getVariable(D, Context); + if (PVConstraint *PVC = dyn_cast_or_null(CV)) + PVC->constrainToWild(CS, Rsn, &PL); } } @@ -534,8 +516,10 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, // declared inside the same macro expansion. Functions are exempt from this // check because they need to be added to the Extern/Static function maps // regardless if they are inside a macro expansion. - CVarSet &S = Variables[PLoc]; - if (S.size() && !isa(D)) return; + if (Variables.find(PLoc) != Variables.end() && !isa(D)) + return; + + ConstraintVariable *NewCV = nullptr; if (FunctionDecl *FD = dyn_cast(D)) { // Function Decls have FVConstraints. @@ -543,7 +527,7 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, F->setValidDecl(); /* Store the FVConstraint in the global and Variables maps */ insertNewFVConstraint(FD, F, astContext); - S.insert(F); + NewCV = F; // Add mappings from the parameters PLoc to the constraint variables for // the parameters. for (unsigned i = 0; i < FD->getNumParams(); i++) { @@ -551,7 +535,7 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, ConstraintVariable *PV = F->getParamVar(i); PV->setValidDecl(); PersistentSourceLoc PSL = PersistentSourceLoc::mkPSL(PVD, *astContext); - Variables[PSL].insert(PV); + Variables[PSL] = PV; specialCaseVarIntros(PVD, astContext); } @@ -560,7 +544,7 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, if (Ty->isPointerType() || Ty->isArrayType()) { PVConstraint *P = new PVConstraint(D, *this, *astContext); P->setValidDecl(); - S.insert(P); + NewCV = P; std::string VarName = VD->getName(); if (VD->hasGlobalStorage()) { // if we see a definition for this global variable, indicate so in ExternGVars @@ -581,13 +565,14 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, if (Ty->isPointerType() || Ty->isArrayType()) { PVConstraint *P = new PVConstraint(D, *this, *astContext); P->setValidDecl(); - S.insert(P); + NewCV = P; specialCaseVarIntros(D, astContext); } } else llvm_unreachable("unknown decl type"); - constrainWildIfMacro(S, D->getLocation()); + constrainWildIfMacro(NewCV, D->getLocation()); + Variables[PLoc] = NewCV; } bool ProgramInfo::hasPersistentConstraints(Expr *E, ASTContext *C) const { @@ -630,12 +615,11 @@ void ProgramInfo::storePersistentConstraints(Expr *E, const CVarSet &Vars, // If it was, we should constrain it to top. This is sad. Hopefully, // someday, the Rewriter will become less lame and let us re-write stuff // in macros. -void ProgramInfo::constrainWildIfMacro(CVarSet &S, +void ProgramInfo::constrainWildIfMacro(ConstraintVariable *CV, SourceLocation Location) { std::string Rsn = "Pointer in Macro declaration."; if (!Rewriter::isRewritable(Location)) - for (const auto &C : S) - C->constrainToWild(CS, Rsn); + CV->constrainToWild(CS, Rsn); } //std::string ProgramInfo::getUniqueDeclKey(Decl *D, ASTContext *C) { @@ -701,36 +685,36 @@ FVConstraint * } // Given a decl, return the variables for the constraints of the Decl. -CVarSet ProgramInfo::getVariable(clang::Decl *D, clang::ASTContext *C) { +// Returns null if a constraint variable could not be found for the decl. +ConstraintVariable *ProgramInfo::getVariable(clang::Decl *D, + clang::ASTContext *C) { assert(!persisted); if (ParmVarDecl *PD = dyn_cast(D)) { DeclContext *DC = PD->getParentFunctionOrMethod(); // This can fail for extern definitions if(!DC) - return std::set(); + return nullptr; FunctionDecl *FD = dyn_cast(DC); // Get the parameter index with in the function. unsigned int PIdx = getParameterIndex(PD, FD); // Get corresponding FVConstraint vars. FVConstraint *FunFVar = getFuncFVConstraint(FD, C); assert(FunFVar != nullptr && "Unable to find function constraints."); - return {FunFVar->getParamVar(PIdx)}; + return FunFVar->getParamVar(PIdx); } else if (FunctionDecl *FD = dyn_cast(D)) { FVConstraint *FunFVar = getFuncFVConstraint(FD, C); if (FunFVar == nullptr) { llvm::errs() << "No fun constraints for " << FD->getName() << "?!\n"; } - return {FunFVar}; + return FunFVar; } else /* neither function nor function parameter */ { - VariableMap::iterator I = - Variables.find(PersistentSourceLoc::mkPSL(D, *C)); - if (I != Variables.end()) { + auto I = Variables.find(PersistentSourceLoc::mkPSL(D, *C)); + if (I != Variables.end()) return I->second; - } - return CVarSet(); + return nullptr; } } @@ -766,11 +750,10 @@ bool ProgramInfo::computeInterimConstraintState for (const auto &I : Variables) { std::string FileName = I.first.getFileName(); if (FilePaths.count(FileName)) { - for (auto &C : I.second) { - if (C->isForValidDecl()) { - CAtoms tmp = getVarsFromConstraint(C); - ValidVarsVec.insert(ValidVarsVec.begin(), tmp.begin(), tmp.end()); - } + ConstraintVariable *C = I.second; + if (C->isForValidDecl()) { + CAtoms tmp = getVarsFromConstraint(C); + ValidVarsVec.insert(ValidVarsVec.begin(), tmp.begin(), tmp.end()); } } } @@ -857,24 +840,22 @@ bool ProgramInfo::computeInterimConstraintState } else { continue; } - const CVarSet &S = I.second; - for (auto *CV : S) { - if (PVConstraint *PV = dyn_cast(CV)) { - for (auto ck : PV->getCvars()) { - if (VarAtom *VA = dyn_cast(ck)) { - CState.PtrSourceMap[VA->getLoc()] = - const_cast(&(I.first)); - } + ConstraintVariable *CV = I.second; + if (PVConstraint *PV = dyn_cast(CV)) { + for (auto ck : PV->getCvars()) { + if (VarAtom *VA = dyn_cast(ck)) { + CState.PtrSourceMap[VA->getLoc()] = + const_cast(&(I.first)); } } - if (FVConstraint *FV = dyn_cast(CV)) { - if (FV->getReturnVar()) { - if (PVConstraint *RPV = dyn_cast(FV->getReturnVar())) { - for (auto ck : RPV->getCvars()) { - if (VarAtom *VA = dyn_cast(ck)) { - CState.PtrSourceMap[VA->getLoc()] = - const_cast(&(I.first)); - } + } + if (FVConstraint *FV = dyn_cast(CV)) { + if (FV->getReturnVar()) { + if (PVConstraint *RPV = dyn_cast(FV->getReturnVar())) { + for (auto ck : RPV->getCvars()) { + if (VarAtom *VA = dyn_cast(ck)) { + CState.PtrSourceMap[VA->getLoc()] = + const_cast(&(I.first)); } } } diff --git a/clang/lib/CConv/StructInit.cpp b/clang/lib/CConv/StructInit.cpp index 0c6786cb26a8..54a54c086d7b 100644 --- a/clang/lib/CConv/StructInit.cpp +++ b/clang/lib/CConv/StructInit.cpp @@ -25,14 +25,12 @@ bool StructVariableInitializer::VariableNeedsInitializer(VarDecl *VD) { for (auto *const D : Definition->fields()) { if (D->getType()->isPointerType() || D->getType()->isArrayType()) { - CVarSet FieldConsVars = I.getVariable(D, Context); - for (auto *CV : FieldConsVars) { - PVConstraint *PV = dyn_cast(CV); - if (PV && PV->isChecked(I.getConstraints().getVariables())) { - // Ok this contains a pointer that is checked. Store it. - RecordsWithCPointers.insert(Definition); - return true; - } + ConstraintVariable *CV = I.getVariable(D, Context); + PVConstraint *PV = dyn_cast_or_null(CV); + if (PV && PV->isChecked(I.getConstraints().getVariables())) { + // Ok this contains a pointer that is checked. Store it. + RecordsWithCPointers.insert(Definition); + return true; } } } From 52b46932673ad10d65a23008bfede24136860e92 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 15 Sep 2020 15:19:02 -0400 Subject: [PATCH 06/15] Return option type for getVariable instead of nullptr --- clang/include/clang/CConv/AVarBoundsInfo.h | 2 +- clang/include/clang/CConv/CheckedRegions.h | 3 +- .../include/clang/CConv/ConstraintResolver.h | 4 +- .../include/clang/CConv/ConstraintVariables.h | 1 + clang/include/clang/CConv/ProgramInfo.h | 3 +- clang/lib/CConv/AVarBoundsInfo.cpp | 10 ++--- .../CConv/ArrayBoundsInferenceConsumer.cpp | 11 +++--- clang/lib/CConv/CheckedRegions.cpp | 38 ++++++++----------- clang/lib/CConv/ConstraintBuilder.cpp | 27 ++++++------- clang/lib/CConv/ConstraintResolver.cpp | 38 +++++++++++-------- clang/lib/CConv/ProgramInfo.cpp | 9 ++--- clang/lib/CConv/StructInit.cpp | 2 +- 12 files changed, 74 insertions(+), 74 deletions(-) diff --git a/clang/include/clang/CConv/AVarBoundsInfo.h b/clang/include/clang/CConv/AVarBoundsInfo.h index ddb586920351..bb99c45f262e 100644 --- a/clang/include/clang/CConv/AVarBoundsInfo.h +++ b/clang/include/clang/CConv/AVarBoundsInfo.h @@ -177,7 +177,7 @@ class AVarBoundsInfo { bool handleAssignment(clang::Expr *L, const CVarSet &LCVars, clang::Expr *R, const CVarSet &RCVars, ASTContext *C, ConstraintResolver *CR); - bool handleAssignment(clang::Decl *L, ConstraintVariable *LCVar, + bool handleAssignment(clang::Decl *L, CVarOption LCVar, clang::Expr *R, const CVarSet &RCVars, ASTContext *C, ConstraintResolver *CR); // Handle context sensitive assignment. diff --git a/clang/include/clang/CConv/CheckedRegions.h b/clang/include/clang/CConv/CheckedRegions.h index 84736da14907..577e0d6ee929 100644 --- a/clang/include/clang/CConv/CheckedRegions.h +++ b/clang/include/clang/CConv/CheckedRegions.h @@ -84,8 +84,7 @@ class CheckedRegionFinder : public clang::RecursiveASTVisitor &Seen); bool isUncheckedStruct(clang::QualType Qt, std::set &Seen); - bool isWild(const ConstraintVariable*); - bool isWild(const FVConstraint*); + bool isWild(CVarOption CVar); clang::ASTContext* Context; clang::Rewriter& Writer; diff --git a/clang/include/clang/CConv/ConstraintResolver.h b/clang/include/clang/CConv/ConstraintResolver.h index 5280f342f700..84797eb52e42 100644 --- a/clang/include/clang/CConv/ConstraintResolver.h +++ b/clang/include/clang/CConv/ConstraintResolver.h @@ -28,6 +28,8 @@ class ConstraintResolver { void constraintAllCVarsToWild(const CVarSet &CSet, const std::string &Rsn, Expr *AtExpr = nullptr); + void constraintCVarToWild(CVarOption CVar, const std::string &Rsn, + Expr *AtExpr = nullptr); // Returns a set of ConstraintVariables which represent the result of // evaluating the expression E. Will explore E recursively, but will @@ -48,7 +50,7 @@ class ConstraintResolver { bool isValidCons(ConstraintVariable *CV); // Try to get the bounds key from the constraint variable set. bool resolveBoundsKey(const CVarSet &CVs, BoundsKey &BK); - bool resolveBoundsKey(ConstraintVariable *CV, BoundsKey &BK); + bool resolveBoundsKey(CVarOption CV, BoundsKey &BK); static bool canFunctionBeSkipped(const std::string &FN); diff --git a/clang/include/clang/CConv/ConstraintVariables.h b/clang/include/clang/CConv/ConstraintVariables.h index 7721855625f3..64c1783eb9b3 100644 --- a/clang/include/clang/CConv/ConstraintVariables.h +++ b/clang/include/clang/CConv/ConstraintVariables.h @@ -166,6 +166,7 @@ class ConstraintVariable { }; typedef std::set CVarSet; +typedef llvm::Optional CVarOption; enum ConsAction { Safe_to_Wild, diff --git a/clang/include/clang/CConv/ProgramInfo.h b/clang/include/clang/CConv/ProgramInfo.h index 089e1379f3d4..8e2d9419b9b7 100644 --- a/clang/include/clang/CConv/ProgramInfo.h +++ b/clang/include/clang/CConv/ProgramInfo.h @@ -74,7 +74,8 @@ class ProgramInfo : public ProgramVariableAdder { ASTContext *C); // Get constraint variable for the provided Decl - ConstraintVariable *getVariable(clang::Decl *D, clang::ASTContext *C); + CVarOption getVariable(clang::Decl *D, + clang::ASTContext *C); // Retrieve a function's constraints by decl, or by name; nullptr if not found FVConstraint *getFuncConstraint (FunctionDecl *D, ASTContext *C) const; diff --git a/clang/lib/CConv/AVarBoundsInfo.cpp b/clang/lib/CConv/AVarBoundsInfo.cpp index fca82c76bc55..2a0aa28cbb96 100644 --- a/clang/lib/CConv/AVarBoundsInfo.cpp +++ b/clang/lib/CConv/AVarBoundsInfo.cpp @@ -338,7 +338,7 @@ bool AVarBoundsInfo::handleAssignment(clang::Expr *L, const CVarSet &LCVars, return false; } -bool AVarBoundsInfo::handleAssignment(clang::Decl *L, ConstraintVariable *LCVars, +bool AVarBoundsInfo::handleAssignment(clang::Decl *L, CVarOption LCVars, clang::Expr *R, const CVarSet &RCVars, ASTContext *C, ConstraintResolver *CR) { BoundsKey LKey, RKey; @@ -371,7 +371,7 @@ bool AVarBoundsInfo::handleContextSensitiveAssignment(CallExpr *CE, } else { // This is the assignment of regular variables. BoundsKey LKey, RKey; - if ((CR->resolveBoundsKey({LCVar}, LKey) || + if ((CR->resolveBoundsKey(LCVar, LKey) || tryGetVariable(L, LKey)) && (CR->resolveBoundsKey(RCVars, RKey) || tryGetVariable(R, *C, RKey))) { @@ -1135,9 +1135,9 @@ ContextSensitiveBoundsKeyVisitor::~ContextSensitiveBoundsKeyVisitor() { bool ContextSensitiveBoundsKeyVisitor::VisitCallExpr(CallExpr *CE) { if (FunctionDecl *FD = dyn_cast_or_null(CE->getCalleeDecl())) { // Contextualize the function at this call-site. - if (ConstraintVariable *CFV = Info.getVariable(FD, Context)) { - Info.getABoundsInfo().contextualizeCVar(CE, {CFV}); - } + CVarOption COpt = Info.getVariable(FD, Context); + if (COpt.hasValue()) + Info.getABoundsInfo().contextualizeCVar(CE, {COpt.getValue()}); } return true; } \ No newline at end of file diff --git a/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp b/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp index f64d5aff8710..449b08b6da01 100644 --- a/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp +++ b/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp @@ -134,9 +134,10 @@ static bool needArrayBounds(Expr *E, ProgramInfo &Info, ASTContext *C) { static bool needArrayBounds(Decl *D, ProgramInfo &Info, ASTContext *C, bool IsNtArr) { const auto &E = Info.getConstraints().getVariables(); - if (ConstraintVariable *CurrCVar = Info.getVariable(D, C)) { - if ((!IsNtArr && needArrayBounds(CurrCVar, E)) || - (IsNtArr && needNTArrayBounds(CurrCVar, E))) + CVarOption CVar = Info.getVariable(D, C); + if (CVar) { + if ((!IsNtArr && needArrayBounds(*CVar, E)) || + (IsNtArr && needNTArrayBounds(*CVar, E))) return true; return false; } @@ -178,9 +179,9 @@ bool tryGetBoundsKeyVar(Expr *E, BoundsKey &BK, ProgramInfo &Info, bool tryGetBoundsKeyVar(Decl *D, BoundsKey &BK, ProgramInfo &Info, ASTContext *Context) { ConstraintResolver CR(Info, Context); - ConstraintVariable *CV = Info.getVariable(D, Context); + CVarOption CV = Info.getVariable(D, Context); auto &ABInfo = Info.getABoundsInfo(); - return (CV && CR.resolveBoundsKey(CV, BK)) || ABInfo.tryGetVariable(D, BK); + return CR.resolveBoundsKey(CV, BK) || ABInfo.tryGetVariable(D, BK); } // Check if the provided expression is a call to one of the known diff --git a/clang/lib/CConv/CheckedRegions.cpp b/clang/lib/CConv/CheckedRegions.cpp index 3500066a2470..683e70b65ba4 100644 --- a/clang/lib/CConv/CheckedRegions.cpp +++ b/clang/lib/CConv/CheckedRegions.cpp @@ -188,8 +188,8 @@ bool CheckedRegionFinder::VisitCallExpr(CallExpr *C) { || containsUncheckedPtr(type) || (std::any_of(FD->param_begin(), FD->param_end(), [this] (Decl *param) { - auto CV = Info.getVariable(param, Context); - return CV && isWild(CV); + CVarOption CV = Info.getVariable(param, Context); + return isWild(CV); })); } handleChildren(C->children()); @@ -200,15 +200,15 @@ bool CheckedRegionFinder::VisitCallExpr(CallExpr *C) { } bool CheckedRegionFinder::VisitVarDecl(VarDecl *VD) { - auto CV = Info.getVariable(VD, Context); - Wild = (CV && isWild(CV)) || containsUncheckedPtr(VD->getType()); + CVarOption CV = Info.getVariable(VD, Context); + Wild = isWild(CV) || containsUncheckedPtr(VD->getType()); return true; } bool CheckedRegionFinder::VisitParmVarDecl(ParmVarDecl *PVD) { // Check if the variable is WILD. - auto CV = Info.getVariable(PVD, Context); - Wild |= (CV && isWild(CV)) || containsUncheckedPtr(PVD->getType()); + CVarOption CV = Info.getVariable(PVD, Context); + Wild |= isWild(CV) || containsUncheckedPtr(PVD->getType()); return true; } @@ -216,8 +216,8 @@ bool CheckedRegionFinder::VisitMemberExpr(MemberExpr *E){ ValueDecl *VD = E->getMemberDecl(); if (VD) { // Check if the variable is WILD. - ConstraintVariable *Cv = Info.getVariable(VD, Context); - if (Cv && Cv->hasWild(Info.getConstraints().getVariables())) + CVarOption Cv = Info.getVariable(VD, Context); + if (Cv && (*Cv)->hasWild(Info.getConstraints().getVariables())) Wild = true; // Check if the variable contains unchecked types. Wild |= containsUncheckedPtr(VD->getType()); @@ -228,15 +228,15 @@ bool CheckedRegionFinder::VisitMemberExpr(MemberExpr *E){ bool CheckedRegionFinder::VisitDeclRefExpr(DeclRefExpr* DR) { auto T = DR->getType(); auto D = DR->getDecl(); - auto CV = Info.getVariable(D, Context); - bool IW = (CV && isWild(CV)) || containsUncheckedPtr(T); + CVarOption CV = Info.getVariable(D, Context); + bool IW = isWild(CV) || containsUncheckedPtr(T); if (auto FD = dyn_cast(D)) { auto *FV = Info.getFuncConstraint(FD, Context); IW |= FV->hasWild(Info.getConstraints().getVariables()); for (const auto& param: FD->parameters()) { - auto CV = Info.getVariable(param, Context); - IW |= (CV && isWild(CV)); + CVarOption CV = Info.getVariable(param, Context); + IW |= isWild(CV); } } @@ -295,14 +295,8 @@ bool CheckedRegionFinder::isInStatementPosition(CallExpr *C) { } } -bool CheckedRegionFinder::isWild(const ConstraintVariable* Cv) { - if (Cv->hasWild(Info.getConstraints().getVariables())) - return true; - return false; -} - -bool CheckedRegionFinder::isWild(const FVConstraint* Fv) { - if (Fv->hasWild(Info.getConstraints().getVariables())) +bool CheckedRegionFinder::isWild(CVarOption Cv) { + if (Cv && (*Cv)->hasWild(Info.getConstraints().getVariables())) return true; return false; } @@ -361,8 +355,8 @@ bool CheckedRegionFinder::isUncheckedStruct(QualType Qt, std::set & for (auto const &Fld : D->fields()) { auto Ftype = Fld->getType(); Unsafe |= containsUncheckedPtrAcc(Ftype, Seen); - ConstraintVariable *Cv = Info.getVariable(Fld, Context); - Unsafe |= (Cv && Cv->hasWild(Info.getConstraints().getVariables())); + CVarOption Cv = Info.getVariable(Fld, Context); + Unsafe |= (Cv && (*Cv)->hasWild(Info.getConstraints().getVariables())); } return Unsafe; } diff --git a/clang/lib/CConv/ConstraintBuilder.cpp b/clang/lib/CConv/ConstraintBuilder.cpp index e21f4c1b4fff..e43192e46801 100644 --- a/clang/lib/CConv/ConstraintBuilder.cpp +++ b/clang/lib/CConv/ConstraintBuilder.cpp @@ -58,10 +58,9 @@ void processRecordDecl(RecordDecl *Declaration, ProgramInfo &Info, // mark field wild if the above is true and the field is a pointer if ((FieldTy->isPointerType() || FieldTy->isArrayType()) && (FieldInUnionOrSysHeader || IsInLineStruct)) { - if (ConstraintVariable *CV = Info.getVariable(F, Context)) { - std::string Rsn = "External struct field or union encountered"; - CB.constraintAllCVarsToWild({CV}, Rsn, nullptr); - } + std::string Rsn = "External struct field or union encountered"; + CVarOption CV = Info.getVariable(F, Context); + CB.constraintCVarToWild(CV, Rsn); } } } @@ -96,9 +95,8 @@ class FunctionVisitor : public RecursiveASTVisitor { (VD->getType()->isPointerType() || VD->getType()->isArrayType())) { if (lastRecordLocation == VD->getBeginLoc().getRawEncoding()) { - if (ConstraintVariable *CV = Info.getVariable(VD, Context)) - CB.constraintAllCVarsToWild({CV}, "Inline struct encountered.", - nullptr); + CVarOption CV = Info.getVariable(VD, Context); + CB.constraintCVarToWild(CV, "Inline struct encountered."); } } } @@ -179,12 +177,12 @@ class FunctionVisitor : public RecursiveASTVisitor { } else if (FunctionDecl *FD = dyn_cast(D)) { FuncName = FD->getNameAsString(); TFD = FD; - if (ConstraintVariable *CV = Info.getVariable(FD, Context)) - FVCons.insert(CV); + if (CVarOption CV = Info.getVariable(FD, Context)) + FVCons.insert(*CV); } else if (DeclaratorDecl *DD = dyn_cast(D)) { FuncName = DD->getNameAsString(); - if (ConstraintVariable *CV = Info.getVariable(DD, Context)) - FVCons.insert(CV); + if (CVarOption CV = Info.getVariable(DD, Context)) + FVCons.insert(*CV); } // Collect type parameters for this function call that are @@ -289,7 +287,7 @@ class FunctionVisitor : public RecursiveASTVisitor { PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(S, *Context); ConstraintVariable *CV = - Info.getVariable(Function, Context); + Info.getVariable(Function, Context).getValueOr(nullptr); // Constrain the value returned (if present) against the return value // of the function. @@ -430,9 +428,8 @@ class ConstraintGenVisitor : public RecursiveASTVisitor { unsigned int BeginLoc = G->getBeginLoc().getRawEncoding(); unsigned int EndLoc = G->getEndLoc().getRawEncoding(); if (lastRecordLocation >= BeginLoc && lastRecordLocation <= EndLoc) { - if (ConstraintVariable *CV = Info.getVariable(G, Context)) - CB.constraintAllCVarsToWild({CV}, "Inline struct encountered.", - nullptr); + CVarOption CV = Info.getVariable(G, Context); + CB.constraintCVarToWild(CV, "Inline struct encountered."); } } diff --git a/clang/lib/CConv/ConstraintResolver.cpp b/clang/lib/CConv/ConstraintResolver.cpp index 3fb11b073d72..641b8df0dd10 100644 --- a/clang/lib/CConv/ConstraintResolver.cpp +++ b/clang/lib/CConv/ConstraintResolver.cpp @@ -41,6 +41,13 @@ void ConstraintResolver::constraintAllCVarsToWild(const CVarSet &CSet, } } +void ConstraintResolver::constraintCVarToWild(CVarOption CVar, + const std::string &Rsn, + Expr *AtExpr) { + if (CVar) + constraintAllCVarsToWild({*CVar}, Rsn, AtExpr); +} + // Return a set of PVConstraints equivalent to the set given, // but dereferenced one level down CVarSet @@ -245,14 +252,14 @@ CVarSet return CVs; // variable (x) } else if (DeclRefExpr *DRE = dyn_cast(E)) { - ConstraintVariable *CV = Info.getVariable(DRE->getDecl(), Context); - assert("Declaration without constraint variable?" && CV); - return {CV}; + CVarOption CV = Info.getVariable(DRE->getDecl(), Context); + assert("Declaration without constraint variable?" && CV.hasValue()); + return {CV.getValue()}; // x.f } else if (MemberExpr *ME = dyn_cast(E)) { - ConstraintVariable *CV = Info.getVariable(ME->getMemberDecl(), Context); - assert("Declaration without constraint variable?" && CV); - return {CV}; + CVarOption CV = Info.getVariable(ME->getMemberDecl(), Context); + assert("Declaration without constraint variable?" && CV.hasValue()); + return {CV.getValue()}; // Checked-C temporary } else if (CHKCBindTemporaryExpr *CE = dyn_cast(E)) { return getExprConstraintVars(CE->getSubExpr()); @@ -471,14 +478,14 @@ CVarSet /* Normal function call */ } else { - ConstraintVariable *J = Info.getVariable(FD, Context); - assert(J && "Function without constraint variable."); + CVarOption CV = Info.getVariable(FD, Context); + assert(CV && "Function without constraint variable."); /* Direct function call */ - if (FVConstraint *FVC = dyn_cast(J)) + if (FVConstraint *FVC = dyn_cast(*CV)) ReturnCVs.insert(FVC->getReturnVar()); /* Call via function pointer */ else { - PVConstraint *tmp = dyn_cast(J); + PVConstraint *tmp = dyn_cast(*CV); assert(tmp != nullptr); if (FVConstraint *FVC = tmp->getFV()) ReturnCVs.insert(FVC->getReturnVar()); @@ -649,14 +656,13 @@ void ConstraintResolver::constrainLocalAssign(Stmt *TSt, DeclaratorDecl *D, PLPtr = &PL; } // Get the in-context local constraints. - ConstraintVariable *V = Info.getVariable(D, Context); + CVarOption V = Info.getVariable(D, Context); auto RHSCons = getExprConstraintVars(RHS); if (V) - constrainConsVarGeq(V, RHSCons, Info.getConstraints(), PLPtr, CAction, + constrainConsVarGeq(*V, RHSCons, Info.getConstraints(), PLPtr, CAction, false, &Info); - if (AllTypes && !(V && isValidCons(V)) && - !containsValidCons(RHSCons)) { + if (AllTypes && !(V && isValidCons(*V)) && !containsValidCons(RHSCons)) { auto &ABI = Info.getABoundsInfo(); ABI.handleAssignment(D, V, RHS, RHSCons, Context, this); } @@ -720,9 +726,9 @@ bool ConstraintResolver::resolveBoundsKey(const CVarSet &CVs, BoundsKey &BK) { return false; } -bool ConstraintResolver::resolveBoundsKey(ConstraintVariable *CV, +bool ConstraintResolver::resolveBoundsKey(CVarOption CV, BoundsKey &BK) { - if (PVConstraint *PV = dyn_cast_or_null(CV)) + if (PVConstraint *PV = dyn_cast_or_null(CV.getValueOr(nullptr))) if (PV->hasBoundsKey()) { BK = PV->getBoundsKey(); return true; diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index f63d89e1b1a7..a5650a09065e 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -496,7 +496,7 @@ void ProgramInfo::specialCaseVarIntros(ValueDecl *D, ASTContext *Context) { PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(D, *Context); if (!D->getType()->isVoidType()) Rsn = "Variable type is va_list."; - ConstraintVariable *CV = getVariable(D, Context); + ConstraintVariable *CV = getVariable(D, Context).getValueOr(nullptr); if (PVConstraint *PVC = dyn_cast_or_null(CV)) PVC->constrainToWild(CS, Rsn, &PL); } @@ -686,15 +686,14 @@ FVConstraint * // Given a decl, return the variables for the constraints of the Decl. // Returns null if a constraint variable could not be found for the decl. -ConstraintVariable *ProgramInfo::getVariable(clang::Decl *D, - clang::ASTContext *C) { +CVarOption ProgramInfo::getVariable(clang::Decl *D, clang::ASTContext *C) { assert(!persisted); if (ParmVarDecl *PD = dyn_cast(D)) { DeclContext *DC = PD->getParentFunctionOrMethod(); // This can fail for extern definitions if(!DC) - return nullptr; + return CVarOption(); FunctionDecl *FD = dyn_cast(DC); // Get the parameter index with in the function. unsigned int PIdx = getParameterIndex(PD, FD); @@ -714,7 +713,7 @@ ConstraintVariable *ProgramInfo::getVariable(clang::Decl *D, auto I = Variables.find(PersistentSourceLoc::mkPSL(D, *C)); if (I != Variables.end()) return I->second; - return nullptr; + return CVarOption(); } } diff --git a/clang/lib/CConv/StructInit.cpp b/clang/lib/CConv/StructInit.cpp index 54a54c086d7b..b514df5c0fe7 100644 --- a/clang/lib/CConv/StructInit.cpp +++ b/clang/lib/CConv/StructInit.cpp @@ -25,7 +25,7 @@ bool StructVariableInitializer::VariableNeedsInitializer(VarDecl *VD) { for (auto *const D : Definition->fields()) { if (D->getType()->isPointerType() || D->getType()->isArrayType()) { - ConstraintVariable *CV = I.getVariable(D, Context); + ConstraintVariable *CV = I.getVariable(D, Context).getValueOr(nullptr); PVConstraint *PV = dyn_cast_or_null(CV); if (PV && PV->isChecked(I.getConstraints().getVariables())) { // Ok this contains a pointer that is checked. Store it. From 87f507dbf1d7b6ef3866287e3cc3b35947342221 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 15 Sep 2020 17:00:17 -0400 Subject: [PATCH 07/15] Remove commented out code that is now truly dead --- clang/lib/CConv/ProgramInfo.cpp | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index a5650a09065e..54998cdd3410 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -302,27 +302,6 @@ bool ProgramInfo::link() { if (Verbose) llvm::errs() << "Linking!\n"; -// MWH: Should never happen: Variables set sizes == 1 - // Multiple Variables can be at the same PersistentSourceLoc. We should - // constrain that everything that is at the same location is explicitly - // equal. -// for (const auto &V : Variables) { -// std::set C = V.second; -// -// if (C.size() > 1) { -// assert(false); // should never get here -// std::set::iterator I = C.begin(); -// std::set::iterator J = C.begin(); -// ++J; -// -// while (J != C.end()) { -// constrainConsVarGeq(*I, *J, CS, nullptr, Same_to_Same, true, this); -// ++I; -// ++J; -// } -// } -// } - // Equate the constraints for all global variables. // This is needed for variables that are defined as extern. for (const auto &V : GlobalVariableSymbols) { From a858a9f894a3e5f66130b84d46a9cbcb9fe90751 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 15 Sep 2020 18:59:43 -0400 Subject: [PATCH 08/15] Fix RUN commands in test --- clang/test/CheckedCRewriter/qualifiers.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/test/CheckedCRewriter/qualifiers.c b/clang/test/CheckedCRewriter/qualifiers.c index 911d7c59f85b..b5ef6fb8b64d 100644 --- a/clang/test/CheckedCRewriter/qualifiers.c +++ b/clang/test/CheckedCRewriter/qualifiers.c @@ -1,6 +1,6 @@ -// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=checkedALL %s %S/qualifiers.c -// RUN: cconv-standalone -base-dir=%S -output-postfix=checkedNOALL %s %S/qualifiers.c -// RUN: %clang -c %S/qualifiers.checkedNOALL.c %S/qualifiers.checkedNOALL.c +// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=checkedALL %s +// RUN: cconv-standalone -base-dir=%S -output-postfix=checkedNOALL %s +// RUN: %clang -c %S/qualifiers.checkedNOALL.c // RUN: FileCheck -match-full-lines --input-file %S/qualifiers.checkedNOALL.c %s // RUN: FileCheck -match-full-lines --input-file %S/qualifiers.checkedALL.c %s // RUN: rm %S/qualifiers.checkedALL.c %S/qualifiers.checkedNOALL.c From 566640c89d431de06ea0b8fba58265e3dfd86f16 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 15 Sep 2020 20:06:17 -0400 Subject: [PATCH 09/15] Handle edge cases for PSL collision --- clang/include/clang/CConv/ProgramInfo.h | 2 +- clang/lib/CConv/ConstraintBuilder.cpp | 8 +-- clang/lib/CConv/ProgramInfo.cpp | 70 ++++++++++++++++++------- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/clang/include/clang/CConv/ProgramInfo.h b/clang/include/clang/CConv/ProgramInfo.h index 8e2d9419b9b7..fdf0a711d39d 100644 --- a/clang/include/clang/CConv/ProgramInfo.h +++ b/clang/include/clang/CConv/ProgramInfo.h @@ -186,7 +186,7 @@ class ProgramInfo : public ProgramVariableAdder { // For each pointer type in the declaration of D, add a variable to the // constraint system for that pointer type. - void addVariable(clang::DeclaratorDecl *D, clang::ASTContext *astContext); + void addVariable(clang::DeclaratorDecl *D, clang::ASTContext *AstContext); }; #endif diff --git a/clang/lib/CConv/ConstraintBuilder.cpp b/clang/lib/CConv/ConstraintBuilder.cpp index e43192e46801..862fef5ddb56 100644 --- a/clang/lib/CConv/ConstraintBuilder.cpp +++ b/clang/lib/CConv/ConstraintBuilder.cpp @@ -498,8 +498,7 @@ class VariableAdderVisitor : public RecursiveASTVisitor { bool VisitVarDecl(VarDecl *D) { FullSourceLoc FL = Context->getFullLoc(D->getBeginLoc()); - if (FL.isValid() && - (!isa(D) || D->getParentFunctionOrMethod() != nullptr)) + if (FL.isValid() && !isa(D)) addVariable(D); return true; } @@ -512,7 +511,10 @@ class VariableAdderVisitor : public RecursiveASTVisitor { } bool VisitRecordDecl(RecordDecl *Declaration) { - if (RecordDecl *Definition = Declaration->getDefinition()) { + if (Declaration->isThisDeclarationADefinition()) { + RecordDecl *Definition = Declaration->getDefinition(); + assert("Declaration is a definition, but getDefinition() is null?" + && Definition); FullSourceLoc FL = Context->getFullLoc(Definition->getBeginLoc()); if (FL.isValid()) { SourceManager &SM = Context->getSourceManager(); diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index 54998cdd3410..ac6404648e7c 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -484,28 +484,53 @@ void ProgramInfo::specialCaseVarIntros(ValueDecl *D, ASTContext *Context) { // For each pointer type in the declaration of D, add a variable to the // constraint system for that pointer type. void ProgramInfo::addVariable(clang::DeclaratorDecl *D, - clang::ASTContext *astContext) { + clang::ASTContext *AstContext) { assert(!persisted); - PersistentSourceLoc PLoc = PersistentSourceLoc::mkPSL(D, *astContext); + PersistentSourceLoc PLoc = PersistentSourceLoc::mkPSL(D, *AstContext); assert(PLoc.valid()); - // We only add a PVConstraint if the set at Variables[PLoc] does not contain - // one already. Two variables can have the same source locations when they are - // declared inside the same macro expansion. Functions are exempt from this - // check because they need to be added to the Extern/Static function maps - // regardless if they are inside a macro expansion. - if (Variables.find(PLoc) != Variables.end() && !isa(D)) + // We only add a PVConstraint if Variables[PLoc] does not exist. + // Functions are exempt from this check because they need to be added to the + // Extern/Static function map even if they are inside a macro expansion. + if (Variables.find(PLoc) != Variables.end() && !isa(D)) { + // Two variables can have the same source locations when they are + // declared inside the same macro expansion. The first instance of the + // source location will have been constrained to WILD, so it's safe to bail + // without doing anymore work. + if (!Rewriter::isRewritable(D->getLocation())) { + // If we're not in a macro, we should make the constraint variable WILD + // anyways. + std::string Rsn = "Duplicate source location. Possibly part of a macro."; + Variables[PLoc]->constrainToWild(CS, Rsn); + } return; + } ConstraintVariable *NewCV = nullptr; if (FunctionDecl *FD = dyn_cast(D)) { // Function Decls have FVConstraints. - FVConstraint *F = new FVConstraint(D, *this, *astContext); + FVConstraint *F = new FVConstraint(D, *this, *AstContext); F->setValidDecl(); + + // Handling of PSL collision for functions is different since we need to + // consider the static and extern function maps. + if (Variables.find(PLoc) != Variables.end()) { + // Try to find a previous definition based on function name + if (!getFuncConstraint(FD, AstContext)) { + constrainWildIfMacro(F, FD->getLocation()); + insertNewFVConstraint(FD, F, AstContext); + } else { + // FIXME: Visiting same function in same source location twice. + // Shouldn't happen but it does. Just bailing for now. + } + return; + } + /* Store the FVConstraint in the global and Variables maps */ - insertNewFVConstraint(FD, F, astContext); + insertNewFVConstraint(FD, F, AstContext); + NewCV = F; // Add mappings from the parameters PLoc to the constraint variables for // the parameters. @@ -513,21 +538,28 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, ParmVarDecl *PVD = FD->getParamDecl(i); ConstraintVariable *PV = F->getParamVar(i); PV->setValidDecl(); - PersistentSourceLoc PSL = PersistentSourceLoc::mkPSL(PVD, *astContext); + PersistentSourceLoc PSL = PersistentSourceLoc::mkPSL(PVD, *AstContext); + // Constraint variable is stored on the parent function, so we need to + // constrain to WILD even if we don't end up storing this in the map. + constrainWildIfMacro(PV, PVD->getLocation()); + specialCaseVarIntros(PVD, AstContext); + // It is possible to have a parameter delc in a macro when function is not + if (Variables.find(PSL) != Variables.end()) + continue; Variables[PSL] = PV; - specialCaseVarIntros(PVD, astContext); } } else if (VarDecl *VD = dyn_cast(D)) { + assert(!isa(VD)); const Type *Ty = VD->getTypeSourceInfo()->getTypeLoc().getTypePtr(); if (Ty->isPointerType() || Ty->isArrayType()) { - PVConstraint *P = new PVConstraint(D, *this, *astContext); + PVConstraint *P = new PVConstraint(D, *this, *AstContext); P->setValidDecl(); NewCV = P; std::string VarName = VD->getName(); if (VD->hasGlobalStorage()) { // if we see a definition for this global variable, indicate so in ExternGVars - if(VD->hasDefinition() || VD->hasDefinition(*astContext)) { + if(VD->hasDefinition() || VD->hasDefinition(*AstContext)) { ExternGVars[VarName] = true; } // if we don't, check that we haven't seen one before before setting to false @@ -536,20 +568,20 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, } GlobalVariableSymbols[VarName].insert(P); } - specialCaseVarIntros(D, astContext); + specialCaseVarIntros(D, AstContext); } } else if (FieldDecl *FlD = dyn_cast(D)) { const Type *Ty = FlD->getTypeSourceInfo()->getTypeLoc().getTypePtr(); if (Ty->isPointerType() || Ty->isArrayType()) { - PVConstraint *P = new PVConstraint(D, *this, *astContext); - P->setValidDecl(); - NewCV = P; - specialCaseVarIntros(D, astContext); + NewCV = new PVConstraint(D, *this, *AstContext); + NewCV->setValidDecl(); + specialCaseVarIntros(D, AstContext); } } else llvm_unreachable("unknown decl type"); + assert("We shouldn't be adding a null CV to Variables map." && NewCV); constrainWildIfMacro(NewCV, D->getLocation()); Variables[PLoc] = NewCV; } From 12d816eb0ffa0ba00599b87fab1f0da461c89aa3 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 16 Sep 2020 09:46:10 -0400 Subject: [PATCH 10/15] Code cleanup and tweaks --- .../include/clang/CConv/ConstraintVariables.h | 19 ------------------- clang/include/clang/CConv/Constraints.h | 15 ++++----------- clang/include/clang/CConv/ProgramInfo.h | 15 ++++++--------- clang/lib/CConv/ProgramInfo.cpp | 9 ++++++--- clang/lib/CConv/RewriteUtils.cpp | 11 +++-------- 5 files changed, 19 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/CConv/ConstraintVariables.h b/clang/include/clang/CConv/ConstraintVariables.h index 64c1783eb9b3..a2bdbb4c6d64 100644 --- a/clang/include/clang/CConv/ConstraintVariables.h +++ b/clang/include/clang/CConv/ConstraintVariables.h @@ -158,10 +158,6 @@ class ConstraintVariable { virtual ~ConstraintVariable() {}; - // Sometimes, constraint variables can be produced that are empty. This - // tests for the existence of those constraint variables. - virtual bool isEmpty(void) const = 0; - virtual bool getIsOriginallyChecked() const = 0; }; @@ -370,8 +366,6 @@ class PointerVariableConstraint : public ConstraintVariable { // Get the set of constraint variables corresponding to the arguments. const std::set &getArgumentConstraints() const; - bool isEmpty(void) const override { return vars.size() == 0; } - ConstraintVariable *getCopy(Constraints &CS) override; // Retrieve the atom at the specified index. This function includes special @@ -481,19 +475,6 @@ class FunctionVariableConstraint : public ConstraintVariable { ConstraintVariable *getCopy(Constraints &CS) override; - // An FVConstraint is empty if every constraint associated is empty. - bool isEmpty(void) const override { - - if (ReturnVar != nullptr) - return false; - - for (const auto &u : ParamVars) - if (!u->isEmpty()) - return false; - - return true; - } - bool getIsOriginallyChecked() const override; ~FunctionVariableConstraint() override {}; diff --git a/clang/include/clang/CConv/Constraints.h b/clang/include/clang/CConv/Constraints.h index 85fc4d1acc3f..40c4076a3aec 100644 --- a/clang/include/clang/CConv/Constraints.h +++ b/clang/include/clang/CConv/Constraints.h @@ -528,7 +528,6 @@ class ConstraintsEnv { public: ConstraintsEnv() : consFreeKey(0), useChecked(true) { environment.clear(); } - EnvironmentMap &getVariables() { return environment; } const EnvironmentMap &getVariables() const { return environment; } void dump() const; void print(llvm::raw_ostream &) const; @@ -573,11 +572,10 @@ class Constraints { // It's important to return these by reference. Programs can have // 10-100-100000 constraints and variables, and copying them each time // a client wants to examine the environment is untenable. - ConstraintSet &getConstraints() { return constraints; } - EnvironmentMap &getVariables() { return environment.getVariables(); } - const EnvironmentMap &getVariables() const { return environment.getVariables(); } - - EnvironmentMap &getitypeVarMap() { return itypeConstraintVars; } + const ConstraintSet &getConstraints() const { return constraints; } + const EnvironmentMap &getVariables() const { + return environment.getVariables(); + } void editConstraintHook(Constraint *C); @@ -619,11 +617,6 @@ class Constraints { ConstraintsGraph *PtrTypCG; std::map constraintsByReason; ConstraintsEnv environment; - // Map of constraint variables, which are identified - // as itype pointers - // These should be the constraint variables of only - // function parameters or returns. - EnvironmentMap itypeConstraintVars; // Confirm a constraint is well-formed bool check(Constraint *C); diff --git a/clang/include/clang/CConv/ProgramInfo.h b/clang/include/clang/CConv/ProgramInfo.h index fdf0a711d39d..95a1065ba58f 100644 --- a/clang/include/clang/CConv/ProgramInfo.h +++ b/clang/include/clang/CConv/ProgramInfo.h @@ -74,8 +74,7 @@ class ProgramInfo : public ProgramVariableAdder { ASTContext *C); // Get constraint variable for the provided Decl - CVarOption getVariable(clang::Decl *D, - clang::ASTContext *C); + CVarOption getVariable(clang::Decl *D, clang::ASTContext *C); // Retrieve a function's constraints by decl, or by name; nullptr if not found FVConstraint *getFuncConstraint (FunctionDecl *D, ASTContext *C) const; @@ -123,8 +122,8 @@ class ProgramInfo : public ProgramVariableAdder { // analysis from compilation unit to compilation unit. VariableMap Variables; - // Map with the same purpose as the Variables map, this stores constraint vars - // non-declaration expressions. + // Map with the same purpose as the Variables map, this stores constraint + // variables for non-declaration expressions. std::map ExprConstraintVars; // Constraint system. @@ -143,8 +142,7 @@ class ProgramInfo : public ProgramVariableAdder { StaticFunctionMapType StaticFunctionFVCons; std::map> GlobalVariableSymbols; - // Object that contains all the bounds information of various - // array variables. + // Object that contains all the bounds information of various array variables. AVarBoundsInfo ArrBInfo; // Constraints state. ConstraintsInfo CState; @@ -153,9 +151,8 @@ class ProgramInfo : public ProgramVariableAdder { // instantiated so they can be inserted during rewriting. TypeParamBindingsT TypeParamBindings; - // Function to check if an external symbol is okay to leave - // constrained. - bool isExternOkay(std::string Ext); + // Function to check if an external symbol is okay to leave constrained. + bool isExternOkay(const std::string &Ext); // Insert the given FVConstraint* set into the provided Map. // Returns true if successful else false. diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index ac6404648e7c..45b1d6928090 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -290,7 +290,7 @@ void ProgramInfo::print_stats(const std::set &F, raw_ostream &O, } } -bool ProgramInfo::isExternOkay(std::string Ext) { +bool ProgramInfo::isExternOkay(const std::string &Ext) { return llvm::StringSwitch(Ext) .Cases("malloc", "free", true) .Default(false); @@ -500,7 +500,8 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, // without doing anymore work. if (!Rewriter::isRewritable(D->getLocation())) { // If we're not in a macro, we should make the constraint variable WILD - // anyways. + // anyways. This happens if the name of the variable is a macro defined + // differently is different parts of the program. std::string Rsn = "Duplicate source location. Possibly part of a macro."; Variables[PLoc]->constrainToWild(CS, Rsn); } @@ -523,7 +524,9 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D, insertNewFVConstraint(FD, F, AstContext); } else { // FIXME: Visiting same function in same source location twice. - // Shouldn't happen but it does. Just bailing for now. + // This shouldn't happen, but it does for some std lib functions + // on our benchmarks programs (vsftpd, lua, etc.). Bailing for + // now, but a real fix will catch and prevent this earlier. } return; } diff --git a/clang/lib/CConv/RewriteUtils.cpp b/clang/lib/CConv/RewriteUtils.cpp index 47ba4c56dc1b..bb78f0b5d359 100644 --- a/clang/lib/CConv/RewriteUtils.cpp +++ b/clang/lib/CConv/RewriteUtils.cpp @@ -233,7 +233,7 @@ class TypeExprRewriter // Macros wil sometimes cause a single expression to have multiple // constraint variables. These should have been constrained to wild, so // there shouldn't be any rewriting required. - EnvironmentMap &Vars = Info.getConstraints().getVariables(); + const EnvironmentMap &Vars = Info.getConstraints().getVariables(); assert(CVSingleton.size() == 1 || llvm::none_of(CVSingleton, [&Vars](ConstraintVariable *CV) { return CV->anyChanges(Vars); @@ -241,15 +241,10 @@ class TypeExprRewriter for (auto *CV : CVSingleton) // Only rewrite if the type has changed. - if (CV->anyChanges(Info.getConstraints().getVariables())){ - // The constraint variable is able to tell us what the new type string - // should be. - std::string - NewType = CV->mkString(Info.getConstraints().getVariables(), false); - + if (CV->anyChanges(Vars)){ // Replace the original type with this new one if (canRewrite(Writer, Range)) - Writer.ReplaceText(Range, NewType); + Writer.ReplaceText(Range, CV->mkString(Vars, false)); } } }; From dad961300d7658a19af07cd0c6eaad49b41d48e0 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 16 Sep 2020 11:24:57 -0400 Subject: [PATCH 11/15] Update comment in PersistentSourceLocation --- clang/lib/CConv/PersistentSourceLoc.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/clang/lib/CConv/PersistentSourceLoc.cpp b/clang/lib/CConv/PersistentSourceLoc.cpp index 1fe629913548..8f1caf11f64b 100644 --- a/clang/lib/CConv/PersistentSourceLoc.cpp +++ b/clang/lib/CConv/PersistentSourceLoc.cpp @@ -14,21 +14,14 @@ using namespace clang; using namespace llvm; -// Given a Decl, look up the source location for that Decl and create a -// PersistentSourceLoc that represents the location of the Decl. -// For Function and Parameter Decls, use the Spelling location, while for -// variables, use the expansion location. +// Given a Decl, look up the source location for that Decl and create a +// PersistentSourceLoc that represents the location of the Decl. +// This currently the expansion location for the declarations source location. +// If we want to add more complete source for macros in the future, I expect we +// will need to the spelling location instead. PersistentSourceLoc PersistentSourceLoc::mkPSL(const Decl *D, ASTContext &C) { - SourceLocation SL = D->getLocation(); - - if (const FunctionDecl *FD = dyn_cast(D)) - SL = C.getSourceManager().getExpansionLoc(FD->getLocation()); - else if (const ParmVarDecl *PV = dyn_cast(D)) - SL = C.getSourceManager().getExpansionLoc(PV->getLocation()); - else if (const VarDecl *V = dyn_cast(D)) - SL = C.getSourceManager().getExpansionLoc(V->getLocation()); - + SourceLocation SL = C.getSourceManager().getExpansionLoc(D->getLocation()); return mkPSL(D->getSourceRange(), SL, C); } From 500cfcc5757ff9a35bca853f3144ea29e28fac7d Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 16 Sep 2020 11:30:53 -0400 Subject: [PATCH 12/15] Add a test case for parameter declarations in macro --- clang/test/CheckedCRewriter/definedType.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clang/test/CheckedCRewriter/definedType.c b/clang/test/CheckedCRewriter/definedType.c index 15c8eaffb93f..7b365a76e84b 100644 --- a/clang/test/CheckedCRewriter/definedType.c +++ b/clang/test/CheckedCRewriter/definedType.c @@ -138,3 +138,9 @@ void test() { // CHECK: int *c = y1; // CHECK: int **d = y2; // CHECK: int ***e = y3; + + +#define parm_decl int *a, int *b + +void parm_test(parm_decl) {} +// CHECK: void parm_test(parm_decl) {} From c97567192e67fe670effa562fa20c6036f6318e0 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 16 Sep 2020 16:25:55 -0400 Subject: [PATCH 13/15] Another test case for WILDness of macros --- clang/test/CheckedCRewriter/definedType.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/clang/test/CheckedCRewriter/definedType.c b/clang/test/CheckedCRewriter/definedType.c index 7b365a76e84b..f5f42a3d8e1d 100644 --- a/clang/test/CheckedCRewriter/definedType.c +++ b/clang/test/CheckedCRewriter/definedType.c @@ -144,3 +144,14 @@ void test() { void parm_test(parm_decl) {} // CHECK: void parm_test(parm_decl) {} + + +#define declare_single_var(x) int *x = 0; +int *another_test(void) { +// CHECK: int *another_test(void) { + declare_single_var(z) + // CHECK: declare_single_var(z) + declare_single_var(y) + // CHECK: declare_single_var(y) + return z; +} From 598504be929a1aafc8ccb5f193406915a9236251 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 16 Sep 2020 17:53:03 -0400 Subject: [PATCH 14/15] Use my own option type for nicer behavior with abstract base class --- .../include/clang/CConv/ConstraintVariables.h | 2 +- clang/include/clang/CConv/Utils.h | 21 ++++++++++ clang/lib/CConv/AVarBoundsInfo.cpp | 4 +- .../CConv/ArrayBoundsInferenceConsumer.cpp | 7 ++-- clang/lib/CConv/CheckedRegions.cpp | 9 ++-- clang/lib/CConv/ConstraintBuilder.cpp | 25 ++++++----- clang/lib/CConv/ConstraintResolver.cpp | 42 +++++++++++-------- clang/lib/CConv/ProgramInfo.cpp | 15 ++++--- clang/lib/CConv/StructInit.cpp | 14 ++++--- 9 files changed, 89 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/CConv/ConstraintVariables.h b/clang/include/clang/CConv/ConstraintVariables.h index a2bdbb4c6d64..ff03b0ddec1c 100644 --- a/clang/include/clang/CConv/ConstraintVariables.h +++ b/clang/include/clang/CConv/ConstraintVariables.h @@ -162,7 +162,7 @@ class ConstraintVariable { }; typedef std::set CVarSet; -typedef llvm::Optional CVarOption; +typedef Option CVarOption; enum ConsAction { Safe_to_Wild, diff --git a/clang/include/clang/CConv/Utils.h b/clang/include/clang/CConv/Utils.h index afa0dd1bd9a9..2747f7bf46a5 100644 --- a/clang/include/clang/CConv/Utils.h +++ b/clang/include/clang/CConv/Utils.h @@ -31,6 +31,27 @@ typedef std::map VariableMap; // Maps a Decl to the DeclStmt that defines the Decl. typedef std::map VariableDecltoStmtMap; +template +class Option { +public: + Option() : Value(nullptr), HasValue(false) {} + Option(ValueT &V) : Value(&V), HasValue(true) {} + + ValueT &getValue() const { + assert("Inconsistent option!" && HasValue && Value != nullptr); + return *Value; + } + + bool hasValue() const { + assert("Inconsistent option!" && HasValue == (Value != nullptr)); + return HasValue; + } + +private: + ValueT *Value; + bool HasValue; +}; + // Replacement for boost:bimap. A wrapper class around two std::maps to enable // map lookup from key to value or from value to key. template diff --git a/clang/lib/CConv/AVarBoundsInfo.cpp b/clang/lib/CConv/AVarBoundsInfo.cpp index 2a0aa28cbb96..57f36910393b 100644 --- a/clang/lib/CConv/AVarBoundsInfo.cpp +++ b/clang/lib/CConv/AVarBoundsInfo.cpp @@ -371,7 +371,7 @@ bool AVarBoundsInfo::handleContextSensitiveAssignment(CallExpr *CE, } else { // This is the assignment of regular variables. BoundsKey LKey, RKey; - if ((CR->resolveBoundsKey(LCVar, LKey) || + if ((CR->resolveBoundsKey(*LCVar, LKey) || tryGetVariable(L, LKey)) && (CR->resolveBoundsKey(RCVars, RKey) || tryGetVariable(R, *C, RKey))) { @@ -1137,7 +1137,7 @@ bool ContextSensitiveBoundsKeyVisitor::VisitCallExpr(CallExpr *CE) { // Contextualize the function at this call-site. CVarOption COpt = Info.getVariable(FD, Context); if (COpt.hasValue()) - Info.getABoundsInfo().contextualizeCVar(CE, {COpt.getValue()}); + Info.getABoundsInfo().contextualizeCVar(CE, {&COpt.getValue()}); } return true; } \ No newline at end of file diff --git a/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp b/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp index 449b08b6da01..6498c9bfa64d 100644 --- a/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp +++ b/clang/lib/CConv/ArrayBoundsInferenceConsumer.cpp @@ -135,9 +135,10 @@ static bool needArrayBounds(Decl *D, ProgramInfo &Info, ASTContext *C, bool IsNtArr) { const auto &E = Info.getConstraints().getVariables(); CVarOption CVar = Info.getVariable(D, C); - if (CVar) { - if ((!IsNtArr && needArrayBounds(*CVar, E)) || - (IsNtArr && needNTArrayBounds(*CVar, E))) + if (CVar.hasValue()) { + ConstraintVariable &CV = CVar.getValue(); + if ((!IsNtArr && needArrayBounds(&CV, E)) || + (IsNtArr && needNTArrayBounds(&CV, E))) return true; return false; } diff --git a/clang/lib/CConv/CheckedRegions.cpp b/clang/lib/CConv/CheckedRegions.cpp index 683e70b65ba4..122e32527de4 100644 --- a/clang/lib/CConv/CheckedRegions.cpp +++ b/clang/lib/CConv/CheckedRegions.cpp @@ -217,7 +217,8 @@ bool CheckedRegionFinder::VisitMemberExpr(MemberExpr *E){ if (VD) { // Check if the variable is WILD. CVarOption Cv = Info.getVariable(VD, Context); - if (Cv && (*Cv)->hasWild(Info.getConstraints().getVariables())) + if (Cv.hasValue() + && Cv.getValue().hasWild(Info.getConstraints().getVariables())) Wild = true; // Check if the variable contains unchecked types. Wild |= containsUncheckedPtr(VD->getType()); @@ -296,7 +297,8 @@ bool CheckedRegionFinder::isInStatementPosition(CallExpr *C) { } bool CheckedRegionFinder::isWild(CVarOption Cv) { - if (Cv && (*Cv)->hasWild(Info.getConstraints().getVariables())) + if (Cv.hasValue() + && Cv.getValue().hasWild(Info.getConstraints().getVariables())) return true; return false; } @@ -356,7 +358,8 @@ bool CheckedRegionFinder::isUncheckedStruct(QualType Qt, std::set & auto Ftype = Fld->getType(); Unsafe |= containsUncheckedPtrAcc(Ftype, Seen); CVarOption Cv = Info.getVariable(Fld, Context); - Unsafe |= (Cv && (*Cv)->hasWild(Info.getConstraints().getVariables())); + Unsafe |= (Cv.hasValue() + && Cv.getValue().hasWild(Info.getConstraints().getVariables())); } return Unsafe; } diff --git a/clang/lib/CConv/ConstraintBuilder.cpp b/clang/lib/CConv/ConstraintBuilder.cpp index 862fef5ddb56..6c21d1b46e4a 100644 --- a/clang/lib/CConv/ConstraintBuilder.cpp +++ b/clang/lib/CConv/ConstraintBuilder.cpp @@ -177,12 +177,14 @@ class FunctionVisitor : public RecursiveASTVisitor { } else if (FunctionDecl *FD = dyn_cast(D)) { FuncName = FD->getNameAsString(); TFD = FD; - if (CVarOption CV = Info.getVariable(FD, Context)) - FVCons.insert(*CV); + CVarOption CV = Info.getVariable(FD, Context); + if (CV.hasValue()) + FVCons.insert(&CV.getValue()); } else if (DeclaratorDecl *DD = dyn_cast(D)) { FuncName = DD->getNameAsString(); - if (CVarOption CV = Info.getVariable(DD, Context)) - FVCons.insert(*CV); + CVarOption CV = Info.getVariable(DD, Context); + if (CV.hasValue()) + FVCons.insert(&CV.getValue()); } // Collect type parameters for this function call that are @@ -286,8 +288,7 @@ class FunctionVisitor : public RecursiveASTVisitor { // Get function variable constraint of the body PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(S, *Context); - ConstraintVariable *CV = - Info.getVariable(Function, Context).getValueOr(nullptr); + CVarOption CVOpt = Info.getVariable(Function, Context); // Constrain the value returned (if present) against the return value // of the function. @@ -296,11 +297,13 @@ class FunctionVisitor : public RecursiveASTVisitor { CVarSet RconsVar = CB.getExprConstraintVars(RetExpr); // Constrain the return type of the function // to the type of the return expression. - if (FVConstraint *FV = dyn_cast_or_null(CV)) { - // This is to ensure that the return type of the function is same - // as the type of return expression. - constrainConsVarGeq(FV->getReturnVar(), RconsVar, Info.getConstraints(), - &PL, Same_to_Same, false, &Info); + if (CVOpt.hasValue()) { + if (FVConstraint *FV = dyn_cast(&CVOpt.getValue())) { + // This is to ensure that the return type of the function is same + // as the type of return expression. + constrainConsVarGeq(FV->getReturnVar(), RconsVar, Info.getConstraints(), + &PL, Same_to_Same, false, &Info); + } } return true; } diff --git a/clang/lib/CConv/ConstraintResolver.cpp b/clang/lib/CConv/ConstraintResolver.cpp index 641b8df0dd10..5e453f3c8eb1 100644 --- a/clang/lib/CConv/ConstraintResolver.cpp +++ b/clang/lib/CConv/ConstraintResolver.cpp @@ -44,8 +44,10 @@ void ConstraintResolver::constraintAllCVarsToWild(const CVarSet &CSet, void ConstraintResolver::constraintCVarToWild(CVarOption CVar, const std::string &Rsn, Expr *AtExpr) { - if (CVar) - constraintAllCVarsToWild({*CVar}, Rsn, AtExpr); + if (CVar.hasValue()) { + ConstraintVariable &T = CVar.getValue(); + constraintAllCVarsToWild({&T}, Rsn, AtExpr); + } } // Return a set of PVConstraints equivalent to the set given, @@ -254,12 +256,12 @@ CVarSet } else if (DeclRefExpr *DRE = dyn_cast(E)) { CVarOption CV = Info.getVariable(DRE->getDecl(), Context); assert("Declaration without constraint variable?" && CV.hasValue()); - return {CV.getValue()}; + return {&CV.getValue()}; // x.f } else if (MemberExpr *ME = dyn_cast(E)) { CVarOption CV = Info.getVariable(ME->getMemberDecl(), Context); assert("Declaration without constraint variable?" && CV.hasValue()); - return {CV.getValue()}; + return {&CV.getValue()}; // Checked-C temporary } else if (CHKCBindTemporaryExpr *CE = dyn_cast(E)) { return getExprConstraintVars(CE->getSubExpr()); @@ -479,13 +481,13 @@ CVarSet /* Normal function call */ } else { CVarOption CV = Info.getVariable(FD, Context); - assert(CV && "Function without constraint variable."); + assert(CV.hasValue() && "Function without constraint variable."); /* Direct function call */ - if (FVConstraint *FVC = dyn_cast(*CV)) + if (FVConstraint *FVC = dyn_cast(&CV.getValue())) ReturnCVs.insert(FVC->getReturnVar()); /* Call via function pointer */ else { - PVConstraint *tmp = dyn_cast(*CV); + PVConstraint *tmp = dyn_cast(&CV.getValue()); assert(tmp != nullptr); if (FVConstraint *FVC = tmp->getFV()) ReturnCVs.insert(FVC->getReturnVar()); @@ -659,10 +661,11 @@ void ConstraintResolver::constrainLocalAssign(Stmt *TSt, DeclaratorDecl *D, CVarOption V = Info.getVariable(D, Context); auto RHSCons = getExprConstraintVars(RHS); - if (V) - constrainConsVarGeq(*V, RHSCons, Info.getConstraints(), PLPtr, CAction, - false, &Info); - if (AllTypes && !(V && isValidCons(*V)) && !containsValidCons(RHSCons)) { + if (V.hasValue()) + constrainConsVarGeq(&V.getValue(), RHSCons, Info.getConstraints(), PLPtr, + CAction, false, &Info); + if (AllTypes && !(V.hasValue() && isValidCons(&V.getValue())) + && !containsValidCons(RHSCons)) { auto &ABI = Info.getABoundsInfo(); ABI.handleAssignment(D, V, RHS, RHSCons, Context, this); } @@ -721,18 +724,21 @@ bool ConstraintResolver::isValidCons(ConstraintVariable *CV) { bool ConstraintResolver::resolveBoundsKey(const CVarSet &CVs, BoundsKey &BK) { if (CVs.size() == 1) { auto *OCons = getOnly(CVs); - return resolveBoundsKey(OCons, BK); + return resolveBoundsKey(*OCons, BK); } return false; } -bool ConstraintResolver::resolveBoundsKey(CVarOption CV, +bool ConstraintResolver::resolveBoundsKey(CVarOption CVOpt, BoundsKey &BK) { - if (PVConstraint *PV = dyn_cast_or_null(CV.getValueOr(nullptr))) - if (PV->hasBoundsKey()) { - BK = PV->getBoundsKey(); - return true; - } + if (CVOpt.hasValue()) { + ConstraintVariable &CV = CVOpt.getValue(); + if (PVConstraint *PV = dyn_cast(&CV)) + if (PV->hasBoundsKey()) { + BK = PV->getBoundsKey(); + return true; + } + } return false; } diff --git a/clang/lib/CConv/ProgramInfo.cpp b/clang/lib/CConv/ProgramInfo.cpp index 45b1d6928090..365aa29d2405 100644 --- a/clang/lib/CConv/ProgramInfo.cpp +++ b/clang/lib/CConv/ProgramInfo.cpp @@ -475,9 +475,12 @@ void ProgramInfo::specialCaseVarIntros(ValueDecl *D, ASTContext *Context) { PersistentSourceLoc PL = PersistentSourceLoc::mkPSL(D, *Context); if (!D->getType()->isVoidType()) Rsn = "Variable type is va_list."; - ConstraintVariable *CV = getVariable(D, Context).getValueOr(nullptr); - if (PVConstraint *PVC = dyn_cast_or_null(CV)) - PVC->constrainToWild(CS, Rsn, &PL); + CVarOption CVOpt = getVariable(D, Context); + if (CVOpt.hasValue()) { + ConstraintVariable &CV = CVOpt.getValue(); + if (PVConstraint *PVC = dyn_cast(&CV)) + PVC->constrainToWild(CS, Rsn, &PL); + } } } @@ -714,19 +717,19 @@ CVarOption ProgramInfo::getVariable(clang::Decl *D, clang::ASTContext *C) { // Get corresponding FVConstraint vars. FVConstraint *FunFVar = getFuncFVConstraint(FD, C); assert(FunFVar != nullptr && "Unable to find function constraints."); - return FunFVar->getParamVar(PIdx); + return CVarOption(*FunFVar->getParamVar(PIdx)); } else if (FunctionDecl *FD = dyn_cast(D)) { FVConstraint *FunFVar = getFuncFVConstraint(FD, C); if (FunFVar == nullptr) { llvm::errs() << "No fun constraints for " << FD->getName() << "?!\n"; } - return FunFVar; + return CVarOption(*FunFVar); } else /* neither function nor function parameter */ { auto I = Variables.find(PersistentSourceLoc::mkPSL(D, *C)); if (I != Variables.end()) - return I->second; + return CVarOption(*I->second); return CVarOption(); } } diff --git a/clang/lib/CConv/StructInit.cpp b/clang/lib/CConv/StructInit.cpp index b514df5c0fe7..a90d080afa2e 100644 --- a/clang/lib/CConv/StructInit.cpp +++ b/clang/lib/CConv/StructInit.cpp @@ -25,12 +25,14 @@ bool StructVariableInitializer::VariableNeedsInitializer(VarDecl *VD) { for (auto *const D : Definition->fields()) { if (D->getType()->isPointerType() || D->getType()->isArrayType()) { - ConstraintVariable *CV = I.getVariable(D, Context).getValueOr(nullptr); - PVConstraint *PV = dyn_cast_or_null(CV); - if (PV && PV->isChecked(I.getConstraints().getVariables())) { - // Ok this contains a pointer that is checked. Store it. - RecordsWithCPointers.insert(Definition); - return true; + CVarOption CV = I.getVariable(D, Context); + if (CV.hasValue()) { + PVConstraint *PV = dyn_cast(&CV.getValue()); + if (PV && PV->isChecked(I.getConstraints().getVariables())) { + // Ok this contains a pointer that is checked. Store it. + RecordsWithCPointers.insert(Definition); + return true; + } } } } From 40f2b525496f3301ecb7c09bcc0e20aeeb77028b Mon Sep 17 00:00:00 2001 From: John Kastner Date: Thu, 17 Sep 2020 11:17:58 -0400 Subject: [PATCH 15/15] Move BaseType code into a static method --- .../include/clang/CConv/ConstraintVariables.h | 6 ++ clang/lib/CConv/ConstraintVariables.cpp | 82 +++++++++++-------- 2 files changed, 52 insertions(+), 36 deletions(-) diff --git a/clang/include/clang/CConv/ConstraintVariables.h b/clang/include/clang/CConv/ConstraintVariables.h index ff03b0ddec1c..61c31ef88a57 100644 --- a/clang/include/clang/CConv/ConstraintVariables.h +++ b/clang/include/clang/CConv/ConstraintVariables.h @@ -236,6 +236,12 @@ class PointerVariableConstraint : public ConstraintVariable { bool &AllArray, bool &ArrayRun, bool Nt) const; void addArrayAnnotations(std::stack &CheckedArrs, std::deque &EndStrs) const; + + // Utility used by the constructor to extract string representation of the + // base type that preserves macros where possible. + static std::string extractBaseType(DeclaratorDecl *D, QualType QT, + const Type *Ty, const ASTContext &C); + // Flag to indicate that this constraint is a part of function prototype // e.g., Parameters or Return. bool partOFFuncPrototype; diff --git a/clang/lib/CConv/ConstraintVariables.cpp b/clang/lib/CConv/ConstraintVariables.cpp index 89ccba4e9d22..5b6364cceb9e 100644 --- a/clang/lib/CConv/ConstraintVariables.cpp +++ b/clang/lib/CConv/ConstraintVariables.cpp @@ -346,42 +346,7 @@ PointerVariableConstraint::PointerVariableConstraint(const QualType &QT, FV = new FVConstraint(Ty, IsDeclTy ? D : nullptr, IsTypedef ? "" : N, I, C); // Get a string representing the type without pointer and array indirection. - bool FoundMatchingType = false; - if (!IsTypedef && D && D->getTypeSourceInfo()) { - // Try to extract the type from original source to preserve defines - TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc(); - if (isa(D)) { - FoundMatchingType = D->getAsFunction()->getReturnType() == QT; - TL = getBaseTypeLoc(TL).getAs(); - // FunctionDecl that doesn't have function type? weird - if (TL.isNull()) - FoundMatchingType = false; - else - TL = TL.getAs().getReturnLoc(); - } else { - FoundMatchingType = D->getType() == QT; - } - TypeLoc BaseLoc = getBaseTypeLoc(TL); - if (!BaseLoc.getAs().isNull()) { - FoundMatchingType = false; - } else { - // Only use this type if the type passed as a parameter to this constructor - // agrees with the actual type of the declaration. - SourceRange SR = BaseLoc.getSourceRange(); - if (FoundMatchingType && SR.isValid()) { - BaseType = getSourceText(SR, C); - - // getSourceText returns the empty string when there's a pointer level - // inside a macro. Not sure how to handle this, so fall back to tyToStr. - if (BaseType.empty()) - FoundMatchingType = false; - } else - FoundMatchingType = false; - } - } - // Fall back to rebuilding the base type based on type passed to constructor - if (!FoundMatchingType) - BaseType = tyToStr(Ty); + BaseType = extractBaseType(D, QT, Ty, C); bool IsWild = !IsGeneric && (isVarArgType(BaseType) || isTypeHasVoid(QT)); if (IsWild) { @@ -424,6 +389,51 @@ PointerVariableConstraint::PointerVariableConstraint(const QualType &QT, } } +std::string PointerVariableConstraint::extractBaseType(DeclaratorDecl *D, + QualType QT, + const Type *Ty, + const ASTContext &C) { + std::string BaseTypeStr; + bool FoundBaseTypeInSrc = false; + if (!Ty->getAs() && D && D->getTypeSourceInfo()) { + // Try to extract the type from original source to preserve defines + TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc(); + if (isa(D)) { + FoundBaseTypeInSrc = D->getAsFunction()->getReturnType() == QT; + TL = getBaseTypeLoc(TL).getAs(); + // FunctionDecl that doesn't have function type? weird + if (TL.isNull()) + FoundBaseTypeInSrc = false; + else + TL = TL.getAs().getReturnLoc(); + } else { + FoundBaseTypeInSrc = D->getType() == QT; + } + TypeLoc BaseLoc = getBaseTypeLoc(TL); + if (!BaseLoc.getAs().isNull()) { + FoundBaseTypeInSrc = false; + } else { + // Only use this type if the type passed as a parameter to this constructor + // agrees with the actual type of the declaration. + SourceRange SR = BaseLoc.getSourceRange(); + if (FoundBaseTypeInSrc && SR.isValid()) { + BaseTypeStr = getSourceText(SR, C); + + // getSourceText returns the empty string when there's a pointer level + // inside a macro. Not sure how to handle this, so fall back to tyToStr. + if (BaseTypeStr.empty()) + FoundBaseTypeInSrc = false; + } else + FoundBaseTypeInSrc = false; + } + } + // Fall back to rebuilding the base type based on type passed to constructor + if (!FoundBaseTypeInSrc) + BaseTypeStr = tyToStr(Ty); + + return BaseTypeStr; +} + void PointerVariableConstraint::print(raw_ostream &O) const { O << "{ "; for (const auto &I : vars) {