To align to longjump() declaration, could you update SetJump()? UINTN EFIAPI RETURNS_TWICE SetJump ( );
==> RETURNS_TWICE UINTN EFIAPI SetJump ( ); Thanks Liming > -----Original Message----- > From: M1cha [mailto:sigmaepsilo...@gmail.com] > Sent: Saturday, December 23, 2017 3:14 AM > To: edk2-devel@lists.01.org > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; Kinney, Michael D > <michael.d.kin...@intel.com>; Gao, Liming > <liming....@intel.com> > Subject: [edk2] [PATCH v2 2/3] MdePkg/BaseLib: add attribute 'RETURNS_TWICE' > to SetJump > > When compiling with any ARM toolchain and Os, registers can get > trashed when returning for the second time from SetJump because GCC > only handles this correctly when using standard names like 'setjmp' or > 'getcontext'. When different names are used you have to use the > attribute 'returns_twice' to tell gcc to be extra careful. > > example: > extern int FN_NAME(void*); > > void jmp_buf_set(void *jmpb, void (*f)(void)) > { > if (!FN_NAME(jmpb)) > f(); > } > > this code produces this wrong code with Os: > 00000000 <jmp_buf_set>: > 0: e92d4010 push {r4, lr} > 4: e1a04001 mov r4, r1 > 8: ebfffffe bl 0 <nonstandard_setjmp> > c: e3500000 cmp r0, #0 > 10: 01a03004 moveq r3, r4 > 14: 08bd4010 popeq {r4, lr} > 18: 012fff13 bxeq r3 > 1c: e8bd4010 pop {r4, lr} > 20: e12fff1e bx lr > > The generated code pushes backups of r4 and lr to the stack and then > saves all registers using nonstandard_setjmp. > Then it pops the stack and jumps to the function in r3 which is the > main problem because now the function can overwrite our register > backups on the stack. > When we return a second time from the call to nonstandard_setjmp, the > stack pointer has it's original(pushed) position and when the code > pops r4 and lr from the stack the values are not guaranteed to be the > same. > > When using a standard name like setjmp or getcontext or adding > '__attribute__((returns_twice))' to nonstandard_setjmp's declaration > the code looks different: > > 00000000 <jmp_buf_set>: > 0: e92d4007 push {r0, r1, r2, lr} > 4: e58d1004 str r1, [sp, #4] > 8: ebfffffe bl 0 <setjmp> > c: e3500000 cmp r0, #0 > 10: 059d3004 ldreq r3, [sp, #4] > 14: 01a0e00f moveq lr, pc > 18: 012fff13 bxeq r3 > 1c: e28dd00c add sp, sp, #12 > 20: e49de004 pop {lr} ; (ldr lr, [sp], #4) > 24: e12fff1e bx lr > > Here the problem is being solved by restoring r3 from the stack > without popping it. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Michael Zimmermann <sigmaepsilo...@gmail.com> > --- > MdePkg/Include/Library/BaseLib.h | 1 + > MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c | 1 + > MdePkg/Library/BaseLib/Ia32/SetJump.c | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/MdePkg/Include/Library/BaseLib.h > b/MdePkg/Include/Library/BaseLib.h > index 2b98af4cd17e..10976032adaa 100644 > --- a/MdePkg/Include/Library/BaseLib.h > +++ b/MdePkg/Include/Library/BaseLib.h > @@ -4905,6 +4905,7 @@ MemoryFence ( > **/ > UINTN > EFIAPI > +RETURNS_TWICE > SetJump ( > OUT BASE_LIBRARY_JUMP_BUFFER *JumpBuffer > ); > diff --git a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c > b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c > index 4c0dba55d52f..e309e8b57d7a 100644 > --- a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c > +++ b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c > @@ -34,6 +34,7 @@ > **/ > UINTN > EFIAPI > +RETURNS_TWICE > SetJump ( > OUT BASE_LIBRARY_JUMP_BUFFER *JumpBuffer > ) > diff --git a/MdePkg/Library/BaseLib/Ia32/SetJump.c > b/MdePkg/Library/BaseLib/Ia32/SetJump.c > index 304f3839b108..40fd16bae8fd 100644 > --- a/MdePkg/Library/BaseLib/Ia32/SetJump.c > +++ b/MdePkg/Library/BaseLib/Ia32/SetJump.c > @@ -51,6 +51,7 @@ InternalAssertJumpBuffer ( > _declspec (naked) > UINTN > EFIAPI > +RETURNS_TWICE > SetJump ( > OUT BASE_LIBRARY_JUMP_BUFFER *JumpBuffer > ) > -- > 2.15.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
