Summary of patch
When multiple symbols are given the same section name, place them into unique sections with the same name but distinguished by both section flags and entity size.
Currently when multiple sections with different flags are implicitly created in IR (for example, a read-only and writable values in the same section) they are emitted in the same section with the flags of the first one seen. Instead, add flags to the ELFUniqueIDMap so that unique sections are emitted for each set of flags.
Ilustrative example
We have an issue with LTO where symbols from separate files are placed in the same section but have incompatible flags:
// file1.c
const volatile int var1 __attribute__((section("foo"))) = 1;
// file2.c
volatile int var2 __attribute__((section("foo"))) = 2;
// file3.c
extern volatile const int var1;
extern volatile int var2;
int main() {
var2 = 1;
return var1 + var2;
}Compiling with clang++ -flto file1.c file2.c file3.c -S --target=arm-unknown-unknown gives the following IR (minimised):
; file1.ll
@var1 = dso_local constant i32 1, section "foo", align 4
; file2.ll
@var2 = dso_local global i32 2, section "foo", align 4
; file3.ll
@var2 = external dso_local global i32, align 4
@var1 = external dso_local constant i32, align 4
; Function Attrs: noinline norecurse nounwind optnone
define dso_local arm_aapcscc i32 @main() #0 {
%1 = alloca i32, align 4
store i32 0, i32* %1, align 4
store volatile i32 1, i32* @var2, align 4
%2 = load volatile i32, i32* @var1, align 4
%3 = load volatile i32, i32* @var2, align 4
%4 = add nsw i32 %2, %3
ret i32 %4
}Both var1 and var2 are placed in the the same RO section (clang 12), resulting in a write to RO data:
...
.type var1,@object # @var1
.section foo,"a",@progbits
.globl var1
.p2align 2
var1:
.long 1 # 0x1
.size var1, 4
.type var2,@object # @var2
.globl var2
.p2align 2
var2:
.long 2 # 0x2
.size var2, 4
...Previous work
This is a based on the work in D72194 which ensures that symbols with different entity sizes are not merged to the same section, by using the unique assembly feature.
We have some mitigation in the frontend for when symbols with incompatible flags are placed in the same section (D93102). However for LTO the conflicting flags come from different translation units and end up in the same LLVM module.
This patch comes out of previous discussion in D93948, where the available options were nicely summarised by @MaskRay. One of the takeaways from that discussion was that the rules for combining flags from different symbols into one set of section flags are unclear/complicated/target dependent, and that outputting separate sections would be a better approach.
Description of the patch
The logic for determining the unique ID had become quite difficult to reason about, so I have factored this out into a separate function. I could put this NFC refactoring up as a separate review if people wanted to see it in isolation.
- I've kept the terminology of D72194: a "default" section is one created by default by MC, and a "generic" section is one that will be emitted without a unique ID
- Remove the current behaviour of outputting all sections with a given name with the first seen set of flags
- Symbols are now placed into separate sections based on the flags, entity size, and section name. This can result in multiple uniqued sections with the same name, but different flags and/or entity size.
- To try to minimise the number of unique sections, the first section seen with a given name becomes the generic section for that name. It doesn't matter whether it is mergeable or not. Therefore the default sections become generic by virtue of being created first. If all symbols in a given section have the same flags and entity size, only one section will be emitted and no unique is required.
- The property of not placing mergeable symbols in non-mergeable sections is preserved because they have different flags.
A small suggestion to emphasise that .rodata has been given an unconventional flag.
; Explicitly placed in a .rodata with writeable set. The writable section should be uniqued, not the default ro section, even if it comes first.