Skip to content

Commit 906138e

Browse files
Aaron Elinemattmccutchen-ccijohn-h-kastner
authored
Typedef implementation improvements (#408)
* Fix #373 * Use a single constraint variable to represent each typedef * Fix rewriting for array typedefs * Fix rewriting for function typedefs Co-authored-by: Matt McCutchen (Correct Computation) <[email protected]> Co-authored-by: John Kastner <[email protected]>
1 parent cc7eec2 commit 906138e

File tree

12 files changed

+116
-54
lines changed

12 files changed

+116
-54
lines changed

‎clang/include/clang/3C/ConstraintVariables.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ class PointerVariableConstraint : public ConstraintVariable {
316316
bool hasSomeSizedArr() const;
317317

318318
bool isTypedef(void);
319-
void setTypedef(TypedefNameDecl *TypedefType, std::string);
319+
void setTypedef(TypedefNameDecl *T, std::string S);
320320

321321
// Return true if this constraint had an itype in the original source code.
322322
bool srcHasItype() const override {
@@ -387,6 +387,8 @@ class PointerVariableConstraint : public ConstraintVariable {
387387
return S->getKind() == PointerVariable;
388388
}
389389

390+
std::string gatherQualStrings(void) const;
391+
390392
std::string mkString(const EnvironmentMap &E, bool EmitName = true,
391393
bool ForItype = false, bool EmitPointee = false,
392394
bool UnmaskTypedef = false) const override;

‎clang/include/clang/3C/ProgramInfo.h

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ class ProgramVariableAdder {
8181

8282
virtual bool seenTypedef(PersistentSourceLoc PSL) = 0;
8383

84-
virtual void addTypedef(PersistentSourceLoc PSL, bool ShouldCheck) = 0;
85-
84+
virtual void addTypedef(PersistentSourceLoc PSL, bool CanRewriteDef,
85+
TypedefDecl *TD, ASTContext &C) = 0;
8686

8787
protected:
8888
virtual AVarBoundsInfo &getABoundsInfo() = 0;
@@ -170,25 +170,23 @@ class ProgramInfo : public ProgramVariableAdder {
170170
void unifyIfTypedef(const clang::Type *, clang::ASTContext &,
171171
clang::DeclaratorDecl *, PVConstraint *);
172172

173-
std::pair<CVarSet, bool> lookupTypedef(PersistentSourceLoc PSL);
173+
CVarOption lookupTypedef(PersistentSourceLoc PSL);
174174

175175
bool seenTypedef(PersistentSourceLoc PSL);
176176

177-
void addTypedef(PersistentSourceLoc PSL, bool ShouldCheck);
177+
void addTypedef(PersistentSourceLoc PSL, bool CanRewriteDef, TypedefDecl *TD,
178+
ASTContext& C);
178179

179180
private:
180181
// List of constraint variables for declarations, indexed by their location in
181182
// the source. This information persists across invocations of the constraint
182183
// analysis from compilation unit to compilation unit.
183184
VariableMap Variables;
184185

185-
// Map storing constraint information for typedefed types,
186-
// The set contains all the constraint variables that also use this typedef.
187-
// TODO this could be replaced w/ a single CVar.
188-
// The bool informs the rewriter whether or not this typedef should be
189-
// rewritten. It will be false for typedefs we don't support rewritting,
190-
// such as typedefs that are pointers to anonymous structs.
191-
std::map<PersistentSourceLoc, std::pair<CVarSet, bool>> TypedefVars;
186+
// Map storing constraint information for typedefed types
187+
// The set contains all the constraint variables that also use this tyepdef
188+
// rewritten.
189+
std::map<PersistentSourceLoc, CVarOption> TypedefVars;
192190

193191
// Map with the same purpose as the Variables map, this stores constraint
194192
// variables for non-declaration expressions.

‎clang/lib/3C/ConstraintBuilder.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,8 @@ class VariableAdderVisitor : public RecursiveASTVisitor<VariableAdderVisitor> {
562562
if (!VarAdder.seenTypedef(PSL))
563563
// Add this typedef to the program info, if it contains a ptr to
564564
// an anonymous struct we mark as not being rewritable
565-
VarAdder.addTypedef(PSL, !PtrToStructDef::containsPtrToStructDef(TD));
566-
565+
VarAdder.addTypedef(PSL, !PtrToStructDef::containsPtrToStructDef(TD),
566+
TD, *Context);
567567
return true;
568568
}
569569

‎clang/lib/3C/ConstraintVariables.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -654,12 +654,18 @@ void PointerVariableConstraint::setTypedef(TypedefNameDecl *T, std::string S) {
654654
// variables and potentially nested function pointer declaration. Produces a
655655
// string that can be replaced in the source code.
656656

657+
std::string PointerVariableConstraint::gatherQualStrings(void) const {
658+
std::ostringstream S;
659+
getQualString(0, S);
660+
return S.str();
661+
}
662+
657663
std::string PointerVariableConstraint::mkString(const EnvironmentMap &E,
658664
bool EmitName, bool ForItype,
659665
bool EmitPointee,
660666
bool UnmaskTypedef) const {
661667
if (IsTypedef && !UnmaskTypedef) {
662-
return TypedefString +
668+
return gatherQualStrings() + TypedefString +
663669
(EmitName && getName() != RETVAR ? (" " + getName()) : " ");
664670
}
665671

@@ -814,14 +820,16 @@ std::string PointerVariableConstraint::mkString(const EnvironmentMap &E,
814820
EndStrs.push_front(" " + getName());
815821
}
816822

817-
if (EmittedBase == false) {
823+
if (!EmittedBase) {
818824
// If we have a FV pointer, then our "base" type is a function pointer.
819825
// type.
820826
if (FV) {
821827
Ss << FV->mkString(E);
822828
} else if (TypedefLevelInfo.HasTypedef) {
829+
std::ostringstream Buf;
830+
getQualString(TypedefLevelInfo.TypedefLevel, Buf);
823831
auto Name = TypedefLevelInfo.TypedefName;
824-
Ss << Name;
832+
Ss << Buf.str() << Name;
825833
} else {
826834
Ss << BaseType;
827835
}
@@ -1407,6 +1415,9 @@ std::string FunctionVariableConstraint::mkString(const EnvironmentMap &E,
14071415
bool UnmaskTypedef) const {
14081416
std::string Ret = ReturnVar.mkTypeStr(E);
14091417
std::string Itype = ReturnVar.mkItypeStr(E);
1418+
// This is done to rewrite the typedef of a function proto
1419+
if (UnmaskTypedef && EmitName)
1420+
Ret += Name;
14101421
Ret = Ret + "(";
14111422
std::vector<std::string> ParmStrs;
14121423
for (const auto &I : this->ParamVars)
@@ -1420,10 +1431,8 @@ std::string FunctionVariableConstraint::mkString(const EnvironmentMap &E,
14201431
Ss << ParmStrs.back();
14211432

14221433
Ret = Ret + Ss.str() + ")";
1423-
} else {
1434+
} else
14241435
Ret = Ret + "void)";
1425-
}
1426-
14271436
return Ret + Itype;
14281437
}
14291438

‎clang/lib/3C/DeclRewriter.cpp

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,19 @@ void DeclRewriter::rewriteDecls(ASTContext &Context, ProgramInfo &Info,
5353
SVI.TraverseDecl(D);
5454
if (const auto &TD = dyn_cast<TypedefDecl>(D)) {
5555
auto PSL = PersistentSourceLoc::mkPSL(TD, Context);
56-
// Don't rewrite base types like int.
57-
if (!TD->getUnderlyingType()->isBuiltinType()) {
58-
const auto Pair = Info.lookupTypedef(PSL);
59-
const auto VSet = Pair.first;
60-
if (!VSet.empty()) { // We ignore typedefs that are never used.
61-
const auto Var = VSet.begin();
56+
if (!TD->getUnderlyingType()->isBuiltinType()) { // Don't rewrite base types like int
57+
const auto O = Info.lookupTypedef(PSL);
58+
if (O.hasValue()) {
59+
const auto &Var = O.getValue();
6260
const auto &Env = Info.getConstraints().getVariables();
63-
if ((*Var)->anyChanges(Env)) {
64-
std::string NewTy =
65-
getStorageQualifierString(D) +
66-
(*Var)->mkString(Info.getConstraints().getVariables(), false,
67-
false, false, true) +
68-
" " + TD->getNameAsString();
69-
RewriteThese.insert(new TypedefDeclReplacement(TD, nullptr, NewTy));
70-
}
61+
if (Var.anyChanges(Env)) {
62+
std::string newTy =
63+
getStorageQualifierString(D) +
64+
Var.mkString(Info.getConstraints().getVariables(), true,
65+
false, false, true);
66+
RewriteThese.insert(
67+
new TypedefDeclReplacement(TD, nullptr, newTy));
68+
}
7169
}
7270
}
7371
}
@@ -208,8 +206,8 @@ void DeclRewriter::rewriteParmVarDecl(ParmVarDeclReplacement *N) {
208206
}
209207
}
210208

211-
void DeclRewriter::rewriteTypedefDecl(TypedefDeclReplacement *TDR,
212-
RSet &ToRewrite) {
209+
210+
void DeclRewriter::rewriteTypedefDecl(TypedefDeclReplacement *TDR, RSet &ToRewrite) {
213211
rewriteSingleDecl(TDR, ToRewrite);
214212
}
215213

‎clang/lib/3C/ProgramInfo.cpp

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -695,17 +695,15 @@ void ProgramInfo::addVariable(clang::DeclaratorDecl *D,
695695
Variables[PLoc] = NewCV;
696696
}
697697

698-
void ProgramInfo::unifyIfTypedef(const Type *Ty, ASTContext &Context,
699-
DeclaratorDecl *Decl, PVConstraint *P) {
700-
if (const auto *const TDT = dyn_cast<TypedefType>(Ty)) {
701-
auto *Decl = TDT->getDecl();
702-
auto PSL = PersistentSourceLoc::mkPSL(Decl, Context);
703-
auto &Pair = TypedefVars[PSL];
704-
CVarSet &Bounds = Pair.first;
705-
if (Pair.second) {
706-
P->setTypedef(Decl, Decl->getNameAsString());
698+
void ProgramInfo::unifyIfTypedef(const Type* Ty, ASTContext& Context, DeclaratorDecl* Decl, PVConstraint* P) {
699+
if (const auto* TDT = dyn_cast<TypedefType>(Ty)) {
700+
auto* TDecl = TDT->getDecl();
701+
auto PSL = PersistentSourceLoc::mkPSL(TDecl, Context);
702+
auto O = lookupTypedef(PSL);
703+
if (O.hasValue()) {
704+
auto *Bounds = &O.getValue();
705+
P->setTypedef(TDecl, TDecl->getNameAsString());
707706
constrainConsVarGeq(P, Bounds, CS, &PSL, Same_to_Same, true, this);
708-
Bounds.insert(P);
709707
}
710708
}
711709
}
@@ -1092,15 +1090,31 @@ ProgramInfo::getTypeParamBindings(CallExpr *CE, ASTContext *C) const {
10921090
return TypeParamBindings.at(PSL);
10931091
}
10941092

1095-
std::pair<CVarSet, bool> ProgramInfo::lookupTypedef(PersistentSourceLoc PSL) {
1093+
CVarOption ProgramInfo::lookupTypedef(PersistentSourceLoc PSL) {
10961094
return TypedefVars[PSL];
10971095
}
10981096

10991097
bool ProgramInfo::seenTypedef(PersistentSourceLoc PSL) {
11001098
return TypedefVars.count(PSL) != 0;
11011099
}
11021100

1103-
void ProgramInfo::addTypedef(PersistentSourceLoc PSL, bool ShouldCheck) {
1104-
CVarSet Empty;
1105-
TypedefVars[PSL] = make_pair(Empty, ShouldCheck);
1101+
void ProgramInfo::addTypedef(PersistentSourceLoc PSL, bool CanRewriteDef,
1102+
TypedefDecl* TD, ASTContext &C) {
1103+
auto Name = TD->getNameAsString();
1104+
ConstraintVariable* V = nullptr;
1105+
const auto T = TD->getUnderlyingType();
1106+
if (isa<clang::FunctionProtoType>(T) || isa<clang::FunctionNoProtoType>(T))
1107+
V = new FunctionVariableConstraint(T.getTypePtr(),
1108+
nullptr, Name, *this, C);
1109+
else
1110+
V = new PointerVariableConstraint(T, nullptr, Name, *this, C);
1111+
auto *const Rsn =
1112+
!CanRewriteDef ?
1113+
"Unable to rewrite a typedef with multiple names"
1114+
: "Declaration in non-writable file";
1115+
if (!(CanRewriteDef && canWrite(PSL.getFileName()))) {
1116+
V->constrainToWild(this->getConstraints(), Rsn, &PSL);
1117+
}
1118+
constrainWildIfMacro(V, TD->getLocation(), &PSL);
1119+
this->TypedefVars[PSL] = {*V};
11061120
}

‎clang/test/3C/base_subdir/canwrite_constraints.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,13 @@ int *bar(int *q) {
3131
foo(q);
3232
return foo_var;
3333
}
34+
35+
36+
int gar(intptr a) {
37+
int* b = a;
38+
//CHECK_LOWER: int* b = a;
39+
//CHECK_HIGHER: _Ptr<int> b = a;
40+
return *b;
41+
}
42+
43+

‎clang/test/3C/basic_checks.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ void mut_pa(PA p) {
3939
p->a = 0;
4040
p->b = 1;
4141
}
42-
//CHECK: void mut_pa(_Ptr<struct _A> p) {
42+
//CHECK: void mut_pa(PA p) {
4343

4444
void pa_driver(void) {
4545
A a = {0};
@@ -49,7 +49,7 @@ void pa_driver(void) {
4949
}
5050
//CHECK: void pa_driver(void) {
5151
//CHECK-NEXT: A a = {0};
52-
//CHECK-NEXT: _Ptr<struct _A> b = &a;
52+
//CHECK-NEXT: PA b = &a;
5353

5454
int *id(int *a) {
5555
return a;

‎clang/test/3C/canwrite_constraints.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ void atol_like() {
2929
inline void no_op() {}
3030
// CHECK_HIGHER: inline void no_op() _Checked {}
3131

32+
// In the lower case, this should stay wild
33+
// In the higher case, this should solve to checked
34+
// expected-warning@+1 {{Declaration in non-writable file}}
35+
typedef int* intptr;
36+
// CHECK_HIGHER: typedef _Ptr<int> intptr;
3237
// Test the unwritable cast internal warning
3338
// (https://github.com/correctcomputation/checkedc-clang/issues/454) using the
3439
// known bug with itypes and function pointers

‎clang/test/3C/root_cause.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,8 @@ void test1() {
4141
extern int *glob; // expected-warning {{External global variable glob has no definition}}
4242

4343
void (*f)(void *); // expected-warning {{Default void* type}}
44+
45+
typedef struct {
46+
int x;
47+
float f;
48+
} A, *PA; // expected-warning {{Unable to rewrite a typedef with multiple names}}

0 commit comments

Comments
 (0)