From d1693af05a6cb57121c27ca6a89703b09c63bc0f Mon Sep 17 00:00:00 2001 From: John Kastner Date: Mon, 1 Mar 2021 15:00:57 -0500 Subject: [PATCH 1/8] Implement rewriting of function declarations with __attributes__ --- clang/include/clang/3C/RewriteUtils.h | 60 ++++++++++++++++++--- clang/include/clang/3C/Utils.h | 2 + clang/lib/3C/DeclRewriter.cpp | 17 ++++-- clang/lib/3C/Utils.cpp | 20 +++++++ clang/test/3C/attr.c | 76 +++++++++++++++++++++++++++ clang/test/3C/valist.c | 13 +++++ 6 files changed, 177 insertions(+), 11 deletions(-) create mode 100644 clang/test/3C/attr.c diff --git a/clang/include/clang/3C/RewriteUtils.h b/clang/include/clang/3C/RewriteUtils.h index 11af89b69f60..6a147f24de01 100644 --- a/clang/include/clang/3C/RewriteUtils.h +++ b/clang/include/clang/3C/RewriteUtils.h @@ -96,20 +96,58 @@ class FunctionDeclReplacement SourceRange getSourceRange(SourceManager &SM) const override { TypeSourceInfo *TSInfo = Decl->getTypeSourceInfo(); - FunctionTypeLoc TypeLoc; + FunctionTypeLoc FTypeLoc = FunctionTypeLoc(); + + SourceLocation AttrBeginLoc = SourceLocation(); + SourceLocation AttrEndLoc = SourceLocation(); if (TSInfo) { auto TSInfoLoc = TSInfo->getTypeLoc(); - TypeLoc = getBaseTypeLoc(TSInfoLoc).getAs(); + TypeLoc TLoc = getBaseTypeLoc(TSInfoLoc); + AttributedTypeLoc ATypeLoc = TLoc.getAs(); + if (!ATypeLoc.isNull()) { + TLoc = ATypeLoc.getNextTypeLoc(); + AttrBeginLoc = ATypeLoc.getSourceRange().getBegin();; + AttrEndLoc = ATypeLoc.getSourceRange().getEnd(); + } + if (Decl->hasAttrs()) { + for (auto *A : Decl->getAttrs()) { + SourceLocation NewAttrBegin = A->getRange().getBegin(); + if (AttrBeginLoc.isInvalid() || + SM.isBeforeInTranslationUnit(NewAttrBegin, AttrBeginLoc)) + AttrBeginLoc = NewAttrBegin; + + SourceLocation NewAttrEnd = A->getRange().getEnd(); + if (AttrEndLoc.isInvalid() || + SM.isBeforeInTranslationUnit(AttrEndLoc, NewAttrEnd)) + AttrEndLoc = NewAttrEnd; + } + } + if (AttrEndLoc.isValid()) { + auto T = Lexer::findNextToken(AttrEndLoc, SM, Decl->getLangOpts()); + AttrEndLoc = T->getEndLoc(); + } + FTypeLoc = TLoc.getAs(); + } + if (FTypeLoc.isNull()) { + // When we can't get a FunctionTypeLoc, we have no way to rewrite in + // specifically parameter or return source ranges. We can still rewrite + // if we're replacing both the parameter and the return. For this we just + // need to get the source range for the entire function. If we are + // replacing only one of the parameter and return, then this case would + // rewriting like this would cause us to clobber the one we aren't + // rewriting. + assert("SourceRange will overwrite function return type or argument." && + RewriteReturn && RewriteParams); + return SourceRange(Decl->getBeginLoc(), + getFunctionDeclRParen(Decl, SM)); } - if (!TSInfo || TypeLoc.isNull()) - return SourceRange(Decl->getBeginLoc(), getFunctionDeclRParen(Decl, SM)); // Function pointer are funky, and require special handling to rewrite the // return type. if (Decl->getReturnType()->isFunctionPointerType()) { if (RewriteParams && RewriteReturn) { auto T = - getBaseTypeLoc(TypeLoc.getReturnLoc()).getAs(); + getBaseTypeLoc(FTypeLoc.getReturnLoc()).getAs(); if (!T.isNull()) return SourceRange(Decl->getBeginLoc(), T.getRParenLoc()); } @@ -119,7 +157,7 @@ class FunctionDeclReplacement // If rewriting the return, then the range starts at the begining of the // decl. Otherwise, skip to the left parenthesis of parameters. SourceLocation Begin = - RewriteReturn ? Decl->getBeginLoc() : TypeLoc.getLParenLoc(); + RewriteReturn ? Decl->getBeginLoc() : FTypeLoc.getLParenLoc(); // If rewriting Parameters, stop at the right parenthesis of the parameters. // Otherwise, stop after the return type. @@ -127,7 +165,7 @@ class FunctionDeclReplacement if (RewriteParams) { // When there are no bounds or itypes on a function, the declaration ends // at the right paren of the declaration parameter list. - End = TypeLoc.getRParenLoc(); + End = FTypeLoc.getRParenLoc(); // If there's a bounds expression, this comes after the right paren of the // function declaration parameter list. @@ -156,6 +194,14 @@ class FunctionDeclReplacement End = Decl->getReturnTypeSourceRange().getEnd(); } + if (RewriteParams && AttrEndLoc.isValid() && + SM.isBeforeInTranslationUnit(End, AttrEndLoc)) + End = AttrEndLoc; + + if (RewriteReturn && AttrBeginLoc.isValid() && + SM.isBeforeInTranslationUnit(AttrBeginLoc, Begin)) + Begin = AttrBeginLoc; + assert("Invalid FunctionDeclReplacement SourceRange!" && Begin.isValid() && End.isValid()); diff --git a/clang/include/clang/3C/Utils.h b/clang/include/clang/3C/Utils.h index 5582731caa14..7040515dee5e 100644 --- a/clang/include/clang/3C/Utils.h +++ b/clang/include/clang/3C/Utils.h @@ -104,6 +104,8 @@ bool hasFunctionBody(clang::Decl *D); std::string getStorageQualifierString(clang::Decl *D); +std::string getAttributeString(clang::Decl *D); + // Use this version for user input that has not yet been validated. std::error_code tryGetCanonicalFilePath(const std::string &FileName, std::string &AbsoluteFp); diff --git a/clang/lib/3C/DeclRewriter.cpp b/clang/lib/3C/DeclRewriter.cpp index 745485396ac8..bcf1f3be8eed 100644 --- a/clang/lib/3C/DeclRewriter.cpp +++ b/clang/lib/3C/DeclRewriter.cpp @@ -118,9 +118,10 @@ void DeclRewriter::rewriteDecls(ASTContext &Context, ProgramInfo &Info, if (VDLToStmtMap.find(D) != VDLToStmtMap.end()) DS = VDLToStmtMap[D]; - std::string NewTy = getStorageQualifierString(D) + - PV->mkString(Info.getConstraints().getVariables()) + - ABRewriter.getBoundsString(PV, D); + std::string NewTy = + getAttributeString(D) + getStorageQualifierString(D) + + PV->mkString(Info.getConstraints().getVariables()) + + ABRewriter.getBoundsString(PV, D); if (auto *VD = dyn_cast(D)) RewriteThese.insert(new VarDeclReplacement(VD, DS, NewTy)); else if (auto *FD = dyn_cast(D)) @@ -563,7 +564,8 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) { std::string Type, IType; this->buildDeclVar(IntCV, ExtCV, PVDecl, Type, IType, RewriteParams, RewriteReturn); - ParmStrs.push_back(Type + IType); + std::string AttrStr = getAttributeString(PVDecl); + ParmStrs.push_back(AttrStr + Type + IType); } if (Defnc->numParams() == 0) { @@ -601,6 +603,13 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) { RewriteReturn = true; } + std::string AttrStr = getAttributeString(FD); + if ((RewriteReturn || RewriteParams) && !AttrStr.empty()) { + ReturnVar = AttrStr + ReturnVar; + RewriteParams = true; + RewriteReturn = true; + } + // Combine parameter and return variables rewritings into a single rewriting // for the entire function declaration. std::string NewSig = ""; diff --git a/clang/lib/3C/Utils.cpp b/clang/lib/3C/Utils.cpp index fc155f5793e3..8ec91abecbe3 100644 --- a/clang/lib/3C/Utils.cpp +++ b/clang/lib/3C/Utils.cpp @@ -13,6 +13,7 @@ #include "clang/3C/ConstraintVariables.h" #include "llvm/Support/Path.h" #include +#include using namespace llvm; using namespace clang; @@ -145,6 +146,25 @@ std::string getStorageQualifierString(Decl *D) { return ""; } +std::string attributeToString(const Attr *A, ASTContext &C) { + return "__attribute__((" + getSourceText(A->getRange(), C) + ")) "; +} + +std::string getAttributeString(Decl *D) { + std::ostringstream AttrStr; + if (D->hasAttrs()) + for (auto *A : D->getAttrs()) + AttrStr << attributeToString(A, D->getASTContext()); + if (auto *FD = dyn_cast(D)) { + if (auto *TSInfo = FD->getTypeSourceInfo()) { + auto ATLoc = TSInfo->getTypeLoc().getAs(); + if (!ATLoc.isNull()) + AttrStr << attributeToString(ATLoc.getAttr(), D->getASTContext()); + } + } + return AttrStr.str(); +} + bool isNULLExpression(clang::Expr *E, ASTContext &C) { QualType Typ = E->getType(); E = removeAuxillaryCasts(E); diff --git a/clang/test/3C/attr.c b/clang/test/3C/attr.c new file mode 100644 index 000000000000..c20dbe100325 --- /dev/null +++ b/clang/test/3C/attr.c @@ -0,0 +1,76 @@ +// RUN: rm -rf %t* +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s +// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s +// RUN: 3c -base-dir=%S -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - +// RUN: 3c -base-dir=%S -output-dir=%t.checked -alltypes %s -- +// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/attr.c -- | diff %t.checked/attr.c - + +// function attributes + +__attribute__((disable_tail_calls)) +int *a() { +//CHECK: __attribute__((disable_tail_calls)) _Ptr a(void) _Checked { + return 0; +} + +__attribute__((disable_tail_calls)) +void b(int *x) { +//CHECK: __attribute__((disable_tail_calls)) void b(_Ptr x) _Checked { + return; +} + +__attribute__((disable_tail_calls)) +int *c(int *x) { +//CHECK: __attribute__((disable_tail_calls)) _Ptr c(_Ptr x) _Checked { + return 0; +} +int *e(int *x) +__attribute__((disable_tail_calls)) +//CHECK: __attribute__((disable_tail_calls)) int *e(_Ptr x) : itype(_Ptr) +{ + return 1; +} + +__attribute__((no_stack_protector)) +int *f(int *x) +__attribute__((disable_tail_calls)) +//CHECK: __attribute__((no_stack_protector)) __attribute__((disable_tail_calls)) _Ptr f(_Ptr x) +{ +//CHECK: _Checked { + while (1){} +} + +// variable attribute on param + +void g(__attribute__((noescape)) int *x) { +//CHECK: void g(__attribute__((noescape)) _Ptr x) _Checked { + return; +} + +void h(__attribute__((noescape)) int *x) { +//CHECK: void h(__attribute__((noescape)) int *x : itype(_Ptr)) { + x = 1; +} + +int *i(__attribute__((noescape)) void *x) { +//CHECK: _Ptr i(__attribute__((noescape)) void *x) { + return 0; +} + +// variable attribute on local + +void j() { + __attribute__((nodebug)) int *a; + __attribute__((nodebug)) int *b = 0; + __attribute__((nodebug)) int *c = 1; + + __attribute__((nodebug)) int *d, *e = 1, **f, g, *h; +//CHECK: __attribute__((nodebug)) _Ptr a = ((void *)0); +//CHECK: __attribute__((nodebug)) _Ptr b = 0; +//CHECK: __attribute__((nodebug)) int *c = 1; +//CHECK: __attribute__((nodebug)) _Ptr d = ((void *)0); +//CHECK: int *e __attribute__((nodebug)) = 1; +//CHECK: __attribute__((nodebug)) _Ptr<_Ptr> f = ((void *)0); +//CHECK: int g __attribute__((nodebug)); +//CHECK: __attribute__((nodebug)) _Ptr h = ((void *)0); +} diff --git a/clang/test/3C/valist.c b/clang/test/3C/valist.c index 11ed61467196..e3bd424e238d 100644 --- a/clang/test/3C/valist.c +++ b/clang/test/3C/valist.c @@ -27,3 +27,16 @@ const char *lua_pushfstring (lua_State *L, const char *fmt, ...) { /*force output*/ int *p; //CHECK: _Ptr p = ((void *)0); + +// This tests va_list correctness using windows builtins +void foo( const char *fmt, __builtin_ms_va_list argp); +__attribute__((ms_abi)) int *bar(const char *fmt, ...) { +//CHECK: __attribute__((ms_abi)) _Ptr bar(const char *fmt : itype(_Ptr), ...) { + __builtin_ms_va_list argp; + __builtin_ms_va_start(argp, fmt); + //CHECK: __builtin_ms_va_start(argp, fmt); + foo(fmt, argp); + __builtin_ms_va_end(argp); + //CHECK: __builtin_ms_va_end(argp); + return 0; +} From bb291cc772d6e055a8c0fefd4c184204b1a96b51 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 2 Mar 2021 13:29:50 -0500 Subject: [PATCH 2/8] cleanup FunctionDeclReplacement::getSourceRange --- clang/include/clang/3C/RewriteUtils.h | 120 ++------------------------ clang/include/clang/3C/Utils.h | 10 +++ clang/lib/3C/RewriteUtils.cpp | 95 ++++++++++++++++++++ clang/lib/3C/Utils.cpp | 35 ++++++-- 4 files changed, 139 insertions(+), 121 deletions(-) diff --git a/clang/include/clang/3C/RewriteUtils.h b/clang/include/clang/3C/RewriteUtils.h index 6a147f24de01..d0ba427bd1ab 100644 --- a/clang/include/clang/3C/RewriteUtils.h +++ b/clang/include/clang/3C/RewriteUtils.h @@ -94,125 +94,17 @@ class FunctionDeclReplacement (RewriteReturn || RewriteParams)); } - SourceRange getSourceRange(SourceManager &SM) const override { - TypeSourceInfo *TSInfo = Decl->getTypeSourceInfo(); - FunctionTypeLoc FTypeLoc = FunctionTypeLoc(); - - SourceLocation AttrBeginLoc = SourceLocation(); - SourceLocation AttrEndLoc = SourceLocation(); - if (TSInfo) { - auto TSInfoLoc = TSInfo->getTypeLoc(); - TypeLoc TLoc = getBaseTypeLoc(TSInfoLoc); - AttributedTypeLoc ATypeLoc = TLoc.getAs(); - if (!ATypeLoc.isNull()) { - TLoc = ATypeLoc.getNextTypeLoc(); - AttrBeginLoc = ATypeLoc.getSourceRange().getBegin();; - AttrEndLoc = ATypeLoc.getSourceRange().getEnd(); - } - if (Decl->hasAttrs()) { - for (auto *A : Decl->getAttrs()) { - SourceLocation NewAttrBegin = A->getRange().getBegin(); - if (AttrBeginLoc.isInvalid() || - SM.isBeforeInTranslationUnit(NewAttrBegin, AttrBeginLoc)) - AttrBeginLoc = NewAttrBegin; - - SourceLocation NewAttrEnd = A->getRange().getEnd(); - if (AttrEndLoc.isInvalid() || - SM.isBeforeInTranslationUnit(AttrEndLoc, NewAttrEnd)) - AttrEndLoc = NewAttrEnd; - } - } - if (AttrEndLoc.isValid()) { - auto T = Lexer::findNextToken(AttrEndLoc, SM, Decl->getLangOpts()); - AttrEndLoc = T->getEndLoc(); - } - FTypeLoc = TLoc.getAs(); - } - if (FTypeLoc.isNull()) { - // When we can't get a FunctionTypeLoc, we have no way to rewrite in - // specifically parameter or return source ranges. We can still rewrite - // if we're replacing both the parameter and the return. For this we just - // need to get the source range for the entire function. If we are - // replacing only one of the parameter and return, then this case would - // rewriting like this would cause us to clobber the one we aren't - // rewriting. - assert("SourceRange will overwrite function return type or argument." && - RewriteReturn && RewriteParams); - return SourceRange(Decl->getBeginLoc(), - getFunctionDeclRParen(Decl, SM)); - } - - // Function pointer are funky, and require special handling to rewrite the - // return type. - if (Decl->getReturnType()->isFunctionPointerType()) { - if (RewriteParams && RewriteReturn) { - auto T = - getBaseTypeLoc(FTypeLoc.getReturnLoc()).getAs(); - if (!T.isNull()) - return SourceRange(Decl->getBeginLoc(), T.getRParenLoc()); - } - // Fall through to standard handling when only rewriting param decls - } - - // If rewriting the return, then the range starts at the begining of the - // decl. Otherwise, skip to the left parenthesis of parameters. - SourceLocation Begin = - RewriteReturn ? Decl->getBeginLoc() : FTypeLoc.getLParenLoc(); - - // If rewriting Parameters, stop at the right parenthesis of the parameters. - // Otherwise, stop after the return type. - SourceLocation End; - if (RewriteParams) { - // When there are no bounds or itypes on a function, the declaration ends - // at the right paren of the declaration parameter list. - End = FTypeLoc.getRParenLoc(); - - // If there's a bounds expression, this comes after the right paren of the - // function declaration parameter list. - if (auto *BoundsE = Decl->getBoundsExpr()) { - SourceLocation BoundsEnd = BoundsE->getEndLoc(); - if (BoundsEnd.isValid()) - End = BoundsEnd; - } - - // If there's an itype, this also comes after the right paren. In the case - // that there is both a bounds expression and an itype, we need check - // which is later in the file and use that as the declaration end. - if (auto *InteropE = Decl->getInteropTypeExpr()) { - SourceLocation InteropEnd = InteropE->getEndLoc(); - if (InteropEnd.isValid() && - (!End.isValid() || SM.isBeforeInTranslationUnit(End, InteropEnd))) - End = InteropEnd; - } - - // SourceLocations are weird and turn up invalid for reasons I don't - // understand. Fallback to extracting r paren location from source - // character buffer. - if (!End.isValid()) - End = getFunctionDeclRParen(Decl, SM); - } else { - End = Decl->getReturnTypeSourceRange().getEnd(); - } - - if (RewriteParams && AttrEndLoc.isValid() && - SM.isBeforeInTranslationUnit(End, AttrEndLoc)) - End = AttrEndLoc; - - if (RewriteReturn && AttrBeginLoc.isValid() && - SM.isBeforeInTranslationUnit(AttrBeginLoc, Begin)) - Begin = AttrBeginLoc; - - assert("Invalid FunctionDeclReplacement SourceRange!" && Begin.isValid() && - End.isValid()); - - return SourceRange(Begin, End); - } + SourceRange getSourceRange(SourceManager &SM) const override; private: // This determines if the full declaration or the return will be replaced. bool RewriteReturn; - bool RewriteParams; + + SourceLocation getDeclBegin(SourceManager &SM) const; + SourceLocation getParamBegin(SourceManager &SM) const; + SourceLocation getReturnEnd(SourceManager &SM) const; + SourceLocation getDeclEnd(SourceManager &SM) const; }; // Compare two DeclReplacement values. The algorithm for comparing them relates diff --git a/clang/include/clang/3C/Utils.h b/clang/include/clang/3C/Utils.h index 7040515dee5e..f09569e2ab2f 100644 --- a/clang/include/clang/3C/Utils.h +++ b/clang/include/clang/3C/Utils.h @@ -104,6 +104,9 @@ bool hasFunctionBody(clang::Decl *D); std::string getStorageQualifierString(clang::Decl *D); +void forEachAttribute(clang::Decl *D, + llvm::function_ref F); + std::string getAttributeString(clang::Decl *D); // Use this version for user input that has not yet been validated. @@ -207,4 +210,11 @@ clang::TypeLoc getBaseTypeLoc(clang::TypeLoc T); // Ignore all CheckedC temporary and clang implicit expression on E. This // combines the behavior of IgnoreExprTmp and IgnoreImplicit. clang::Expr *ignoreCheckedCImplicit(clang::Expr *E); + +// Get a FunctionTypeLoc object from the declaration/type location. This is a +// little complicated due to various clang wrapper types that come from +// parenthesised types and function attributes. +clang::FunctionTypeLoc getFunctionTypeLoc(clang::TypeLoc TLoc); +clang::FunctionTypeLoc getFunctionTypeLoc(clang::DeclaratorDecl *Decl); + #endif diff --git a/clang/lib/3C/RewriteUtils.cpp b/clang/lib/3C/RewriteUtils.cpp index e2e935fde893..984ac5c7f305 100644 --- a/clang/lib/3C/RewriteUtils.cpp +++ b/clang/lib/3C/RewriteUtils.cpp @@ -464,6 +464,101 @@ class TypeArgumentAdder : public clang::RecursiveASTVisitor { } }; +SourceRange FunctionDeclReplacement::getSourceRange(SourceManager &SM) const { + SourceLocation Begin = RewriteReturn ? getDeclBegin(SM) : getParamBegin(SM); + SourceLocation End = RewriteParams ? getDeclEnd(SM) : getReturnEnd(SM); + assert("Invalid FunctionDeclReplacement SourceRange!" && Begin.isValid() && + End.isValid() && SM.isBeforeInTranslationUnit(Begin, End)); + return SourceRange(Begin, End); +} + +SourceLocation FunctionDeclReplacement::getDeclBegin(SourceManager &SM) const { + SourceLocation Begin = Decl->getBeginLoc(); + forEachAttribute(Decl, [&Begin, &SM](const clang::Attr *A) { + SourceLocation NewAttrBegin = A->getRange().getBegin(); + if (NewAttrBegin.isValid() && + SM.isBeforeInTranslationUnit(NewAttrBegin, Begin)) + Begin = NewAttrBegin; + }); + return Begin; +} + +SourceLocation FunctionDeclReplacement::getParamBegin(SourceManager &SM) const { + FunctionTypeLoc FTypeLoc = getFunctionTypeLoc(Decl); + // If we can't get a FunctionTypeLoc instance, then we'll guess that the + // l-paren is the token following the function name. This can clobber some + // comments and formatting. + if (FTypeLoc.isNull()) + return Lexer::getLocForEndOfToken(Decl->getLocation(), 0, SM, + Decl->getLangOpts()); + return FTypeLoc.getLParenLoc(); +} + +SourceLocation FunctionDeclReplacement::getReturnEnd(SourceManager &SM) const { + return Decl->getReturnTypeSourceRange().getEnd(); +} + +SourceLocation FunctionDeclReplacement::getDeclEnd(SourceManager &SM) const { + FunctionTypeLoc FTypeLoc = getFunctionTypeLoc(Decl); + + SourceLocation End; + if (FTypeLoc.isNull()) { + // Without a FunctionTypeLocation, we have to approximate the end of the + // declaration as the location of the first r-paren before the start of the + // function body. This is messed up by comments and ifdef blocks containing + // r-paren, but works correctly most of the time. + End = getFunctionDeclRParen(Decl, SM); + } else if (Decl->getReturnType()->isFunctionPointerType()) { + // If a function returns a function pointer type, the paramter list for the + // returned function type comes after the top-level functions parameter + // list. Of course, this FunctionTypeLoc can also be null, so we have + // another fall back to the r-paren approximation. + FunctionTypeLoc T = getFunctionTypeLoc(FTypeLoc.getReturnLoc()); + if (!T.isNull()) + End = T.getRParenLoc(); + else + End = getFunctionDeclRParen(Decl, SM); + } else { + End = FTypeLoc.getRParenLoc(); + } + + // If there's a bounds expression, this comes after the right paren of the + // function declaration parameter list. + if (auto *BoundsE = Decl->getBoundsExpr()) { + SourceLocation BoundsEnd = BoundsE->getEndLoc(); + if (BoundsEnd.isValid()) + End = BoundsEnd; + } + + // If there's an itype, this also comes after the right paren. In the case + // that there is both a bounds expression and an itype, we need check + // which is later in the file and use that as the declaration end. + if (auto *InteropE = Decl->getInteropTypeExpr()) { + SourceLocation InteropEnd = InteropE->getEndLoc(); + if (InteropEnd.isValid() && + (!End.isValid() || SM.isBeforeInTranslationUnit(End, InteropEnd))) + End = InteropEnd; + } + + // Functions attributes can appear after the the closing paren for the + // parameter list. + forEachAttribute(Decl, [&End, &SM, this](const clang::Attr *A) { + auto NewEnd = Lexer::findNextToken(A->getRange().getEnd(), SM, + Decl->getLangOpts())->getEndLoc(); + if (!End.isValid() || + (NewEnd.isValid() && SM.isBeforeInTranslationUnit(End, NewEnd))) + End = NewEnd; + }); + + // SourceLocations are weird and turn up invalid for reasons I don't + // understand. Fallback to extracting r paren location from source + // character buffer. + if (!End.isValid()) + End = getFunctionDeclRParen(Decl, SM); + + return End; +} + std::string ArrayBoundsRewriter::getBoundsString(const PVConstraint *PV, Decl *D, bool Isitype) { auto &ABInfo = Info.getABoundsInfo(); diff --git a/clang/lib/3C/Utils.cpp b/clang/lib/3C/Utils.cpp index 8ec91abecbe3..d7eb2f9de049 100644 --- a/clang/lib/3C/Utils.cpp +++ b/clang/lib/3C/Utils.cpp @@ -146,22 +146,29 @@ std::string getStorageQualifierString(Decl *D) { return ""; } -std::string attributeToString(const Attr *A, ASTContext &C) { - return "__attribute__((" + getSourceText(A->getRange(), C) + ")) "; -} - -std::string getAttributeString(Decl *D) { +void forEachAttribute(Decl *D, llvm::function_ref F) { std::ostringstream AttrStr; if (D->hasAttrs()) for (auto *A : D->getAttrs()) - AttrStr << attributeToString(A, D->getASTContext()); + F(A); if (auto *FD = dyn_cast(D)) { if (auto *TSInfo = FD->getTypeSourceInfo()) { auto ATLoc = TSInfo->getTypeLoc().getAs(); if (!ATLoc.isNull()) - AttrStr << attributeToString(ATLoc.getAttr(), D->getASTContext()); + F(ATLoc.getAttr()); } } +} + +std::string attributeToString(const Attr *A, ASTContext &C) { + return "__attribute__((" + getSourceText(A->getRange(), C) + ")) "; +} + +std::string getAttributeString(Decl *D) { + std::ostringstream AttrStr; + forEachAttribute(D, [&AttrStr, D](const clang::Attr *A) { + AttrStr << attributeToString(A, D->getASTContext()); + }); return AttrStr.str(); } @@ -491,3 +498,17 @@ Expr *ignoreCheckedCImplicit(Expr *E) { } return New; } + +FunctionTypeLoc getFunctionTypeLoc(TypeLoc TLoc) { + TLoc = getBaseTypeLoc(TLoc); + auto ATLoc = TLoc.getAs(); + if (!ATLoc.isNull()) + TLoc = ATLoc.getNextTypeLoc(); + return TLoc.getAs(); +} + +FunctionTypeLoc getFunctionTypeLoc(DeclaratorDecl *Decl) { + if (auto *TSInfo = Decl->getTypeSourceInfo()) + return getFunctionTypeLoc(TSInfo->getTypeLoc()); + return FunctionTypeLoc(); +} From 6baa07640473e893001e29ba6b186c21bcaacc00 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 2 Mar 2021 16:50:57 -0500 Subject: [PATCH 3/8] Fixes for libarchive --- clang/lib/3C/RewriteUtils.cpp | 20 ++++++++++---------- clang/lib/3C/Utils.cpp | 7 ++++--- clang/test/3C/attr.c | 11 +++++++++++ 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/clang/lib/3C/RewriteUtils.cpp b/clang/lib/3C/RewriteUtils.cpp index 984ac5c7f305..a44ca8b376b8 100644 --- a/clang/lib/3C/RewriteUtils.cpp +++ b/clang/lib/3C/RewriteUtils.cpp @@ -473,14 +473,7 @@ SourceRange FunctionDeclReplacement::getSourceRange(SourceManager &SM) const { } SourceLocation FunctionDeclReplacement::getDeclBegin(SourceManager &SM) const { - SourceLocation Begin = Decl->getBeginLoc(); - forEachAttribute(Decl, [&Begin, &SM](const clang::Attr *A) { - SourceLocation NewAttrBegin = A->getRange().getBegin(); - if (NewAttrBegin.isValid() && - SM.isBeforeInTranslationUnit(NewAttrBegin, Begin)) - Begin = NewAttrBegin; - }); - return Begin; + return Decl->getBeginLoc(); } SourceLocation FunctionDeclReplacement::getParamBegin(SourceManager &SM) const { @@ -543,8 +536,15 @@ SourceLocation FunctionDeclReplacement::getDeclEnd(SourceManager &SM) const { // Functions attributes can appear after the the closing paren for the // parameter list. forEachAttribute(Decl, [&End, &SM, this](const clang::Attr *A) { - auto NewEnd = Lexer::findNextToken(A->getRange().getEnd(), SM, - Decl->getLangOpts())->getEndLoc(); + SourceLocation AttrEnd = A->getRange().getEnd(); + + llvm::Optional NextTok = Lexer::findNextToken(AttrEnd, SM, + Decl->getLangOpts()); + + SourceLocation NewEnd = NextTok.hasValue() ? NextTok->getEndLoc() + : A->getRange().getEnd(); + NewEnd = SM.getExpansionLoc(NewEnd); + if (!End.isValid() || (NewEnd.isValid() && SM.isBeforeInTranslationUnit(End, NewEnd))) End = NewEnd; diff --git a/clang/lib/3C/Utils.cpp b/clang/lib/3C/Utils.cpp index d7eb2f9de049..430238d320b2 100644 --- a/clang/lib/3C/Utils.cpp +++ b/clang/lib/3C/Utils.cpp @@ -146,14 +146,14 @@ std::string getStorageQualifierString(Decl *D) { return ""; } -void forEachAttribute(Decl *D, llvm::function_ref F) { +void forEachAttribute(Decl *D, llvm::function_ref F) { std::ostringstream AttrStr; if (D->hasAttrs()) for (auto *A : D->getAttrs()) F(A); if (auto *FD = dyn_cast(D)) { if (auto *TSInfo = FD->getTypeSourceInfo()) { - auto ATLoc = TSInfo->getTypeLoc().getAs(); + auto ATLoc = getBaseTypeLoc(TSInfo->getTypeLoc()).getAs(); if (!ATLoc.isNull()) F(ATLoc.getAttr()); } @@ -161,7 +161,7 @@ void forEachAttribute(Decl *D, llvm::function_ref F) { } std::string attributeToString(const Attr *A, ASTContext &C) { - return "__attribute__((" + getSourceText(A->getRange(), C) + ")) "; + return "__attribute__((" + A->getAttrName()->getName().str() + ")) "; } std::string getAttributeString(Decl *D) { @@ -484,6 +484,7 @@ TypeLoc getBaseTypeLoc(TypeLoc T) { assert(!T.isNull() && "Can't get base location from Null."); while (!T.getNextTypeLoc().isNull() && (!T.getAs().isNull() || + !T.getAs().isNull() || T.getTypePtr()->isPointerType() || T.getTypePtr()->isArrayType())) T = T.getNextTypeLoc(); return T; diff --git a/clang/test/3C/attr.c b/clang/test/3C/attr.c index c20dbe100325..6f50a6457944 100644 --- a/clang/test/3C/attr.c +++ b/clang/test/3C/attr.c @@ -74,3 +74,14 @@ void j() { //CHECK: int g __attribute__((nodebug)); //CHECK: __attribute__((nodebug)) _Ptr h = ((void *)0); } + +#define FOO __attribute__((ms_abi)) +int *foo() FOO ; +int *foo() { return 0; } +//CHECK: __attribute__((ms_abi)) _Ptr foo(void) ; +//CHECK: __attribute__((ms_abi)) _Ptr foo(void) _Checked { return 0; } + +__attribute__((deprecated)) int *bar(); +int *bar() { return 0; } +//CHECK: __attribute__((deprecated)) _Ptr bar(void); +//CHECK: __attribute__((deprecated)) _Ptr bar(void) _Checked { return 0; } From 328161dabc8314d25597ed46bb03d74d5f892391 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Tue, 2 Mar 2021 17:33:10 -0500 Subject: [PATCH 4/8] Fixes for zlib --- clang/lib/3C/RewriteUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/3C/RewriteUtils.cpp b/clang/lib/3C/RewriteUtils.cpp index a44ca8b376b8..52e785275809 100644 --- a/clang/lib/3C/RewriteUtils.cpp +++ b/clang/lib/3C/RewriteUtils.cpp @@ -468,7 +468,7 @@ SourceRange FunctionDeclReplacement::getSourceRange(SourceManager &SM) const { SourceLocation Begin = RewriteReturn ? getDeclBegin(SM) : getParamBegin(SM); SourceLocation End = RewriteParams ? getDeclEnd(SM) : getReturnEnd(SM); assert("Invalid FunctionDeclReplacement SourceRange!" && Begin.isValid() && - End.isValid() && SM.isBeforeInTranslationUnit(Begin, End)); + End.isValid()); return SourceRange(Begin, End); } From 5c442f978e346cb06f660e8e80143d55164df46a Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 3 Mar 2021 12:04:46 -0500 Subject: [PATCH 5/8] Fix vsftpd --- clang/lib/3C/Utils.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clang/lib/3C/Utils.cpp b/clang/lib/3C/Utils.cpp index 430238d320b2..10b96dd0d404 100644 --- a/clang/lib/3C/Utils.cpp +++ b/clang/lib/3C/Utils.cpp @@ -161,7 +161,9 @@ void forEachAttribute(Decl *D, llvm::function_ref F) { } std::string attributeToString(const Attr *A, ASTContext &C) { - return "__attribute__((" + A->getAttrName()->getName().str() + ")) "; + if (A->getAttrName() && !A->getAttrName()->getName().empty()) + return "__attribute__((" + A->getAttrName()->getName().str() + ")) "; + return ""; } std::string getAttributeString(Decl *D) { From e9e34f20d651681006141fd1362a7645177f1381 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 3 Mar 2021 12:10:58 -0500 Subject: [PATCH 6/8] Add test for benchmark failures --- clang/test/3C/attr.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clang/test/3C/attr.c b/clang/test/3C/attr.c index 6f50a6457944..82f38b42d493 100644 --- a/clang/test/3C/attr.c +++ b/clang/test/3C/attr.c @@ -85,3 +85,10 @@ __attribute__((deprecated)) int *bar(); int *bar() { return 0; } //CHECK: __attribute__((deprecated)) _Ptr bar(void); //CHECK: __attribute__((deprecated)) _Ptr bar(void) _Checked { return 0; } + +// Because toupper is a standard libary function, it has attributes in the AST +// even though there are none in the source. This was causing issues when +// trying to get the name of the attribute. + +int toupper(int c) { return 0; } +//CHECK: int toupper(int c) _Checked { return 0; } From 31b90e4286a95d7d209bdc490b73f3e79e716c9e Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 3 Mar 2021 13:26:01 -0500 Subject: [PATCH 7/8] Preserve attribute arguments --- clang/lib/3C/Utils.cpp | 27 ++++++++++++++++----------- clang/test/3C/attr.c | 7 ++++--- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/clang/lib/3C/Utils.cpp b/clang/lib/3C/Utils.cpp index 10b96dd0d404..7f74a6ee9005 100644 --- a/clang/lib/3C/Utils.cpp +++ b/clang/lib/3C/Utils.cpp @@ -153,25 +153,30 @@ void forEachAttribute(Decl *D, llvm::function_ref F) { F(A); if (auto *FD = dyn_cast(D)) { if (auto *TSInfo = FD->getTypeSourceInfo()) { - auto ATLoc = getBaseTypeLoc(TSInfo->getTypeLoc()).getAs(); + auto ATLoc = getBaseTypeLoc( + TSInfo->getTypeLoc()).getAs(); if (!ATLoc.isNull()) F(ATLoc.getAttr()); } } } -std::string attributeToString(const Attr *A, ASTContext &C) { - if (A->getAttrName() && !A->getAttrName()->getName().empty()) - return "__attribute__((" + A->getAttrName()->getName().str() + ")) "; - return ""; -} - std::string getAttributeString(Decl *D) { - std::ostringstream AttrStr; - forEachAttribute(D, [&AttrStr, D](const clang::Attr *A) { - AttrStr << attributeToString(A, D->getASTContext()); + std::string AttrStr; + llvm::raw_string_ostream AttrStream(AttrStr); + forEachAttribute(D, [&AttrStream, D](const clang::Attr *A) { + A->printPretty(AttrStream, PrintingPolicy(D->getLangOpts())); }); - return AttrStr.str(); + AttrStream.flush(); + + // Attr::printPretty puts a space before each attribute resulting in a space + // appearing before any attributes, and no space following the attributes. + // I need this to be the other way around. + if (!AttrStr.empty()) { + assert(AttrStr[0] == ' '); + return AttrStr.substr(1) + ' '; + } + return AttrStr; } bool isNULLExpression(clang::Expr *E, ASTContext &C) { diff --git a/clang/test/3C/attr.c b/clang/test/3C/attr.c index 82f38b42d493..87d1e0f8ec1f 100644 --- a/clang/test/3C/attr.c +++ b/clang/test/3C/attr.c @@ -81,10 +81,11 @@ int *foo() { return 0; } //CHECK: __attribute__((ms_abi)) _Ptr foo(void) ; //CHECK: __attribute__((ms_abi)) _Ptr foo(void) _Checked { return 0; } -__attribute__((deprecated)) int *bar(); +// Attribute parameter is preserved +__attribute__((deprecated("bar"))) int *bar(); int *bar() { return 0; } -//CHECK: __attribute__((deprecated)) _Ptr bar(void); -//CHECK: __attribute__((deprecated)) _Ptr bar(void) _Checked { return 0; } +//CHECK: __attribute__((deprecated("bar"))) _Ptr bar(void); +//CHECK: __attribute__((deprecated("bar"))) _Ptr bar(void) _Checked { return 0; } // Because toupper is a standard libary function, it has attributes in the AST // even though there are none in the source. This was causing issues when From c665560190eb9c52a3b4747283d86d7130078f72 Mon Sep 17 00:00:00 2001 From: John Kastner Date: Wed, 22 Sep 2021 10:35:29 -0400 Subject: [PATCH 8/8] update test --- clang/test/3C/attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/3C/attr.c b/clang/test/3C/attr.c index 31fe4fea0feb..6deccc30a9e0 100644 --- a/clang/test/3C/attr.c +++ b/clang/test/3C/attr.c @@ -53,7 +53,7 @@ void h(__attribute__((noescape)) int *x) { } int *i(__attribute__((noescape)) void *x) { -//CHECK: _Ptr i(__attribute__((noescape)) void *x) { +//CHECK: _For_any(T) _Ptr i(__attribute__((noescape)) _Ptr x) { return 0; }