[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-05-07 Thread Lucas Prates via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d39df03a984: [Clang][Sema] Capturing section type conflicts 
between #pragma clang section… (authored by pratlucas).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

https://reviews.llvm.org/D78573

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  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,10 @@
 #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'}}
+const int z __attribute__((section("myrodata.6"))) = 11;
+
 int a;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12754,7 +12754,7 @@
   if (GlobalStorage && var->isThisDeclarationADefinition() &&
   !inTemplateInstantiation()) {
 PragmaStack *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Implicit | ASTContext::PSF_Read;
+int SectionFlags = ASTContext::PSF_Read;
 if (var->getType().isConstQualified())
   Stack = 
 else if (!var->getInit()) {
@@ -12764,14 +12764,19 @@
   Stack = 
   SectionFlags |= ASTContext::PSF_Write;
 }
-if (Stack->CurrentValue && !var->hasAttr())
+if (const SectionAttr *SA = var->getAttr()) {
+  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())
-  if (UnifySection(SA->getName(), SectionFlags, var))
+  Context, SectionName, Stack->CurrentPragmaLocation,
+  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+  if (UnifySection(SectionName, SectionFlags, var))
 var->dropAttr();
+}
 
 // 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
@@ -530,42 +530,49 @@
 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())
+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  = 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())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
-  if (auto A = OtherDecl->getAttr())
-

[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-05-07 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas updated this revision to Diff 262594.
pratlucas added a comment.

Addressing review comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

https://reviews.llvm.org/D78573

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  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,10 @@
 #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'}}
+const int z __attribute__((section("myrodata.6"))) = 11;
+
 int a;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12754,7 +12754,7 @@
   if (GlobalStorage && var->isThisDeclarationADefinition() &&
   !inTemplateInstantiation()) {
 PragmaStack *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Implicit | ASTContext::PSF_Read;
+int SectionFlags = ASTContext::PSF_Read;
 if (var->getType().isConstQualified())
   Stack = 
 else if (!var->getInit()) {
@@ -12764,14 +12764,19 @@
   Stack = 
   SectionFlags |= ASTContext::PSF_Write;
 }
-if (Stack->CurrentValue && !var->hasAttr())
+if (const SectionAttr *SA = var->getAttr()) {
+  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())
-  if (UnifySection(SA->getName(), SectionFlags, var))
+  Context, SectionName, Stack->CurrentPragmaLocation,
+  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+  if (UnifySection(SectionName, SectionFlags, var))
 var->dropAttr();
+}
 
 // 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
@@ -530,42 +530,49 @@
 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())
+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  = 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())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
-  if (auto A = OtherDecl->getAttr())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
+  

[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-05-05 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/include/clang/AST/ASTContext.h:3008
+/// Insertion operator for diagnostics.
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder ,

pratlucas wrote:
> rnk wrote:
> > It seems like there is no need for this to be defined inline, since it is 
> > presumably cold code.
> The `inline`'s purpose here is just to allow the definition in the header 
> file, avoiding multiple definition errors.
Yes, I am suggesting that the body be moved out of line, but this is optional. 
Less code in headers --> faster builds.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

https://reviews.llvm.org/D78573



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-05-05 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

https://reviews.llvm.org/D78573



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-04-28 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas updated this revision to Diff 260630.
pratlucas added a comment.

Updateing test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

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,10 @@
 #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'}}
+const int z __attribute__((section("myrodata.6"))) = 11;
+
 int a;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12754,7 +12754,7 @@
   if (GlobalStorage && var->isThisDeclarationADefinition() &&
   !inTemplateInstantiation()) {
 PragmaStack *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Implicit | ASTContext::PSF_Read;
+int SectionFlags = ASTContext::PSF_Read;
 if (var->getType().isConstQualified())
   Stack = 
 else if (!var->getInit()) {
@@ -12764,14 +12764,19 @@
   Stack = 
   SectionFlags |= ASTContext::PSF_Write;
 }
-if (Stack->CurrentValue && !var->hasAttr())
+if (const SectionAttr *SA = var->getAttr()) {
+  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())
-  if (UnifySection(SA->getName(), SectionFlags, var))
+  Context, SectionName, Stack->CurrentPragmaLocation,
+  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+  if (UnifySection(SectionName, SectionFlags, var))
 var->dropAttr();
+}
 
 // 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,49 @@
 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())
+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  = 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())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
-  if (auto A = OtherDecl->getAttr())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
+  Diag(Decl->getLocation(), diag::err_section_conflict) << Decl 

