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 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
