[llvm-branch-commits] [llvm] release/22.x: [MC][ARM] Don't set funcs to Thumb as a side effect of .hidden (#181156) (PR #181799)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-arm

Author: None (llvmbot)

<details>
<summary>Changes</summary>

Backport d2d620d4beafa53acfab6d689214913705776aa6

Requested by: @<!-- -->nikic

---
Full diff: https://github.com/llvm/llvm-project/pull/181799.diff


2 Files Affected:

- (modified) llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp (+23-13) 
- (added) llvm/test/MC/ARM/thumb-state-on-hidden-func.s (+33) 


``````````diff
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
index 94b511a09e400..42b0569c2882b 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
@@ -600,22 +600,32 @@ class ARMELFStreamer : public MCELFStreamer {
     MCELFStreamer::emitValueImpl(Value, Size, Loc);
   }
 
-  /// If a label is defined before the .type directive sets the label's type
-  /// then the label can't be recorded as thumb function when the label is
-  /// defined. We override emitSymbolAttribute() which is called as part of the
-  /// parsing of .type so that if the symbol has already been defined we can
-  /// record the label as Thumb. FIXME: there is a corner case where the state
-  /// is changed in between the label definition and the .type directive, this
-  /// is not expected to occur in practice and handling it would require the
-  /// backend to track IsThumb for every label.
+  /// Called to set any attribute on a symbol.
+  ///
+  /// If this function is called for the .type directive that marks the symbol
+  /// as a function, and the label has been defined already without being typed
+  /// as a function, then this is the first opportunity we have to mark the
+  /// label as Thumb rather than Arm (if we're in Thumb mode).
+  ///
+  /// FIXME: there is a corner case where the state is changed in between the
+  /// label definition and the .type directive. This is not expected to occur
+  /// in practice, and handling it would require the backend to track IsThumb
+  /// for every label.
+  ///
+  /// We do not mark the symbol as Thumb due to any attributes other than
+  /// setting its type to 'function', because there _are_ cases in practice
+  /// where an attribute directive such as .hidden can be widely separated from
+  /// the symbol definition. (For example, bug #180358: rustc targeting mostly
+  /// Thumb generates a top-level Arm function entirely in inline assembly, and
+  /// then uses an LLVM IR `declare` statement to mark it as hidden symbol
+  /// visibility, which causes LLVM to emit a `.hidden` directive after having
+  /// switched back to Thumb mode.)
   bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override {
     bool Val = MCELFStreamer::emitSymbolAttribute(Symbol, Attribute);
 
-    if (!IsThumb)
-      return Val;
-
-    unsigned Type = static_cast<MCSymbolELF *>(Symbol)->getType();
-    if ((Type == ELF::STT_FUNC || Type == ELF::STT_GNU_IFUNC) &&
+    if (IsThumb &&
+        (Attribute == MCSA_ELF_TypeFunction ||
+         Attribute == MCSA_ELF_TypeIndFunction) &&
         Symbol->isDefined())
       getAssembler().setIsThumbFunc(Symbol);
 
diff --git a/llvm/test/MC/ARM/thumb-state-on-hidden-func.s b/llvm/test/MC/ARM/thumb-state-on-hidden-func.s
new file mode 100644
index 0000000000000..ab43583d91059
--- /dev/null
+++ b/llvm/test/MC/ARM/thumb-state-on-hidden-func.s
@@ -0,0 +1,33 @@
+// RUN: llvm-mc --triple=thumbv7-none-eabi -filetype=obj %s | llvm-readelf -s - | FileCheck %s
+
+// Switch to Arm state to define a function, then switch back to Thumb
+// state before marking it as .hidden. We expect that the function is
+// still listed as Arm in the symbol table (low bit clear).
+
+        .arm
+        .type   hidden_arm_func, %function
+hidden_arm_func: bx lr
+
+        .thumb
+        .hidden hidden_arm_func
+
+// CHECK: 00000000 0 FUNC LOCAL HIDDEN {{[0-9]+}} hidden_arm_func
+
+// Define two function symbols in Thumb state, with the .type
+// directive before the label in one case and after it in the other.
+// We expect that both are marked as Thumb. (This was the _intended_
+// use of the 'mark as Thumb' behavior that was accidentally applying
+// to .hidden as well.)
+
+        .balign 4
+thumb_symbol_before_type:
+        .type thumb_symbol_before_type, %function
+        bx lr
+
+        .balign 4
+        .type thumb_symbol_after_type, %function
+thumb_symbol_after_type:
+        bx lr
+
+// CHECK: 00000005 0 FUNC LOCAL DEFAULT {{[0-9]+}} thumb_symbol_before_type
+// CHECK: 00000009 0 FUNC LOCAL DEFAULT {{[0-9]+}} thumb_symbol_after_type

``````````

</details>


https://github.com/llvm/llvm-project/pull/181799