[Clang][Sema] Detect section type conflicts between functions and variables

[tmatheson]

Summary

If two variables are declared with attribute((section(name))) and
the implicit section types (e.g. read only vs writeable) conflict, an
error is raised. Extend this mechanism so that an error is raised if the
section type implied by a function's attribute((section)) conflicts
with that of another variable.

[tmatheson]

Rebase

[aaron.ballman]

clang/lib/Sema/SemaDeclAttr.cpp

3086

Does this need to be limited to function declarations? It seems to me that this should also be used on say ObjCMethod declarations or global variables as well, right?

If so, then Sema::UnifySection() may need to be updated as well as it currently accepts a DeclaratorDecl but some of these constructs (like ObjcMethodDecl and ObjCPropertyDecl) are NamedDecls and not declarators.

3087

NewAttr is already declared as being a SectionAttr, so this isn't needed, you can use NewAttr->getName() below instead.

clang/test/Sema/attr-section.c

42

Some other tests that may be interesting:

const int whatever1 __attribute__((section("seg4, sec4"))) = 10;
void test5(void) __attribute__((section("seg4, sec4")));
void test5(void) {}

void test6(void);
const int whatever2 __attribute__((section("seg5, sec5"))) = 10;
void test6(void) __attribute__((section("seg5, sec5")));
void test6(void) {}

[tmatheson]

Extended to Objective-C methods and properties.
Added suggested C tests, C++ template function test and Objective-C tests.
I had to removed PSF_Implicit flag so that cases where the function was declared before the global variable were caught.

[tmatheson]

clang/lib/Sema/SemaDeclAttr.cpp

3086

I've extended this to include Objective-C methods and properties and template functions. Gobal variables are handled by Sema::CheckCompleteVariableDeclaration.

[aaron.ballman]

LGTM aside from a minor nit. Thank you for the patch! Do you need me to commit on your behalf? If so, are you okay with Tomas Matheson <[e-mail]> for patch attribution?

clang/lib/Sema/SemaDeclAttr.cpp

3086–3087

[tmatheson]

Minor syntax change, fix CodeGen/attributes.c

[tmatheson]

In D93102#2458433, @aaron.ballman wrote:

LGTM aside from a minor nit. Thank you for the patch! Do you need me to commit on your behalf? If so, are you okay with Tomas Matheson <[e-mail]> for patch attribution?

Thanks for the review. I will ask for commit access for future but if you could commit this one in the meantime that would be appreciated. That's fine for attribution.

[aaron.ballman]

In D93102#2460034, @tmatheson wrote:

In D93102#2458433, @aaron.ballman wrote:

LGTM aside from a minor nit. Thank you for the patch! Do you need me to commit on your behalf? If so, are you okay with Tomas Matheson <[e-mail]> for patch attribution?

Thanks for the review. I will ask for commit access for future but if you could commit this one in the meantime that would be appreciated. That's fine for attribution.

I've commit on your behalf in f50066292477fb26806336e5604615d0eddde399, thank you for the patch!

clang/include/clang/AST/ASTContext.h

enum PragmaSectionFlag : unsigned {
     PSF_Write = 0x2,
     PSF_Execute = 0x4,
     PSF_Implicit = 0x8,
     PSF_ZeroInit = 0x10,
     PSF_Invalid = 0x80000000U,
   };
 
   struct SectionInfo {
-    DeclaratorDecl *Decl;
+    NamedDecl *Decl;
     SourceLocation PragmaSectionLocation;
     int SectionFlags;
 
     SectionInfo() = default;
-    SectionInfo(DeclaratorDecl *Decl,
-                SourceLocation PragmaSectionLocation,
+    SectionInfo(NamedDecl *Decl, SourceLocation PragmaSectionLocation,
                 int SectionFlags)
         : Decl(Decl), PragmaSectionLocation(PragmaSectionLocation),
           SectionFlags(SectionFlags) {}
   };
 
   llvm::StringMap<SectionInfo> SectionInfos;
 
   /// Return a new OMPTraitInfo object owned by this context.

clang/include/clang/Sema/Sema.h

 
   enum PragmaSectionKind {
     PSK_DataSeg,
     PSK_BSSSeg,
     PSK_ConstSeg,
     PSK_CodeSeg,
   };
 
-  bool UnifySection(StringRef SectionName,
-                    int SectionFlags,
-                    DeclaratorDecl *TheDecl);
+  bool UnifySection(StringRef SectionName, int SectionFlags,
+                    NamedDecl *TheDecl);
   bool UnifySection(StringRef SectionName,
                     int SectionFlags,
                     SourceLocation PragmaSectionLocation);
 
   /// Called on well formed \#pragma bss_seg/data_seg/const_seg/code_seg.
   void ActOnPragmaMSSeg(SourceLocation PragmaLocation,
                         PragmaMsStackAction Action,
                         llvm::StringRef StackSlotLabel,

clang/lib/Sema/SemaAttr.cpp

void Sema::ActOnPragmaMSVtorDisp(PragmaMsStackAction Action,
                                  SourceLocation PragmaLoc,
                                  MSVtorDispMode Mode) {
   if (Action & PSK_Pop && VtorDispStack.Stack.empty())
     Diag(PragmaLoc, diag::warn_pragma_pop_failed) << "vtordisp"
                                                   << "stack empty";
   VtorDispStack.Act(PragmaLoc, Action, StringRef(), Mode);
 }
 
-bool Sema::UnifySection(StringRef SectionName,
-                        int SectionFlags,
-                        DeclaratorDecl *Decl) {
+bool Sema::UnifySection(StringRef SectionName, int SectionFlags,
+                        NamedDecl *Decl) {
   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, PragmaLocation, SectionFlags);

clang/lib/Sema/SemaDeclAttr.cpp

static void handleSectionAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   std::string Error = S.Context.getTargetInfo().isValidSectionSpecifier(Str);
   if (!Error.empty()) {
     S.Diag(LiteralLoc, diag::err_attribute_section_invalid_for_target)
     << Error;
     return;
   }
 
   SectionAttr *NewAttr = S.mergeSectionAttr(D, AL, Str);
-  if (NewAttr)
+  if (NewAttr) {
     D->addAttr(NewAttr);
+    if (isa<FunctionDecl, FunctionTemplateDecl, ObjCMethodDecl,
+            ObjCPropertyDecl>(D))
+      S.UnifySection(NewAttr->getName(),
+                     ASTContext::PSF_Execute | ASTContext::PSF_Read,
+                     cast<NamedDecl>(D));
+  }
 }
 
 // This is used for `__declspec(code_seg("segname"))` on a decl.
 // `#pragma code_seg("segname")` uses checkSectionName() instead.
 static bool checkCodeSegName(Sema &S, SourceLocation LiteralLoc,
                              StringRef CodeSegName) {
   std::string Error =
       S.Context.getTargetInfo().isValidSectionSpecifier(CodeSegName);

clang/test/CodeGen/attributes.c

 
 // CHECK: define void @t72() [[COLDDEF:#[0-9]+]] {
 void t71(void) __attribute__((cold));
 void t72() __attribute__((cold));
 void t72() { t71(); }
 // CHECK: call void @t71() [[COLDSITE:#[0-9]+]]
 // CHECK: declare void @t71() [[COLDDECL:#[0-9]+]]
 
-// CHECK: define void @t10() [[NUW]] section "SECT" {
-void t10(void) __attribute__((section("SECT")));
+// CHECK: define void @t10() [[NUW]] section "xSECT" {
+void t10(void) __attribute__((section("xSECT")));
 void t10(void) {}
-// CHECK: define void @t11() [[NUW]] section "SECT" {
-void __attribute__((section("SECT"))) t11(void) {}
+// CHECK: define void @t11() [[NUW]] section "xSECT" {
+void __attribute__((section("xSECT"))) t11(void) {}
 
 // CHECK: define i32 @t19() [[NUW]] {
 extern int t19(void) __attribute__((weak_import));
 int t19(void) {
   return 10;
 }
 
 // CHECK:define void @t20() [[NUW]] {

clang/test/Sema/attr-section.c

 enum __attribute__((section("NEAR,x"))) e { one }; // expected-error {{'section' attribute only applies to}}
 
 extern int a; // expected-note {{previous declaration is here}}
 int *b = &a;
 extern int a __attribute__((section("foo,zed"))); // expected-warning {{section attribute is specified on redeclared variable}}
 
 // Not a warning.
 int c;
-int c __attribute__((section("foo,zed")));
+int c __attribute__((section("seg1,sec1")));
 
 // Also OK.
 struct r_debug {};
 extern struct r_debug _r_debug;
 struct r_debug _r_debug __attribute__((nocommon, section(".r_debug,bar")));
+
+// Section type conflicts between functions and variables
+void test3(void) __attribute__((section("seg3,sec3"))); // expected-note {{declared here}}
+void test3(void) {}
+const int const_global_var __attribute__((section("seg3,sec3"))) = 10; // expected-error {{'const_global_var' causes a section type conflict with 'test3'}}
+
+void test4(void) __attribute__((section("seg4,sec4"))); // expected-note {{declared here}}
+void test4(void) {}
+int mut_global_var __attribute__((section("seg4,sec4"))) = 10; // expected-error {{'mut_global_var' causes a section type conflict with 'test4'}}
+
+const int global_seg5sec5 __attribute__((section("seg5,sec5"))) = 10; // expected-note {{declared here}}
+void test5(void) __attribute__((section("seg5,sec5")));               // expected-error {{'test5' causes a section type conflict with 'global_seg5sec5'}}
+void test5(void) {}
+
+void test6(void);
+const int global_seg6sec6 __attribute__((section("seg6,sec6"))) = 10; // expected-note {{declared here}}
+void test6(void) __attribute__((section("seg6,sec6")));               // expected-error {{'test6' causes a section type conflict with 'global_seg6sec6'}}
+void test6(void) {}

clang/test/SemaCXX/attr-section.cpp

struct B : A {
     void f() {} // ok
   };
   struct C : A {
     __attribute__((section("bar"))) void f(); // expected-note {{previous}}
   };
   __attribute__((section("baz"))) // expected-warning {{section does not match}}
   void C::f() {}
 }
+
+// Check for section type conflicts between global variables and function templates
+template <typename> __attribute__((section("template_fn1"))) void template_fn1() {} // expected-note {{declared here}}
+const int const_global_var __attribute__((section("template_fn1"))) = 42;           // expected-error {{'const_global_var' causes a section type conflict with 'template_fn1'}}
+int mut_global_var __attribute__((section("template_fn2"))) = 42;                   // expected-note {{declared here}}
+template <typename> __attribute__((section("template_fn2"))) void template_fn2() {} // expected-error {{'template_fn2' causes a section type conflict with 'mut_global_var'}}

clang/test/SemaObjC/method-attributes.m

 // rdar://15450637
 @interface rdar15450637 : NSObject
 @property int p __attribute__((section("__TEXT,foo")));
 
 - (id) IMethod :(int) count, ...  __attribute__((section("__TEXT,foo")));
 
 + (void) CMethod : (id) Obj __attribute__((section("__TEXT,fee")));
 @end
+
+// Section type conflicts between methods/properties and global variables
+const int global1 __attribute__((section("seg1,sec1"))) = 10; // expected-note {{declared here}} expected-note {{declared here}} expected-note {{declared here}}
+int global2 __attribute__((section("seg2,sec2"))) = 10;       // expected-note {{declared here}} expected-note {{declared here}} expected-note {{declared here}}
+
+@interface section_conflicts : NSObject
+@property int p1 __attribute__((section("seg1,sec1"))); // expected-error {{'p1' causes a section type conflict with 'global1'}}
+@property int p2 __attribute__((section("seg2,sec2"))); // expected-error {{'p2' causes a section type conflict with 'global2'}}
+
+- (void)imethod1 __attribute__((section("seg1,sec1"))); // expected-error {{'imethod1' causes a section type conflict with 'global1'}}
+- (void)imethod2 __attribute__((section("seg2,sec2"))); // expected-error {{'imethod2' causes a section type conflict with 'global2'}}
+
++ (void)cmethod1:(id)Obj __attribute__((section("seg1,sec1"))); // expected-error {{'cmethod1:' causes a section type conflict with 'global1'}}
++ (void)cmethod2:(id)Obj __attribute__((section("seg2,sec2"))); // expected-error {{'cmethod2:' causes a section type conflict with 'global2'}}
+@end