[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-04-28 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas marked 4 inline comments as done.
pratlucas added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:3008
+/// Insertion operator for diagnostics.
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder ,

rnk wrote:
> It seems like there is no need for this to be defined inline, since it is 
> presumably cold code.
The `inline`'s purpose here is just to allow the definition in the header file, 
avoiding multiple definition errors.



Comment at: clang/test/Sema/pragma-clang-section.c:28
+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'}}
+

rnk wrote:
> Please add a case like:
>   const int y __attribute__((section("myrodata.6"))) = 11;
> There should be no diagnostics in this case, and I expect myrodata.6 to 
> override the pragma, since it is more specific to the declaration.
I've added this case to the test.
The overriding behaviour is already checked by 
`clang/test/CodeGen/clang-sections-attribute.c`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

https://reviews.llvm.org/D78573



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-04-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:3008
+/// Insertion operator for diagnostics.
+inline const DiagnosticBuilder &
+operator<<(const DiagnosticBuilder ,

It seems like there is no need for this to be defined inline, since it is 
presumably cold code.



Comment at: clang/test/Sema/pragma-clang-section.c:28
+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'}}
+

Please add a case like:
  const int y __attribute__((section("myrodata.6"))) = 11;
There should be no diagnostics in this case, and I expect myrodata.6 to 
override the pragma, since it is more specific to the declaration.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

https://reviews.llvm.org/D78573



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-04-23 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas updated this revision to Diff 259594.
pratlucas added a comment.

Rebasing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

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
@@ -12746,7 +12746,7 @@
   if (GlobalStorage && var->isThisDeclarationADefinition() &&
   !inTemplateInstantiation()) {
 PragmaStack *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Implicit | ASTContext::PSF_Read;
+int SectionFlags = ASTContext::PSF_Read;
 if (var->getType().isConstQualified())
   Stack = 
 else if (!var->getInit()) {
@@ -12756,14 +12756,19 @@
   Stack = 
   SectionFlags |= ASTContext::PSF_Write;
 }
-if (Stack->CurrentValue && !var->hasAttr())
+if (const SectionAttr *SA = var->getAttr()) {
+  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())
-  if (UnifySection(SA->getName(), SectionFlags, var))
+  Context, SectionName, Stack->CurrentPragmaLocation,
+  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+  if (UnifySection(SectionName, SectionFlags, var))
 var->dropAttr();
+}
 
 // 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,49 @@
 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())
+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  = 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())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
-  if (auto A = OtherDecl->getAttr())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
+  Diag(Decl->getLocation(), diag::err_section_conflict) << Decl << Section;
+  if (Section.Decl)
+

[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-04-21 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas updated this revision to Diff 259048.
pratlucas added a comment.

Removing unnecessary function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

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 *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Implicit | ASTContext::PSF_Read;
+int SectionFlags = ASTContext::PSF_Read;
 if (var->getType().isConstQualified())
   Stack = 
 else if (!var->getInit()) {
@@ -12746,14 +12746,19 @@
   Stack = 
   SectionFlags |= ASTContext::PSF_Write;
 }
-if (Stack->CurrentValue && !var->hasAttr())
+if (const SectionAttr *SA = var->getAttr()) {
+  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())
-  if (UnifySection(SA->getName(), SectionFlags, var))
+  Context, SectionName, Stack->CurrentPragmaLocation,
+  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+  if (UnifySection(SectionName, SectionFlags, var))
 var->dropAttr();
+}
 
 // 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,49 @@
 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())
+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  = 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())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
-  if (auto A = OtherDecl->getAttr())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
+  Diag(Decl->getLocation(), diag::err_section_conflict) << Decl << Section;
+  if (Section.Decl)
+

[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-04-21 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas updated this revision to Diff 259040.
pratlucas added a comment.

Fixing "mising clang-format" messages.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78573/new/

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 *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Implicit | ASTContext::PSF_Read;
+int SectionFlags = ASTContext::PSF_Read;
 if (var->getType().isConstQualified())
   Stack = 
 else if (!var->getInit()) {
@@ -12746,14 +12746,19 @@
   Stack = 
   SectionFlags |= ASTContext::PSF_Write;
 }
-if (Stack->CurrentValue && !var->hasAttr())
+if (const SectionAttr *SA = var->getAttr()) {
+  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())
-  if (UnifySection(SA->getName(), SectionFlags, var))
+  Context, SectionName, Stack->CurrentPragmaLocation,
+  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+  if (UnifySection(SectionName, SectionFlags, var))
 var->dropAttr();
+}
 
 // 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,49 @@
 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())
+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  = 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())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
-  if (auto A = OtherDecl->getAttr())
-if (A->isImplicit())
-  Diag(A->getLocation(), diag::note_pragma_entered_here);
+  Diag(Decl->getLocation(), diag::err_section_conflict) << Decl << Section;
+  if (Section.Decl)
+

[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

2020-04-21 Thread Lucas Prates via Phabricator via cfe-commits
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 *Stack = nullptr;
-int SectionFlags = ASTContext::PSF_Implicit | ASTContext::PSF_Read;
+int SectionFlags = ASTContext::PSF_Read;
 if (var->getType().isConstQualified())
   Stack = 
 else if (!var->getInit()) {
@@ -12746,14 +12746,19 @@
   Stack = 
   SectionFlags |= ASTContext::PSF_Write;
 }
-if (Stack->CurrentValue && !var->hasAttr())
+if (const SectionAttr *SA = var->getAttr()) {
+  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())
-  if (UnifySection(SA->getName(), SectionFlags, var))
+  Context, SectionName, Stack->CurrentPragmaLocation,
+  AttributeCommonInfo::AS_Pragma, SectionAttr::Declspec_allocate));
+  if (UnifySection(SectionName, SectionFlags, var))
 var->dropAttr();
+}
 
 // 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())
+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  = 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)
-  <<