[llvm-branch-commits] [llvm] 431a597 - [MC][ARM] Don't set funcs to Thumb as a side effect of .hidden (#181156)

[Cullen Rhodes via llvm-branch-commits]

Author: Simon Tatham
Date: 2026-02-20T09:31:26Z
New Revision: 431a597e7ecf84e31275bf9732a9be89a28d4e1a

URL: https://github.com/llvm/llvm-project/commit/431a597e7ecf84e31275bf9732a9be89a28d4e1a
DIFF: https://github.com/llvm/llvm-project/commit/431a597e7ecf84e31275bf9732a9be89a28d4e1a.diff

LOG: [MC][ARM] Don't set funcs to Thumb as a side effect of .hidden (#181156)

When assembling a source file which switches between Arm and Thumb state
using `.arm` and `.thumb`, if you defined a function in Arm state and
mark it as hidden at dynamic link time using `.hidden`, but don't
actually issue the `.hidden` directive until you have switched back to
Thumb state, then the function would be accidentally marked as Thumb as
a side effect of making it hidden.

This happened in `ARMELFStreamer::emitSymbolAttribute`, and the comment
suggests that it was semi-deliberate: it was intended to happen as a
side effect of `.type foo,%function`, because the function label might
have already been defined without a type, and shouldn't be marked as
Thumb until it's known that it's a function. But I think it was an
accident that the same behavior also applies to any other addition of a
symbol attribute, such as `.hidden`: the call to `setIsThumbFunc` was
conditioned on whether the symbol has function type after setting the
attribute, not whether function type was the attribute _actually being
set_. So if you set the symbol to function type and _then_ use
`.hidden`, the condition would match again.

Fixes #180358, in which this came up in real-world code: rustc's IR
output included a top-level function in the form of inline asm, defined
in Arm state, and marked it as hidden via the LLVM IR `declare`, so that
LLVM didn't emit the `.hidden` directive until after it had switched
back to Thumb state.

The new test case includes a check for the _intended_ behavior of the
code in `emitSymbolAttribute`, to confirm that that still works.

(cherry picked from commit d2d620d4beafa53acfab6d689214913705776aa6)

Added: 
    llvm/test/MC/ARM/thumb-state-on-hidden-func.s

Modified: 
    llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp

Removed: 
    


################################################################################
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