FYI I've fixed a memory leak in the new test in r372925.
On Wed, Sep 25, 2019 at 10:58 AM Vedant Kumar via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > Author: vedantk > Date: Wed Sep 25 11:00:31 2019 > New Revision: 372903 > > URL: http://llvm.org/viewvc/llvm-project?rev=372903&view=rev > Log: > [Mangle] Add flag to asm labels to disable '\01' prefixing > > LLDB synthesizes decls using asm labels. These decls cannot have a mangle > different than the one specified in the label name. I.e., the '\01' prefix > should not be added. > > Fixes an expression evaluation failure in lldb's TestVirtual.py on iOS. > > rdar://45827323 > > Differential Revision: https://reviews.llvm.org/D67774 > > Modified: > cfe/trunk/include/clang/Basic/Attr.td > cfe/trunk/include/clang/Basic/AttrDocs.td > cfe/trunk/lib/AST/Mangle.cpp > cfe/trunk/lib/Sema/SemaDecl.cpp > cfe/trunk/unittests/AST/DeclTest.cpp > > Modified: cfe/trunk/include/clang/Basic/Attr.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=372903&r1=372902&r2=372903&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/Attr.td (original) > +++ cfe/trunk/include/clang/Basic/Attr.td Wed Sep 25 11:00:31 2019 > @@ -722,9 +722,25 @@ def AVRSignal : InheritableAttr, TargetS > > def AsmLabel : InheritableAttr { > let Spellings = [Keyword<"asm">, Keyword<"__asm__">]; > - let Args = [StringArgument<"Label">]; > + let Args = [ > + // Label specifies the mangled name for the decl. > + StringArgument<"Label">, > + > + // IsLiteralLabel specifies whether the label is literal (i.e. suppresses > + // the global C symbol prefix) or not. If not, the mangle-suppression > prefix > + // ('\01') is omitted from the decl name at the LLVM IR level. > + // > + // Non-literal labels are used by some external AST sources like LLDB. > + BoolArgument<"IsLiteralLabel", /*optional=*/0, /*fake=*/1> > + ]; > let SemaHandler = 0; > - let Documentation = [Undocumented]; > + let Documentation = [AsmLabelDocs]; > + let AdditionalMembers = > +[{ > +bool isEquivalent(AsmLabelAttr *Other) const { > + return getLabel() == Other->getLabel() && getIsLiteralLabel() == > Other->getIsLiteralLabel(); > +} > +}]; > } > > def Availability : InheritableAttr { > > Modified: cfe/trunk/include/clang/Basic/AttrDocs.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=372903&r1=372902&r2=372903&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/AttrDocs.td (original) > +++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Sep 25 11:00:31 2019 > @@ -2558,6 +2558,30 @@ manipulating bits of the enumerator when > }]; > } > > +def AsmLabelDocs : Documentation { > + let Category = DocCatDecl; > + let Content = [{ > +This attribute can be used on a function or variable to specify its symbol > name. > + > +On some targets, all C symbols are prefixed by default with a single > character, typically ``_``. This was done historically to distinguish them > from symbols used by other languages. (This prefix is also added to the > standard Itanium C++ ABI prefix on "mangled" symbol names, so that e.g. on > such targets the true symbol name for a C++ variable declared as ``int > cppvar;`` would be ``__Z6cppvar``; note the two underscores.) This prefix is > *not* added to the symbol names specified by the ``asm`` attribute; > programmers wishing to match a C symbol name must compensate for this. > + > +For example, consider the following C code: > + > +.. code-block:: c > + > + int var1 asm("altvar") = 1; // "altvar" in symbol table. > + int var2 = 1; // "_var2" in symbol table. > + > + void func1(void) asm("altfunc"); > + void func1(void) {} // "altfunc" in symbol table. > + void func2(void) {} // "_func2" in symbol table. > + > +Clang's implementation of this attribute is compatible with GCC's, > `documented here <https://gcc.gnu.org/onlinedocs/gcc/Asm-Labels.html>`_. > + > +While it is possible to use this attribute to name a special symbol used > internally by the compiler, such as an LLVM intrinsic, this is neither > recommended nor supported and may cause the compiler to crash or miscompile. > Users who wish to gain access to intrinsic behavior are strongly encouraged > to request new builtin functions. > + }]; > +} > + > def EnumExtensibilityDocs : Documentation { > let Category = DocCatDecl; > let Content = [{ > > Modified: cfe/trunk/lib/AST/Mangle.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Mangle.cpp?rev=372903&r1=372902&r2=372903&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/Mangle.cpp (original) > +++ cfe/trunk/lib/AST/Mangle.cpp Wed Sep 25 11:00:31 2019 > @@ -122,15 +122,21 @@ void MangleContext::mangleName(const Nam > if (const AsmLabelAttr *ALA = D->getAttr<AsmLabelAttr>()) { > // If we have an asm name, then we use it as the mangling. > > + // If the label isn't literal, or if this is an alias for an LLVM > intrinsic, > + // do not add a "\01" prefix. > + if (!ALA->getIsLiteralLabel() || ALA->getLabel().startswith("llvm.")) { > + Out << ALA->getLabel(); > + return; > + } > + > // Adding the prefix can cause problems when one file has a "foo" and > // another has a "\01foo". That is known to happen on ELF with the > // tricks normally used for producing aliases (PR9177). Fortunately the > // llvm mangler on ELF is a nop, so we can just avoid adding the \01 > - // marker. We also avoid adding the marker if this is an alias for an > - // LLVM intrinsic. > + // marker. > char GlobalPrefix = > getASTContext().getTargetInfo().getDataLayout().getGlobalPrefix(); > - if (GlobalPrefix && !ALA->getLabel().startswith("llvm.")) > + if (GlobalPrefix) > Out << '\01'; // LLVM IR Marker for __asm("foo") > > Out << ALA->getLabel(); > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=372903&r1=372902&r2=372903&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Sep 25 11:00:31 2019 > @@ -2766,7 +2766,7 @@ void Sema::mergeDeclAttributes(NamedDecl > > if (AsmLabelAttr *NewA = New->getAttr<AsmLabelAttr>()) { > if (AsmLabelAttr *OldA = Old->getAttr<AsmLabelAttr>()) { > - if (OldA->getLabel() != NewA->getLabel()) { > + if (!OldA->isEquivalent(NewA)) { > // This redeclaration changes __asm__ label. > Diag(New->getLocation(), diag::err_different_asm_label); > Diag(OldA->getLocation(), diag::note_previous_declaration); > @@ -6983,8 +6983,8 @@ NamedDecl *Sema::ActOnVariableDeclarator > } > } > > - NewVD->addAttr(::new (Context) > - AsmLabelAttr(Context, SE->getStrTokenLoc(0), Label)); > + NewVD->addAttr(::new (Context) AsmLabelAttr( > + Context, SE->getStrTokenLoc(0), Label, /*IsLiteralLabel=*/true)); > } else if (!ExtnameUndeclaredIdentifiers.empty()) { > llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I = > ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier()); > @@ -8882,8 +8882,9 @@ Sema::ActOnFunctionDeclarator(Scope *S, > if (Expr *E = (Expr*) D.getAsmLabel()) { > // The parser guarantees this is a string. > StringLiteral *SE = cast<StringLiteral>(E); > - NewFD->addAttr(::new (Context) AsmLabelAttr(Context, > SE->getStrTokenLoc(0), > - SE->getString())); > + NewFD->addAttr(::new (Context) > + AsmLabelAttr(Context, SE->getStrTokenLoc(0), > + SE->getString(), > /*IsLiteralLabel=*/true)); > } else if (!ExtnameUndeclaredIdentifiers.empty()) { > llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I = > ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier()); > @@ -17555,8 +17556,8 @@ void Sema::ActOnPragmaRedefineExtname(Id > LookupOrdinaryName); > AttributeCommonInfo Info(AliasName, SourceRange(AliasNameLoc), > AttributeCommonInfo::AS_Pragma); > - AsmLabelAttr *Attr = > - AsmLabelAttr::CreateImplicit(Context, AliasName->getName(), Info); > + AsmLabelAttr *Attr = AsmLabelAttr::CreateImplicit( > + Context, AliasName->getName(), /*LiteralLabel=*/true, Info); > > // If a declaration that: > // 1) declares a function or a variable > > Modified: cfe/trunk/unittests/AST/DeclTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/DeclTest.cpp?rev=372903&r1=372902&r2=372903&view=diff > ============================================================================== > --- cfe/trunk/unittests/AST/DeclTest.cpp (original) > +++ cfe/trunk/unittests/AST/DeclTest.cpp Wed Sep 25 11:00:31 2019 > @@ -10,12 +10,16 @@ > // > > //===----------------------------------------------------------------------===// > > +#include "clang/AST/ASTContext.h" > +#include "clang/AST/Mangle.h" > #include "clang/ASTMatchers/ASTMatchFinder.h" > +#include "clang/Basic/LLVM.h" > #include "clang/Tooling/Tooling.h" > #include "gtest/gtest.h" > > using namespace clang::ast_matchers; > using namespace clang::tooling; > +using namespace clang; > > TEST(Decl, CleansUpAPValues) { > MatchFinder Finder; > @@ -56,3 +60,48 @@ TEST(Decl, CleansUpAPValues) { > "constexpr _Complex __uint128_t c = 0xffffffffffffffff;", > Args)); > } > + > +TEST(Decl, AsmLabelAttr) { > + // Create two method decls: `f` and `g`. > + StringRef Code = R"( > + struct S { > + void f() {} > + void g() {} > + }; > + )"; > + auto AST = > + tooling::buildASTFromCodeWithArgs(Code, {"-target", > "i386-apple-darwin"}); > + ASTContext &Ctx = AST->getASTContext(); > + assert(Ctx.getTargetInfo().getDataLayout().getGlobalPrefix() && > + "Expected target to have a global prefix"); > + DiagnosticsEngine &Diags = AST->getDiagnostics(); > + SourceManager &SM = AST->getSourceManager(); > + FileID MainFileID = SM.getMainFileID(); > + > + // Find the method decls within the AST. > + SmallVector<Decl *, 1> Decls; > + AST->findFileRegionDecls(MainFileID, Code.find('{'), 0, Decls); > + ASSERT_TRUE(Decls.size() == 1); > + CXXRecordDecl *DeclS = cast<CXXRecordDecl>(Decls[0]); > + NamedDecl *DeclF = *DeclS->method_begin(); > + NamedDecl *DeclG = *(++DeclS->method_begin()); > + > + // Attach asm labels to the decls: one literal, and one not. > + DeclF->addAttr(::new (Ctx) AsmLabelAttr(Ctx, SourceLocation(), "foo", > + /*LiteralLabel=*/true)); > + DeclG->addAttr(::new (Ctx) AsmLabelAttr(Ctx, SourceLocation(), "goo", > + /*LiteralLabel=*/false)); > + > + // Mangle the decl names. > + std::string MangleF, MangleG; > + MangleContext *MC = ItaniumMangleContext::create(Ctx, Diags); > + { > + llvm::raw_string_ostream OS_F(MangleF); > + llvm::raw_string_ostream OS_G(MangleG); > + MC->mangleName(DeclF, OS_F); > + MC->mangleName(DeclG, OS_G); > + } > + > + ASSERT_TRUE(0 == MangleF.compare("\x01" "foo")); > + ASSERT_TRUE(0 == MangleG.compare("goo")); > +} > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
