From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.20; helo=mga02.intel.com; envelope-from=liming.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C5D5A21CB2E5F for ; Tue, 2 Jan 2018 04:26:16 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Jan 2018 04:31:18 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,497,1508828400"; d="scan'208";a="191605470" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga005.fm.intel.com with ESMTP; 02 Jan 2018 04:31:17 -0800 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 2 Jan 2018 04:31:17 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 2 Jan 2018 04:31:16 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.152]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.213]) with mapi id 14.03.0319.002; Tue, 2 Jan 2018 20:31:14 +0800 From: "Gao, Liming" To: M1cha , "edk2-devel@lists.01.org" CC: Ard Biesheuvel , "Kinney, Michael D" Thread-Topic: [edk2] [PATCH v2 2/3] MdePkg/BaseLib: add attribute 'RETURNS_TWICE' to SetJump Thread-Index: AQHTe1kV2kJ9F2AKWEW1sLWRAx66NKNgk+Uw Date: Tue, 2 Jan 2018 12:31:13 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E19D288@SHSMSX104.ccr.corp.intel.com> References: <20171222191416.5105-1-sigmaepsilon92@gmail.com> In-Reply-To: <20171222191416.5105-1-sigmaepsilon92@gmail.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2 2/3] MdePkg/BaseLib: add attribute 'RETURNS_TWICE' to SetJump X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Jan 2018 12:26:17 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable To align to longjump() declaration, could you update SetJump()? UINTN EFIAPI RETURNS_TWICE SetJump ( ); =3D=3D> RETURNS_TWICE UINTN EFIAPI SetJump ( ); Thanks Liming > -----Original Message----- > From: M1cha [mailto:sigmaepsilon92@gmail.com] > Sent: Saturday, December 23, 2017 3:14 AM > To: edk2-devel@lists.01.org > Cc: Ard Biesheuvel ; Kinney, Michael D ; Gao, Liming > > Subject: [edk2] [PATCH v2 2/3] MdePkg/BaseLib: add attribute 'RETURNS_TWI= CE' to SetJump >=20 > 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. >=20 > example: > extern int FN_NAME(void*); >=20 > void jmp_buf_set(void *jmpb, void (*f)(void)) > { > if (!FN_NAME(jmpb)) > f(); > } >=20 > this code produces this wrong code with Os: > 00000000 : > 0: e92d4010 push {r4, lr} > 4: e1a04001 mov r4, r1 > 8: ebfffffe bl 0 > 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 >=20 > 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. >=20 > When using a standard name like setjmp or getcontext or adding > '__attribute__((returns_twice))' to nonstandard_setjmp's declaration > the code looks different: >=20 > 00000000 : > 0: e92d4007 push {r0, r1, r2, lr} > 4: e58d1004 str r1, [sp, #4] > 8: ebfffffe bl 0 > 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 >=20 > Here the problem is being solved by restoring r3 from the stack > without popping it. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Michael Zimmermann > --- > 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(+) >=20 > diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/Ba= seLib.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/Librar= y/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/BaseL= ib/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