Page MenuHomePhabricator

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

Authored by petpav01 on Jul 4 2018, 1:01 AM.



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.


$ 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

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.

Diff Detail

rC Clang

Event Timeline

petpav01 created this revision.Jul 4 2018, 1:01 AM
chill added inline comments.Jul 4 2018, 1:35 AM

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

petpav01 added inline comments.Jul 4 2018, 2:17 AM

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 accepted this revision.Jul 5 2018, 12:51 AM

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

This revision is now accepted and ready to land.Jul 5 2018, 12:51 AM
Closed by commit rC336842: Fix setting of empty implicit-section-name attribute (authored by petr.pavlu, committed by ). · Explain WhyJul 11 2018, 1:22 PM
This revision was automatically updated to reflect the committed changes.