On 10/1/20 10:55 PM, Leif Lindholm wrote:
On Thu, Oct 01, 2020 at 22:49:05 +0200, Ard Biesheuvel wrote:
On 10/1/20 8:37 PM, Leif Lindholm wrote:
The SetJump comment header states that:
    If JumpBuffer is NULL, then ASSERT().

However, this was not currently done.
Add a call to InternalAssertJumpBuffer.

Signed-off-by: Leif Lindholm <l...@nuviainc.com>
---
   MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S   | 3 +++
   MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm | 3 +++
   MdePkg/Library/BaseLib/Arm/SetJumpLongJump.S       | 3 +++
   MdePkg/Library/BaseLib/Arm/SetJumpLongJump.asm     | 3 +++
   4 files changed, 12 insertions(+)

diff --git a/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S 
b/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S
index 989736cee74c..34765a676430 100644
--- a/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S
+++ b/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S
@@ -45,6 +45,9 @@ GCC_ASM_EXPORT(InternalLongJump)
   #  );
   #
   ASM_PFX(SetJump):
+        stp     x30, x0, [sp, #-16]!
+        bl      InternalAssertJumpBuffer
+        ldp     x30, x0, [sp], #16

I think we should make this more idiomatic for AArch64 function calls, i.e.

         stp     x29, x30, [sp, #-32]!
         mov     x29, sp
         str     x0, [sp, #16]
         bl      InternalAssertJumpBuffer
         ldr     x0, [sp, #16]
         ldp     x29, x30, [sp], #32

That way, we'll have a well formed call stack with all the frame records
linked together, allowing the debugger to show you where SetJump() was
called from to begin with if you are stuck in the deadloop or hit the
breakpoint (depending on how the PCD was configured to begin with)

Mmm, you have a point.

I wouldn't mind putting the whole thing inside #ifndef MDEPKG_NDEBUG /#endif
btw

Well...
At that point we might as well go and un-bonkers-ify the interfaces and
make the C function the wrapper that does the assert check before
calling this function renamed to InternalSetJump - making it symmetric
with the LongJump side of the equation.
Which I was hoping to avoid, since that messes with all architectures.

Urgh, that is actually the sensible thing to do here, isn't it?


Do rhetorical questions expect an answer?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65820): https://edk2.groups.io/g/devel/message/65820
Mute This Topic: https://groups.io/mt/77247140/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to