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..9eca32d39976 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 between pointers with different depths"; + 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,11 @@ void PointerVariableConstraint::brainTransplant(ConstraintVariable *FromCV, ArgumentConstraints = From->getArgumentConstraints(); if (FV) { assert(From->FV); - FV->brainTransplant(From->FV, I); + FV->brainTransplant(From->FV, I, ReasonFailed); + if (ReasonFailed != "") { + ReasonFailed += " within the referenced function"; + return; + } } } @@ -1755,7 +1764,7 @@ void PointerVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV, CAtoms::iterator I = Vars.begin(); CAtoms::iterator J = CFrom.begin(); if (CFrom.size() != Vars.size()) { - ReasonFailed = "conflicting types "; + ReasonFailed = "transplanting between pointers with different depths"; return; } while (I != Vars.end()) { @@ -1783,7 +1792,7 @@ void PointerVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV, ++I; ++J; } - assert(Vars.size() == NewVatoms.size() && "Merging Failed"); + assert(Vars.size() == NewVatoms.size() && "Merging Error"); Vars = NewVatoms; SrcHasItype = SrcHasItype || From->SrcHasItype; if (!From->ItypeStr.empty()) @@ -1795,6 +1804,10 @@ void PointerVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV, if (FV) { assert(From->FV); FV->mergeDeclaration(From->FV, Info, ReasonFailed); + if (ReasonFailed != "") { + ReasonFailed += " within the referenced function"; + return; + } } } @@ -1816,15 +1829,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(); @@ -1839,7 +1863,7 @@ void FunctionVariableConstraint::brainTransplant(ConstraintVariable *FromCV, } } } else { - llvm_unreachable("Brain Transplant on empty params"); + ReasonFailed = "transplant on differing param count"; } } @@ -1855,7 +1879,7 @@ void FunctionVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV, // Transplant returns. ReturnVar.mergeDeclaration(&From->ReturnVar, I, ReasonFailed); if (ReasonFailed != "") { - ReasonFailed += "for return value"; + ReasonFailed += " for return value"; return; } @@ -1865,7 +1889,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()) { @@ -1875,7 +1899,7 @@ void FunctionVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV, for (unsigned J = 0; J < From->numParams(); J++) { ParamVars[J].mergeDeclaration(&From->ParamVars[J], I, ReasonFailed); if (ReasonFailed != "") { - ReasonFailed += "for parameter " + std::to_string(J); + ReasonFailed += " for parameter " + std::to_string(J); return; } } @@ -1908,19 +1932,30 @@ void FVComponentVariable::mergeDeclaration(FVComponentVariable *From, InternalConstraint->mergeDeclaration(From->InternalConstraint, I, ReasonFailed); } + if (ReasonFailed != ""){ + ReasonFailed += " during internal merge"; + return; + } ExternalConstraint->mergeDeclaration(From->ExternalConstraint, I, ReasonFailed); } 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); + if (ReasonFailed != ""){ + ReasonFailed += " during internal merge"; + return; + } + ExternalConstraint->brainTransplant(From->ExternalConstraint, I, + ReasonFailed); } std::string diff --git a/clang/lib/3C/ProgramInfo.cpp b/clang/lib/3C/ProgramInfo.cpp index 5d55d7ac79dd..c319c8997631 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 has not yet been seen, 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 has not yet been seen, 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 + { // 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 + // with Mike's (2) listed on the original issue (#283) + exit(1); } void ProgramInfo::specialCaseVarIntros(ValueDecl *D, ASTContext *Context) { diff --git a/clang/test/3C/difftypes1a.c b/clang/test/3C/difftypes1a.c new file mode 100644 index 000000000000..2a4f38bfef66 --- /dev/null +++ b/clang/test/3C/difftypes1a.c @@ -0,0 +1,7 @@ +//RUN: rm -rf %t* +//RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1b.c -- 2>%t.stderr +//RUN: grep -q "merging failed" %t.stderr + +// The desired behavior in this case is to fail, so other checks are omitted + +_Ptr foo(int, char); diff --git a/clang/test/3C/difftypes1b.c b/clang/test/3C/difftypes1b.c new file mode 100644 index 000000000000..1ce14ed97bc6 --- /dev/null +++ b/clang/test/3C/difftypes1b.c @@ -0,0 +1,7 @@ +//RUN: rm -rf %t* +//RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1a.c -- 2>%t.stderr +//RUN: grep -q "merging failed" %t.stderr + +// The desired behavior in this case is to fail, so other checks are omitted + +int * foo(int, char *); diff --git a/clang/test/3C/difftypes2.c b/clang/test/3C/difftypes2.c new file mode 100644 index 000000000000..e4ebdcd92fc6 --- /dev/null +++ b/clang/test/3C/difftypes2.c @@ -0,0 +1,14 @@ +// RUN: rm -rf %t* +// RUN: not 3c -base-dir=%S -output-dir=%t.checked %s -- 2>%t.stderr +// RUN: grep -q "merging failed" %t.stderr + +// The desired behavior in this case is to fail, so other checks are omitted + +// Test no body vs body + +void foo(char *x); + +void foo(char **y) { + // this is the body +} + diff --git a/clang/test/3C/difftypes3.c b/clang/test/3C/difftypes3.c new file mode 100644 index 000000000000..607606672f82 --- /dev/null +++ b/clang/test/3C/difftypes3.c @@ -0,0 +1,14 @@ +// RUN: rm -rf %t* +// RUN: not 3c -base-dir=%S -output-dir=%t.checked %s -- 2>%t.stderr +// RUN: grep -q "merging failed" %t.stderr + +// The desired behavior in this case is to fail, so other checks are omitted + +// Test body vs no body + +void foo(char **y) { + // this is the body +} + +void foo(char *x); + diff --git a/clang/test/3C/difftypes_xfail1.c b/clang/test/3C/difftypes_xfail1.c deleted file mode 100644 index f672510486af..000000000000 --- a/clang/test/3C/difftypes_xfail1.c +++ /dev/null @@ -1,8 +0,0 @@ -//RUN: rm -rf %t* -//RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes_xfail2.c -- - -// XFAIL: * - -// The desired behavior in this case is to fail, so other checks are omitted - -_Ptr foo(int, char); diff --git a/clang/test/3C/difftypes_xfail2.c b/clang/test/3C/difftypes_xfail2.c deleted file mode 100644 index 56f00b03a46a..000000000000 --- a/clang/test/3C/difftypes_xfail2.c +++ /dev/null @@ -1,8 +0,0 @@ -//RUN: rm -rf %t* -//RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes_xfail1.c -- - -// XFAIL: * - -// The desired behavior in this case is to fail, so other checks are omitted - -int * foo(int, char *); diff --git a/clang/test/3C/multidef_xfail1.c b/clang/test/3C/multidef1a.c similarity index 59% rename from clang/test/3C/multidef_xfail1.c rename to clang/test/3C/multidef1a.c index f3d33ee3f711..4a7de7bef839 100644 --- a/clang/test/3C/multidef_xfail1.c +++ b/clang/test/3C/multidef1a.c @@ -1,7 +1,6 @@ // RUN: rm -rf %t* -// RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/multidef_xfail2.c -- - -// XFAIL: * +// RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/multidef1b.c -- 2>%t.stderr +// RUN: grep "merging failed" %t.stderr // The desired behavior in this case is to fail, so other checks are omitted diff --git a/clang/test/3C/multidef_xfail2.c b/clang/test/3C/multidef1b.c similarity index 58% rename from clang/test/3C/multidef_xfail2.c rename to clang/test/3C/multidef1b.c index fd6ea0e97df1..1f993fc72048 100644 --- a/clang/test/3C/multidef_xfail2.c +++ b/clang/test/3C/multidef1b.c @@ -1,7 +1,6 @@ // RUN: rm -rf %t* -// RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/multidef_xfail1.c -- - -// XFAIL: * +// RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/multidef1a.c -- 2>%t.stderr +// RUN: grep -q "merging failed" %t.stderr // The desired behavior in this case is to fail, so other checks are omitted