[llvm] aff1ba1 - [X86] Fix null dereference in optimizeCompareInstr after lzcnt/tzcnt (#224282)

[via llvm-commits]

Author: tfzee
Date: 2026-09-22T11:58:18Z
New Revision: aff1ba103ab3006242c72af33420a3437f4ff1db

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

LOG: [X86] Fix null dereference in optimizeCompareInstr after lzcnt/tzcnt (#224282)

#210069 added `LTZCNTInst` to the exit condition of the backward scan of
`optimizeCompareInstr()`:

```c++
    if (MI || Sub || LTZCNTInst)
      break;
```

but it assigns `MI = LTZCNTInst` only inside the *forward* scan's
"EFLAGS is
used by this instruction" branch. When the forward scan never reaches
that
branch, both `MI` and `Sub` stay null.
However later code was built on the idea that one of them is not null.
 
Added a MIR test that would crash before but gets fixed by this
additional check.

Used AI for code review and comment/PR/test formatting/writing

Added: 
    llvm/test/CodeGen/X86/lzcnt-cmp-null-deref.mir

Modified: 
    llvm/lib/Target/X86/X86InstrInfo.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index 09a8ae6107020..d8d4b8e8ecf31 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -5623,6 +5623,14 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
           break;
         }
 
+        // Try to use CF produced by an LZCNT/TZCNT reading %SrcReg: it and
+        // "cmp $1, %SrcReg" both set CF iff %SrcReg is zero. The other flags
+        // 
diff er, so all EFLAGS users need to read CF only (ADC/SBB/RCL/RCR).
+        // Example:
+        //     lzcntq %rdi, %rax
+        //     ...                 // EFLAGS not changed
+        //     cmpq $1, %rdi       // <-- can be removed
+        //     adcq $0, %rax       // reads CF only
         if (isCmpRedundantAfterLTZCNT(SrcReg, SrcReg2, CmpMask, CmpValue,
                                       Inst)) {
           LTZCNTInst = &Inst;
@@ -5838,6 +5846,9 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
     }
   }
 
+  if (LTZCNTInst && !MI)
+    return false;
+
   // If we have to update users but EFLAGS is live-out abort, since we cannot
   // easily find all of the users.
   if ((MI != nullptr || ShouldUpdateCC) && FlagsMayLiveOut) {

diff  --git a/llvm/test/CodeGen/X86/lzcnt-cmp-null-deref.mir b/llvm/test/CodeGen/X86/lzcnt-cmp-null-deref.mir
new file mode 100644
index 0000000000000..73e259cbc2478
--- /dev/null
+++ b/llvm/test/CodeGen/X86/lzcnt-cmp-null-deref.mir
@@ -0,0 +1,54 @@
+# RUN: llc -mtriple=x86_64-- -mattr=+lzcnt -run-pass=peephole-opt -o - %s | FileCheck %s
+#
+# CID 3654645 / FORWARD_NULL: optimizeCompareInstr() breaks out of the backward
+# scan when only LTZCNTInst is set, but assigns MI = LTZCNTInst only after it has
+# found an ADC/SBB/RCL/RCR user of EFLAGS in the forward scan.  When no such user
+# exists in the compare's block, both MI and Sub stay null and
+# "Sub->getParent()" dereferences null.
+
+--- |
+  define i32 @flags_clobbered_after_cmp(i64 %x) { ret i32 0 }
+  define i64 @flags_live_out_of_cmp_block(i64 %x) { ret i64 0 }
+...
+
+# Case A: EFLAGS is redefined right after the compare by an instruction that does
+# not read it, so the forward scan breaks before the LTZCNT handling.
+---
+name:            flags_clobbered_after_cmp
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $rdi
+    ; CHECK-LABEL: name: flags_clobbered_after_cmp
+    ; CHECK: CMP64ri32 %0, 1, implicit-def $eflags
+    %0:gr64 = COPY $rdi
+    %1:gr64 = LZCNT64rr %0, implicit-def $eflags
+    CMP64ri32 %0, 1, implicit-def $eflags
+    %2:gr32 = MOV32r0 implicit-def dead $eflags
+    $eax = COPY %2
+    RET64 implicit $eax
+...
+
+# Case B: no EFLAGS user at all in the compare's block; EFLAGS is live out to a
+# successor.  The "(MI != nullptr || ShouldUpdateCC) && FlagsMayLiveOut" guard
+# that is meant to reject this is skipped because MI is null.
+---
+name:            flags_live_out_of_cmp_block
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    successors: %bb.1
+    liveins: $rdi
+    ; CHECK-LABEL: name: flags_live_out_of_cmp_block
+    ; CHECK: CMP64ri32 %0, 1, implicit-def $eflags
+    %0:gr64 = COPY $rdi
+    %1:gr64 = LZCNT64rr %0, implicit-def $eflags
+    CMP64ri32 %0, 1, implicit-def $eflags
+    JMP_1 %bb.1
+
+  bb.1:
+    liveins: $eflags
+    %2:gr64 = ADC64ri32 %1, 0, implicit-def dead $eflags, implicit $eflags
+    $rax = COPY %2
+    RET64 implicit $rax
+...