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

[AArch64] Fix register reversing crash on MSVC with 1 pair. #82392

Closed
wants to merge 3 commits into from

Conversation

ScottTodd
Copy link
Member

This fixes the crash reported here: #79623 (comment) where this assert was hit:

#if _ITERATOR_DEBUG_LEVEL != 0
template <class _Ty>
constexpr void _Verify_range(const _Ty* const _First, const _Ty* const _Last) noexcept {
    // special case range verification for pointers
    _STL_VERIFY(_First <= _Last, "transposed pointer range");
}
#endif // _ITERATOR_DEBUG_LEVEL != 0

Info from the debugger on the test case that crashed:
image

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 20, 2024

@llvm/pr-subscribers-backend-aarch64

Author: Scott Todd (ScottTodd)

Changes

This fixes the crash reported here: #79623 (comment) where this assert was hit:

#if _ITERATOR_DEBUG_LEVEL != 0
template &lt;class _Ty&gt;
constexpr void _Verify_range(const _Ty* const _First, const _Ty* const _Last) noexcept {
    // special case range verification for pointers
    _STL_VERIFY(_First &lt;= _Last, "transposed pointer range");
}
#endif // _ITERATOR_DEBUG_LEVEL != 0

Info from the debugger on the test case that crashed:
image


Full diff: https://github.com/llvm/llvm-project/pull/82392.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+10-8)
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index c013bbe9926fef..6c8117cb978df5 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -3115,14 +3115,16 @@ bool AArch64FrameLowering::restoreCalleeSavedRegisters(
   }
 
   // For performance reasons restore SVE register in increasing order
-  auto IsPPR = [](const RegPairInfo &c) { return c.Type == RegPairInfo::PPR; };
-  auto PPRBegin = std::find_if(RegPairs.begin(), RegPairs.end(), IsPPR);
-  auto PPREnd = std::find_if(RegPairs.rbegin(), RegPairs.rend(), IsPPR);
-  std::reverse(PPRBegin, PPREnd.base());
-  auto IsZPR = [](const RegPairInfo &c) { return c.Type == RegPairInfo::ZPR; };
-  auto ZPRBegin = std::find_if(RegPairs.begin(), RegPairs.end(), IsZPR);
-  auto ZPREnd = std::find_if(RegPairs.rbegin(), RegPairs.rend(), IsZPR);
-  std::reverse(ZPRBegin, ZPREnd.base());
+  if (RegPairs.size() > 1) {
+    auto IsPPR = [](const RegPairInfo &c) { return c.Type == RegPairInfo::PPR; };
+    auto PPRBegin = std::find_if(RegPairs.begin(), RegPairs.end(), IsPPR);
+    auto PPREnd = std::find_if(RegPairs.rbegin(), RegPairs.rend(), IsPPR);
+    std::reverse(PPRBegin, PPREnd.base());
+    auto IsZPR = [](const RegPairInfo &c) { return c.Type == RegPairInfo::ZPR; };
+    auto ZPRBegin = std::find_if(RegPairs.begin(), RegPairs.end(), IsZPR);
+    auto ZPREnd = std::find_if(RegPairs.rbegin(), RegPairs.rend(), IsZPR);
+    std::reverse(ZPRBegin, ZPREnd.base());
+  }
 
   for (const RegPairInfo &RPI : RegPairs) {
     unsigned Reg1 = RPI.Reg1;

Copy link

github-actions bot commented Feb 20, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@momchil-velikov momchil-velikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect the issue is with dereferencing PPREnd.base() when PPREnd == RegPairs.rend() (same with ZPR).
And it can happen even if RegPairs.size() > 1, for example if there are no PPR pairs at all.
(it'll be a couple of hours before I get my hands on a Windows machine to confirm and/or investigate).

@ScottTodd
Copy link
Member Author

I suspect the issue is with dereferencing PPREnd.base() when PPREnd == RegPairs.rend() (same with ZPR). And it can happen even if RegPairs.size() > 1, for example if there are no PPR pairs at all. (it'll be a couple of hours before I get my hands on a Windows machine to confirm and/or investigate).

Ah, interesting. Should we revert for now and then land with these cases fixed?

@ScottTodd
Copy link
Member Author

Commits were reverted. LMK if I can help test other fixes - not sure I have enough context here to iterate on this patch.

@ScottTodd ScottTodd closed this Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants