[PATCH] D138477: MC: Add .data. and .rodata. prefixes to MCContext section classification

[Dave Marchevsky via Phabricator via llvm-commits]

davemarchevsky created this revision.
davemarchevsky added reviewers: yonghong-song, compnerd, MaskRay, MatzeB, echristo, ast.
davemarchevsky added a project: LLVM.
Herald added subscribers: StephenFan, hiraditya, emaste.
Herald added a project: All.
davemarchevsky requested review of this revision.
Herald added a subscriber: llvm-commits.

Commit 463da422f019 <https://reviews.llvm.org/rG463da422f019a417b791d7db9adae1ad7d089802> ("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.

Signed-off-by: Dave Marchevsky <davemarchevsky at fb.com>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138477

Files:
  llvm/lib/MC/MCContext.cpp
  llvm/test/MC/ELF/data-section-prefix.ll


Index: llvm/test/MC/ELF/data-section-prefix.ll
===================================================================
--- /dev/null
+++ llvm/test/MC/ELF/data-section-prefix.ll
@@ -0,0 +1,7 @@
+; RUN: llc -filetype obj -o - %s
+
+target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
+target triple = "bpf"
+
+ at glock = dso_local local_unnamed_addr global i32 0, section ".data.A", align 8
+ at ghead = dso_local local_unnamed_addr global i32 0, section ".data.A", align 8
Index: llvm/lib/MC/MCContext.cpp
===================================================================
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -577,8 +577,10 @@
                .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())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138477.477075.patch
Type: text/x-patch
Size: 1301 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221122/02bb93e6/attachment.bin>