[PATCH] Implementation of #pragma (data|code|const|bss)_seg
Aaron Ballman
aaron at aaronballman.com
Thu Apr 3 07:26:12 PDT 2014
Would it make sense to also add __declspec(code_seg) as part of this
patch? http://msdn.microsoft.com/en-us/library/dn636922.aspx
Especially since MSDN recommends it over #pragma code_seg.
The patch is missing parser test cases.
More comments below.
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -1069,7 +1069,7 @@
> }
>
> def Section : InheritableAttr {
> - let Spellings = [GCC<"section">];
> + let Spellings = [GCC<"section">, Declspec<"allocate">];
> let Args = [StringArgument<"Name">];
> let Subjects = SubjectList<[Function, GlobalVar,
> ObjCMethod, ObjCProperty], ErrorDiag,
I'd love to see:
+ let Documentation = [Anything other than Undocumented];
;-)
> Index: include/clang/Basic/DiagnosticParseKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticParseKinds.td
> +++ include/clang/Basic/DiagnosticParseKinds.td
> @@ -771,6 +771,8 @@
> "missing ')' after '#pragma %0' - ignoring">, InGroup<IgnoredPragmas>;
> def warn_pragma_expected_identifier : Warning<
> "expected identifier in '#pragma %0' - ignored">, InGroup<IgnoredPragmas>;
> +def warn_pragma_expected_string : Warning<
> + "expected string literal in '#pragma %0' - ignored">, InGroup<IgnoredPragmas>;
This could be combined with warn_pragma_expected_identifier using a %select.
> def warn_pragma_expected_integer : Warning<
> "expected integer between %0 and %1 inclusive in '#pragma %2' - ignored">,
> InGroup<IgnoredPragmas>;
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -516,6 +516,7 @@
> Warning<"ms_struct may not produce MSVC-compatible layouts for classes "
> "with base classes or virtual functions">,
> DefaultError, InGroup<IncompatibleMSStruct>;
> +def err_section_conflict : Error<"%0 causes a section type conflict with %1">;
>
> def warn_pragma_unused_undeclared_var : Warning<
> "undeclared variable %0 used as an argument for '#pragma unused'">,
> Index: include/clang/Basic/TokenKinds.def
> ===================================================================
> --- include/clang/Basic/TokenKinds.def
> +++ include/clang/Basic/TokenKinds.def
> @@ -684,6 +684,11 @@
> // handles them.
> ANNOTATION(pragma_ms_vtordisp)
>
> +// Annotation for all microsoft #pragmas...
> +// The lexer produces these so that they only take effect when the parser
> +// handles them.
> +ANNOTATION(pragma_ms_pragma)
> +
> // Annotation for #pragma OPENCL EXTENSION...
> // The lexer produces these so that they only take effect when the parser
> // handles them.
> Index: include/clang/Parse/Parser.h
> ===================================================================
> --- include/clang/Parse/Parser.h
> +++ include/clang/Parse/Parser.h
> @@ -154,6 +154,11 @@
> std::unique_ptr<PragmaHandler> MSDetectMismatchHandler;
> std::unique_ptr<PragmaHandler> MSPointersToMembers;
> std::unique_ptr<PragmaHandler> MSVtorDisp;
> + std::unique_ptr<PragmaHandler> MSDataSeg;
> + std::unique_ptr<PragmaHandler> MSBSSSeg;
> + std::unique_ptr<PragmaHandler> MSConstSeg;
> + std::unique_ptr<PragmaHandler> MSCodeSeg;
> + std::unique_ptr<PragmaHandler> MSSection;
>
> std::unique_ptr<CommentHandler> CommentSemaHandler;
>
> @@ -475,6 +480,12 @@
>
> void HandlePragmaMSVtorDisp();
>
> + void HandlePragmaMSPragma();
> + unsigned HandlePragmaMSSection(llvm::StringRef PragmaName,
> + SourceLocation PragmaLocation);
> + unsigned HandlePragmaMSXXXXSeg(llvm::StringRef PragmaName,
> + SourceLocation PragmaLocation);
I'm not too keen on XXXXSeg as the name. Perhaps
HandlePragmaMSSegment? Sorry, did I mention I like my bikesheds red?
> +
> /// \brief Handle the annotation token produced for
> /// #pragma align...
> void HandlePragmaAlign();
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h
> +++ include/clang/Sema/Sema.h
> @@ -274,6 +274,15 @@
> PVDK_Reset ///< #pragma vtordisp()
> };
>
> + enum PragmaMsStackAction {
> + PSK_Reset, // #pragma ()
> + PSK_Set, // #pragma ("name")
> + PSK_Push, // #pragma (push[, id])
> + PSK_Push_Set, // #pragma (push[, id], "name")
> + PSK_Pop, // #pragma (pop[, id])
> + PSK_Pop_Set, // #pragma (pop[, id], "name")
> + };
> +
> /// \brief Whether to insert vtordisps prior to virtual bases in the Microsoft
> /// C++ ABI. Possible values are 0, 1, and 2, which mean:
> ///
> @@ -289,6 +298,30 @@
> /// \brief Source location for newly created implicit MSInheritanceAttrs
> SourceLocation ImplicitMSInheritanceAttrLoc;
>
> + template<typename ValueType>
> + struct PragmaStack {
> + struct Slot {
> + llvm::StringRef StackSlotLabel;
> + ValueType Value;
> + SourceLocation PragmaLocation;
> + };
> + void Act(SourceLocation PragmaLocation,
> + PragmaMsStackAction Action,
> + llvm::StringRef StackSlotLabel,
> + ValueType Value);
> + explicit PragmaStack(const ValueType& Value)
According to the style guidelines, the reference goes with the
identifier, not the type. This applies across the entire patch (same
with pointers).
> + : CurrentValue(Value) {}
> + SmallVector<Slot, 2> Stack;
> + ValueType CurrentValue;
> + SourceLocation CurrentPragmaLocation;
> + };
> + // FIXME: We should serialize / deserialize these if they occur in a PCH (but
> + // we shouldn't do so if they're in a module).
> + PragmaStack<StringLiteral *> DataSegStack;
> + PragmaStack<StringLiteral *> BSSSegStack;
> + PragmaStack<StringLiteral *> ConstSegStack;
> + PragmaStack<StringLiteral *> CodeSegStack;
> +
> /// VisContext - Manages the stack for \#pragma GCC visibility.
> void *VisContext; // Really a "PragmaVisStack*"
>
> @@ -7030,6 +7063,46 @@
> void ActOnPragmaMSVtorDisp(PragmaVtorDispKind Kind, SourceLocation PragmaLoc,
> MSVtorDispAttr::Mode Value);
>
> + enum PragmaMSKind {
> + PSK_DataSeg,
> + PSK_BSSSeg,
> + PSK_ConstSeg,
> + PSK_CodeSeg,
> + };
> +
> + enum PragmaSectionFlag {
> + PSF_None = 0,
> + PSF_Read = 1,
> + PSF_Write = 2,
> + PSF_Execute = 4,
> + PSF_Implicit = 8,
> + };
> +
> + struct SectionInfo {
> + DeclaratorDecl* Decl;
> + SourceLocation PragmaSectionLocation;
> + int SectionFlags;
> + };
> +
> + llvm::StringMap<SectionInfo> SectionInfos;
This doesn't need to be a public member, does it?
> + bool UnifySection(const StringRef& SectionName,
> + int SectionFlags,
> + DeclaratorDecl* TheDecl);
> + bool UnifySection(const StringRef& SectionName,
> + int SectionFlags,
> + SourceLocation PragmaSectionLocation);
> +
> + /// \brief Called on well formed \#pragma bss_seg/data_seg/const_seg/code_seg.
> + void ActOnPragmaMSSeg(SourceLocation PragmaLocation,
> + PragmaMsStackAction Action,
> + llvm::StringRef StackSlotLabel,
> + StringLiteral *SegmentName,
> + llvm::StringRef PragmaName);
> +
> + /// \brief Called on well formed \#pragma section().
> + void ActOnPragmaMSSection(SourceLocation PragmaLocation,
> + int SectionFlags, StringLiteral *SegmentName);
> +
> /// ActOnPragmaDetectMismatch - Call on well-formed \#pragma detect_mismatch
> void ActOnPragmaDetectMismatch(StringRef Name, StringRef Value);
>
> Index: lib/Parse/ParseDeclCXX.cpp
> ===================================================================
> --- lib/Parse/ParseDeclCXX.cpp
> +++ lib/Parse/ParseDeclCXX.cpp
> @@ -2646,6 +2646,11 @@
> continue;
> }
>
> + if (Tok.is(tok::annot_pragma_ms_pragma)) {
> + HandlePragmaMSPragma();
> + continue;
> + }
> +
> // If we see a namespace here, a close brace was missing somewhere.
> if (Tok.is(tok::kw_namespace)) {
> DiagnoseUnexpectedNamespace(cast<NamedDecl>(TagDecl));
> Index: lib/Parse/ParsePragma.cpp
> ===================================================================
> --- lib/Parse/ParsePragma.cpp
> +++ lib/Parse/ParsePragma.cpp
> @@ -11,6 +11,7 @@
> //
> //===----------------------------------------------------------------------===//
>
> +#include "RAIIObjectsForParser.h"
> #include "clang/Lex/Preprocessor.h"
> #include "clang/Parse/ParseDiagnostic.h"
> #include "clang/Parse/Parser.h"
> @@ -124,6 +125,12 @@
> Token &FirstToken) override;
> };
>
> +struct PragmaMSPragma : public PragmaHandler {
> + explicit PragmaMSPragma(const char *name) : PragmaHandler(name) {}
> + virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
> + Token &FirstToken);
Marked as override and drop the virtual?
> +};
> +
> } // end namespace
>
> void Parser::initializePragmaHandlers() {
> @@ -175,6 +182,16 @@
> PP.AddPragmaHandler(MSPointersToMembers.get());
> MSVtorDisp.reset(new PragmaMSVtorDisp());
> PP.AddPragmaHandler(MSVtorDisp.get());
> + MSDataSeg.reset(new PragmaMSPragma("data_seg"));
> + PP.AddPragmaHandler(MSDataSeg.get());
> + MSBSSSeg.reset(new PragmaMSPragma("bss_seg"));
> + PP.AddPragmaHandler(MSBSSSeg.get());
> + MSConstSeg.reset(new PragmaMSPragma("const_seg"));
> + PP.AddPragmaHandler(MSConstSeg.get());
> + MSCodeSeg.reset(new PragmaMSPragma("code_seg"));
> + PP.AddPragmaHandler(MSCodeSeg.get());
> + MSSection.reset(new PragmaMSPragma("section"));
> + PP.AddPragmaHandler(MSSection.get());
> }
> }
>
> @@ -214,6 +231,16 @@
> MSPointersToMembers.reset();
> PP.RemovePragmaHandler(MSVtorDisp.get());
> MSVtorDisp.reset();
> + PP.RemovePragmaHandler(MSDataSeg.get());
> + MSDataSeg.reset();
> + PP.RemovePragmaHandler(MSBSSSeg.get());
> + MSBSSSeg.reset();
> + PP.RemovePragmaHandler(MSConstSeg.get());
> + MSConstSeg.reset();
> + PP.RemovePragmaHandler(MSCodeSeg.get());
> + MSCodeSeg.reset();
> + PP.RemovePragmaHandler(MSSection.get());
> + MSSection.reset();
> }
>
> PP.RemovePragmaHandler("STDC", FPContractHandler.get());
> @@ -400,6 +427,120 @@
> Actions.ActOnPragmaMSVtorDisp(Kind, PragmaLoc, Mode);
> }
>
> +void Parser::HandlePragmaMSPragma() {
> + assert(Tok.is(tok::annot_pragma_ms_pragma));
> + // Grab the tokens out of the annotation and enter them into the stream.
> + auto TheTokens = (std::pair<Token*, size_t> *)Tok.getAnnotationValue();
> + PP.EnterTokenStream(TheTokens->first, TheTokens->second, true, true);
> + SourceLocation PragmaLocation = ConsumeToken(); // The annotation token.
> + assert(Tok.isAnyIdentifier());
> + llvm::StringRef PragmaName = Tok.getIdentifierInfo()->getName();
> + PP.Lex(Tok); // pragma kind
> + // Figure out which #pragma we're dealing with. The switch has no default
> + // because lex shouldn't emit the annotation token for unrecognized pragmas.
> + typedef unsigned (Parser::*PragmaHandler)(llvm::StringRef, SourceLocation);
> + PragmaHandler Handler = llvm::StringSwitch<PragmaHandler>(PragmaName)
> + .Case("data_seg", &Parser::HandlePragmaMSXXXXSeg)
> + .Case("bss_seg", &Parser::HandlePragmaMSXXXXSeg)
> + .Case("const_seg", &Parser::HandlePragmaMSXXXXSeg)
> + .Case("code_seg", &Parser::HandlePragmaMSXXXXSeg)
> + .Case("section", &Parser::HandlePragmaMSSection);
> + if (auto DiagID = (this->*Handler)(PragmaName, PragmaLocation)) {
> + PP.Diag(PragmaLocation, DiagID) << PragmaName;
> + SkipUntil(tok::eof);
> + PP.Lex(Tok); // tok::eof
> + }
> +}
> +
> +unsigned Parser::HandlePragmaMSSection(llvm::StringRef PragmaName,
> + SourceLocation PragmaLocation) {
This could use Parser tests.
> + BalancedDelimiterTracker Parens(*this, tok::l_paren, tok::eof);
> + if (Parens.consumeOpen())
> + return diag::warn_pragma_expected_lparen;
> + // Parsing code for pragma section
> + if (Tok.isNot(tok::string_literal))
Should you be calling Parser::isTokenStringLiteral() here instead? If
not, there should be a test that shows wide string literals fail to
parse.
> + return diag::warn_pragma_expected_string;
> + StringLiteral *SegmentName =
> + cast<StringLiteral>(ParseStringLiteralExpression().get());
> + int SectionFlags = 0;
> + while (Tok.is(tok::comma)) {
> + PP.Lex(Tok); // ,
> + if (!Tok.isAnyIdentifier())
> + return diag::warn_pragma_invalid_action;
> + Sema::PragmaSectionFlag Flag =
> + llvm::StringSwitch<Sema::PragmaSectionFlag>(
> + Tok.getIdentifierInfo()->getName())
> + .Case("read", Sema::PSF_Read)
> + .Case("write", Sema::PSF_Write)
> + .Case("execute", Sema::PSF_Execute)
What about shared, nopage, nocache, discard and remove? I know they're
covered by returning PSF_None, but I would feel more comfortable
seeing them explicitly spelled out as case statements with a comment
saying that we don't handle them yet (otherwise, they look a bit
forgotten).
> + .Default(Sema::PSF_None);
> + if (Flag == Sema::PSF_None)
> + return diag::warn_pragma_invalid_action;
> + PP.Lex(Tok); // Identifier
> + SectionFlags |= Flag;
> + }
> + if (Parens.consumeClose())
> + return diag::warn_pragma_expected_rparen;
> + if (Tok.isNot(tok::eof))
> + return diag::warn_pragma_extra_tokens_at_eol;
> + PP.Lex(Tok); // eof
Shouldn't this be tok::eod instead of tok::eof?
> + Actions.ActOnPragmaMSSection(PragmaLocation, SectionFlags, SegmentName);
> + return 0;
> +}
> +
> +unsigned Parser::HandlePragmaMSXXXXSeg(llvm::StringRef PragmaName,
> + SourceLocation PragmaLocation) {
This could use Parser tests.
> + BalancedDelimiterTracker Parens(*this, tok::l_paren, tok::eof);
> + if (Parens.consumeOpen())
> + return diag::warn_pragma_expected_lparen;
> + Sema::PragmaMsStackAction Action = Sema::PSK_Reset;
> + llvm::StringRef SlotLabel;
> + if (Tok.isAnyIdentifier()) {
> + llvm::StringRef PushPop = Tok.getIdentifierInfo()->getName();
> + if (PushPop == "push")
> + Action = Sema::PSK_Push;
> + else if (PushPop == "pop")
> + Action = Sema::PSK_Pop;
> + else
> + return diag::warn_pragma_invalid_action;
> + if (Action != Sema::PSK_Reset) {
> + PP.Lex(Tok); // push | pop
> + if (Tok.is(tok::comma)) {
> + PP.Lex(Tok); // ,
> + // If we've got a comma, we either need a
I think you forgot to finish that sentence. ;-)
> + if (Tok.isAnyIdentifier()) {
> + SlotLabel = Tok.getIdentifierInfo()->getName();
> + PP.Lex(Tok); // identifier
> + if (Tok.is(tok::comma))
> + PP.Lex(Tok);
> + else if (Tok.isNot(tok::r_paren))
> + return diag::warn_pragma_expected_punc;
> + }
> + }
> + else if (Tok.isNot(tok::r_paren))
Should be on the same line as the closing curly brace.
> + return diag::warn_pragma_expected_punc;
> + }
> + }
> + // Grab the string literal for our section name.
> + StringLiteral *SegmentName = nullptr;
> + if (Tok.isNot(tok::r_paren)) {
> + if (Tok.isNot(tok::string_literal))
Same question about string literals here as above.
> + return diag::warn_pragma_expected_string;
> + SegmentName = cast<StringLiteral>(ParseStringLiteralExpression().get());
> + // Setting section "" has no effect
> + if (SegmentName->getLength())
> + Action = (Sema::PragmaMsStackAction)(Action | Sema::PSK_Set);
> + }
> + if (Parens.consumeClose())
> + return diag::warn_pragma_expected_rparen;
> + if (Tok.isNot(tok::eof))
> + return diag::warn_pragma_extra_tokens_at_eol;
> + PP.Lex(Tok); // eof
tok::eod question here, as above.
> + Actions.ActOnPragmaMSSeg(PragmaLocation, Action, SlotLabel,
> + SegmentName, PragmaName);
> + return 0;
> +}
> +
> // #pragma GCC visibility comes in two variants:
> // 'push' '(' [visibility] ')'
> // 'pop'
> @@ -1204,6 +1345,33 @@
> PP.EnterToken(AnnotTok);
> }
>
> +/// \brief Handle all MS pragmas. Simply forwards the tokens after inserting
> +/// an annotation token.
> +void PragmaMSPragma::HandlePragma(Preprocessor &PP,
> + PragmaIntroducerKind Introducer,
> + Token &Tok) {
> + Token EoF, AnnotTok;
> + EoF.startToken();
> + EoF.setKind(tok::eof);
> + AnnotTok.startToken();
> + AnnotTok.setKind(tok::annot_pragma_ms_pragma);
> + AnnotTok.setLocation(Tok.getLocation());
> + SmallVector<Token, 8> TokenVector;
> + // Suck up all of the tokens before the eod.
> + for (; Tok.isNot(tok::eod); PP.Lex(Tok))
> + TokenVector.push_back(Tok);
> + // Add a sentinal EoF token to the end of the list.
> + TokenVector.push_back(EoF);
I think this is usually eod for pragmas, isn't it?
> + // We must allocate this array with new because EnterTokenStream is going to
> + // delete it later.
> + Token *TokenArray = new Token[TokenVector.size()];
> + memcpy(TokenArray, TokenVector.data(), sizeof(Token) * TokenVector.size());
Bikeshed: std::copy instead of memcpy since Token is used more like a
class than just a struct?
> + auto Value = new (PP.getPreprocessorAllocator())
> + std::pair<Token*, size_t>(std::make_pair(TokenArray, TokenVector.size()));
> + AnnotTok.setAnnotationValue(Value);
> + PP.EnterToken(AnnotTok);
> +}
> +
> /// \brief Handle the Microsoft \#pragma detect_mismatch extension.
> ///
> /// The syntax is:
> Index: lib/Parse/ParseStmt.cpp
> ===================================================================
> --- lib/Parse/ParseStmt.cpp
> +++ lib/Parse/ParseStmt.cpp
> @@ -350,6 +350,10 @@
> HandlePragmaMSPointersToMembers();
> return StmtEmpty();
>
> + case tok::annot_pragma_ms_pragma:
> + ProhibitAttributes(Attrs);
> + HandlePragmaMSPragma();
> + return StmtEmpty();
> }
>
> // If we reached this code, the statement must end in a semicolon.
> @@ -828,6 +832,9 @@
> case tok::annot_pragma_ms_pointers_to_members:
> HandlePragmaMSPointersToMembers();
> break;
> + case tok::annot_pragma_ms_pragma:
> + HandlePragmaMSPragma();
> + break;
> default:
> checkForPragmas = false;
> break;
> Index: lib/Parse/Parser.cpp
> ===================================================================
> --- lib/Parse/Parser.cpp
> +++ lib/Parse/Parser.cpp
> @@ -631,6 +631,9 @@
> case tok::annot_pragma_ms_vtordisp:
> HandlePragmaMSVtorDisp();
> return DeclGroupPtrTy();
> + case tok::annot_pragma_ms_pragma:
> + HandlePragmaMSPragma();
> + return DeclGroupPtrTy();
> case tok::semi:
> // Either a C++11 empty-declaration or attribute-declaration.
> SingleDecl = Actions.ActOnEmptyDeclaration(getCurScope(),
> Index: lib/Sema/Sema.cpp
> ===================================================================
> --- lib/Sema/Sema.cpp
> +++ lib/Sema/Sema.cpp
> @@ -79,7 +79,8 @@
> MSPointerToMemberRepresentationMethod(
> LangOpts.getMSPointerToMemberRepresentationMethod()),
> VtorDispModeStack(1, MSVtorDispAttr::Mode(LangOpts.VtorDispMode)),
> - VisContext(0),
> + DataSegStack(nullptr), BSSSegStack(nullptr), ConstSegStack(nullptr),
> + CodeSegStack(nullptr), VisContext(0),
> IsBuildingRecoveryCallExpr(false),
> ExprNeedsCleanups(false), LateTemplateParser(0), OpaqueParser(0),
> IdResolver(pp), StdInitializerList(0), CXXTypeInfoDecl(0), MSVCGuidDecl(0),
> Index: lib/Sema/SemaAttr.cpp
> ===================================================================
> --- lib/Sema/SemaAttr.cpp
> +++ lib/Sema/SemaAttr.cpp
> @@ -325,6 +325,127 @@
> }
> }
>
> +template<typename Type> Type
> +PopToName(std::vector<llvm::StringRef, Type>& Stack, llvm::StringRef Key) {
> + if (!ActionTarget.empty()) {
> + auto I = std::find_if(DataSegStack.rbegin(), DataSegStack.rend(),
> + [&](const std::pair<llvm::StringRef, StringLiteral *> &x) {
Why not capture just ActionTarget?
> + return x.first == ActionTarget;
> + });
> + CurrentDataSegment = I->second;
> + if (I != DataSegStack.rend())
> + DataSegStack.erase(std::prev(I.base()), DataSegStack.end());
> + }
> + else if (!DataSegStack.empty()) {
Should be on the same line as the closing curly brace.
> + CurrentDataSegment = DataSegStack.back().second;
> + DataSegStack.pop_back();
> + }
> +}
> +
> +template<typename ValueType>
> +void Sema::PragmaStack<ValueType>::Act(SourceLocation PragmaLocation,
> + PragmaMsStackAction Action,
> + llvm::StringRef StackSlotLabel,
> + ValueType Value) {
> + if (Action == PSK_Reset) {
> + CurrentValue = nullptr;
> + return;
> + }
> + if (Action & PSK_Push)
> + Stack.push_back({StackSlotLabel, CurrentValue, CurrentPragmaLocation});
This will break MSVC 2012.
> + else if (Action & PSK_Pop) {
> + if (!StackSlotLabel.empty()) {
> + // If we've got a label, try to find it and jump there.
> + auto I = std::find_if(Stack.rbegin(), Stack.rend(),
> + [&](const Slot &x) { return x.StackSlotLabel == StackSlotLabel; });
Why not capture just StackSlotLabel?
> + // If we found the label so pop from there.
> + if (I != Stack.rend()) {
> + CurrentValue = I->Value;
> + CurrentPragmaLocation = I->PragmaLocation;
> + Stack.erase(std::prev(I.base()), Stack.end());
> + }
> + } else if (!Stack.empty()) {
> + // We don't have a label, just pop the last entry.
> + CurrentValue = Stack.back().Value;
> + CurrentPragmaLocation = Stack.back().PragmaLocation;
> + Stack.pop_back();
> + }
> + }
> + if (Action & PSK_Set) {
> + CurrentValue = Value;
> + CurrentPragmaLocation = PragmaLocation;
> + }
> +}
> +
> +bool Sema::UnifySection(const StringRef& SectionName,
> + int SectionFlags,
> + DeclaratorDecl* Decl) {
> + auto Section = SectionInfos.find(SectionName);
> + if (Section == SectionInfos.end()) {
> + SectionInfos[SectionName] = { Decl, SourceLocation(), SectionFlags };
This will break MSVC 2012.
> + return false;
> + }
> + // A pre-declared section takes precedence w/o diagnostic
Missing period at the end of the comment.
> + if (Section->second.SectionFlags == SectionFlags ||
> + !(Section->second.SectionFlags & PSF_Implicit))
> + return false;
> + auto OtherDecl = Section->second.Decl;
> + Diag(Decl->getLocation(), diag::err_section_conflict)
> + << Decl->getName() << OtherDecl->getName();
> + Diag(OtherDecl->getLocation(), diag::note_declared_at)
> + << OtherDecl->getName();
No need to call getName() -- the diagnostics engine already
understands how to print NamedDecls.
> + if (auto attr = Decl->getAttr<SectionAttr>())
Should be something like "A" instead of "attr" (can't use Attr since
that's a class name).
> + if (attr->isImplicit())
> + Diag(attr->getLocation(), diag::note_pragma_entered_here);
> + if (auto attr = OtherDecl->getAttr<SectionAttr>())
Same here.
> + if (attr->isImplicit())
> + Diag(attr->getLocation(), diag::note_pragma_entered_here);
> + return false;
> +}
> +
> +bool Sema::UnifySection(const StringRef& SectionName,
> + int SectionFlags,
> + SourceLocation PragmaSectionLocation) {
> + auto Section = SectionInfos.find(SectionName);
> + if (Section != SectionInfos.end()) {
> + if (Section->second.SectionFlags == SectionFlags)
> + return false;
> + if (!(Section->second.SectionFlags & PSF_Implicit)) {
> + Diag(PragmaSectionLocation, diag::err_section_conflict)
> + << "this" << "a prior #pragma section";
> + Diag(Section->second.PragmaSectionLocation,
> + diag::note_pragma_entered_here);
> + return true;
> + }
> + }
> + SectionInfos[SectionName] = {nullptr, PragmaSectionLocation, SectionFlags};
This will break MSVC 2012.
> + return false;
> +}
> +
> +/// \brief Called on well formed \#pragma bss_seg().
> +void Sema::ActOnPragmaMSSeg(SourceLocation PragmaLocation,
> + PragmaMsStackAction Action,
> + llvm::StringRef StackSlotLabel,
> + StringLiteral *SegmentName,
> + llvm::StringRef PragmaName) {
> + PragmaStack<StringLiteral *> *Stack =
> + llvm::StringSwitch<PragmaStack<StringLiteral *> *>(PragmaName)
> + .Case("data_seg", &DataSegStack)
> + .Case("bss_seg", &BSSSegStack)
> + .Case("const_seg", &ConstSegStack)
> + .Case("code_seg", &CodeSegStack);
> + if (Action & PSK_Pop && Stack->Stack.empty())
> + Diag(PragmaLocation, diag::warn_pragma_pop_failed) << PragmaName
> + << "stack empty";
Formatting.
> + Stack->Act(PragmaLocation, Action, StackSlotLabel, SegmentName);
> +}
> +
> +/// \brief Called on well formed \#pragma bss_seg().
> +void Sema::ActOnPragmaMSSection(SourceLocation PragmaLocation,
> + int SectionFlags, StringLiteral *SegmentName) {
> + UnifySection(SegmentName->getString(), SectionFlags, PragmaLocation);
> +}
> +
> void Sema::ActOnPragmaUnused(const Token &IdTok, Scope *curScope,
> SourceLocation PragmaLoc) {
>
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -6974,6 +6974,17 @@
> NewFD->setInvalidDecl();
> }
>
> + if (D.isFunctionDefinition() && CodeSegStack.CurrentValue &&
> + !NewFD->hasAttr<SectionAttr>()) {
> + NewFD->addAttr(SectionAttr::CreateImplicit(Context,
> + SectionAttr::Declspec_allocate,
> + CodeSegStack.CurrentValue->getString(),
> + CodeSegStack.CurrentPragmaLocation));
> + if (UnifySection(CodeSegStack.CurrentValue->getString(),
> + PSF_Implicit | PSF_Execute | PSF_Read, NewFD))
> + NewFD->dropAttr<SectionAttr>();
I think it might make more sense to only add the attribute if
UnifySection returns false, instead of adding it unilaterally, and
then dropping it.
> + }
> +
> // Handle attributes.
> ProcessDeclAttributes(S, NewFD, D);
>
> @@ -8838,6 +8849,30 @@
> Diag(var->getLocation(), diag::note_use_thread_local);
> }
>
> + if (var->isThisDeclarationADefinition() &&
> + ActiveTemplateInstantiations.empty()) {
> + PragmaStack<StringLiteral *> *Stack = nullptr;
> + int SectionFlags = PSF_Implicit | PSF_Read;
> + if (var->getType().isConstQualified())
> + Stack = &ConstSegStack;
> + else if (!var->getInit()) {
> + Stack = &BSSSegStack;
> + SectionFlags |= PSF_Write;
> + } else {
> + Stack = &DataSegStack;
> + SectionFlags |= PSF_Write;
> + }
> + if (!var->hasAttr<SectionAttr>() && Stack->CurrentValue) {
> + var->addAttr(SectionAttr::CreateImplicit(
> + Context, SectionAttr::Declspec_allocate,
> + Stack->CurrentValue->getString(),
> + Stack->CurrentPragmaLocation));
> + }
The curly braces can be elided (plus, there's an identation issue with
the closing brace).
> + if (const SectionAttr *SA = var->getAttr<SectionAttr>())
> + if (UnifySection(SA->getName(), SectionFlags, var))
> + var->dropAttr<SectionAttr>();
Same comment about add/drop attribute as above.
> + }
> +
> // All the following checks are C++ only.
> if (!getLangOpts().CPlusPlus) return;
>
> Index: lib/Sema/SemaObjCProperty.cpp
> ===================================================================
> --- lib/Sema/SemaObjCProperty.cpp
> +++ lib/Sema/SemaObjCProperty.cpp
> @@ -1969,8 +1969,9 @@
> ObjCReturnsInnerPointerAttr::CreateImplicit(Context, Loc));
>
> if (const SectionAttr *SA = property->getAttr<SectionAttr>())
> - GetterMethod->addAttr(SectionAttr::CreateImplicit(Context, SA->getName(),
> - Loc));
> + GetterMethod->addAttr(
> + SectionAttr::CreateImplicit(Context, SectionAttr::GNU_section,
> + SA->getName(), Loc));
>
> if (getLangOpts().ObjCAutoRefCount)
> CheckARCMethodDecl(GetterMethod);
> @@ -2022,8 +2023,9 @@
> if (lexicalDC)
> SetterMethod->setLexicalDeclContext(lexicalDC);
> if (const SectionAttr *SA = property->getAttr<SectionAttr>())
> - SetterMethod->addAttr(SectionAttr::CreateImplicit(Context,
> - SA->getName(), Loc));
> + SetterMethod->addAttr(
> + SectionAttr::CreateImplicit(Context, SectionAttr::GNU_section,
> + SA->getName(), Loc));
> // It's possible for the user to have set a very odd custom
> // setter selector that causes it to have a method family.
> if (getLangOpts().ObjCAutoRefCount)
> Index: test/CodeGen/function-sections.c
> ===================================================================
> --- /dev/null
> +++ test/CodeGen/function-sections.c
> @@ -0,0 +1,28 @@
> +// REQUIRES: x86-registered-target
> +
> +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -o - < %s | FileCheck %s --check-prefix=PLAIN
> +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -ffunction-sections -fno-function-sections -o - < %s | FileCheck %s --check-prefix=PLAIN
> +
> +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -ffunction-sections -o - < %s | FileCheck %s --check-prefix=FUNC_SECT
> +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fno-function-sections -ffunction-sections -o - < %s | FileCheck %s --check-prefix=FUNC_SECT
> +
> +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fdata-sections -o - < %s | FileCheck %s --check-prefix=DATA_SECT
> +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fno-data-sections -fdata-sections -o - < %s | FileCheck %s --check-prefix=DATA_SECT
> +
> +const int hello = 123;
> +void world() {}
> +
> +// PLAIN-NOT: section
> +// PLAIN: world:
> +// PLAIN: section .rodata,
> +// PLAIN: hello:
> +
> +// FUNC_SECT: section .text.world,
> +// FUNC_SECT: world:
> +// FUNC_SECT: section .rodata,
> +// FUNC_SECT: hello:
> +
> +// DATA_SECT-NOT: section
> +// DATA_SECT: world:
> +// DATA_SECT: .section .rodata.hello,
> +// DATA_SECT: hello:
> Index: test/CodeGen/sections.c
> ===================================================================
> --- test/CodeGen/sections.c
> +++ test/CodeGen/sections.c
> @@ -1,28 +1,51 @@
> -// REQUIRES: x86-registered-target
> +// RUN: %clang_cc1 -emit-llvm -fms-extensions -xc++ -o - < %s | FileCheck %s
>
> -// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -o - < %s | FileCheck %s --check-prefix=PLAIN
> -// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -ffunction-sections -fno-function-sections -o - < %s | FileCheck %s --check-prefix=PLAIN
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +#pragma const_seg(".my_const")
> +#pragma bss_seg(".my_bss")
> +int D = 1;
> +#pragma data_seg(".data")
> +int a = 1;
> +#pragma data_seg(push, label, ".data2")
> +extern const int b;
> +const int b = 1;
> +const char* s = "my string!";
> +#pragma data_seg(push, ".my_seg")
> +int c = 1;
> +#pragma data_seg(pop, label)
> +int d = 1;
> +int e;
> +#pragma bss_seg(".c")
> +int f;
> +void g(void){}
> +#pragma code_seg(".my_code")
> +void h(void){}
> +#pragma bss_seg()
> +int i;
> +#pragma bss_seg(".bss1")
> +#pragma bss_seg(push, test, ".bss2")
> +#pragma bss_seg()
> +#pragma bss_seg()
> +int TEST1;
> +#pragma bss_seg(pop)
> +int TEST2;
> +#ifdef __cplusplus
> +}
> +#endif
>
> -// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -ffunction-sections -o - < %s | FileCheck %s --check-prefix=FUNC_SECT
> -// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fno-function-sections -ffunction-sections -o - < %s | FileCheck %s --check-prefix=FUNC_SECT
> -
> -// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fdata-sections -o - < %s | FileCheck %s --check-prefix=DATA_SECT
> -// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fno-data-sections -fdata-sections -o - < %s | FileCheck %s --check-prefix=DATA_SECT
> -
> -const int hello = 123;
> -void world() {}
> -
> -// PLAIN-NOT: section
> -// PLAIN: world:
> -// PLAIN: section .rodata,
> -// PLAIN: hello:
> -
> -// FUNC_SECT: section .text.world,
> -// FUNC_SECT: world:
> -// FUNC_SECT: section .rodata,
> -// FUNC_SECT: hello:
> -
> -// DATA_SECT-NOT: section
> -// DATA_SECT: world:
> -// DATA_SECT: .section .rodata.hello,
> -// DATA_SECT: hello:
> +//CHECK: @D = global i32 1
> +//CHECK: @a = global i32 1, section ".data"
> +//CHECK: @b = constant i32 1, section ".my_const"
> +//CHECK: @[[MYSTR:.*]] = linkonce_odr unnamed_addr constant [11 x i8] c"my string!\00"
> +//CHECK: @s = global i8* getelementptr inbounds ([11 x i8]* @[[MYSTR]], i32 0, i32 0), section ".data2"
> +//CHECK: @c = global i32 1, section ".my_seg"
> +//CHECK: @d = global i32 1, section ".data"
> +//CHECK: @e = global i32 0, section ".my_bss"
> +//CHECK: @f = global i32 0, section ".c"
> +//CHECK: @i = global i32 0
> +//CHECK: @TEST1 = global i32 0
> +//CHECK: @TEST2 = global i32 0, section ".bss1"
> +//CHECK: define void @g()
> +//CHECK: define void @h() {{.*}} section ".my_code"
> Index: test/Sema/pragma-section.c
> ===================================================================
> --- /dev/null
> +++ test/Sema/pragma-section.c
> @@ -0,0 +1,19 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s
> +
> +#pragma const_seg(".my_const") // expected-note 2 {{#pragma entered here}}
> +extern const int a;
> +const int a = 1; // expected-note 2 {{declared here}}
> +#pragma data_seg(".my_const") // expected-note {{#pragma entered here}}
> +int b = 1; // expected-error {{b causes a section type conflict with a}}
> +#pragma data_seg()
> +int c = 1;
> +__declspec(allocate(".my_const")) int d = 1; // expected-error {{d causes a section type conflict with a}}
> +
> +#pragma section(".my_seg", execute)
> +__declspec(allocate(".my_seg")) int int_my_seg;
> +#pragma code_seg(".my_seg")
> +void fn_my_seg(void){}
> +
> +__declspec(allocate(".bad_seg")) int int_bad_seg = 1; // expected-note {{declared here}}
> +#pragma code_seg(".bad_seg") // expected-note {{#pragma entered here}}
> +void fn_bad_seg(void){} // expected-error {{fn_bad_seg causes a section type conflict with int_bad_seg}}
>
Thanks for working on this!
~Aaron
On Wed, Apr 2, 2014 at 9:21 PM, Warren Hunt <whunt at google.com> wrote:
> A major overhaul. The patch now considers __attribute__((section())), __declspec(allocate()) and #pragma section. I've also added gcc style diagnostics for section conflicts (a behavior clang lacked for __attribute__((section()))). I've added test cases for both Sema and CodeGen. The CodeGen test case reuses an old file-name and moves its contents to a more appropriately named test.
>
> Note: There is one known outstanding issue with the patch: In C mode, due to tentative declarations, we don't actually process uninitialized variables until the end of the TU, which means they get stuck in whatever section the last #pragma bss_seg() dictates, rather than the one that was active when they were defined.
>
> Hi rsmith, majnemer, rnk, aaron.ballman,
>
> http://llvm-reviews.chandlerc.com/D3065
>
> CHANGE SINCE LAST DIFF
> http://llvm-reviews.chandlerc.com/D3065?vs=8116&id=8320#toc
>
> Files:
> include/clang/Basic/Attr.td
> include/clang/Basic/DiagnosticParseKinds.td
> include/clang/Basic/DiagnosticSemaKinds.td
> include/clang/Basic/TokenKinds.def
> include/clang/Parse/Parser.h
> include/clang/Sema/Sema.h
> lib/Parse/ParseDeclCXX.cpp
> lib/Parse/ParsePragma.cpp
> lib/Parse/ParseStmt.cpp
> lib/Parse/Parser.cpp
> lib/Sema/Sema.cpp
> lib/Sema/SemaAttr.cpp
> lib/Sema/SemaDecl.cpp
> lib/Sema/SemaObjCProperty.cpp
> test/CodeGen/function-sections.c
> test/CodeGen/sections.c
> test/Sema/pragma-section.c
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list