[MC,COFF] .safeseh: avoid changeSection

[MaskRay]

The directive temporarily switches to the .sxdata section to emit data,
and then calls insert, which makes CurFrag out of sync of the
current section. Call push/switch/pop instead.

Related to #132464

[llvmbot]

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-mc

Author: Fangrui Song (MaskRay)

Changes

The directive temporarily switches to the .sxdata section to emit data,
and then calls insert, which makes CurFrag out of sync of the
current section. Call push/switch/pop instead.

Related to #132464

---

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

2 Files Affected:

• (modified) llvm/lib/MC/MCWinCOFFStreamer.cpp (+3-1)
• (modified) llvm/test/CodeGen/X86/win32-eh.ll (+9)

diff --git a/llvm/lib/MC/MCWinCOFFStreamer.cpp b/llvm/lib/MC/MCWinCOFFStreamer.cpp
index d406bc58ccf9d..5d84546b0c175 100644
--- a/llvm/lib/MC/MCWinCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCWinCOFFStreamer.cpp
@@ -287,7 +287,8 @@ void MCWinCOFFStreamer::emitCOFFSafeSEH(MCSymbol const *Symbol) {
     return;
 
   MCSection *SXData = getContext().getObjectFileInfo()->getSXDataSection();
-  changeSection(SXData);
+  pushSection();
+  switchSection(SXData);
   SXData->ensureMinAlignment(Align(4));
 
   insert(getContext().allocFragment<MCSymbolIdFragment>(Symbol));
@@ -298,6 +299,7 @@ void MCWinCOFFStreamer::emitCOFFSafeSEH(MCSymbol const *Symbol) {
   // function. Go ahead and oblige it here.
   CSymbol->setType(COFF::IMAGE_SYM_DTYPE_FUNCTION
                    << COFF::SCT_COMPLEX_TYPE_SHIFT);
+  popSection();
 }
 
 void MCWinCOFFStreamer::emitCOFFSymbolIndex(MCSymbol const *Symbol) {
diff --git a/llvm/test/CodeGen/X86/win32-eh.ll b/llvm/test/CodeGen/X86/win32-eh.ll
index 82dc4beaf972b..d3d19ede546d6 100644
--- a/llvm/test/CodeGen/X86/win32-eh.ll
+++ b/llvm/test/CodeGen/X86/win32-eh.ll
@@ -1,4 +1,5 @@
 ; RUN: llc -mtriple=i686-pc-windows-msvc < %s | FileCheck %s
+; RUN: llc -mtriple=i686-pc-windows-msvc -filetype=obj < %s -o %t
 
 declare void @may_throw_or_crash()
 declare i32 @_except_handler3(...)
@@ -208,6 +209,14 @@ catch:
 ; CHECK-NEXT:  .long   0
 ; CHECK-NEXT:  .long   1
 
+; CHECK-LABEL: inlineasm:
+; CHECK: .safeseh my_handler
+define i32 @inlineasm() {
+entry:
+  call void asm sideeffect ".safeseh my_handler", "~{dirflag},~{fpsr},~{flags}"()
+  ret i32 0
+}
+
 ; CHECK-LABEL: ___ehhandler$use_CxxFrameHandler3:
 ; CHECK: movl $L__ehtable$use_CxxFrameHandler3, %eax
 ; CHECK-NEXT: jmp  ___CxxFrameHandler3 # TAILCALL

[MaskRay]

(While I believe this is improvement, I don't really know .safeseh (invented by LLVM, not in binutils). I think we lack -filetype=obj -mtriple=i686-windows-msvc tests to actually exercise the MCWinCOFFStreamer code path (object file emission), but I don't know how to properly test it. )

[lhmouse]

.safeseh is not invented by LLVM; it's invented by Microsoft: https://learn.microsoft.com/en-us/cpp/assembler/masm/dot-safeseh?view=msvc-170

It creates some metadata (indexes of symbols to safe exception handlers) in the .sxdata section.

[mstorsjo]

.safeseh is not invented by LLVM; it's invented by Microsoft: https://learn.microsoft.com/en-us/cpp/assembler/masm/dot-safeseh?view=msvc-170

It creates some metadata (indexes of symbols to safe exception handlers) in the .sxdata section.

The point was not that LLVM has invented what the .safeseh directive produces - however Microsoft don't use GAS style assembly and don't relate to such directives. In that realm, GAS/binutils is the reference, defining the semantics of the directives. But as binutils doesn't support any .safeseh directive, it is an LLVM invented GAS directive.

[lhmouse]

Here's an updated test program that compiles with any of GCC, Clang or Clang-CL.

# gcc -m32 test.S -nostdlib /mingw32/lib/libkernel32.a && ./a.exe
# clang -target i686-w64-mingw32 test.S -nostdlib /mingw32/lib/libkernel32.a && ./a.exe
# clang -target i686-windows-msvc  test.S -nostdlib -Wl,-subsystem:console  \
#    "C:/Program Files (x86)/Windows Kits/10/Lib/10.0.26100.0/um/x86/kernel32.Lib" && ./a.exe

# CODE SECTION
.intel_syntax noprefix
.text

  # static void __stdcall print(const char* msg)
  .def _print@4; .scl 3; .type 32; .endef
  _print@4:
    push esi
    sub esp, 24

    # %esi = GetStdHandle(STD_ERROR_HANDLE)
    mov DWORD PTR [esp], -12
    call _GetStdHandle@4
    push ecx
    mov esi, eax

    # %eax = lstrlenA(msg)
    mov edx, DWORD PTR [esp + 32]
    mov DWORD PTR [esp], edx
    call _lstrlenA@4
    push ecx

    # WriteFile(%esi, msg, %eax, ignored, nullptr)
    mov DWORD PTR [esp], esi
    mov edx, DWORD PTR [esp + 32]
    mov DWORD PTR [esp + 4], edx
    mov DWORD PTR [esp + 8], eax
    lea eax, [esp + 20]
    mov DWORD PTR [esp + 12], eax
    mov DWORD PTR [esp + 16], 0
    call _WriteFile@20
    sub esp, 20

    # return
    add esp, 24
    pop esi
    ret 4

  # EXCEPTION_DISPOSITION my_handler(...)
  .def _my_handler; .scl 2; .type 32; .endef
  .globl _my_handler
#if defined __clang__ || defined _MSC_VER
  .safeseh _my_handler
#endif
  _my_handler:
    sub esp, 12

    # print(msg_002)
    mov DWORD PTR [esp], OFFSET _msg_002
    call _print@4
    push ecx

    # return ExceptionContinueExecution
    xor eax, eax
    add esp, 12
    ret

  # [[noreturn]] int mainCRTStartup(void)
  .globl _mainCRTStartup
  _mainCRTStartup:
    sub esp, 60

    # EXCEPTION_REGISTRATION_RECORD record
    # record.Next = *%fs:0
    # record.Handler = &my_handler
    # *%fs:0 = &record
    lea ecx, [esp + 52]
    mov eax, DWORD PTR fs:[0]
    mov DWORD PTR [ecx], eax
    mov DWORD PTR [ecx + 4], OFFSET _my_handler
    mov DWORD PTR fs:[0], ecx

    # print(msg_000)
    mov DWORD PTR [esp], OFFSET _msg_000
    call _print@4
    push ecx

    # RaiseException(0x20616263, 0, 0, NULL)
    mov DWORD PTR [esp], 0x20616263
    mov DWORD PTR [esp + 4], 0
    mov DWORD PTR [esp + 8], 0
    mov DWORD PTR [esp + 12], 0
    call _RaiseException@16
    sub esp, 16

    # print(msg_001)
    mov DWORD PTR [esp], OFFSET _msg_001
    call _print@4
    push ecx

    # ExitProcess(0)
    mov DWORD PTR [esp], 0
    call _ExitProcess@4

# DATA SECTION
.section .rdata, "dr"

  _msg_000:  .asciz "my_handler installed\n"
  _msg_001:  .asciz "exiting\n"
  _msg_002:  .asciz "my_handler invoked\n"

  # @feat.00 = 1;  safeseh
  .def @feat.00; .scl 2; .type 0; .endef
  .globl @feat.00
  .set @feat.00, 1

  # IMAGE_LOAD_CONFIG_DIRECTORY32 _load_config_used =
  #   { .Size = 72,
  #     .SEHandlerTable = __safe_se_handler_table,
  #     .SEHandlerCount = &__safe_se_handler_count }
  .def __load_config_used; .scl 2; .type 0; .endef
  .globl __load_config_used
  .align 4
  __load_config_used:
    .long 72
    .fill 16, 4, 0
#if defined __clang__ || defined _MSC_VER
    .long ___safe_se_handler_table
    .long ___safe_se_handler_count
#else
    .long 0
    .long 0
#endif

Before the patch:

• CLANG64 ~/Desktop
  $ clang -target i686-windows-msvc test.S -nostdlib -Wl,-subsystem:console
  "C:/Program Files (x86)/Windows Kits/10/Lib/10.0.26100.0/um/x86/kernel32.Lib" && ./a.exe
  lld-link: warning: ignoring invalid symbol table index in section .sxdata in object C:/MSYS64/tmp/test-5008c8.o
  lld-link: warning: ignoring invalid symbol table index in section .sxdata in object C:/MSYS64/tmp/test-5008c8.o
  lld-link: warning: ignoring invalid symbol table index in section .sxdata in object C:/MSYS64/tmp/test-5008c8.o
  ...

After the patch:

• CLANG64 ~/Desktop
  $ clang -target i686-windows-msvc test.S -nostdlib -Wl,-subsystem:console "C:/Program Files (x86)/Windows Kits/10/Lib/10.0.26100.0/um/x86/kernel32.Lib" && ./a.exe
  my_handler installed
  my_handler invoked
  exiting