Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inliner asserts with cleanuppad #53206

Closed
nikic opened this issue Jan 14, 2022 · 4 comments
Closed

Inliner asserts with cleanuppad #53206

nikic opened this issue Jan 14, 2022 · 4 comments
Assignees

Comments

@nikic
Copy link
Contributor

nikic commented Jan 14, 2022

; RUN: opt -S -inline < %s
define void @foo() personality i32 (...)* undef {
entry:
  br i1 false, label %join, label %split

split:                                            ; preds = %entry
  br label %join

join:                                             ; preds = %split, %entry
  %phi = phi i64 [ 1, %split ], [ 0, %entry ]
  %cmp = icmp ugt i64 1, %phi
  br i1 %cmp, label %invoke1, label %exit

invoke1:                                          ; preds = %join
  invoke void undef()
          to label %exit unwind label %cleanup1

cleanup1:                                         ; preds = %invoke1
  %pad1 = cleanuppad within none []
  br label %cleanup1.cont

cleanup1.cont:                                    ; preds = %invoke2, %cleanup1
  br i1 undef, label %cleanupret, label %invoke2

invoke2:                                          ; preds = %cleanup1.cont
  invoke void undef()
          to label %cleanup1.cont unwind label %cleanup2

cleanup2:                                         ; preds = %invoke2
  %pad2 = cleanuppad within none []
  br label %cleanupret

cleanupret:                                       ; preds = %cleanup2, %cleanup1.cont
  cleanupret from %pad1 unwind to caller

exit:                                             ; preds = %invoke1, %join
  ret void
}

define void @test() personality i32 (...)* undef {
  invoke void @foo()
          to label %exit unwind label %cleanup

exit:                                             ; preds = %0
  ret void

cleanup:                                          ; preds = %0
  %pad = cleanuppad within none []
  cleanupret from %pad unwind to caller
}

Asserts:

opt: /home/npopov/repos/llvm-project/llvm/include/llvm/Support/Casting.h:255: std::enable_if_t<(! llvm::is_simple_type<Y>::value), typename llvm::cast_retty<X, const Y>::ret_type> llvm::cast(const Y&) [with X = llvm::CleanupPadInst; Y = llvm::Use; std::enable_if_t<(! llvm::is_simple_type<Y>::value), typename llvm::cast_retty<X, const Y>::ret_type> = llvm::CleanupPadInst*; typename llvm::cast_retty<X, const Y>::ret_type = llvm::CleanupPadInst*]: Assertion `isa<X>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build/bin/opt -S -inline -debug-only=inline test064.ll
 #0 0x000000000316a451 PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
 #1 0x0000000003167c2e SignalHandler(int) Signals.cpp:0:0
 #2 0x00007ffabebb8750 __restore_rt (/lib64/libc.so.6+0x42750)
 #3 0x00007ffabec0584c __pthread_kill_implementation (/lib64/libc.so.6+0x8f84c)
 #4 0x00007ffabebb86a6 gsignal (/lib64/libc.so.6+0x426a6)
 #5 0x00007ffabeba27d3 abort (/lib64/libc.so.6+0x2c7d3)
 #6 0x00007ffabeba26fb _nl_load_domain.cold (/lib64/libc.so.6+0x2c6fb)
 #7 0x00007ffabebb1396 (/lib64/libc.so.6+0x3b396)
 #8 0x0000000003211cdf HandleInlinedEHPad(llvm::InvokeInst*, llvm::BasicBlock*, llvm::ClonedCodeInfo&) InlineFunction.cpp:0:0
 #9 0x0000000003217fd3 llvm::InlineFunction(llvm::CallBase&, llvm::InlineFunctionInfo&, llvm::AAResults*, bool, llvm::Function*) (build/bin/opt+0x3217fd3)
#10 0x0000000002976948 llvm::InlinerPass::run(llvm::LazyCallGraph::SCC&, llvm::AnalysisManager<llvm::LazyCallGraph::SCC, llvm::LazyCallGraph&>&, llvm::LazyCallGraph&, llvm::CGSCCUpdateResult&) (build/bin/opt+0x2976948)
@nikic
Copy link
Contributor Author

nikic commented Jan 14, 2022

A possibly helpful variant is this:

entry:
  br i1 false, label %join, label %split

split:                                            ; preds = %entry
  br label %join

join:                                             ; preds = %split, %entry
  %phi = phi i64 [ 1, %split ], [ 0, %entry ]
  %cmp = icmp ugt i64 1, %phi
  br i1 %cmp, label %invoke1, label %exit

invoke1:                                          ; preds = %join
  invoke void undef()
          to label %exit unwind label %cleanup1

cleanup1:                                         ; preds = %invoke1
  %pad1 = cleanuppad within none []
  br label %cleanup1.cont

cleanup1.cont:                                    ; preds = %invoke2, %cleanup1
  br i1 undef, label %cleanupret, label %invoke2

invoke2:                                          ; preds = %cleanup1.cont
  invoke void undef()
          to label %cleanup1.cont unwind label %cleanup2

cleanup2:                                         ; preds = %invoke2
  %pad2 = cleanuppad within none []
  br label %cleanupret

cleanupret:                                       ; preds = %cleanup2, %cleanup1.cont
  cleanupret from %pad1 unwind to caller

exit:                                             ; preds = %invoke1, %join
  ret void
}

define void @test() personality i32 (...)* undef {
  call void @foo()
  ret void
}

This results in a verifier error rather than an assert, with the following IR produced for @test():

define void @test() personality i32 (...)* undef {
  br label %foo.exit

cleanup1.cont.i:                                  ; preds = %invoke2.i
  br i1 undef, label %cleanupret.i, label %invoke2.i

invoke2.i:                                        ; preds = %cleanup1.cont.i
  invoke void undef()
          to label %cleanup1.cont.i unwind label %cleanup2.i

cleanup2.i:                                       ; preds = %invoke2.i
  %pad2.i = cleanuppad within none []
  br label %cleanupret.i

cleanupret.i:                                     ; preds = %cleanup2.i, %cleanup1.cont.i
  cleanupret from undef unwind to caller

foo.exit:                                         ; preds = %0
  ret void
}

@efriedma-quic
Copy link
Collaborator

The testcases given are not quite valid... missing required "funclet" marking on invoke in cleanup. Verifier should be checking for that, I think? Anyway, fixed testcase:

define internal void @foo() personality i32 (...)* undef {
entry:
  br i1 false, label %join, label %split

split:
  br label %join

join:
  %phi = phi i64 [ 1, %split ], [ 0, %entry ]
  %cmp = icmp ugt i64 1, %phi
  br i1 %cmp, label %invoke1, label %exit

invoke1:
  invoke void undef()
          to label %exit unwind label %cleanup1

cleanup1:
  %pad1 = cleanuppad within none []
  br label %cleanup1.cont

cleanup1.cont:
  br i1 undef, label %cleanupret, label %invoke2

invoke2:
  invoke void undef() [ "funclet"(token %pad1) ]
          to label %cleanup1.cont unwind label %cleanup2

cleanup2:
  %pad2 = cleanuppad within %pad1 []
  unreachable

cleanupret:
  unreachable

exit:
  ret void
}

define void @test() personality i32 (...)* undef {
  call void @foo()
  ret void
}

I think the inliner's code for updating the EH pads is getting confused because the inlined code is unreachable. The simplest way to fix this might be to just teach the inliner to erase unreachable code...

@nikic
Copy link
Contributor Author

nikic commented Jan 28, 2022

I think the inliner's code for updating the EH pads is getting confused because the inlined code is unreachable. The simplest way to fix this might be to just teach the inliner to erase unreachable code...

I just looked into this a bit. The pruning function cloner will only clone blocks that are reachable -- the problem is that PHI nodes only get resolved and folded after that, and then there's a second pass for folding terminators and dropping dead blocks:

ConstantFoldTerminator(&*I);
// Check if this block has become dead during inlining or other
// simplifications. Note that the first block will appear dead, as it has
// not yet been wired up properly.
if (I != Begin && (pred_empty(&*I) || I->getSinglePredecessor() == &*I)) {
BasicBlock *DeadBB = &*I++;
DeleteDeadBlock(DeadBB);
continue;
}

But that code will only drop trivially dead blocks, not those that are in unreachable cycles.

@nikic nikic self-assigned this Jan 28, 2022
@nikic
Copy link
Contributor Author

nikic commented Jan 28, 2022

Candidate patch: https://reviews.llvm.org/D118449

@nikic nikic closed this as completed in 4810051 Jan 31, 2022
Amanieu pushed a commit to Amanieu/llvm-project that referenced this issue Feb 1, 2022
…PR53206)

The pruning cloner already tries to remove unreachable blocks. The
original cloning process will simplify instructions and constant
terminators, and only clone blocks that are reachable at that point.
However, phi nodes can only be simplified after everything has been
cloned. For that reason, additional blocks may become unreachable
after phi simplification.

The code does try to handle this as well, but only removes blocks
that don't have predecessors. It misses unreachable cycles. This
can cause issues if SEH exception handling code is part of an
unreachable cycle, as the inliner is not prepared to deal with that.

This patch instead performs an explicit scan for reachable blocks,
and drops everything else.

Fixes llvm#53206.

Differential Revision: https://reviews.llvm.org/D118449
nikic added a commit to rust-lang/llvm-project that referenced this issue Feb 1, 2022
…PR53206)

The pruning cloner already tries to remove unreachable blocks. The
original cloning process will simplify instructions and constant
terminators, and only clone blocks that are reachable at that point.
However, phi nodes can only be simplified after everything has been
cloned. For that reason, additional blocks may become unreachable
after phi simplification.

The code does try to handle this as well, but only removes blocks
that don't have predecessors. It misses unreachable cycles. This
can cause issues if SEH exception handling code is part of an
unreachable cycle, as the inliner is not prepared to deal with that.

This patch instead performs an explicit scan for reachable blocks,
and drops everything else.

Fixes llvm#53206.

Differential Revision: https://reviews.llvm.org/D118449
hedgar2017 pushed a commit to matter-labs/era-compiler-llvm that referenced this issue Jun 28, 2022
…PR53206)

The pruning cloner already tries to remove unreachable blocks. The
original cloning process will simplify instructions and constant
terminators, and only clone blocks that are reachable at that point.
However, phi nodes can only be simplified after everything has been
cloned. For that reason, additional blocks may become unreachable
after phi simplification.

The code does try to handle this as well, but only removes blocks
that don't have predecessors. It misses unreachable cycles. This
can cause issues if SEH exception handling code is part of an
unreachable cycle, as the inliner is not prepared to deal with that.

This patch instead performs an explicit scan for reachable blocks,
and drops everything else.

Fixes llvm/llvm-project#53206.

Differential Revision: https://reviews.llvm.org/D118449
arichardson pushed a commit to CTSRD-CHERI/llvm-project that referenced this issue Jan 3, 2023
…PR53206)

The pruning cloner already tries to remove unreachable blocks. The
original cloning process will simplify instructions and constant
terminators, and only clone blocks that are reachable at that point.
However, phi nodes can only be simplified after everything has been
cloned. For that reason, additional blocks may become unreachable
after phi simplification.

The code does try to handle this as well, but only removes blocks
that don't have predecessors. It misses unreachable cycles. This
can cause issues if SEH exception handling code is part of an
unreachable cycle, as the inliner is not prepared to deal with that.

This patch instead performs an explicit scan for reachable blocks,
and drops everything else.

Fixes llvm/llvm-project#53206.

Differential Revision: https://reviews.llvm.org/D118449
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants