[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

3048

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.

3049

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

3048

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

3048–3049

[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/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 (auto FD = dyn_cast<FunctionDecl>(D))
+      if (auto SA = dyn_cast<SectionAttr>(NewAttr))
+        S.UnifySection(SA->getName(),
+                       ASTContext::PSF_Implicit | ASTContext::PSF_Execute |
+                           ASTContext::PSF_Read,
+                       FD);
+  }
 }
 
 // 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/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("seg2,sec2"))); // expected-note {{declared here}}
+void test3(void) {}
+const int ro __attribute__((section("seg2,sec2"))) = 10; // expected-error {{'ro' causes a section type conflict with 'test3'}}
+void test4(void) __attribute__((section("seg3,sec3"))); // expected-note {{declared here}}
+void test4(void) {}
+int rw __attribute__((section("seg3,sec3"))) = 10;       // expected-error {{'rw' causes a section type conflict with 'test4'}}