MC: Add .data. and .rodata. prefixes to MCContext section classification
authorDave Marchevsky <davemarchevsky@fb.com>
Tue, 27 Dec 2022 23:56:47 +0000 (15:56 -0800)
committerYonghong Song <yhs@fb.com>
Wed, 28 Dec 2022 00:03:33 +0000 (16:03 -0800)
Commit 463da422f019 ("MC: make section classification a bit more
thorough") changed MCContext::getELFSection section classification logic
to default to SectionKind::getText (previously default was
SectionKind::getReadOnly) and added some matching based on section name
to determine internal section classification.

The BPF runtime implements global variables using 'BPF map'
datastructures, specifically the arraymap BPF map type. Global variables
in a section are placed in a single arraymap value at predictable byte
offsets. Variables in different sections are placed in separate
arraymaps, so in this example:

  #define SEC(name) __attribute__((section(name)))
  SEC(".data.A") u32 one;
  SEC(".data.A") u32 two;
  SEC(".data.B") u32 three;
  SEC(".data.B") u32 four;

variables one and two would correspond to some byte offsets (probably 0
and 4) in one arraymap, while three and four would be in a separate
arraymap. Variables of a bpf_spin_lock type are considered to protect
next-generation BPF datastructure types in the same arraymap value and
there can only be a single bpf_spin_lock variable per arraymap value -
and thus per section.

As a result it's necessary to keep bpf_spin_locks and the datastructures
they guard in separate data sections. Before the aforementioned commit,
a section whose name starts with ".data." - like ".data.A" - would be
classified as SectionKind::getReadOnly, whereas after it is
SectionKind::getText. If 4-byte padding is required in such a section due to
alignment of some symbol within it, classification of the section as
SectionKind::getText will result in compilation of those variables to
BPF backend failing with an error like "unable to write nop sequence of
4 bytes". This is due to nop instruction emitted in
BPFAsmBackend::writeNopData being 8 bytes, so the function fails since
it cannot emit a 4-byte nop instruction.

Let's follow the pattern of matching section names starting with ".bss."
and ".tbss." prefixes resulting in proper classification of the section
as data by adding similar matches for ".data." and ".rodata." prefixes.
This will bring padding behavior for these sections back to what it was
before that commit and fix the crash.

Differential Revision: https://reviews.llvm.org/D138477

llvm/lib/MC/MCContext.cpp
llvm/test/MC/ELF/data-section-prefix.ll [new file with mode: 0644]

index 29dd0e4..40e5e0f 100644 (file)
@@ -577,8 +577,10 @@ MCSectionELF *MCContext::getELFSection(const Twine &Section, unsigned Type,
                .Case(".data", SectionKind::getData())
                .Case(".data1", SectionKind::getData())
                .Case(".data.rel.ro", SectionKind::getReadOnlyWithRel())
+               .StartsWith(".data.", SectionKind::getData())
                .Case(".rodata", SectionKind::getReadOnly())
                .Case(".rodata1", SectionKind::getReadOnly())
+               .StartsWith(".rodata.", SectionKind::getReadOnly())
                .Case(".tbss", SectionKind::getThreadBSS())
                .StartsWith(".tbss.", SectionKind::getThreadData())
                .StartsWith(".gnu.linkonce.tb.", SectionKind::getThreadData())
diff --git a/llvm/test/MC/ELF/data-section-prefix.ll b/llvm/test/MC/ELF/data-section-prefix.ll
new file mode 100644 (file)
index 0000000..5a13000
--- /dev/null
@@ -0,0 +1,25 @@
+; RUN: llc -filetype obj -o - %s | llvm-readobj --sections - | FileCheck --check-prefix="SECTIONS" %s
+;
+; SECTIONS:         Name: .data.A
+; SECTIONS-NEXT:    Type: SHT_PROGBITS (0x1)
+; SECTIONS-NEXT:        Flags [ (0x3)
+; SECTIONS-NEXT:          SHF_ALLOC (0x2)
+; SECTIONS-NEXT:          SHF_WRITE (0x1)
+; SECTIONS-NEXT:    ]
+;
+; SECTIONS:         Name: .rodata.A
+; SECTIONS-NEXT:    Type: SHT_PROGBITS (0x1)
+; SECTIONS-NEXT:        Flags [ (0x3)
+; SECTIONS-NEXT:          SHF_ALLOC (0x2)
+; SECTIONS-NEXT:          SHF_WRITE (0x1)
+; SECTIONS-NEXT:    ]
+
+
+target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
+target triple = "bpf"
+
+@glock = dso_local local_unnamed_addr global i32 0, section ".data.A", align 8
+@ghead = dso_local local_unnamed_addr global i32 0, section ".data.A", align 8
+
+@glock2 = dso_local local_unnamed_addr global i32 0, section ".rodata.A", align 8
+@ghead2 = dso_local local_unnamed_addr global i32 0, section ".rodata.A", align 8