Skip to content

Commit 671e01b

Browse files
john-h-kastnerjohn-h-kastner
committed
Fix #205
FunctionDecl replacment now only replaces the return/parameters when they have changed. This alows macros to appear in these locations as long as they don't require rewritting.
1 parent 47dd9c9 commit 671e01b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+242
-148
lines changed

‎clang/include/clang/CConv/RewriteUtils.h

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class DeclReplacement {
2828

2929
std::string getReplacement() const { return Replacement; }
3030

31-
virtual SourceRange getSourceRange(SourceManager &SR) const {
31+
virtual SourceRange getSourceRange() const {
3232
return getDecl()->getSourceRange();
3333
}
3434

@@ -85,24 +85,56 @@ class FunctionDeclReplacement :
8585
public DeclReplacementTempl<FunctionDecl,
8686
DeclReplacement::DRK_FunctionDecl> {
8787
public:
88-
explicit FunctionDeclReplacement(FunctionDecl *D, std::string R, bool Full)
89-
: DeclReplacementTempl(D, nullptr, R), FullDecl(Full) {}
90-
91-
SourceRange getSourceRange(SourceManager &SM) const override {
92-
if (FullDecl) {
93-
SourceRange Range = Decl->getSourceRange();
94-
Range.setEnd(getFunctionDeclarationEnd(Decl, SM));
95-
return Range;
96-
} else
97-
return Decl->getReturnTypeSourceRange();
88+
explicit FunctionDeclReplacement(FunctionDecl *D, std::string R, bool Return,
89+
bool Params)
90+
: DeclReplacementTempl(D, nullptr, R), RewriteReturn(Return),
91+
RewriteParams(Params) {
92+
assert("Doesn't make sense to rewrite nothing!"
93+
&& (RewriteReturn || RewriteParams));
9894
}
9995

100-
bool isFullDecl() const {
101-
return FullDecl;
96+
SourceRange getSourceRange() const override {
97+
FunctionTypeLoc TypeLoc =
98+
Decl->getTypeSourceInfo()->getTypeLoc().getAs<clang::FunctionTypeLoc>();
99+
100+
// Function pointer are funky, and require special handling to rewrite the
101+
// return type.
102+
if (Decl->getReturnType()->isFunctionPointerType()){
103+
if (RewriteParams && RewriteReturn) {
104+
SourceLocation End = TypeLoc.getReturnLoc().getNextTypeLoc()
105+
.getAs<clang::ParenTypeLoc>().getInnerLoc()
106+
.getAs<clang::FunctionTypeLoc>()
107+
.getRParenLoc();
108+
return SourceRange(Decl->getBeginLoc(), End);
109+
}
110+
assert("RewriteReturn implies RewriteParams for function pointer return."
111+
&& !RewriteReturn);
112+
// Fall through to standard handling when only rewriting param decls
113+
}
114+
115+
// If rewriting the return, then the range starts at the begining of the
116+
// decl. Otherwise, skip to the left parenthesis of parameters.
117+
SourceLocation Begin = RewriteReturn ?
118+
Decl->getBeginLoc() :
119+
TypeLoc.getLParenLoc();
120+
121+
// If rewriting Parameters, stop at the right parenthesis of the parameters.
122+
// Otherwise, stop after the return type.
123+
SourceLocation End = RewriteParams ?
124+
TypeLoc.getRParenLoc() :
125+
Decl->getReturnTypeSourceRange().getEnd();
126+
127+
assert("Invalid FunctionDeclReplacement SourceRange!"
128+
&& Begin.isValid() && End.isValid());
129+
130+
return SourceRange(Begin, End);
102131
}
132+
103133
private:
104134
// This determines if the full declaration or the return will be replaced.
105-
bool FullDecl;
135+
bool RewriteReturn;
136+
137+
bool RewriteParams;
106138
};
107139

108140
// Compare two DeclReplacement values. The algorithm for comparing them relates

‎clang/lib/CConv/DeclRewriter.cpp

Lines changed: 62 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,8 @@ void DeclRewriter::rewriteDecls(ASTContext &Context, ProgramInfo &Info,
103103
// If this function already has a modified signature? and it is not
104104
// visited by our cast placement visitor then rewrite it.
105105
std::string NewSig = NewFuncSig[FV->getName()];
106-
RewriteThese.insert(new FunctionDeclReplacement(FD, NewSig, true));
106+
RewriteThese.insert(new FunctionDeclReplacement(FD, NewSig, true,
107+
true));
107108
}
108109
}
109110
}
@@ -335,9 +336,23 @@ void DeclRewriter::rewriteFunctionDecl(FunctionDeclReplacement *N) {
335336
// Additionally, a source range can be (mis) identified as
336337
// spanning multiple files. We don't know how to re-write that,
337338
// so don't.
338-
SourceRange SR = N->getSourceRange(A.getSourceManager());
339-
if (canRewrite(R, SR))
339+
SourceRange SR = N->getSourceRange();
340+
if (canRewrite(R, SR)) {
340341
R.ReplaceText(SR, N->getReplacement());
342+
} else {
343+
SourceRange Possible(R.getSourceMgr().getExpansionLoc(SR.getBegin()),
344+
SR.getEnd());
345+
if (canRewrite(R, Possible)) {
346+
R.ReplaceText(Possible, N->getReplacement());
347+
} else if (Verbose) {
348+
errs() << "Don't know how to re-write FunctionDecl\n";
349+
N->getDecl()->dump();
350+
errs() << "at\n";
351+
if (N->getStatement())
352+
N->getStatement()->dump();
353+
errs() << "with " << N->getReplacement() << "\n";
354+
}
355+
}
341356
}
342357

343358
bool DeclRewriter::areDeclarationsOnSameLine(DeclReplacement *N1,
@@ -457,10 +472,10 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
457472
if (!Defnc->hasBody())
458473
return true;
459474

460-
// DidAny tracks if we have made any changes to this function declaration.
461-
// If no changes are made, then there is no need to rewrite anything, and the
462-
// declaration is not added to RewriteThese.
463-
bool DidAny = false;
475+
// DidAnyParams tracks if we have made any changes to the parameters for this
476+
// declarations. If no changes are made, then there is no need to rewrite the
477+
// parameter declarations.
478+
bool DidAnyParams = false;
464479

465480
// Get rewritten parameter variable declarations
466481
std::vector<std::string> ParmStrs;
@@ -471,7 +486,7 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
471486
if (isAValidPVConstraint(Defn) && Defn->anyChanges(CS.getVariables())) {
472487
// This means Defn has a checked type, so we should rewrite to use this
473488
// type with an itype if applicable.
474-
DidAny = true;
489+
DidAnyParams = true;
475490

476491
if (Defn->hasItype() || !Defn->anyArgumentIsWild(CS.getVariables())) {
477492
// If the definition already has itype or there are no WILD arguments.
@@ -505,16 +520,29 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
505520
}
506521
}
507522

523+
524+
if (Defnc->numParams() == 0) {
525+
ParmStrs.push_back("void");
526+
QualType ReturnTy = FD->getReturnType();
527+
QualType Ty = FD->getType();
528+
if (!Ty->isFunctionProtoType() && ReturnTy->isPointerType())
529+
DidAnyParams = true;
530+
}
531+
508532
// Get rewritten return variable
509533
auto *Defn = dyn_cast<PVConstraint>(Defnc->getReturnVar());
510534

511535
std::string ReturnVar = "";
512536
std::string ItypeStr = "";
513537

538+
// Does the same job as DidAnyParams, but with respect to the return value. If
539+
// the return does not change, there is no need to rewrite it.
540+
bool DidAnyReturn = false;
541+
514542
// Insert a bounds safe interface for the return.
515543
if (isAValidPVConstraint(Defn) && Defn->anyChanges(CS.getVariables())) {
516544
// This means we can infer that the return type is a checked type.
517-
DidAny = true;
545+
DidAnyReturn = true;
518546
// If the definition has itype or there is no argument which is WILD?
519547
if (Defn->hasItype() || !Defn->anyArgumentIsWild(CS.getVariables())) {
520548
// Just get the checked itype
@@ -526,6 +554,10 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
526554
Defn->mkString(Info.getConstraints().getVariables(), false, true);
527555
ReturnVar = Defn->getRewritableOriginalTy();
528556
ItypeStr = " : itype(" + Itype + ")";
557+
558+
// A small hack here. The inserted itype comes after param declarations,
559+
// so we have to rewrite the parameters if we want to insert an itype.
560+
DidAnyParams = true;
529561
}
530562
} else {
531563
// This means inside the function, the return value is WILD so the return
@@ -535,37 +567,43 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
535567
ItypeStr = getExistingIType(Defn);
536568
}
537569

570+
// If the return is a function pointer, we need to rewrite the whole
571+
// declaration even if no actual changes were made to the parameters. It could
572+
// probably be done better, but getting the correct source locations is
573+
// painful.
574+
if (FD->getReturnType()->isFunctionPointerType() && DidAnyReturn)
575+
DidAnyParams = true;
576+
538577
// Combine parameter and return variables rewritings into a single rewriting
539578
// for the entire function declaration.
540-
std::string NewSig =
541-
getStorageQualifierString(Definition) + ReturnVar + Defnc->getName()
542-
+ "(";
543-
if (!ParmStrs.empty()) {
579+
std::string NewSig = "";
580+
if (DidAnyReturn)
581+
NewSig = getStorageQualifierString(Definition) + ReturnVar;
582+
583+
if (DidAnyReturn && DidAnyParams)
584+
NewSig += Defnc->getName();
585+
586+
if (DidAnyParams && !ParmStrs.empty()) {
544587
// Gather individual parameter strings into a single buffer
545588
std::ostringstream ConcatParamStr;
546589
copy(ParmStrs.begin(), ParmStrs.end() - 1,
547590
std::ostream_iterator<std::string>(ConcatParamStr, ", "));
548591
ConcatParamStr << ParmStrs.back();
549592

550-
NewSig = NewSig + ConcatParamStr.str();
593+
NewSig += "(" + ConcatParamStr.str();
551594
// Add varargs.
552595
if (functionHasVarArgs(Definition))
553-
NewSig = NewSig + ", ...";
554-
NewSig = NewSig + ")";
555-
} else {
556-
NewSig = NewSig + "void)";
557-
QualType ReturnTy = FD->getReturnType();
558-
QualType Ty = FD->getType();
559-
if (!Ty->isFunctionProtoType() && ReturnTy->isPointerType())
560-
DidAny = true;
596+
NewSig += ", ...";
597+
NewSig += ")";
561598
}
562599
if (!ItypeStr.empty())
563600
NewSig = NewSig + ItypeStr;
564601

565602
// Add new declarations to RewriteThese if it has changed
566-
if (DidAny) {
603+
if (DidAnyReturn || DidAnyParams) {
567604
for (auto *const RD : Definition->redecls())
568-
RewriteThese.insert(new FunctionDeclReplacement(RD, NewSig, true));
605+
RewriteThese.insert(new FunctionDeclReplacement(RD, NewSig, DidAnyReturn,
606+
DidAnyParams));
569607
// Save the modified function signature.
570608
if(FD->isStatic()) {
571609
auto FileName = PersistentSourceLoc::mkPSL(FD, *Context).getFileName();

‎clang/lib/CConv/RewriteUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ SourceLocation DComp::getDeclBegin(DeclReplacement *D) const {
3131
}
3232

3333
SourceRange DComp::getReplacementSourceRange(DeclReplacement *D) const {
34-
SourceRange Range = D->getSourceRange(SM);
34+
SourceRange Range = D->getSourceRange();
3535

3636
// Also take into account whether or not there is a multi-statement
3737
// decl, because the generated ranges will overlap.

‎clang/test/CheckedCRewriter/b11_calleestructnp.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ struct np *sus(struct p x, struct p y) {
4545
}
4646

4747
struct np *foo() {
48-
//CHECK: struct np * foo(void) {
48+
//CHECK: struct np *foo(void) {
4949
struct p x, y;
5050
x.x = 1;
5151
x.y = 2;
@@ -57,7 +57,7 @@ struct np *foo() {
5757
}
5858

5959
struct np *bar() {
60-
//CHECK: struct np * bar(void) {
60+
//CHECK: struct np *bar(void) {
6161
struct p x, y;
6262
x.x = 1;
6363
x.y = 2;

‎clang/test/CheckedCRewriter/b12_callerstructnp.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,20 @@ struct np *sus(struct p x, struct p y) {
4747

4848
struct np *foo() {
4949
//CHECK_NOALL: _Ptr<struct np> foo(void) {
50-
//CHECK_ALL: struct np * foo(void) {
50+
//CHECK_ALL: struct np *foo(void) {
5151
struct p x, y;
5252
x.x = 1;
5353
x.y = 2;
5454
y.x = 3;
5555
y.y = 4;
5656
struct np *z = sus(x, y);
57-
//CHECK_NOALL: _Ptr<struct np> z = sus(x, y);
57+
//CHECK_NOALL: _Ptr<struct np> z = sus(x, y);
5858
//CHECK_ALL: struct np *z = sus(x, y);
5959
return z;
6060
}
6161

6262
struct np *bar() {
63-
//CHECK: struct np * bar(void) {
63+
//CHECK: struct np *bar(void) {
6464
struct p x, y;
6565
x.x = 1;
6666
x.y = 2;

‎clang/test/CheckedCRewriter/b15_calleepointerstruct.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ struct r {
3535

3636

3737
struct p *sus(struct p *x, struct p *y) {
38-
//CHECK: struct p * sus(_Ptr<struct p> x, _Ptr<struct p> y) {
38+
//CHECK: struct p *sus(_Ptr<struct p> x, _Ptr<struct p> y) {
3939
x->y += 1;
4040
struct p *z = malloc(sizeof(struct p));
4141
//CHECK: struct p *z = malloc<struct p>(sizeof(struct p));
@@ -44,7 +44,7 @@ struct p *sus(struct p *x, struct p *y) {
4444
}
4545

4646
struct p *foo() {
47-
//CHECK: struct p * foo(void) {
47+
//CHECK: struct p *foo(void) {
4848
int ex1 = 2, ex2 = 3;
4949
struct p *x;
5050
//CHECK: _Ptr<struct p> x = ((void *)0);
@@ -60,7 +60,7 @@ struct p *foo() {
6060
}
6161

6262
struct p *bar() {
63-
//CHECK: struct p * bar(void) {
63+
//CHECK: struct p *bar(void) {
6464
int ex1 = 2, ex2 = 3;
6565
struct p *x;
6666
//CHECK: _Ptr<struct p> x = ((void *)0);

‎clang/test/CheckedCRewriter/b16_callerpointerstruct.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,17 @@ struct r {
3636

3737
struct p *sus(struct p *x, struct p *y) {
3838
//CHECK_NOALL: struct p *sus(_Ptr<struct p> x, _Ptr<struct p> y) : itype(_Ptr<struct p>) {
39-
//CHECK_ALL: struct p * sus(_Ptr<struct p> x, _Ptr<struct p> y) {
39+
//CHECK_ALL: struct p *sus(_Ptr<struct p> x, _Ptr<struct p> y) {
4040
x->y += 1;
4141
struct p *z = malloc(sizeof(struct p));
42-
//CHECK_NOALL: _Ptr<struct p> z = malloc<struct p>(sizeof(struct p));
42+
//CHECK_NOALL: _Ptr<struct p> z = malloc<struct p>(sizeof(struct p));
4343
//CHECK_ALL: struct p *z = malloc<struct p>(sizeof(struct p));
4444
return z;
4545
}
4646

4747
struct p *foo() {
4848
//CHECK_NOALL: _Ptr<struct p> foo(void) {
49-
//CHECK_ALL: struct p * foo(void) {
49+
//CHECK_ALL: struct p *foo(void) {
5050
int ex1 = 2, ex2 = 3;
5151
struct p *x;
5252
//CHECK: _Ptr<struct p> x = ((void *)0);
@@ -57,13 +57,13 @@ struct p *foo() {
5757
x->y = &ex2;
5858
y->y = &ex1;
5959
struct p *z = (struct p *) sus(x, y);
60-
//CHECK_NOALL: _Ptr<struct p> z = (_Ptr<struct p>) sus(x, y);
60+
//CHECK_NOALL: _Ptr<struct p> z = (_Ptr<struct p>) sus(x, y);
6161
//CHECK_ALL: struct p *z = (struct p *) sus(x, y);
6262
return z;
6363
}
6464

6565
struct p *bar() {
66-
//CHECK: struct p * bar(void) {
66+
//CHECK: struct p *bar(void) {
6767
int ex1 = 2, ex2 = 3;
6868
struct p *x;
6969
//CHECK: _Ptr<struct p> x = ((void *)0);

‎clang/test/CheckedCRewriter/b17_bothstructnp.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ struct np *sus(struct p x, struct p y) {
4545
}
4646

4747
struct np *foo() {
48-
//CHECK: struct np * foo(void) {
48+
//CHECK: struct np *foo(void) {
4949
struct p x, y;
5050
x.x = 1;
5151
x.y = 2;
@@ -57,7 +57,7 @@ struct np *foo() {
5757
}
5858

5959
struct np *bar() {
60-
//CHECK: struct np * bar(void) {
60+
//CHECK: struct np *bar(void) {
6161
struct p x, y;
6262
x.x = 1;
6363
x.y = 2;

‎clang/test/CheckedCRewriter/b19_bothpointerstruct.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ struct r {
3535

3636

3737
struct p *sus(struct p *x, struct p *y) {
38-
//CHECK: struct p * sus(_Ptr<struct p> x, _Ptr<struct p> y) {
38+
//CHECK: struct p *sus(_Ptr<struct p> x, _Ptr<struct p> y) {
3939
x->y += 1;
4040
struct p *z = malloc(sizeof(struct p));
4141
//CHECK: struct p *z = malloc<struct p>(sizeof(struct p));
@@ -44,7 +44,7 @@ struct p *sus(struct p *x, struct p *y) {
4444
}
4545

4646
struct p *foo() {
47-
//CHECK: struct p * foo(void) {
47+
//CHECK: struct p *foo(void) {
4848
int ex1 = 2, ex2 = 3;
4949
struct p *x;
5050
//CHECK: _Ptr<struct p> x = ((void *)0);
@@ -60,7 +60,7 @@ struct p *foo() {
6060
}
6161

6262
struct p *bar() {
63-
//CHECK: struct p * bar(void) {
63+
//CHECK: struct p *bar(void) {
6464
int ex1 = 2, ex2 = 3;
6565
struct p *x;
6666
//CHECK: _Ptr<struct p> x = ((void *)0);

0 commit comments

Comments
 (0)