Skip to content

Conversation

@mtrofin
Copy link
Member

@mtrofin mtrofin commented Dec 2, 2025

We were not marking the .cfi.jumptable​ functions as naked​ on windows. The referenced bug (https://llvm.org/bugs/show_bug.cgi?id=28641#c3) appears to be fixed:

build/bin/opt -S -passes=lowertypetests -mtriple=i686-pc-win32 llvm/test/Transforms/LowerTypeTests/function.ll | build/bin/llc -O0
L_.cfi.jumptable:                       # @.cfi.jumptable
# %bb.0:                                # %entry
        #APP
        jmp     _f.cfi@PLT
        int3
        int3
        int3

        #NO_APP
        #APP
        jmp     _g.cfi@PLT
        int3
        int3
        int3

        #NO_APP
                                        # -- End function
        .section        .rdata,"dr"
        .p2align        4, 0x0                          # @0

Not seeing the spilled registers described in the bug anymore.

Copy link
Member Author

mtrofin commented Dec 2, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@mtrofin mtrofin requested a review from pcc December 2, 2025 21:56
@mtrofin mtrofin marked this pull request as ready for review December 2, 2025 21:56
@llvmbot
Copy link
Member

llvmbot commented Dec 2, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

We were not marking the .cfi.jumptable​ functions as naked​ on windows. The referenced bug (https://llvm.org/bugs/show_bug.cgi?id=28641#c3) appears to be fixed:

build/bin/opt -S -passes=lowertypetests -mtriple=i686-pc-win32 llvm/test/Transforms/LowerTypeTests/function.ll | build/bin/llc -O0
L_.cfi.jumptable:                       # @.cfi.jumptable
# %bb.0:                                # %entry
        #APP
        jmp     _f.cfi@<!-- -->PLT
        int3
        int3
        int3

        #NO_APP
        #APP
        jmp     _g.cfi@<!-- -->PLT
        int3
        int3
        int3

        #NO_APP
                                        # -- End function
        .section        .rdata,"dr"
        .p2align        4, 0x0                          # @<!-- -->0

Not seeing the spilled registers described in the bug anymore, no assertions are raised either.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/LowerTypeTests.cpp (+1-6)
  • (modified) llvm/test/Transforms/LowerTypeTests/function.ll (+1-1)
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index fa35eef2c00f5..f7aeda95e41b3 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1554,12 +1554,7 @@ void LowerTypeTestsModule::createJumpTable(
 
   // Align the whole table by entry size.
   F->setAlignment(Align(getJumpTableEntrySize(JumpTableArch)));
-  // Skip prologue.
-  // Disabled on win32 due to https://llvm.org/bugs/show_bug.cgi?id=28641#c3.
-  // Luckily, this function does not get any prologue even without the
-  // attribute.
-  if (OS != Triple::Win32)
-    F->addFnAttr(Attribute::Naked);
+  F->addFnAttr(Attribute::Naked);
   if (JumpTableArch == Triple::arm)
     F->addFnAttr("target-features", "-thumb-mode");
   if (JumpTableArch == Triple::thumb) {
diff --git a/llvm/test/Transforms/LowerTypeTests/function.ll b/llvm/test/Transforms/LowerTypeTests/function.ll
index ab3cfb6acccf8..e654da656362f 100644
--- a/llvm/test/Transforms/LowerTypeTests/function.ll
+++ b/llvm/test/Transforms/LowerTypeTests/function.ll
@@ -115,7 +115,7 @@ define i1 @foo(ptr %p) {
 ; NATIVE-SAME: "s"(ptr @g.cfi)
 
 ; X86-LINUX: attributes #[[ATTR]] = { naked nocf_check noinline }
-; X86-WIN32: attributes #[[ATTR]] = { nocf_check noinline }
+; X86-WIN32: attributes #[[ATTR]] = { naked nocf_check noinline }
 ; ARM: attributes #[[ATTR]] = { naked noinline
 ; THUMB: attributes #[[ATTR]] = { naked noinline "target-cpu"="cortex-a8" "target-features"="+thumb-mode" }
 ; THUMBV6M: attributes #[[ATTR]] = { naked noinline "target-features"="+thumb-mode" }

@mtrofin mtrofin requested a review from eugenis December 2, 2025 21:57
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Title needs to s/[LLT]/[LTT]

@mtrofin mtrofin changed the title [LLT] mark the CFI jumptable naked on Windows [LTT] mark the CFI jumptable naked on Windows Dec 2, 2025
@mtrofin mtrofin force-pushed the users/mtrofin/12-02-_llt_mark_the_cfi_jumptable_naked_on_windows branch from 4dc36ce to fe3a86e Compare December 2, 2025 22:45
Copy link
Member Author

mtrofin commented Dec 2, 2025

Merge activity

  • Dec 2, 11:46 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Dec 2, 11:47 PM UTC: @mtrofin merged this pull request with Graphite.

@mtrofin mtrofin merged commit e9c1274 into main Dec 2, 2025
10 checks passed
@mtrofin mtrofin deleted the users/mtrofin/12-02-_llt_mark_the_cfi_jumptable_naked_on_windows branch December 2, 2025 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants