[CodeGen][CodeLayout] Fix segfault on access to deleted block in MBP. #142357
Conversation
|
We have caught the bug on the custom out-of-tree backend in a ~200Kb function of ~70 basic blocks with several loops, switch instructions etc.. Any attempt to reduce the test case results in bug disappearing. I'm afraid, I'm not able to provide the reproducer / regression test for any in-tree backend in a reasonable time. |
|
Could you please review the PR? |
From function markBlockSuccessors, a BB can be added to BlockWorkList only when UnscheduledPredecessors reaches 0. So how was B added to BlockWorkList in the first place? |
Yes, it is a bit tricky:
Work lists might contain blocks from chains with non-zero |
|
Some test in flang failed. Need to be investigated. |
ivafanas
force-pushed
the
dev/fix-segfault-in-block-placement
branch
from
501954d to
034458b
Compare
|
PR is ready for review. Seems like flang test is flaky. I've failed to reproduce the problem locally and it disappeared after rerun. |
|
Missing test |
If test is a mandatory requirement for this PR, it is ok for me to reject PR. |
Did you try llvm-reduce on it? |
Yes. No effect. I've spent ~1 day in different attempts to reduce the test case =) |
|
Ping. Should PR be closed or merged? |
|
Ping. It's ok for me to leave the fix in our customer's fork. |
|
Ok, I'll close the PR so as no response. |
The volume of PRs is impossible to keep up with, you should not expect prompt replies |
Did you try llvm-reduce on the MIR? |
…llvm#142357) Problem 1: There is a typo which reassigns `BlockWorkList` to `EHPadWorkList` on attempt to remove `RemBB` from work lists. Problem 2: `Chain->UnscheduledPredecessors == 0` is an incorrect way to check whether `RemBB` is enqueued or not. The root cause is a postponed deletion of `WorkList` from already scheduled blocks in `selectBestCandidateBlock`. Bug happens in the following scenario: * `FunctionChain` is being processed with non-zero `UnscheduledPredecessors` * Block `B'` is added to the `BlockWorkList` * Block `B'` is chosen as the best successor (`selectBestSuccessor`) for some another block and added into `Chain` * Block `B'` is removed by tail duplicator. `RemovalCallback` erroneously won't erase `B'` from `BlockWorkList`, because `UnscheduledPredecessors` value of `FunctionChain` is not zero (and it is allowed to be non-zero). Proposed solution is to always cleanup worklists on block deletion by tail duplicator.
…llvm#142357) Problem 1: There is a typo which reassigns `BlockWorkList` to `EHPadWorkList` on attempt to remove `RemBB` from work lists. Problem 2: `Chain->UnscheduledPredecessors == 0` is an incorrect way to check whether `RemBB` is enqueued or not. The root cause is a postponed deletion of `WorkList` from already scheduled blocks in `selectBestCandidateBlock`. Bug happens in the following scenario: * `FunctionChain` is being processed with non-zero `UnscheduledPredecessors` * Block `B'` is added to the `BlockWorkList` * Block `B'` is chosen as the best successor (`selectBestSuccessor`) for some another block and added into `Chain` * Block `B'` is removed by tail duplicator. `RemovalCallback` erroneously won't erase `B'` from `BlockWorkList`, because `UnscheduledPredecessors` value of `FunctionChain` is not zero (and it is allowed to be non-zero). Proposed solution is to always cleanup worklists on block deletion by tail duplicator.
…llvm#142357) Problem 1: There is a typo which reassigns `BlockWorkList` to `EHPadWorkList` on attempt to remove `RemBB` from work lists. Problem 2: `Chain->UnscheduledPredecessors == 0` is an incorrect way to check whether `RemBB` is enqueued or not. The root cause is a postponed deletion of `WorkList` from already scheduled blocks in `selectBestCandidateBlock`. Bug happens in the following scenario: * `FunctionChain` is being processed with non-zero `UnscheduledPredecessors` * Block `B'` is added to the `BlockWorkList` * Block `B'` is chosen as the best successor (`selectBestSuccessor`) for some another block and added into `Chain` * Block `B'` is removed by tail duplicator. `RemovalCallback` erroneously won't erase `B'` from `BlockWorkList`, because `UnscheduledPredecessors` value of `FunctionChain` is not zero (and it is allowed to be non-zero). Proposed solution is to always cleanup worklists on block deletion by tail duplicator.
Problem 1: There is a typo which reassigns
BlockWorkListtoEHPadWorkListon attempt to removeRemBBfrom work lists.Problem 2:
Chain->UnscheduledPredecessors == 0is an incorrect way to check whetherRemBBis enqueued or not. The root cause is a postponed deletion ofWorkListfrom already scheduled blocks inselectBestCandidateBlock. Bug happens in the following scenario:FunctionChainis being processed with non-zeroUnscheduledPredecessorsB'is added to theBlockWorkListB'is chosen as the best successor (selectBestSuccessor) for some another block and added intoChainB'is removed by tail duplicator.RemovalCallbackerroneously won't eraseB'fromBlockWorkList, becauseUnscheduledPredecessorsvalue ofFunctionChainis not zero (and it is allowed to be non-zero).Proposed solution is to always cleanup worklists on block deletion by tail duplicator.