From e846ae2bb151bc3160f6a773e8af8d723d567d24 Mon Sep 17 00:00:00 2001 From: Kyle Headley Date: Mon, 8 Feb 2021 15:26:25 -0500 Subject: [PATCH 1/2] Add ReasonFailed to brainTransplant; Refactor insertNewFVConstraint --- clang/include/clang/3C/ConstraintVariables.h | 12 +- clang/include/clang/3C/ProgramInfo.h | 15 +- clang/lib/3C/ConstraintVariables.cpp | 43 ++++-- clang/lib/3C/ProgramInfo.cpp | 149 +++++++++---------- 4 files changed, 113 insertions(+), 106 deletions(-) diff --git a/clang/include/clang/3C/ConstraintVariables.h b/clang/include/clang/3C/ConstraintVariables.h index 3b777108d052..f969856c6c0e 100644 --- a/clang/include/clang/3C/ConstraintVariables.h +++ b/clang/include/clang/3C/ConstraintVariables.h @@ -138,7 +138,8 @@ class ConstraintVariable { virtual void equateArgumentConstraints(ProgramInfo &I) = 0; // Update this CV with information from duplicate declaration CVs - virtual void brainTransplant(ConstraintVariable *, ProgramInfo &) = 0; + virtual void brainTransplant(ConstraintVariable *, ProgramInfo &, + std::string &ReasonFailed) = 0; virtual void mergeDeclaration(ConstraintVariable *, ProgramInfo &, std::string &ReasonFailed) = 0; @@ -383,7 +384,8 @@ class PointerVariableConstraint : public ConstraintVariable { const CAtoms &getCvars() const { return Vars; } - void brainTransplant(ConstraintVariable *From, ProgramInfo &I) override; + void brainTransplant(ConstraintVariable *From, ProgramInfo &I, + std::string &ReasonFailed) override; void mergeDeclaration(ConstraintVariable *From, ProgramInfo &I, std::string &ReasonFailed) override; @@ -466,7 +468,8 @@ class FVComponentVariable { void mergeDeclaration(FVComponentVariable *From, ProgramInfo &I, std::string &ReasonFailed); - void brainTransplant(FVComponentVariable *From, ProgramInfo &I); + void brainTransplant(FVComponentVariable *From, ProgramInfo &I, + std::string &ReasonFailed); std::string mkItypeStr(const EnvironmentMap &E) const; std::string mkTypeStr(const EnvironmentMap &E) const; @@ -536,7 +539,8 @@ class FunctionVariableConstraint : public ConstraintVariable { return S->getKind() == FunctionVariable; } - void brainTransplant(ConstraintVariable *From, ProgramInfo &I) override; + void brainTransplant(ConstraintVariable *From, ProgramInfo &I, + std::string &ReasonFailed) override; void mergeDeclaration(ConstraintVariable *FromCV, ProgramInfo &I, std::string &ReasonFailed) override; diff --git a/clang/include/clang/3C/ProgramInfo.h b/clang/include/clang/3C/ProgramInfo.h index ad05721766bc..c20d208b5c2b 100644 --- a/clang/include/clang/3C/ProgramInfo.h +++ b/clang/include/clang/3C/ProgramInfo.h @@ -167,20 +167,7 @@ class ProgramInfo : public ProgramVariableAdder { // For each call to a generic function, remember how the type parameters were // instantiated so they can be inserted during rewriting. TypeParamBindingsT TypeParamBindings; - - // Insert the given FVConstraint* set into the provided Map. - void insertIntoExternalFunctionMap(ExternalFunctionMapType &Map, - const std::string &FuncName, - FVConstraint *NewC, FunctionDecl *FD, - ASTContext *C); - - // Inserts the given FVConstraint* set into the provided static map. - void insertIntoStaticFunctionMap(StaticFunctionMapType &Map, - const std::string &FuncName, - const std::string &FileName, - FVConstraint *ToIns, FunctionDecl *FD, - ASTContext *C); - + // Special-case handling for decl introductions. For the moment this covers: // * void-typed variables // * va_list-typed variables diff --git a/clang/lib/3C/ConstraintVariables.cpp b/clang/lib/3C/ConstraintVariables.cpp index 29eeda821589..5b78f242c713 100644 --- a/clang/lib/3C/ConstraintVariables.cpp +++ b/clang/lib/3C/ConstraintVariables.cpp @@ -1724,16 +1724,21 @@ bool isAValidPVConstraint(const ConstraintVariable *C) { // Replace CVars and ArgumentConstraints with those in [FromCV]. void PointerVariableConstraint::brainTransplant(ConstraintVariable *FromCV, - ProgramInfo &I) { + ProgramInfo &I, + std::string &ReasonFailed) { PVConstraint *From = dyn_cast(FromCV); assert(From != nullptr); CAtoms CFrom = From->getCvars(); - assert(Vars.size() == CFrom.size()); + if(Vars.size() != CFrom.size()) { + ReasonFailed = "transplanting different pointer levels"; + return; + } if (From->hasBoundsKey()) { // If this has bounds key!? Then do brain transplant of // bound keys as well. if (hasBoundsKey()) - I.getABoundsInfo().brainTransplant(getBoundsKey(), From->getBoundsKey()); + I.getABoundsInfo().brainTransplant(getBoundsKey(), + From->getBoundsKey()); ValidBoundsKey = From->hasBoundsKey(); BKey = From->getBoundsKey(); @@ -1742,7 +1747,7 @@ void PointerVariableConstraint::brainTransplant(ConstraintVariable *FromCV, ArgumentConstraints = From->getArgumentConstraints(); if (FV) { assert(From->FV); - FV->brainTransplant(From->FV, I); + FV->brainTransplant(From->FV, I, ReasonFailed); } } @@ -1816,15 +1821,26 @@ Atom *PointerVariableConstraint::getAtom(unsigned AtomIdx, Constraints &CS) { // Brain Transplant params and returns in [FromCV], recursively. void FunctionVariableConstraint::brainTransplant(ConstraintVariable *FromCV, - ProgramInfo &I) { + ProgramInfo &I, + std::string &ReasonFailed) { FVConstraint *From = dyn_cast(FromCV); assert(From != nullptr); // Transplant returns. - ReturnVar.brainTransplant(&From->ReturnVar, I); + ReturnVar.brainTransplant(&From->ReturnVar, I, ReasonFailed); + if (ReasonFailed != "") { + ReasonFailed += "for return value"; + return; + } + // Transplant params. if (numParams() == From->numParams()) { - for (unsigned J = 0; J < From->numParams(); J++) - ParamVars[J].brainTransplant(&From->ParamVars[J], I); + for (unsigned J = 0; J < From->numParams(); J++) { + ParamVars[J].brainTransplant(&From->ParamVars[J], I, ReasonFailed); + if (ReasonFailed != "") { + ReasonFailed += "for parameter " + std::to_string(J); + return; + } + } } else if (numParams() != 0 && From->numParams() == 0) { auto &CS = I.getConstraints(); const std::vector &Defers = From->getDeferredParams(); @@ -1865,7 +1881,7 @@ void FunctionVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV, } if (this->numParams() == 0) { // This is an untyped declaration, we need to perform a transplant - From->brainTransplant(this, I); + From->brainTransplant(this, I, ReasonFailed); } else { // Standard merge if (this->numParams() != From->numParams()) { @@ -1913,14 +1929,17 @@ void FVComponentVariable::mergeDeclaration(FVComponentVariable *From, } void FVComponentVariable::brainTransplant(FVComponentVariable *From, - ProgramInfo &I) { + ProgramInfo &I, + std::string &ReasonFailed) { // As in mergeDeclaration, special handling is required if the original // declaration did not allocate split constraint variables. if (InternalConstraint == ExternalConstraint) InternalConstraint = From->InternalConstraint; else - InternalConstraint->brainTransplant(From->InternalConstraint, I); - ExternalConstraint->brainTransplant(From->ExternalConstraint, I); + InternalConstraint->brainTransplant(From->InternalConstraint, I, + ReasonFailed); + ExternalConstraint->brainTransplant(From->ExternalConstraint, I, + ReasonFailed); } std::string diff --git a/clang/lib/3C/ProgramInfo.cpp b/clang/lib/3C/ProgramInfo.cpp index 5d55d7ac79dd..a347d818a567 100644 --- a/clang/lib/3C/ProgramInfo.cpp +++ b/clang/lib/3C/ProgramInfo.cpp @@ -402,87 +402,84 @@ void ProgramInfo::exitCompilationUnit() { return; } -void ProgramInfo::insertIntoExternalFunctionMap(ExternalFunctionMapType &Map, - const std::string &FuncName, - FVConstraint *NewC, - FunctionDecl *FD, - ASTContext *C) { - if (Map.find(FuncName) == Map.end()) { - Map[FuncName] = NewC; - } else { - auto *OldC = Map[FuncName]; - if (!OldC->hasBody()) { - if (NewC->hasBody() || - (OldC->numParams() == 0 && NewC->numParams() != 0)) { - NewC->brainTransplant(OldC, *this); - Map[FuncName] = NewC; - } else { - // if the current FV constraint is not a definition? - // then merge. - std::string ReasonFailed = ""; - OldC->mergeDeclaration(NewC, *this, ReasonFailed); - bool MergingFailed = ReasonFailed != ""; - if (MergingFailed) { - clang::DiagnosticsEngine &DE = C->getDiagnostics(); - unsigned MergeFailID = DE.getCustomDiagID( - DiagnosticsEngine::Fatal, "merging failed for %q0 due to %1"); - const auto Pointer = reinterpret_cast(FD); - const auto Kind = - clang::DiagnosticsEngine::ArgumentKind::ak_nameddecl; - auto DiagBuilder = DE.Report(FD->getLocation(), MergeFailID); - DiagBuilder.AddTaggedVal(Pointer, Kind); - DiagBuilder.AddString(ReasonFailed); - } - if (MergingFailed) { - // Kill the process and stop conversion - // Without this code here, 3C simply ignores this pair of functions - // and converts the rest of the files as it will (in semi-compliance - // with Mike's (2) listed on the original issue (#283) - exit(1); - } - } - } else if (NewC->hasBody()) { - clang::DiagnosticsEngine &DE = C->getDiagnostics(); - unsigned DuplicateDefinitionsID = DE.getCustomDiagID( - DiagnosticsEngine::Fatal, "duplicate definition for function %0"); - DE.Report(FD->getLocation(), DuplicateDefinitionsID).AddString(FuncName); - exit(1); - } else { - // The old constraint has a body, but we've encountered another prototype - // for the function. - assert(OldC->hasBody() && !NewC->hasBody()); - // By transplanting the atoms of OldC into NewC, we ensure that any - // constraints applied to NewC later on constrain the atoms of OldC. - NewC->brainTransplant(OldC, *this); +void ProgramInfo::insertNewFVConstraint(FunctionDecl *FD, FVConstraint *NewC, + ASTContext *C) { + std::string FuncName = FD->getNameAsString(); + + // Choose a storage location + + // assume a global function, but change to a static if not + ExternalFunctionMapType *Map = &ExternalFunctionFVCons; + if (!FD->isGlobal()) { + // if the filename is new, just insert and we're done + auto Psl = PersistentSourceLoc::mkPSL(FD, *C); + std::string FileName = Psl.getFileName(); + if (StaticFunctionFVCons.find(FileName) == StaticFunctionFVCons.end()){ + StaticFunctionFVCons[FileName][FuncName] = NewC; + return; } + + // store in static map + Map = &StaticFunctionFVCons[FileName]; } -} -void ProgramInfo::insertIntoStaticFunctionMap(StaticFunctionMapType &Map, - const std::string &FuncName, - const std::string &FileName, - FVConstraint *ToIns, - FunctionDecl *FD, ASTContext *C) { - if (Map.find(FileName) == Map.end()) - Map[FileName][FuncName] = ToIns; - else - insertIntoExternalFunctionMap(Map[FileName], FuncName, ToIns, FD, C); -} + // if the function is new, just insert and we're done + if (Map->find(FuncName) == Map->end()) { + (*Map)[FuncName] = NewC; + return; + } -void ProgramInfo::insertNewFVConstraint(FunctionDecl *FD, FVConstraint *FVCon, - ASTContext *C) { - std::string FuncName = FD->getNameAsString(); - if (FD->isGlobal()) { - // external method. - insertIntoExternalFunctionMap(ExternalFunctionFVCons, FuncName, FVCon, FD, - C); - } else { - // static method - auto Psl = PersistentSourceLoc::mkPSL(FD, *C); - std::string FuncFileName = Psl.getFileName(); - insertIntoStaticFunctionMap(StaticFunctionFVCons, FuncName, FuncFileName, - FVCon, FD, C); + // Resolve conflicts + + // We need to keep the version with a body, if it exists, + // so branch based on it + auto *OldC = (*Map)[FuncName]; + bool NewHasBody = NewC->hasBody(); + bool OldHasBody = OldC->hasBody(); + std::string ReasonFailed = ""; + + if (OldHasBody && NewHasBody) { + // Two separate bodies for a function is irreconcilable + ReasonFailed = "multiple function bodies"; + } else if (OldHasBody && !NewHasBody) { + // By transplanting the atoms of OldC into NewC, we ensure that any + // constraints applied to NewC later on constrain the atoms of OldC. + NewC->brainTransplant(OldC, *this, ReasonFailed); + } else if (!OldHasBody && NewHasBody) { + // as above, but the new version has the body + NewC->brainTransplant(OldC, *this, ReasonFailed); + (*Map)[FuncName] = NewC; + } else if (!OldHasBody && !NewHasBody) { + // special case for undeclared params + if (OldC->numParams() == 0 && NewC->numParams() != 0) { + NewC->brainTransplant(OldC, *this, ReasonFailed); + (*Map)[FuncName] = NewC; + } else { + // Merging favors the checked types. + // It assumes the author changed a few and missed a few. + OldC->mergeDeclaration(NewC, *this, ReasonFailed); + } } + + // If successful, we're done and can skip error reporting + if (ReasonFailed == "") return; + + // Error reporting + + clang::DiagnosticsEngine &DE = C->getDiagnostics(); + unsigned FailID = DE.getCustomDiagID(DiagnosticsEngine::Fatal, + "merging failed for %q0 due to %1"); + const auto Pointer = reinterpret_cast(FD); + const auto Kind = clang::DiagnosticsEngine::ArgumentKind::ak_nameddecl; + auto DiagBuilder = DE.Report(FD->getLocation(), FailID); + DiagBuilder.AddTaggedVal(Pointer, Kind); + DiagBuilder.AddString(ReasonFailed); + + // Kill the process and stop conversion + // Without this code here, 3C simply ignores this pair of functions + // and converts the rest of the files as it will (in semi-compliance + // with Mike's (2) listed on the original issue (#283) + exit(1); } void ProgramInfo::specialCaseVarIntros(ValueDecl *D, ASTContext *Context) { From 717fb0fee913dc9fcf3c5b21bd1aa75eea4468e3 Mon Sep 17 00:00:00 2001 From: Kyle Headley Date: Mon, 8 Feb 2021 17:09:18 -0500 Subject: [PATCH 2/2] another assert --- clang/lib/3C/ConstraintVariables.cpp | 2 +- clang/lib/3C/ProgramInfo.cpp | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/clang/lib/3C/ConstraintVariables.cpp b/clang/lib/3C/ConstraintVariables.cpp index 5b78f242c713..878ce28daced 100644 --- a/clang/lib/3C/ConstraintVariables.cpp +++ b/clang/lib/3C/ConstraintVariables.cpp @@ -1855,7 +1855,7 @@ void FunctionVariableConstraint::brainTransplant(ConstraintVariable *FromCV, } } } else { - llvm_unreachable("Brain Transplant on empty params"); + ReasonFailed = "transplant on differing param count"; } } diff --git a/clang/lib/3C/ProgramInfo.cpp b/clang/lib/3C/ProgramInfo.cpp index a347d818a567..8b53e6b77ab9 100644 --- a/clang/lib/3C/ProgramInfo.cpp +++ b/clang/lib/3C/ProgramInfo.cpp @@ -465,16 +465,16 @@ void ProgramInfo::insertNewFVConstraint(FunctionDecl *FD, FVConstraint *NewC, if (ReasonFailed == "") return; // Error reporting - - clang::DiagnosticsEngine &DE = C->getDiagnostics(); - unsigned FailID = DE.getCustomDiagID(DiagnosticsEngine::Fatal, - "merging failed for %q0 due to %1"); - const auto Pointer = reinterpret_cast(FD); - const auto Kind = clang::DiagnosticsEngine::ArgumentKind::ak_nameddecl; - auto DiagBuilder = DE.Report(FD->getLocation(), FailID); - DiagBuilder.AddTaggedVal(Pointer, Kind); - DiagBuilder.AddString(ReasonFailed); - + { // block to force DiagBuilder destructor and emit message + clang::DiagnosticsEngine &DE = C->getDiagnostics(); + unsigned FailID = DE.getCustomDiagID(DiagnosticsEngine::Fatal, + "merging failed for %q0 due to %1"); + const auto Pointer = reinterpret_cast(FD); + const auto Kind = clang::DiagnosticsEngine::ArgumentKind::ak_nameddecl; + auto DiagBuilder = DE.Report(FD->getLocation(), FailID); + DiagBuilder.AddTaggedVal(Pointer, Kind); + DiagBuilder.AddString(ReasonFailed); + } // Kill the process and stop conversion // Without this code here, 3C simply ignores this pair of functions // and converts the rest of the files as it will (in semi-compliance