Skip to content

293 convert partial #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions clang/include/clang/CConv/ConstraintVariables.h
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,6 @@ class PointerVariableConstraint : public ConstraintVariable {
// pointers.
bool IsZeroWidthArray;

// Was this variable a checked pointer in the input program?
// This is important for two reasons: (1) externs that are checked should be
// kept that way during solving, (2) nothing that was originally checked
// should be modified during rewriting.
bool OriginallyChecked;

public:
// Constructor for when we know a CVars and a type string.
PointerVariableConstraint(CAtoms V, std::string T, std::string Name,
Expand All @@ -290,8 +284,8 @@ class PointerVariableConstraint : public ConstraintVariable {
ConstraintVariable(PointerVariable, "" /*not used*/, Name),
BaseType(T),vars(V),FV(F), ArrPresent(isArr), ItypeStr(is),
partOFFuncPrototype(false), Parent(nullptr),
BoundsAnnotationStr(""), IsGeneric(Generic), IsZeroWidthArray(false),
OriginallyChecked(false) {}
BoundsAnnotationStr(""), IsGeneric(Generic), IsZeroWidthArray(false)
{}

std::string getTy() const { return BaseType; }
bool getArrPresent() const { return ArrPresent; }
Expand All @@ -311,7 +305,9 @@ class PointerVariableConstraint : public ConstraintVariable {

bool getIsGeneric() const { return IsGeneric; }

bool getIsOriginallyChecked() const override { return OriginallyChecked; }
bool getIsOriginallyChecked() const override {
return llvm::any_of(vars, [](Atom *A) { return isa<ConstAtom>(A); });
}

bool solutionEqualTo(Constraints &CS,
const ConstraintVariable *CV) const override;
Expand Down Expand Up @@ -381,7 +377,7 @@ class PointerVariableConstraint : public ConstraintVariable {
// Get the set of constraint variables corresponding to the arguments.
const std::set<ConstraintVariable *> &getArgumentConstraints() const;

ConstraintVariable *getCopy(Constraints &CS) override;
PointerVariableConstraint *getCopy(Constraints &CS) override;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in C++ you can override a method whose return type is T with a method whose return type is S <: T ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure of this myself, but it does seem to be the case. The override annotation will trigger an error if try you an invalid override but, it doesn't complain here.


// Retrieve the atom at the specified index. This function includes special
// handling for generic constraint variables to create deeper pointers as
Expand All @@ -408,10 +404,10 @@ class FunctionVariableConstraint : public ConstraintVariable {
FunctionVariableConstraint(FunctionVariableConstraint *Ot,
Constraints &CS);
// N constraints on the return value of the function.
ConstraintVariable *ReturnVar;
PVConstraint *ReturnVar;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I recall thinking in the past that this was always true and we should refactor.

// A vector of K sets of N constraints on the parameter values, for
// K parameters accepted by the function.
std::vector<ConstraintVariable *> ParamVars;
std::vector<PVConstraint *> ParamVars;
// Storing of parameters in the case of untyped prototypes
std::vector<ParamDeferment> deferredParams;
// File name in which this declaration is found.
Expand All @@ -438,7 +434,7 @@ class FunctionVariableConstraint : public ConstraintVariable {
clang::DeclaratorDecl *D, std::string N,
ProgramInfo &I, const clang::ASTContext &C);

ConstraintVariable *getReturnVar() const {
PVConstraint *getReturnVar() const {
return ReturnVar;
}

Expand All @@ -462,7 +458,7 @@ class FunctionVariableConstraint : public ConstraintVariable {
void brainTransplant(ConstraintVariable *From, ProgramInfo &I) override;
void mergeDeclaration(ConstraintVariable *FromCV, ProgramInfo &I) override;

ConstraintVariable *getParamVar(unsigned i) const {
PVConstraint *getParamVar(unsigned i) const {
assert(i < ParamVars.size());
return ParamVars.at(i);
}
Expand All @@ -488,7 +484,7 @@ class FunctionVariableConstraint : public ConstraintVariable {

void equateArgumentConstraints(ProgramInfo &P) override;

ConstraintVariable *getCopy(Constraints &CS) override;
FunctionVariableConstraint *getCopy(Constraints &CS) override;

bool getIsOriginallyChecked() const override;

Expand Down
66 changes: 37 additions & 29 deletions clang/lib/CConv/ConstraintVariables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ PointerVariableConstraint::
this->Parent = Ot;
this->IsGeneric = Ot->IsGeneric;
this->IsZeroWidthArray = Ot->IsZeroWidthArray;
this->OriginallyChecked = Ot->OriginallyChecked;
// We need not initialize other members.
}

Expand Down Expand Up @@ -217,7 +216,6 @@ PointerVariableConstraint::PointerVariableConstraint(const QualType &QT,
bool IsArr = false;
bool IsIncompleteArr = false;
bool IsTopMost = true;
OriginallyChecked = false;
uint32_t TypeIdx = 0;
std::string Npre = inFunc ? ((*inFunc)+":") : "";
VarAtom::VarKind VK =
Expand All @@ -236,13 +234,12 @@ PointerVariableConstraint::PointerVariableConstraint(const QualType &QT,
break;
}

if (Ty->isCheckedPointerType()) {
OriginallyChecked = true;
if (Ty->isCheckedPointerType() || Ty->isCheckedArrayType()) {
ConstAtom *CAtom = nullptr;
if (Ty->isCheckedPointerNtArrayType()) {
if (Ty->isCheckedPointerNtArrayType() || Ty->isNtCheckedArrayType()) {
// This is an NT array type.
CAtom = CS.getNTArr();
} else if (Ty->isCheckedPointerArrayType()) {
} else if (Ty->isCheckedPointerArrayType() || Ty->isCheckedArrayType()) {
// This is an array type.
CAtom = CS.getArr();
} else if (Ty->isCheckedPointerPtrType()) {
Expand Down Expand Up @@ -412,7 +409,8 @@ std::string PointerVariableConstraint::extractBaseType(DeclaratorDecl *D,
const ASTContext &C) {
std::string BaseTypeStr;
bool FoundBaseTypeInSrc = false;
if (!Ty->getAs<TypedefType>() && D && D->getTypeSourceInfo()) {
if (!QT->isOrContainsCheckedType() && !Ty->getAs<TypedefType>() && D &&
D->getTypeSourceInfo()) {
// Try to extract the type from original source to preserve defines
TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc();
if (isa<FunctionDecl>(D)) {
Expand Down Expand Up @@ -901,7 +899,10 @@ void FunctionVariableConstraint::constrainToWild
}

bool FunctionVariableConstraint::anyChanges(const EnvironmentMap &E) const {
return ReturnVar->anyChanges(E);
return ReturnVar->anyChanges(E) ||
llvm::any_of(ParamVars, [&E](ConstraintVariable *CV) {
return CV->anyChanges(E);
});
}

bool FunctionVariableConstraint::hasWild(const EnvironmentMap &E,
Expand All @@ -919,7 +920,7 @@ bool FunctionVariableConstraint::hasNtArr(const EnvironmentMap &E,
return ReturnVar->hasNtArr(E, AIdx);
}

ConstraintVariable *FunctionVariableConstraint::getCopy(Constraints &CS) {
FVConstraint *FunctionVariableConstraint::getCopy(Constraints &CS) {
return new FVConstraint(this, CS);
}

Expand Down Expand Up @@ -980,13 +981,19 @@ void PointerVariableConstraint::constrainToWild(Constraints &CS,
void PointerVariableConstraint::constrainToWild(Constraints &CS,
const std::string &Rsn,
PersistentSourceLoc *PL) const {
// Constrains the outer pointer level to WILD. Inner pointer levels are
// Find the first VarAtom. All atoms before this are ConstAtoms, so
// constraining them isn't useful;
VarAtom *FirstVA = nullptr;
for (Atom *A : vars)
if (isa<VarAtom>(A)) {
FirstVA = cast<VarAtom>(A);
break;
}

// Constrains the outer VarAtom to WILD. Inner pointer levels are
// implicitly WILD because of implication constraints.
if (!vars.empty()) {
Atom *A = *vars.begin();
if (auto *VA = dyn_cast<VarAtom>(A))
CS.addConstraint(CS.createGeq(VA, CS.getWild(), Rsn, PL, true));
}
if (FirstVA)
CS.addConstraint(CS.createGeq(FirstVA, CS.getWild(), Rsn, PL, true));

if (FV)
FV->constrainToWild(CS, Rsn, PL);
Expand Down Expand Up @@ -1031,29 +1038,30 @@ bool PointerVariableConstraint::anyArgumentIsWild(const EnvironmentMap &E) {
}

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)
return false;

// If it was not checked in the input, then it has changed if it now has a
// checked type.
bool Ret = false;
bool PtrChanged = false;

// Are there any non-WILD pointers?
for (const auto &C : vars) {
const ConstAtom *CS = getSolution(C, E);
assert(CS != nullptr && "Atom should be either const or var");
Ret |= !(isa<WildAtom>(CS));
for (unsigned I = 0; I < vars.size(); I++) {
const Atom *VA = vars[I];
const ConstAtom *CA = getSolution(VA, E);
assert(CA != nullptr && "Atom should be either const or var");
bool OriginallyChecked = isa<ConstAtom>(VA);

// Atom has changed if it was not originally checked, and it did not solve
// to WILD. The pointer has changed if the atom has changed.
bool AtomChanged = !OriginallyChecked && !isa<WildAtom>(CA);
PtrChanged |= AtomChanged;
}

if (FV)
Ret |= FV->anyChanges(E);
PtrChanged |= FV->anyChanges(E);

return Ret;
return PtrChanged;
}

ConstraintVariable *PointerVariableConstraint::getCopy(Constraints &CS) {
PVConstraint *PointerVariableConstraint::getCopy(Constraints &CS) {
return new PointerVariableConstraint(this, CS);
}

Expand Down Expand Up @@ -1267,7 +1275,7 @@ FunctionVariableConstraint::mkString(const EnvironmentMap &E,

Ret = Ret + Ss.str() + ")";
} else {
Ret = Ret + ")";
Ret = Ret + "void)";
}

return Ret;
Expand Down
6 changes: 2 additions & 4 deletions clang/lib/CConv/DeclRewriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -498,9 +498,7 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
// Get rewritten parameter variable declarations
std::vector<std::string> ParmStrs;
for (unsigned I = 0; I < Defnc->numParams(); ++I) {
auto *Defn = dyn_cast<PVConstraint>(Defnc->getParamVar(I));
assert(Defn);

PVConstraint *Defn = Defnc->getParamVar(I);
ParmVarDecl *PVDecl = Definition->getParamDecl(I);
std::string Type, IType;
this->buildDeclVar(Defn, PVDecl, Type, IType, RewriteParams, RewriteReturn);
Expand All @@ -516,7 +514,7 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
}

// Get rewritten return variable
auto *Defn = dyn_cast<PVConstraint>(Defnc->getReturnVar());
PVConstraint *Defn = Defnc->getReturnVar();
std::string ReturnVar, ItypeStr;
this->buildDeclVar(Defn, FD, ReturnVar, ItypeStr, RewriteParams, RewriteReturn);

Expand Down
15 changes: 7 additions & 8 deletions clang/lib/CConv/ProgramInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,15 +340,14 @@ bool ProgramInfo::link() {
// If there was a checked type on a variable in the input program, it
// should stay that way. Otherwise, we shouldn't be adding a checked type
// to an extern function.
if (!G->getReturnVar()->getIsOriginallyChecked()) {
std::string Rsn = "Return value of an external function:" + FuncName;
std::string Rsn =
"Unchecked pointer in parameter or return of external function " +
FuncName;
if (!G->getReturnVar()->getIsGeneric())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this exception? I guess if it's generic, we are assuming the underlying pointer, even if void *, should not be constrained to WILD?

G->getReturnVar()->constrainToWild(CS, Rsn);
}

std::string rsn = "Inner pointer of a parameter to external function.";
for (unsigned i = 0; i < G->numParams(); i++)
if (!G->getParamVar(i)->getIsOriginallyChecked())
G->getParamVar(i)->constrainToWild(CS, rsn);
for (unsigned I = 0; I < G->numParams(); I++)
if (!G->getParamVar(I)->getIsGeneric())
G->getParamVar(I)->constrainToWild(CS, Rsn);
}
}

Expand Down
4 changes: 2 additions & 2 deletions clang/test/CheckedCRewriter/arrboth.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// 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/arrboth.checked.c -- | diff %S/arrboth.checked.c -
// RUN: cconv-standalone -alltypes %S/arrboth.checked.c -- | count 0
// RUN: rm %S/arrboth.checked.c


Expand Down Expand Up @@ -108,7 +108,7 @@ x = (int *) 5;
//CHECK: x = (int *) 5;
int *z = calloc(5, sizeof(int));
//CHECK_NOALL: int *z = calloc<int>(5, sizeof(int));
//CHECK_ALL: _Array_ptr<int> z = calloc<int>(5, sizeof(int));
//CHECK_ALL: _Array_ptr<int> z = calloc<int>(5, sizeof(int));
int i, fac;
int *p;
//CHECK_NOALL: int *p;
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CheckedCRewriter/arrbothmulti1.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
// RUN: FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" --input-file %S/arrbothmulti1.checkedALL.c %s
// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=checked %S/arrbothmulti2.c %s
// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=convert_again %S/arrbothmulti1.checked.c %S/arrbothmulti2.checked.c
// RUN: diff %S/arrbothmulti1.checked.convert_again.c %S/arrbothmulti1.checked.c
// RUN: diff %S/arrbothmulti2.checked.convert_again.c %S/arrbothmulti2.checked.c
// RUN: test ! -f %S/arrbothmulti1.checked.convert_again.c
// RUN: test ! -f %S/arrbothmulti2.checked.convert_again.c
// RUN: rm %S/arrbothmulti1.checkedALL.c %S/arrbothmulti2.checkedALL.c
// RUN: rm %S/arrbothmulti1.checkedNOALL.c %S/arrbothmulti2.checkedNOALL.c
// RUN: rm %S/arrbothmulti1.checked.c %S/arrbothmulti2.checked.c %S/arrbothmulti1.checked.convert_again.c %S/arrbothmulti2.checked.convert_again.c
// RUN: rm %S/arrbothmulti1.checked.c %S/arrbothmulti2.checked.c


/*********************************************************************************/
Expand Down
8 changes: 4 additions & 4 deletions clang/test/CheckedCRewriter/arrbothmulti2.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
// RUN: FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" --input-file %S/arrbothmulti2.checkedALL2.c %s
// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=checked2 %S/arrbothmulti1.c %s
// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=convert_again %S/arrbothmulti1.checked2.c %S/arrbothmulti2.checked2.c
// RUN: diff %S/arrbothmulti1.checked2.convert_again.c %S/arrbothmulti1.checked2.c
// RUN: diff %S/arrbothmulti2.checked2.convert_again.c %S/arrbothmulti2.checked2.c
// RUN: test ! -f %S/arrbothmulti1.checked2.convert_again.c
// RUN: test ! -f %S/arrbothmulti2.checked2.convert_again.c
// RUN: rm %S/arrbothmulti1.checkedALL2.c %S/arrbothmulti2.checkedALL2.c
// RUN: rm %S/arrbothmulti1.checkedNOALL2.c %S/arrbothmulti2.checkedNOALL2.c
// RUN: rm %S/arrbothmulti1.checked2.c %S/arrbothmulti2.checked2.c %S/arrbothmulti1.checked2.convert_again.c %S/arrbothmulti2.checked2.convert_again.c
// RUN: rm %S/arrbothmulti1.checked2.c %S/arrbothmulti2.checked2.c


/*********************************************************************************/
Expand Down Expand Up @@ -116,7 +116,7 @@ x = (int *) 5;
//CHECK: x = (int *) 5;
int *z = calloc(5, sizeof(int));
//CHECK_NOALL: int *z = calloc<int>(5, sizeof(int));
//CHECK_ALL: _Array_ptr<int> z = calloc<int>(5, sizeof(int));
//CHECK_ALL: _Array_ptr<int> z = calloc<int>(5, sizeof(int));
int i, fac;
int *p;
//CHECK_NOALL: int *p;
Expand Down
4 changes: 2 additions & 2 deletions clang/test/CheckedCRewriter/arrcallee.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// 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/arrcallee.checked.c -- | diff %S/arrcallee.checked.c -
// RUN: cconv-standalone -alltypes %S/arrcallee.checked.c -- | count 0
// RUN: rm %S/arrcallee.checked.c


Expand Down Expand Up @@ -108,7 +108,7 @@ x = (int *) 5;
//CHECK: x = (int *) 5;
int *z = calloc(5, sizeof(int));
//CHECK_NOALL: int *z = calloc<int>(5, sizeof(int));
//CHECK_ALL: _Array_ptr<int> z = calloc<int>(5, sizeof(int));
//CHECK_ALL: _Array_ptr<int> z = calloc<int>(5, sizeof(int));
int i, fac;
int *p;
//CHECK_NOALL: int *p;
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CheckedCRewriter/arrcalleemulti1.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
// RUN: FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" --input-file %S/arrcalleemulti1.checkedALL.c %s
// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=checked %S/arrcalleemulti2.c %s
// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=convert_again %S/arrcalleemulti1.checked.c %S/arrcalleemulti2.checked.c
// RUN: diff %S/arrcalleemulti1.checked.convert_again.c %S/arrcalleemulti1.checked.c
// RUN: diff %S/arrcalleemulti2.checked.convert_again.c %S/arrcalleemulti2.checked.c
// RUN: test ! -f %S/arrcalleemulti1.checked.convert_again.c
// RUN: test ! -f %S/arrcalleemulti2.checked.convert_again.c
// RUN: rm %S/arrcalleemulti1.checkedALL.c %S/arrcalleemulti2.checkedALL.c
// RUN: rm %S/arrcalleemulti1.checkedNOALL.c %S/arrcalleemulti2.checkedNOALL.c
// RUN: rm %S/arrcalleemulti1.checked.c %S/arrcalleemulti2.checked.c %S/arrcalleemulti1.checked.convert_again.c %S/arrcalleemulti2.checked.convert_again.c
// RUN: rm %S/arrcalleemulti1.checked.c %S/arrcalleemulti2.checked.c


/*********************************************************************************/
Expand Down
8 changes: 4 additions & 4 deletions clang/test/CheckedCRewriter/arrcalleemulti2.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
// RUN: FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" --input-file %S/arrcalleemulti2.checkedALL2.c %s
// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=checked2 %S/arrcalleemulti1.c %s
// RUN: cconv-standalone -base-dir=%S -alltypes -output-postfix=convert_again %S/arrcalleemulti1.checked2.c %S/arrcalleemulti2.checked2.c
// RUN: diff %S/arrcalleemulti1.checked2.convert_again.c %S/arrcalleemulti1.checked2.c
// RUN: diff %S/arrcalleemulti2.checked2.convert_again.c %S/arrcalleemulti2.checked2.c
// RUN: test ! -f %S/arrcalleemulti1.checked2.convert_again.c
// RUN: test ! -f %S/arrcalleemulti2.checked2.convert_again.c
// RUN: rm %S/arrcalleemulti1.checkedALL2.c %S/arrcalleemulti2.checkedALL2.c
// RUN: rm %S/arrcalleemulti1.checkedNOALL2.c %S/arrcalleemulti2.checkedNOALL2.c
// RUN: rm %S/arrcalleemulti1.checked2.c %S/arrcalleemulti2.checked2.c %S/arrcalleemulti1.checked2.convert_again.c %S/arrcalleemulti2.checked2.convert_again.c
// RUN: rm %S/arrcalleemulti1.checked2.c %S/arrcalleemulti2.checked2.c


/*********************************************************************************/
Expand Down Expand Up @@ -116,7 +116,7 @@ x = (int *) 5;
//CHECK: x = (int *) 5;
int *z = calloc(5, sizeof(int));
//CHECK_NOALL: int *z = calloc<int>(5, sizeof(int));
//CHECK_ALL: _Array_ptr<int> z = calloc<int>(5, sizeof(int));
//CHECK_ALL: _Array_ptr<int> z = calloc<int>(5, sizeof(int));
int i, fac;
int *p;
//CHECK_NOALL: int *p;
Expand Down
Loading