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)
I wouldn't mind putting the whole thing inside #ifndef MDEPKG_NDEBUG
/#endif btw
mov x16, sp // use IP0 so save SP
#define REG_PAIR(REG1, REG2, OFFS) stp REG1, REG2, [x0, OFFS]
#define REG_ONE(REG1, OFFS) str REG1, [x0, OFFS]
diff --git a/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm
b/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm
index 8922128e8c62..f2729a8bb03e 100644
--- a/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm
+++ b/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm
@@ -44,6 +44,9 @@
; );
;
SetJump
+ stp x30, x0, [sp, #-16]!
+ bl InternalAssertJumpBuffer
+ ldp x30, x0, [sp], #16
Same here
mov x16, sp // use IP0 so save SP
#define REG_PAIR(REG1, REG2, OFFS) stp REG1, REG2, [x0, OFFS]
#define REG_ONE(REG1, OFFS) str REG1, [x0, OFFS]
diff --git a/MdePkg/Library/BaseLib/Arm/SetJumpLongJump.S
b/MdePkg/Library/BaseLib/Arm/SetJumpLongJump.S
index e4c1946a28ff..54b11ad2197c 100644
--- a/MdePkg/Library/BaseLib/Arm/SetJumpLongJump.S
+++ b/MdePkg/Library/BaseLib/Arm/SetJumpLongJump.S
@@ -31,6 +31,9 @@ GCC_ASM_EXPORT(InternalLongJump)
# );
#
ASM_PFX(SetJump):
+ push {r0, lr}
+ bl InternalAssertJumpBuffer
+ pop {r0, lr}
... but not here (but the #ifndef would still be an improvement imo)
mov r3, r13
stmia r0, {r3-r12,r14}
eor r0, r0, r0
diff --git a/MdePkg/Library/BaseLib/Arm/SetJumpLongJump.asm
b/MdePkg/Library/BaseLib/Arm/SetJumpLongJump.asm
index e1eff758f7ab..6d47033975f2 100644
--- a/MdePkg/Library/BaseLib/Arm/SetJumpLongJump.asm
+++ b/MdePkg/Library/BaseLib/Arm/SetJumpLongJump.asm
@@ -31,6 +31,9 @@
; )
;
SetJump
+ PUSH {R0, LR}
+ BL InternalAssertJumpBuffer
+ POP {R0, LR}
MOV R3, R13
STM R0, {R3-R12,R14}
EOR R0, R0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65818): https://edk2.groups.io/g/devel/message/65818
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]
-=-=-=-=-=-=-=-=-=-=-=-
