pratlucas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Conflicting types for the same section name defined in clang section
pragmas and GNU-style section attributes were not properly captured by
Clang's Sema. The lack of diagnostics was caused by the fact the section
specification coming from attributes was handled by Sema as implicit,
even though explicitly defined by the user.

This patch enables the diagnostics for section type conflicts between
those specifications by making sure sections defined in section
attributes are correctly handled as explicit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78573

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/pragma-clang-section.c

Index: clang/test/Sema/pragma-clang-section.c
===================================================================
--- clang/test/Sema/pragma-clang-section.c
+++ clang/test/Sema/pragma-clang-section.c
@@ -22,4 +22,9 @@
 #pragma clang section bss="myrodata.1" // expected-error {{this causes a section type conflict with a prior #pragma section}}
 #pragma clang section text="mybss.3" // expected-error {{this causes a section type conflict with a prior #pragma section}}
 
+#pragma clang section rodata="myrodata.4" // expected-note {{#pragma entered here}}
+int x __attribute__((section("myrodata.4"))); // expected-error {{'x' causes a section type conflict with a prior #pragma section}}
+const int y __attribute__((section("myrodata.5"))) = 10; // expected-note {{declared here}}
+#pragma clang section data="myrodata.5" // expected-error {{this causes a section type conflict with 'y'}}
+
 int a;
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12736,7 +12736,7 @@
   if (GlobalStorage && var->isThisDeclarationADefinition() &&
       !inTemplateInstantiation()) {
     PragmaStack<StringLiteral *> *Stack = nullptr;
-    int SectionFlags = ASTContext::PSF_Implicit | ASTContext::PSF_Read;
+    int SectionFlags = ASTContext::PSF_Read;
     if (var->getType().isConstQualified())
       Stack = &ConstSegStack;
     else if (!var->getInit()) {
@@ -12746,14 +12746,19 @@
       Stack = &DataSegStack;
       SectionFlags |= ASTContext::PSF_Write;
     }
-    if (Stack->CurrentValue && !var->hasAttr<SectionAttr>())
+    if (const SectionAttr *SA = var->getAttr<SectionAttr>()) {
+      if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec)
+        SectionFlags |= ASTContext::PSF_Implicit;
+      UnifySection(SA->getName(), SectionFlags, var);
+    } else if (Stack->CurrentValue) {
+      SectionFlags |= ASTContext::PSF_Implicit;
+      auto SectionName = Stack->CurrentValue->getString();
       var->addAttr(SectionAttr::CreateImplicit(
-          Context, Stack->CurrentValue->getString(),
-          Stack->CurrentPragmaLocation, AttributeCommonInfo::AS_Pragma,
-          SectionAttr::Declspec_allocate));
-    if (const SectionAttr *SA = var->getAttr<SectionAttr>())
-      if (UnifySection(SA->getName(), SectionFlags, var))
+          Context, SectionName, Stack->CurrentPragmaLocation,
+          AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+      if (UnifySection(SectionName, SectionFlags, var))
         var->dropAttr<SectionAttr>();
+    }
 
     // Apply the init_seg attribute if this has an initializer.  If the
     // initializer turns out to not be dynamic, we'll end up ignoring this
Index: clang/lib/Sema/SemaAttr.cpp
===================================================================
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -471,42 +471,50 @@
 bool Sema::UnifySection(StringRef SectionName,
                         int SectionFlags,
                         DeclaratorDecl *Decl) {
-  auto Section = Context.SectionInfos.find(SectionName);
-  if (Section == Context.SectionInfos.end()) {
+  SourceLocation PragmaLocation;
+  if (auto A = Decl->getAttr<SectionAttr>())
+    if (A->isImplicit())
+      PragmaLocation = A->getLocation();
+  auto SectionIt = Context.SectionInfos.find(SectionName);
+  if (SectionIt == Context.SectionInfos.end()) {
     Context.SectionInfos[SectionName] =
-        ASTContext::SectionInfo(Decl, SourceLocation(), SectionFlags);
+        ASTContext::SectionInfo(Decl, PragmaLocation, SectionFlags);
     return false;
   }
   // A pre-declared section takes precedence w/o diagnostic.
-  if (Section->second.SectionFlags == SectionFlags ||
-      !(Section->second.SectionFlags & ASTContext::PSF_Implicit))
+  const auto &Section = SectionIt->second;
+  if (Section.SectionFlags == SectionFlags ||
+      ((SectionFlags & ASTContext::PSF_Implicit) &&
+       !(Section.SectionFlags & ASTContext::PSF_Implicit)))
     return false;
-  auto OtherDecl = Section->second.Decl;
   Diag(Decl->getLocation(), diag::err_section_conflict)
-      << Decl << OtherDecl;
-  Diag(OtherDecl->getLocation(), diag::note_declared_at)
-      << OtherDecl->getName();
-  if (auto A = Decl->getAttr<SectionAttr>())
-    if (A->isImplicit())
-      Diag(A->getLocation(), diag::note_pragma_entered_here);
-  if (auto A = OtherDecl->getAttr<SectionAttr>())
-    if (A->isImplicit())
-      Diag(A->getLocation(), diag::note_pragma_entered_here);
+      << Decl << Section;
+  if (Section.Decl)
+    Diag(Section.Decl->getLocation(), diag::note_declared_at)
+        << Section.Decl->getName();
+  if (PragmaLocation.isValid())
+      Diag(PragmaLocation, diag::note_pragma_entered_here);
+  if (Section.PragmaSectionLocation.isValid())
+      Diag(Section.PragmaSectionLocation, diag::note_pragma_entered_here);
   return true;
 }
 
 bool Sema::UnifySection(StringRef SectionName,
                         int SectionFlags,
                         SourceLocation PragmaSectionLocation) {
-  auto Section = Context.SectionInfos.find(SectionName);
-  if (Section != Context.SectionInfos.end()) {
-    if (Section->second.SectionFlags == SectionFlags)
+  auto SectionIt = Context.SectionInfos.find(SectionName);
+  if (SectionIt != Context.SectionInfos.end()) {
+    const auto &Section = SectionIt->second;
+    if (Section.SectionFlags == SectionFlags)
       return false;
-    if (!(Section->second.SectionFlags & ASTContext::PSF_Implicit)) {
+    if (!(Section.SectionFlags & ASTContext::PSF_Implicit)) {
       Diag(PragmaSectionLocation, diag::err_section_conflict)
-          << "this" << "a prior #pragma section";
-      Diag(Section->second.PragmaSectionLocation,
-           diag::note_pragma_entered_here);
+          << "this" << Section.getSectionReference();
+      if (Section.Decl)
+        Diag(Section.Decl->getLocation(), diag::note_declared_at)
+            << Section.Decl->getName();
+      if (Section.PragmaSectionLocation.isValid())
+        Diag(Section.PragmaSectionLocation, diag::note_pragma_entered_here);
       return true;
     }
   }
Index: clang/include/clang/AST/ASTContext.h
===================================================================
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -2961,6 +2961,12 @@
                 int SectionFlags)
         : Decl(Decl), PragmaSectionLocation(PragmaSectionLocation),
           SectionFlags(SectionFlags) {}
+    std::string getSectionReference() const {
+      if (Decl)
+        return "'" + Decl->getName().str() + "'";
+      else
+        return "a prior #pragma section";
+    }
   };
 
   llvm::StringMap<SectionInfo> SectionInfos;
@@ -2974,6 +2980,14 @@
   SmallVector<OMPTraitInfo *, 4> OMPTraitInfoVector;
 };
 
+/// Insertion operator for diagnostics.
+inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
+                                           const ASTContext::SectionInfo& Section) {
+  if (Section.Decl)
+    return DB << Section.Decl;
+  return DB << "a prior #pragma section";
+}
+
 /// Utility function for constructing a nullary selector.
 inline Selector GetNullarySelector(StringRef name, ASTContext &Ctx) {
   IdentifierInfo* II = &Ctx.Idents.get(name);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to