r332470 - Add support for __declspec(code_seg("segname"))

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri May 18 13:03:46 PDT 2018


On 16 May 2018 at 06:57, Erich Keane via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: erichkeane
> Date: Wed May 16 06:57:17 2018
> New Revision: 332470
>
> URL: http://llvm.org/viewvc/llvm-project?rev=332470&view=rev
> Log:
> Add support for __declspec(code_seg("segname"))
>
> Add support for __declspec(code_seg("segname"))
>
> This patch is built on the existing support for #pragma code_seg. The
> code_seg
> declspec is allowed on functions and classes. The attribute enables the
> placement of code into separate named segments, including
> compiler-generated
> members and template instantiations.
>
> For more information, please see the following:
> https://msdn.microsoft.com/en-us/library/dn636922.aspx
>
> A new CodeSeg attribute is used instead of adding a new spelling to the
> existing
> Section attribute since they don’t apply to the same Subjects. Section
> attributes are also added for the code_seg declspec since they are used for
> #pragma code_seg. No CodeSeg attributes are added to the AST.
>

This approach really doesn't work. You're incorrectly applying the
__declspec(code_seg) restrictions to __attribute__((section)), regressing
our support for the latter. Also, code_seg has fundamentally different
semantics from the section attribute -- the former exists in part to
support paged addressing (which is why it has the virtual function
restriction) and the latter exists merely to control properties of the
object file. This approach also violates our policy of maintaining source
fidelity.

I'm going to revert this for now to fix the regression; when it comes back,
I think you should just use CodeSegAttr to represent __declspec(code_seg)
rather than trying to treat it as -- effectively -- a different spelling of
SectionAttr.


