[PATCH] D122959: [ARM] Only update the successor edges for immediate predecessors of PrologueMBB.

[Zhiyao Ma via Phabricator via llvm-commits]

ZhiyaoMa98 created this revision.
ZhiyaoMa98 added reviewers: asl, ostannard, chill, john.brawn, efriedma.
Herald added subscribers: JDevlieghere, hiraditya, kristof.beyls.
Herald added a project: All.
ZhiyaoMa98 requested review of this revision.
Herald added a project: LLVM.

When adjusting the function prologue for segmented stacks, only update the successor edges of the immediate predecessors of the original prologue.

I encountered a segmentation fault in LLVM while compiling the Rust core targeting `armv7em` with segmented stack enabled. Below is my attempt to fix it. I am not very certain about the correctness of my patch, because I do not fully understand the code.

Looking at the do-while loop

  SmallPtrSet<MachineBasicBlock *, 8> BeforePrologueRegion;
  SmallVector<MachineBasicBlock *, 2> WalkList;
  WalkList.push_back(&PrologueMBB);
  
  do {
    MachineBasicBlock *CurMBB = WalkList.pop_back_val();
    for (MachineBasicBlock *PredBB : CurMBB->predecessors()) {
      if (BeforePrologueRegion.insert(PredBB).second)
        WalkList.push_back(PredBB);
    }
  } while (!WalkList.empty());

it seems `BeforePrologueRegion` contains non-immediate predecessors of `PrologueMBB `.

However, in the following while loop, `ReplaceUsesOfBlockWith` assumes that `MBB` is an immediate predecessor of `PrologueMBB`.

  for (MachineBasicBlock *MBB : BeforePrologueRegion) {
    // Make sure the LiveIns are still sorted and unique.
    MBB->sortUniqueLiveIns();
    // Replace the edges to PrologueMBB by edges to the sequences
    // we are about to add.
    MBB->ReplaceUsesOfBlockWith(&PrologueMBB, AddedBlocks[0]);
  }

The assumption comes from the function call chain

  MBB->ReplaceUsesOfBlockWith(&PrologueMBB, AddedBlocks[0]);
  --> replaceSuccessor(Old, New);
  ----> Old->removePredecessor(this);
  ------> assert(I != Predecessors.end()); Predecessors.erase(I);

I ran into this problem when I was compiling Rust core in release mode targeting `armv7em` with segmented stack enabled. The `Predecessors.erase(I);` line led to a segmentation fault.

I fixed the problem by adding the if-continue statement. It still passed all of the tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122959

Files:
  llvm/lib/Target/ARM/ARMFrameLowering.cpp


Index: llvm/lib/Target/ARM/ARMFrameLowering.cpp
===================================================================
--- llvm/lib/Target/ARM/ARMFrameLowering.cpp
+++ llvm/lib/Target/ARM/ARMFrameLowering.cpp
@@ -2570,6 +2570,9 @@
   for (MachineBasicBlock *MBB : BeforePrologueRegion) {
     // Make sure the LiveIns are still sorted and unique.
     MBB->sortUniqueLiveIns();
+    // Only update for immediate predecessors.
+    if (!PrologueMBB.isPredecessor(MBB))
+      continue;
     // Replace the edges to PrologueMBB by edges to the sequences
     // we are about to add.
     MBB->ReplaceUsesOfBlockWith(&PrologueMBB, AddedBlocks[0]);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D122959.419918.patch
Type: text/x-patch
Size: 642 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220402/37f075ab/attachment.bin>