Fix setting of empty implicit-section-name attribute for functions affected by '#pragma clang section'

[petpav01]

Summary

Code in CodeGenModule::SetFunctionAttributes() can set an empty attribute implicit-section-name on a function that is affected by #pragma clang text="section". This is incorrect because the attribute should contain a valid section name. If the function additionally also uses __attribute__((section("section"))) then this results in emitting the function in a section with an empty name.

Example:

$ cat repr.c
#pragma clang section text=".text_pragma"
void f(void) __attribute__((section(".text_attr")));
void f(void) {}
$ clang -target arm-none-eabi -march=armv7-a -c repr.c
$ llvm-objdump -h repr.o

repr.o: file format ELF32-arm-little

Sections:
Idx Name          Size      Address          Type
  0               00000000 0000000000000000
  1 .strtab       0000005d 0000000000000000
  2 .text         00000000 0000000000000000 TEXT
  3               00000004 0000000000000000 TEXT
  4 .ARM.exidx    00000008 0000000000000000
  5 .rel.ARM.exidx 00000008 0000000000000000
  6 .comment      000000b2 0000000000000000
  7 .note.GNU-stack 00000000 0000000000000000
  8 .ARM.attributes 0000003a 0000000000000000
  9 .symtab       00000050 0000000000000000

Section with index 3 contains the code for function f(). The name of the section should be .text_attr but is instead empty.

The following happens in this example:

• CodeGenModule::EmitGlobalFunctionDefinition() is called to emit IR for f(). It invokes GetAddrOfFunction() -> GetOrCreateLLVMFunction() -> SetFunctionAttributes(). The last method notices that the FunctionDecl has the PragmaClangTextSectionAttr attribute and wrongly sets empty implicit-section-name attribute on the resulting llvm::Function.
• EmitGlobalFunctionDefinition() calls setNonAliasAttributes() which normally also sets the implicit-section-name attribute but because the Decl has SectionAttr it gets skipped.

The patch fixes the issue by removing the problematic code that sets empty implicit-section-name from CodeGenModule::SetFunctionAttributes(). The idea is that it is sufficient to set this attribute only from setNonAliasAttributes() when a function is emitted.

[chill]

test/CodeGen/clang-sections-attribute.c

2

Isn't it possible for the test to fail if the Arm target is not configured?

[petpav01]

test/CodeGen/clang-sections-attribute.c

2

I think it should not be necessary to add a REQUIRES line because the test emits and checks only LLVM IR but no target codegen is done. An experiment without having the ARM target configured shows that the test still works in such a setting. Note also that the two already present tests for #pragma clang section (clang-sections.cpp and clang-sections-tentative.c) have a similar header as this new test as well.

[chill]

LGTM. Please, wait a couple of days before committing to give a chance to someone else to weigh in.

lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
   }
 
   // Only a few attributes are set on declarations; these may later be
   // overridden by a definition.
 
   setLinkageForGV(F, FD);
   setGVProperties(F, FD);
 
-  if (FD->getAttr<PragmaClangTextSectionAttr>()) {
-    F->addFnAttr("implicit-section-name");
-  }
-
   if (const SectionAttr *SA = FD->getAttr<SectionAttr>())
     F->setSection(SA->getName());
 
   if (FD->isReplaceableGlobalAllocationFunction()) {
     // A replaceable global allocation function does not act like a builtin by
     // default, only if it is invoked by a new-expression or delete-expression.
     F->addAttribute(llvm::AttributeList::FunctionIndex,
                     llvm::Attribute::NoBuiltin);

test/CodeGen/clang-sections-attribute.c

+// RUN: %clang_cc1 -emit-llvm -triple arm-none-eabi -o - %s | FileCheck %s
+
+// Test interaction between __attribute__((section())) and '#pragma clang
+// section' directives. The attribute should always have higher priority than
+// the pragma.
+
+// Text tests.
+#pragma clang section text=".ext_fun_pragma"
+void ext_fun(void) __attribute__((section(".ext_fun_attr")));
+void ext_fun(void) {}
+#pragma clang section text=""
+
+void ext_fun2(void) __attribute__((section(".ext_fun2_attr")));
+#pragma clang section text=".ext_fun2_pragma"
+void ext_fun2(void) {}
+#pragma clang section text=""
+
+#pragma clang section text=".int_fun_pragma"
+static void int_fun(void) __attribute__((section(".int_fun_attr"), used));
+static void int_fun(void) {}
+#pragma clang section text=""
+
+static void int_fun2(void) __attribute__((section(".int_fun2_attr"), used));
+#pragma clang section text=".int_fun2_pragma"
+static void int_fun2(void) {}
+#pragma clang section text=""
+
+// Rodata tests.
+#pragma clang section rodata=".ext_const_pragma"
+__attribute__((section(".ext_const_attr")))
+const int ext_const = 1;
+#pragma clang section rodata=""
+
+#pragma clang section rodata=".int_const_pragma"
+__attribute__((section(".int_const_attr"), used))
+static const int int_const = 1;
+#pragma clang section rodata=""
+
+// Data tests.
+#pragma clang section data=".ext_var_pragma"
+__attribute__((section(".ext_var_attr")))
+int ext_var = 1;
+#pragma clang section data=""
+
+#pragma clang section data=".int_var_pragma"
+__attribute__((section(".int_var_attr"), used))
+static int int_var = 1;
+#pragma clang section data=""
+
+// Bss tests.
+#pragma clang section bss=".ext_zvar_pragma"
+__attribute__((section(".ext_zvar_attr")))
+int ext_zvar;
+#pragma clang section bss=""
+
+#pragma clang section bss=".int_zvar_pragma"
+__attribute__((section(".int_zvar_attr"), used))
+static int int_zvar;
+#pragma clang section bss=""
+
+// CHECK: @ext_const = constant i32 1, section ".ext_const_attr", align 4{{$}}
+// CHECK: @int_const = internal constant i32 1, section ".int_const_attr", align 4{{$}}
+// CHECK: @ext_var = global i32 1, section ".ext_var_attr", align 4{{$}}
+// CHECK: @int_var = internal global i32 1, section ".int_var_attr", align 4{{$}}
+// CHECK: @ext_zvar = global i32 0, section ".ext_zvar_attr", align 4{{$}}
+// CHECK: @int_zvar = internal global i32 0, section ".int_zvar_attr", align 4{{$}}
+// CHECK: define void @ext_fun() #0 section ".ext_fun_attr"
+// CHECK: define void @ext_fun2() #0 section ".ext_fun2_attr"
+// CHECK: define internal void @int_fun() #0 section ".int_fun_attr"
+// CHECK: define internal void @int_fun2() #0 section ".int_fun2_attr"
+//
+// Function attributes should not include implicit-section-name.
+// CHECK-NOT: attributes #0 = {{.*}}implicit-section-name
+//
+// No other attribute group should be present in the file.
+// CHECK-NOT: attributes #1