[THUMB2] default .text alignment to 2B

[nickdesaulniers]

Summary

Similar to GAS, default the alignment of .text to 2B. This difference
between clang vs GAS was frustrating efforts to debug pr/51929.

Similar to D102052.

Does not regress Linux kernel boots in QEMU. Tested with Linux kernel
tag v5.15-rc3 with
https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9122/1
applied, and QEMU tag v5.2.0. In pr/51929 we're working to dig our way
out of boot failures with ToT QEMU (v6.1.0+).

Link: https://github.com/ClangBuiltLinux/linux/issues/1447

[nickdesaulniers]

• add llvm-mc based test comparing arm vs thumb

[MaskRay]

llvm/lib/Target/ARM/MCTargetDesc/ARMMCObjectFileInfo.h

2

80 column

[MaskRay]

The approach looks good to me.

[nickdesaulniers]

• expand header comment to 80 lines

[MaskRay]

Like the RISCV patch, the test should be in llvm/test/MC/ARM. test/CodeGen/ARM does not test pure lib/MC behavior.

llvm/test/CodeGen/ARM/thumb-text-align.s

1 ↗

(On Diff #375399)

< and -o - can be removed

7 ↗

(On Diff #375399)

Align the keys (Name, Type, etc)

[nickdesaulniers]

• mv test, use @ style comments, remove < and -o -, realign CHECKs

[MaskRay]

LGTM.

llvm/test/MC/ARM/thumb-text-align.s

2

@@ Test the default alignment for text sections

[nickdesaulniers]

• add comment explaining point of test

[peter.smith]

No objections as I think a defaut alignment of 2 for Thumb is the right thing to do.

One thing that may cause problems is an immediate inline change of state to Arm something like the following with a thumb target.

.text
.arm
nop

Whereas the default alignment of 4 would save us beforehand. There is a small risk that some existing projects could be affected. I think that an eventual fix for pr/51929 could address this case though. Such projects could also add an align directive as well.

[nickdesaulniers]

llvm/test/MC/ARM/thumb-text-align.s

5–7

I can drop --sd; that's not needed.

[nickdesaulniers]

• drop --sd arg to llvm-readobj

[nickdesaulniers]

In D110580#3027025, @peter.smith wrote:

One thing that may cause problems is an immediate inline change of state to Arm something like the following with a thumb target.

.text
.arm
nop

I think I can extend this patch (preferably a child patch on top); it seems that GAS will:

1. use 4B alignment for .text if .arm and .thumb are encountered, regardless of -mthumb.
2. use 4B alignment if only .arm is encountered regardless of -mthumb, or 2B alignment if only .thumb is encountered regardless of -marm.
3. use 2B alignment for -mthumb, 4B otherwise (if neither .arm or .thumb are encountered).

I'll bet this happens with other TEXT sections, too, not just .text. (unverified)

though for pr/51929, there are no .arm or .thumb directives, so pr/51929 is the third case above.

@ $ arm-linux-gnueabi-as -march=armv7-a x.s -o x.o
@ $ llvm-readelf -S x.o
@ [ 1] .text             PROGBITS        00000000 000034 000004 00  AX  0   0  4
.text
nop

@ $ arm-linux-gnueabi-as -mthumb -march=armv7-a x.s -o x.o
@ $ llvm-readelf -S x.o
@ [ 1] .text             PROGBITS        00000000 000034 000002 00  AX  0   0  2
.text
nop

@ $ arm-linux-gnueabi-as -mthumb -march=armv7-a x.s -o x.o
@ $ llvm-readelf -S x.o
@ [ 1] .text             PROGBITS        00000000 000034 000004 00  AX  0   0  4
.text
.arm
nop

@ $ arm-linux-gnueabi-as -march=armv7-a x.s -o x.o
@ $ llvm-readelf -S x.o
@ [ 1] .text             PROGBITS        00000000 000034 000002 00  AX  0   0  2
.text
.thumb
nop

@ $ arm-linux-gnueabi-as -march=armv7-a x.s -o x.o
@ $ llvm-readelf -S x.o
@ [ 1] .text             PROGBITS        00000000 000034 000008 00  AX  0   0  4
.text
.thumb
nop
.arm
nop

@ $ arm-linux-gnueabi-as -march=armv7-a -mthumb x.s -o x.o
@ $ llvm-readelf -S x.o
@ [ 1] .text             PROGBITS        00000000 000034 000008 00  AX  0   0  4
.text
.thumb
nop
.arm
nop

That said, looking at the case from pr/51929, it still doesn't have a 2B alignment for .text. Trying my best with cvise, it seems the presence of a .align 0 directive also forces GAS to emit .text with 4B alignment. So that's yet a fourth case...(but also what's occurring for pr/51929, so I suspect I'll need this plus handling .align 0 better in llvm).

[nickdesaulniers]

https://lore.kernel.org/linux-arm-kernel/[e-mail]/ is the kernel patch to address pr/51929. At this point, I don't care to see this land, but happy to land it if folks prefer.

lld/test/ELF/arm-thumb-branch.s

 // CHECK-NEXT:   10010:       00 f0 0a b8     b.w     0x10028 <callee_high>
 // CHECK-NEXT:   10014:       40 f0 08 80     bne.w   0x10028 <callee_high>
 /// far_uncond = 0x101001b
 // CHECK-NEXT:   10018:       ff f3 ff d7     bl      0x101001a
 // CHECK-NEXT:   1001c:       ff f3 fd 97     b.w     0x101001a
 /// far_cond = 0x110023
 // CHECK-NEXT:   10020:       3f f3 ff af     bgt.w   0x110022
 // CHECK-NEXT:   10024:       70 47   bx      lr
-// CHECK-NEXT:   10026:
 // CHECK-EMPTY:
 // CHECK-NEXT: Disassembly of section .callee2:
 // CHECK-EMPTY:
 // CHECK-NEXT: <callee_high>:
 // CHECK-NEXT:   10028:       70 47   bx      lr

lld/test/ELF/arm-thunk-linkerscript-orphan.s

 // CHECK: <low_target>:
 // CHECK-NEXT:   100002:        00 f0 03 f8     bl      0x10000c <__Thumbv7ABSLongThunk_high_target>
 // CHECK-NEXT:   100006:        00 f0 06 f8     bl      0x100016 <__Thumbv7ABSLongThunk_orphan_target>
 // CHECK: <__Thumbv7ABSLongThunk_high_target>:
 // CHECK-NEXT:   10000c:        40 f2 01 0c     movw    r12, #1
 // CHECK-NEXT:   100010:        c0 f2 00 2c     movt    r12, #512
 // CHECK-NEXT:   100014:        60 47   bx      r12
 // CHECK: <__Thumbv7ABSLongThunk_orphan_target>:
-// CHECK-NEXT:   100016:        40 f2 15 0c     movw    r12, #21
+// CHECK-NEXT:   100016:        40 f2 13 0c     movw    r12, #19
 // CHECK-NEXT:   10001a:        c0 f2 00 2c     movt    r12, #512
 // CHECK-NEXT:   10001e:        60 47   bx      r12
   .section .text_high, "ax", %progbits
  .thumb
  .globl high_target
  .type high_target, %function
 high_target:
  bl low_target
  bl orphan_target
 // CHECK: Disassembly of section .text_high:
 // CHECK-EMPTY:
 // CHECK-NEXT: <high_target>:
 // CHECK-NEXT:  2000000:        00 f0 02 f8     bl      0x2000008 <__Thumbv7ABSLongThunk_low_target>
-// CHECK-NEXT:  2000004:        00 f0 06 f8     bl      0x2000014 <orphan_target>
+// CHECK-NEXT:  2000004:        00 f0 05 f8     bl      0x2000012 <orphan_target>
 // CHECK: <__Thumbv7ABSLongThunk_low_target>:
 // CHECK-NEXT:  2000008:        40 f2 03 0c     movw    r12, #3
 // CHECK-NEXT:  200000c:        c0 f2 10 0c     movt    r12, #16
 // CHECK-NEXT:  2000010:        60 47   bx      r12
 
  .section orphan, "ax", %progbits
  .thumb
  .globl orphan_target
  .type orphan_target, %function
 orphan_target:
  bl low_target
  bl high_target
 // CHECK: Disassembly of section orphan:
 // CHECK-EMPTY:
 // CHECK-NEXT: <orphan_target>:
-// CHECK-NEXT:  2000014:        ff f7 f8 ff     bl      0x2000008 <__Thumbv7ABSLongThunk_low_target>
-// CHECK-NEXT:  2000018:        ff f7 f2 ff     bl      0x2000000 <high_target>
+// CHECK-NEXT:  2000012:        ff f7 f9 ff     bl      0x2000008 <__Thumbv7ABSLongThunk_low_target>
+// CHECK-NEXT:  2000016:        ff f7 f3 ff     bl      0x2000000 <high_target>
 
  .data
  .word 10

llvm/lib/Target/ARM/MCTargetDesc/ARMMCObjectFileInfo.h

+//===-- ARMMCObjectFileInfo.h - ARM object file Info -----------*- C++ -*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains the declaration of the ARMMCObjectFileInfo class.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TARGET_ARM_MCTARGETDESC_ARMMCOBJECTFILEINFO_H
+#define LLVM_LIB_TARGET_ARM_MCTARGETDESC_ARMMCOBJECTFILEINFO_H
+
+#include "llvm/MC/MCObjectFileInfo.h"
+
+namespace llvm {
+
+class ARMMCObjectFileInfo : public MCObjectFileInfo {
+public:
+  unsigned getTextSectionAlignment() const override;
+};
+
+} // namespace llvm
+
+#endif

llvm/lib/Target/ARM/MCTargetDesc/ARMMCObjectFileInfo.cpp

+//===-- ARMMCObjectFileInfo.cpp - ARM object file properties ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains the declarations of the ARMMCObjectFileInfo properties.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ARMMCObjectFileInfo.h"
+#include "llvm/MC/MCContext.h"
+
+using namespace llvm;
+
+unsigned ARMMCObjectFileInfo::getTextSectionAlignment() const {
+  return getContext().getTargetTriple().isThumb() ? 2 : 4;
+}

llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "ARMMCTargetDesc.h"
 #include "ARMAddressingModes.h"
 #include "ARMBaseInfo.h"
 #include "ARMInstPrinter.h"
 #include "ARMMCAsmInfo.h"
+#include "ARMMCObjectFileInfo.h"
 #include "TargetInfo/ARMTargetInfo.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/DebugInfo/CodeView/CodeView.h"
 #include "llvm/MC/MCAsmBackend.h"
 #include "llvm/MC/MCCodeEmitter.h"
 #include "llvm/MC/MCELFStreamer.h"
 #include "llvm/MC/MCInstrAnalysis.h"
 #include "llvm/MC/MCInstrInfo.h"

 bool ARM::isCDECoproc(size_t Coproc, const MCSubtargetInfo &STI) {
   // Unfortunately we don't have ARMTargetInfo in the disassembler, so we have
   // to rely on feature bits.
   if (Coproc >= 8)
     return false;
   return STI.getFeatureBits()[ARM::FeatureCoprocCDE0 + Coproc];
 }
 
+static MCObjectFileInfo *
+createARMMCObjectFileInfo(MCContext &Ctx, bool PIC,
+                          bool LargeCodeModel = false) {
+  MCObjectFileInfo *MOFI = new ARMMCObjectFileInfo();
+  MOFI->initMCObjectFileInfo(Ctx, PIC, LargeCodeModel);
+  return MOFI;
+}
+
 // Force static initialization.
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeARMTargetMC() {
   for (Target *T : {&getTheARMLETarget(), &getTheARMBETarget(),
                     &getTheThumbLETarget(), &getTheThumbBETarget()}) {
     // Register the MC asm info.
     RegisterMCAsmInfoFn X(*T, createARMMCAsmInfo);
 
     // Register the MC instruction info.
for (Target *T : {&getTheARMLETarget(), &getTheARMBETarget(),
     // Register the null TargetStreamer.
     TargetRegistry::RegisterNullTargetStreamer(*T, createARMNullTargetStreamer);
 
     // Register the MCInstPrinter.
     TargetRegistry::RegisterMCInstPrinter(*T, createARMMCInstPrinter);
 
     // Register the MC relocation info.
     TargetRegistry::RegisterMCRelocationInfo(*T, createARMMCRelocationInfo);
+
+    // Register the MCObjectFileInfo.
+    TargetRegistry::RegisterMCObjectFileInfo(*T, createARMMCObjectFileInfo);
   }
 
   // Register the MC instruction analyzer.
   for (Target *T : {&getTheARMLETarget(), &getTheARMBETarget(),
                     &getTheThumbLETarget(), &getTheThumbBETarget()})
     TargetRegistry::RegisterMCInstrAnalysis(*T, createARMMCInstrAnalysis);
 
   for (Target *T : {&getTheARMLETarget(), &getTheThumbLETarget()}) {
     TargetRegistry::RegisterMCCodeEmitter(*T, createARMLEMCCodeEmitter);
     TargetRegistry::RegisterMCAsmBackend(*T, createARMLEAsmBackend);
   }
   for (Target *T : {&getTheARMBETarget(), &getTheThumbBETarget()}) {
     TargetRegistry::RegisterMCCodeEmitter(*T, createARMBEMCCodeEmitter);
     TargetRegistry::RegisterMCAsmBackend(*T, createARMBEAsmBackend);
   }
 }

llvm/lib/Target/ARM/MCTargetDesc/CMakeLists.txt

 add_llvm_component_library(LLVMARMDesc
   ARMAsmBackend.cpp
   ARMELFObjectWriter.cpp
   ARMELFStreamer.cpp
   ARMInstPrinter.cpp
-  ARMMachObjectWriter.cpp
-  ARMMachORelocationInfo.cpp
   ARMMCAsmInfo.cpp
   ARMMCCodeEmitter.cpp
   ARMMCExpr.cpp
+  ARMMCObjectFileInfo.cpp
   ARMMCTargetDesc.cpp
+  ARMMachORelocationInfo.cpp
+  ARMMachObjectWriter.cpp
   ARMTargetStreamer.cpp
   ARMUnwindOpAsm.cpp
   ARMWinCOFFObjectWriter.cpp
   ARMWinCOFFStreamer.cpp
 
   LINK_COMPONENTS
   ARMInfo
   ARMUtils
   MC
   MCDisassembler
   Support
   BinaryFormat
 
   ADD_TO_COMPONENT
   ARM
   )

llvm/test/MC/ARM/thumb-text-align.s

+@@ Test the default alignment for text sections
+
+@ RUN: llvm-mc %s -triple=thumbv7-linux-gnueabi -filetype=obj \
+@ RUN:   | llvm-readobj -S - | FileCheck %s --check-prefixes=CHECK,CHECK-THUMB
+@ RUN: llvm-mc %s -triple=armv7-linux-gnueabi -filetype=obj \
+@ RUN:   | llvm-readobj -S - | FileCheck %s --check-prefixes=CHECK,CHECK-ARM
+mov pc, lr
+
+@ CHECK:            Name: .text (1)
+@ CHECK-NEXT:       Type: SHT_PROGBITS (0x1)
+@ CHECK-NEXT:       Flags [ (0x6)
+@ CHECK-NEXT:         SHF_ALLOC (0x2)
+@ CHECK-NEXT:         SHF_EXECINSTR (0x4)
+@ CHECK-NEXT:       ]
+@ CHECK-NEXT:       Address: 0x0
+@ CHECK-NEXT:       Offset: 0x34
+@ CHECK-THUMB-NEXT: Size: 2
+@ CHECK-ARM-NEXT:   Size: 4
+@ CHECK-NEXT:       Link: 0
+@ CHECK-NEXT:       Info: 0
+@ CHECK-THUMB-NEXT: AddressAlignment: 2
+@ CHECK-ARM-NEXT:   AddressAlignment: 4

llvm/utils/gn/secondary/llvm/lib/Target/ARM/MCTargetDesc/BUILD.gn

static_library("MCTargetDesc") {
   sources = [
     "ARMAsmBackend.cpp",
     "ARMELFObjectWriter.cpp",
     "ARMELFStreamer.cpp",
     "ARMInstPrinter.cpp",
     "ARMMCAsmInfo.cpp",
     "ARMMCCodeEmitter.cpp",
     "ARMMCExpr.cpp",
+    "ARMMCObjectFileInfo.cpp",
     "ARMMCTargetDesc.cpp",
     "ARMMachORelocationInfo.cpp",
     "ARMMachObjectWriter.cpp",
     "ARMTargetStreamer.cpp",
     "ARMUnwindOpAsm.cpp",
     "ARMWinCOFFObjectWriter.cpp",
     "ARMWinCOFFStreamer.cpp",
   ]
 }