> The patch is written to match with the Microsoft compiler’s behavior even
> where
> that behavior is a little complicated (see https://reviews.llvm.org/D22931,
> the
> Microsoft feedback page is no longer available since MS has removed the
> page).
> That code is in getImplicitSectionAttrFromClass routine.
>
> Diagnostics messages are added to match with the Microsoft compiler for
> code-seg
> attribute mismatches on base and derived classes and virtual overrides.
>
>
> Differential Revision: https://reviews.llvm.org/D43352
>
> Modified:
>     cfe/trunk/include/clang/Basic/Attr.td
>     cfe/trunk/include/clang/Basic/AttrDocs.td
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/include/clang/Sema/Sema.h
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>     cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>     cfe/trunk/lib/Sema/SemaLambda.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/Attr.td?rev=332470&r1=332469&r2=332470&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Wed May 16 06:57:17 2018
> @@ -1769,6 +1769,13 @@ def Section : InheritableAttr {
>    let Documentation = [SectionDocs];
>  }
>
> +def CodeSeg : InheritableAttr {
> +  let Spellings = [Declspec<"code_seg">];
> +  let Args = [StringArgument<"Name">];
> +  let Subjects = SubjectList<[Function, CXXRecord], ErrorDiag>;
> +  let Documentation = [CodeSegDocs];
> +}
> +
>  def PragmaClangBSSSection : InheritableAttr {
>    // This attribute has no spellings as it is only ever created
> implicitly.
>    let Spellings = [];
>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/AttrDocs.td?rev=332470&r1=332469&r2=332470&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed May 16 06:57:17 2018
> @@ -306,6 +306,18 @@ An example of how to use ``alloc_size``
>    }];
>  }
>
> +def CodeSegDocs : Documentation {
> +  let Category = DocCatFunction;
> +  let Content = [{
> +The ``__declspec(code_seg)`` attribute enables the placement of code into
> separate
> +named segments that can be paged or locked in memory individually. This
> attribute
> +is used to control the placement of instantiated templates and
> compiler-generated
> +code. See the documentation for `__declspec(code_seg)`_ on MSDN.
> +
> +.. _`__declspec(code_seg)`: http://msdn.microsoft.com/en-
> us/library/dn636922.aspx
> +  }];
> +}
> +
>  def AllocAlignDocs : Documentation {
>    let Category = DocCatFunction;
>    let Content = [{
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> DiagnosticSemaKinds.td?rev=332470&r1=332469&r2=332470&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed May 16
> 06:57:17 2018
> @@ -2668,6 +2668,14 @@ def warn_mismatched_section : Warning<
>  def warn_attribute_section_on_redeclaration : Warning<
>    "section attribute is specified on redeclared variable">,
> InGroup<Section>;
>
> +def err_mismatched_code_seg_base : Error<
> +  "derived class must specify the same code segment as its base classes">;
> +def err_mismatched_code_seg_override : Error<
> +  "overriding virtual function must specify the same code segment as its
> overridden function">;
> +def err_conflicting_codeseg_attribute : Error<
> +  "conflicting code segment specifiers">;
> +def warn_duplicate_codeseg_attribute : Warning<
> +  "duplicate code segment specifiers">, InGroup<Section>;
>  def err_anonymous_property: Error<
>    "anonymous property is not supported">;
>  def err_property_is_variably_modified : Error<
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Sema/Sema.h?rev=332470&r1=332469&r2=332470&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Wed May 16 06:57:17 2018
> @@ -1934,6 +1934,7 @@ public:
>    bool shouldLinkDependentDeclWithPrevious(Decl *D, Decl *OldDecl);
>    void CheckMain(FunctionDecl *FD, const DeclSpec &D);
>    void CheckMSVCRTEntryPoint(FunctionDecl *FD);
> +  Attr *getImplicitSectionAttrForFunction(const FunctionDecl *FD, bool
> IsDefinition = true);
>    Decl *ActOnParamDeclarator(Scope *S, Declarator &D);
>    ParmVarDecl *BuildParmVarDeclForTypedef(DeclContext *DC,
>                                            SourceLocation Loc,
> @@ -5836,6 +5837,7 @@ public:
>    /// ensure that referenceDLLExportedClassMethods is called some point
> later
>    /// when all outer classes of Class are complete.
>    void checkClassLevelDLLAttribute(CXXRecordDecl *Class);
> +  void checkClassLevelSectionAttribute(CXXRecordDecl *Class);
>
>    void referenceDLLExportedClassMethods();
>
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDecl.cpp?rev=332470&r1=332469&r2=332470&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed May 16 06:57:17 2018
> @@ -2667,9 +2667,14 @@ void Sema::mergeDeclAttributes(NamedDecl
>          Diag(New->getLocation(), diag::warn_attribute_section_
> on_redeclaration);
>          Diag(Old->getLocation(), diag::note_previous_declaration);
>        }
> +    } else if (isa<CXXMethodDecl>(New)) {
> +      const auto *NewSA = New->getAttr<SectionAttr>();
> +      if (!NewSA->isImplicit()) {
> +        Diag(New->getLocation(), diag::warn_mismatched_section);
> +        Diag(Old->getLocation(), diag::note_previous_declaration);
> +      }
>

This, in particular, results in bogus warnings for __attribute__((section)).


>      }
>    }
> -
>    if (!Old->hasAttrs())
>      return;
>
> @@ -8716,18 +8721,18 @@ Sema::ActOnFunctionDeclarator(Scope *S,
>                                                   PragmaClangTextSection.
> PragmaLocation));
>    }
>
> -  // Apply an implicit SectionAttr if #pragma code_seg is active.
> -  if (CodeSegStack.CurrentValue && D.isFunctionDefinition() &&
> -      !NewFD->hasAttr<SectionAttr>()) {
> -    NewFD->addAttr(
> -        SectionAttr::CreateImplicit(Context, SectionAttr::Declspec_
> allocate,
> -                                    CodeSegStack.CurrentValue->
> getString(),
> -                                    CodeSegStack.CurrentPragmaLocation));
> -    if (UnifySection(CodeSegStack.CurrentValue->getString(),
> -                     ASTContext::PSF_Implicit | ASTContext::PSF_Execute |
> -                         ASTContext::PSF_Read,
> -                     NewFD))
> -      NewFD->dropAttr<SectionAttr>();
> +  // Apply an implicit SectionAttr from class declspec or from
> +  // #pragma code_seg if active.
> +  if (!NewFD->hasAttr<SectionAttr>()) {
> +    if (Attr *SAttr = getImplicitSectionAttrForFunction(NewFD,
> +
> D.isFunctionDefinition())) {
> +      NewFD->addAttr(SAttr);
> +      if (UnifySection(cast<SectionAttr>(SAttr)->getName(),
> +                       ASTContext::PSF_Implicit | ASTContext::PSF_Execute
> |
> +                       ASTContext::PSF_Read,
> +                       NewFD))
> +        NewFD->dropAttr<SectionAttr>();
> +    }
>    }
>
>    // Handle attributes.
> @@ -9177,6 +9182,64 @@ Sema::ActOnFunctionDeclarator(Scope *S,
>    return NewFD;
>  }
>
> +/// Return a SectionAttr from a containing class.  The Microsoft docs say
> +/// when __declspec(code_seg) "is applied to a class, all member
> functions of
> +/// the class and nested classes -- this includes compiler-generated
> special
> +/// member functions -- are put in the specified segment."
> +/// The actual behavior is a little more complicated. The Microsoft
> compiler
> +/// won't check outer classes if there is an active value from #pragma
> code_seg.
> +/// The section is always applied from the direct parent but only from
> outer
> +/// classes when the #pragma code_seg stack is empty. See:
> +/// https://reviews.llvm.org/D22931, the Microsoft feedback page is no
> longer
> +/// available since MS has removed the page.
> +static Attr *getImplicitSectionAttrFromClass(Sema &S, const FunctionDecl
> *FD) {
> +  const auto *Method = dyn_cast<CXXMethodDecl>(FD);
> +  if (!Method)
> +    return nullptr;
> +  const CXXRecordDecl *Parent = Method->getParent();
> +  if (const auto *SAttr = Parent->getAttr<SectionAttr>()) {
> +    Attr *NewAttr = SAttr->clone(S.getASTContext());
> +    NewAttr->setImplicit(true);
> +    return NewAttr;
> +  }
> +
> +  // The Microsoft compiler won't check outer classes for the section
> +  // when the #pragma code_seg stack is active.
> +  if (S.CodeSegStack.CurrentValue)
> +    return nullptr;
> +
> +  while ((Parent = dyn_cast<CXXRecordDecl>(Parent->getParent()))) {
> +    if (const auto *SAttr = Parent->getAttr<SectionAttr>()) {
> +      Attr *NewAttr = SAttr->clone(S.getASTContext());
> +      NewAttr->setImplicit(true);
> +      return NewAttr;
> +    }
> +  }
> +  return nullptr;
> +}
> +
> +/// \brief Returns an implicit SectionAttr for a function.
> +///
> +/// \param FD Function being declared.
> +/// \param IsDefinition Whether it is a definition or just a declarartion.
> +/// \returns A SectionAttr to apply to the function or nullptr if no
> +///          attribute should be added.
> +///
> +/// First tries to find a SectionAttr on a containing class (from
> +/// a __declspec(code_seg)).  If not found on the class, and if the
> function is
> +/// also a definition it will use the current #pragma code_seg value.
> +Attr *Sema::getImplicitSectionAttrForFunction(const FunctionDecl *FD,
> bool IsDefinition) {
> +  if (Attr *A = getImplicitSectionAttrFromClass(*this, FD))
> +    return A;
> +  if (IsDefinition && CodeSegStack.CurrentValue) {
> +    return SectionAttr::CreateImplicit(getASTContext(),
> +                                       SectionAttr::Declspec_allocate,
> +                                       CodeSegStack.CurrentValue->
> getString(),
> +                                       CodeSegStack.
> CurrentPragmaLocation);
> +  }
> +  return nullptr;
> +}
> +
>  /// Checks if the new declaration declared in dependent context must be
>  /// put in the same redeclaration chain as the specified declaration.
>  ///
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDeclAttr.cpp?rev=332470&r1=332469&r2=332470&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed May 16 06:57:17 2018
> @@ -2853,6 +2853,13 @@ static void handleVecTypeHint(Sema &S, D
>  SectionAttr *Sema::mergeSectionAttr(Decl *D, SourceRange Range,
>                                      StringRef Name,
>                                      unsigned AttrSpellingListIndex) {
> +  // Explicit or partial specializations do not inherit
> +  // the code_seg attribute from the primary template.
> +  if (const auto *FD = dyn_cast<FunctionDecl>(D)){
> +    if (FD->isFunctionTemplateSpecialization())
> +      return nullptr;
> +  }
> +
>    if (SectionAttr *ExistingAttr = D->getAttr<SectionAttr>()) {
>      if (ExistingAttr->getName() == Name)
>        return nullptr;
> @@ -2942,6 +2949,27 @@ static void handleTargetAttr(Sema &S, De
>    D->addAttr(NewAttr);
>  }
>
> +static void handleCodeSegAttr(Sema &S, Decl *D, const AttributeList &AL) {
> +  StringRef Str;
> +  SourceLocation LiteralLoc;
> +  if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &LiteralLoc))
> +    return;
> +  if (!S.checkSectionName(LiteralLoc, Str))
> +    return;
> +  if (const auto *ExistingAttr = D->getAttr<SectionAttr>()) {
> +    if (!ExistingAttr->isImplicit()) {
> +      S.Diag(AL.getLoc(),
> +             ExistingAttr->getName() == Str
> +             ? diag::warn_duplicate_codeseg_attribute
> +             : diag::err_conflicting_codeseg_attribute);
> +      return;
> +    }
> +    D->dropAttr<SectionAttr>();
> +  }
> +  D->addAttr(::new (S.Context) SectionAttr(
> +      AL.getRange(), S.Context, Str, AL.getAttributeSpellingListIndex(
> )));
> +}
> +
>  static void handleCleanupAttr(Sema &S, Decl *D, const AttributeList &AL) {
>    Expr *E = AL.getArgAsExpr(0);
>    SourceLocation Loc = E->getExprLoc();
> @@ -6297,6 +6325,9 @@ static void ProcessDeclAttribute(Sema &S
>    case AttributeList::AT_Uuid:
>      handleUuidAttr(S, D, AL);
>      break;
> +  case AttributeList::AT_CodeSeg:
> +    handleCodeSegAttr(S, D, AL);
> +    break;
>    case AttributeList::AT_MSInheritance:
>      handleMSInheritanceAttr(S, D, AL);
>      break;
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDeclCXX.cpp?rev=332470&r1=332469&r2=332470&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed May 16 06:57:17 2018
> @@ -2231,6 +2231,20 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *
>    CXXRecordDecl *CXXBaseDecl = cast<CXXRecordDecl>(BaseDecl);
>    assert(CXXBaseDecl && "Base type is not a C++ type");
>
> +  // Microsoft docs say:
> +  // "If a base-class has a code_seg attribute, derived classes must have
> the
> +  // same attribute."
> +   const auto *BaseSA = CXXBaseDecl->getAttr<SectionAttr>();
> +   const auto *DerivedSA = Class->getAttr<SectionAttr>();
> +   if (BaseSA || DerivedSA) {
> +     if (!BaseSA || !DerivedSA || BaseSA->getName() !=
> DerivedSA->getName()) {
> +       Diag(Class->getLocation(), diag::err_mismatched_code_seg_base);
> +       Diag(CXXBaseDecl->getLocation(), diag::note_base_class_
> specified_here)
> +         << CXXBaseDecl;
> +       return nullptr;
> +     }
> +   }
> +
>    // A class which contains a flexible array member is not suitable for
> use as a
>    // base class:
>    //   - If the layout determines that a base comes before another base,
> @@ -5576,6 +5590,16 @@ static void checkForMultipleExportedDefa
>    }
>  }
>
> +void Sema::checkClassLevelSectionAttribute(CXXRecordDecl *Class) {
> +  // Mark any compiler-generated routines with the implicit Section
> attribute.
> +  for (auto *Method : Class->methods()) {
> +    if (Method->isUserProvided())
> +      continue;
> +    if (Attr *A = getImplicitSectionAttrForFunction(Method))
> +      Method->addAttr(A);
> +  }
> +}
> +
>  /// Check class-level dllimport/dllexport attribute.
>  void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
>    Attr *ClassAttr = getDLLAttr(Class);
> @@ -6079,6 +6103,7 @@ void Sema::CheckCompletedCXXClass(CXXRec
>    }
>
>    checkClassLevelDLLAttribute(Record);
> +  checkClassLevelSectionAttribute(Record);
>
>    bool ClangABICompat4 =
>        Context.getLangOpts().getClangABICompat() <=
> LangOptions::ClangABI::Ver4;
> @@ -14531,6 +14556,16 @@ bool Sema::CheckOverridingFunctionAttrib
>               diag::note_overridden_marked_noescape);
>        }
>    }
> +  // Virtual overrides must have the same code_seg.
> +  const auto *OldSA = Old->getAttr<SectionAttr>();
> +  const auto *NewSA = New->getAttr<SectionAttr>();
> +  if (OldSA || NewSA) {
> +    if (!OldSA || !NewSA || NewSA->getName() != OldSA->getName()) {
> +      Diag(New->getLocation(), diag::err_mismatched_code_seg_override);
> +      Diag(Old->getLocation(), diag::note_previous_declaration);
> +      return true;
> +    }
> +  }
>
>    CallingConv NewCC = NewFT->getCallConv(), OldCC = OldFT->getCallConv();
>
>
> Modified: cfe/trunk/lib/Sema/SemaLambda.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaLambda.cpp?rev=332470&r1=332469&r2=332470&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaLambda.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaLambda.cpp Wed May 16 06:57:17 2018
> @@ -910,6 +910,10 @@ void Sema::ActOnStartOfLambdaDefinition(
>    AddRangeBasedOptnone(Method);
>
>    // Attributes on the lambda apply to the method.
> +  if (Attr *A = getImplicitSectionAttrForFunction(Method))
> +    Method->addAttr(A);
> +
> +  // Attributes on the lambda apply to the method.
>    ProcessDeclAttributes(CurScope, Method, ParamInfo);
>
>    // CUDA lambdas get implicit attributes based on the scope in which
> they're
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180518/c10a7a2a/attachment-0001.html>


More information about the cfe-commits mailing list