[DebugInfo] Assertion failure `!NodePtr->isKnownSentinel()' when fixing up debug info #105571
Comments
|
@llvm/issue-subscribers-debuginfo Author: Jonathon Penix (jonathonpenix)
I'm seeing a failed assert '!NodePtr->isKnownSentinel()' when debug info is being handled as part of the BasicBlock `splice()` API. I think we are able to dereference an `end()` iterator (`Dest`) here under certain circumstances:
llvm-project/llvm/lib/IR/BasicBlock.cpp Line 974 in 47e0212 I was originally seeing this with a pass that is not upstream, but please see below for a unittest that should demonstrate the issue using only upstream LLVM. I tested this using LLVM at f7bbc40 and apologies in advance if there is just something wrong in my test! Test in commit form: jonathonpenix@fab9bba Test in text-form: |
|
Thanks for the report and the unittest. You're right, it looks like we erroneously dereference |
OCHyams
added a commit
to OCHyams/llvm-project
that referenced
this issue
Aug 22, 2024
Fix llvm#105571 which demonstrates an end() iterator dereference when performing a non-empty splice to end() from a region that ends at Src::end(). Rather than calling Instruction::adoptDbgRecords from Dest, create a marker (which takes an iterator) and absorbDebugValues onto that. The "absorb" variant doesn't clean up the source marker, which in this case we know is a trailing marker, so we have to do that manually.
OCHyams
added a commit
that referenced
this issue
Aug 28, 2024
…105670) Not quite NFC, fixes splitBasicBlockBefore case when we split before an instruction with debug records (but without the headBit set, i.e., we are splitting before the instruction but after the debug records that come before it). splitBasicBlockBefore splices the instructions before the split point into a new block. Prior to this patch, the debug records would get shifted up to the front of the spliced instructions (as seen in the modified unittest - I believe the unittest was checking erroneous behaviour). We instead want to leave those debug records at the end of the spliced instructions. The functionality of the deleted `else if` branch is covered by the remaining `if` now that `DestMarker` is set to the trailing marker if `Dest` is `end()`. Previously the "===" markers were sometimes detached, now we always detach them and always reattach them. Note: `deleteTrailingDbgRecords` only "unlinks" the tailing marker from the block, it doesn't delete anything. The trailing marker is still cleaned up properly inside the final `if` body with `DestMarker->eraseFromParent();`. Part 1 of 2 needed for #105571
OCHyams
added a commit
to OCHyams/llvm-project
that referenced
this issue
Aug 29, 2024
Fix llvm#105571 which demonstrates an end() iterator dereference when performing a non-empty splice to end() from a region that ends at Src::end(). Rather than calling Instruction::adoptDbgRecords from Dest, create a marker (which takes an iterator) and absorbDebugValues onto that. The "absorb" variant doesn't clean up the source marker, which in this case we know is a trailing marker, so we have to do that manually.
OCHyams
added a commit
that referenced
this issue
Aug 29, 2024
Fix #105571 which demonstrates an end() iterator dereference when performing a non-empty splice to end() from a region that ends at Src::end(). Rather than calling Instruction::adoptDbgRecords from Dest, create a marker (which takes an iterator) and absorbDebugValues onto that. The "absorb" variant doesn't clean up the source marker, which in this case we know is a trailing marker, so we have to do that manually.
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this issue
Aug 30, 2024
…lvm#105670) Not quite NFC, fixes splitBasicBlockBefore case when we split before an instruction with debug records (but without the headBit set, i.e., we are splitting before the instruction but after the debug records that come before it). splitBasicBlockBefore splices the instructions before the split point into a new block. Prior to this patch, the debug records would get shifted up to the front of the spliced instructions (as seen in the modified unittest - I believe the unittest was checking erroneous behaviour). We instead want to leave those debug records at the end of the spliced instructions. The functionality of the deleted `else if` branch is covered by the remaining `if` now that `DestMarker` is set to the trailing marker if `Dest` is `end()`. Previously the "===" markers were sometimes detached, now we always detach them and always reattach them. Note: `deleteTrailingDbgRecords` only "unlinks" the tailing marker from the block, it doesn't delete anything. The trailing marker is still cleaned up properly inside the final `if` body with `DestMarker->eraseFromParent();`. Part 1 of 2 needed for llvm#105571 (cherry picked from commit f581553)
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this issue
Aug 30, 2024
Fix llvm#105571 which demonstrates an end() iterator dereference when performing a non-empty splice to end() from a region that ends at Src::end(). Rather than calling Instruction::adoptDbgRecords from Dest, create a marker (which takes an iterator) and absorbDebugValues onto that. The "absorb" variant doesn't clean up the source marker, which in this case we know is a trailing marker, so we have to do that manually. (cherry picked from commit 43661a1)
|
There is a leak in patches, we need either revert or fix it somehow. https://lab.llvm.org/buildbot/#/builders/169/builds/2622/steps/9/logs/stdio |
This was referenced Aug 30, 2024
|
The leak is already fixed. |
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this issue
Sep 1, 2024
…lvm#105670) Not quite NFC, fixes splitBasicBlockBefore case when we split before an instruction with debug records (but without the headBit set, i.e., we are splitting before the instruction but after the debug records that come before it). splitBasicBlockBefore splices the instructions before the split point into a new block. Prior to this patch, the debug records would get shifted up to the front of the spliced instructions (as seen in the modified unittest - I believe the unittest was checking erroneous behaviour). We instead want to leave those debug records at the end of the spliced instructions. The functionality of the deleted `else if` branch is covered by the remaining `if` now that `DestMarker` is set to the trailing marker if `Dest` is `end()`. Previously the "===" markers were sometimes detached, now we always detach them and always reattach them. Note: `deleteTrailingDbgRecords` only "unlinks" the tailing marker from the block, it doesn't delete anything. The trailing marker is still cleaned up properly inside the final `if` body with `DestMarker->eraseFromParent();`. Part 1 of 2 needed for llvm#105571 (cherry picked from commit f581553)
OCHyams
added a commit
to OCHyams/llvm-project
that referenced
this issue
Sep 2, 2024
Fix llvm#105571 which demonstrates an end() iterator dereference when performing a non-empty splice to end() from a region that ends at Src::end(). Rather than calling Instruction::adoptDbgRecords from Dest, create a marker (which takes an iterator) and absorbDebugValues onto that. The "absorb" variant doesn't clean up the source marker, which in this case we know is a trailing marker, so we have to do that manually. (cherry picked from commit 43661a1)
tru
pushed a commit
to OCHyams/llvm-project
that referenced
this issue
Sep 10, 2024
Fix llvm#105571 which demonstrates an end() iterator dereference when performing a non-empty splice to end() from a region that ends at Src::end(). Rather than calling Instruction::adoptDbgRecords from Dest, create a marker (which takes an iterator) and absorbDebugValues onto that. The "absorb" variant doesn't clean up the source marker, which in this case we know is a trailing marker, so we have to do that manually. (cherry picked from commit 43661a1)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I'm seeing a failed assert '!NodePtr->isKnownSentinel()' when debug info is being handled as part of the BasicBlock
splice()API. I think we are able to dereference anend()iterator (Dest) here under certain circumstances:llvm-project/llvm/lib/IR/BasicBlock.cpp
Line 974 in 47e0212
I was originally seeing this with a pass that is not upstream, but please see below for a unittest that should demonstrate the issue using only upstream LLVM. I tested this using LLVM at f7bbc40 and apologies in advance if there is just something wrong in my test!
Test in commit form: jonathonpenix@fab9bba
Test in text-form:
The text was updated successfully, but these errors were encountered: