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 5BF872217CE58 for ; Sun, 7 Jan 2018 21:29:50 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Jan 2018 21:34:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,329,1511856000"; d="scan'208";a="9423773" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga006.jf.intel.com with ESMTP; 07 Jan 2018 21:34:59 -0800 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 7 Jan 2018 21:34:58 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 7 Jan 2018 21:34:58 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.152]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Mon, 8 Jan 2018 13:34:56 +0800 From: "Gao, Liming" To: M1cha , "edk2-devel@lists.01.org" CC: Ard Biesheuvel , "Kinney, Michael D" Thread-Topic: [edk2] [PATCH v3 2/3] MdePkg/BaseLib: add attribute 'RETURNS_TWICE' to SetJump Thread-Index: AQHTh91WfA8ZPGvGfk2b9ahEEXIrfaNpdRIQ Date: Mon, 8 Jan 2018 05:34:55 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E1A023C@SHSMSX104.ccr.corp.intel.com> References: <20180107173103.29032-1-sigmaepsilon92@gmail.com> In-Reply-To: <20180107173103.29032-1-sigmaepsilon92@gmail.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v3 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: Mon, 08 Jan 2018 05:29:50 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Liming Gao >-----Original Message----- >From: M1cha [mailto:sigmaepsilon92@gmail.com] >Sent: Monday, January 08, 2018 1:31 AM >To: edk2-devel@lists.01.org >Cc: Ard Biesheuvel ; Kinney, Michael D >; Gao, Liming >Subject: [edk2] [PATCH v3 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 : > 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 > >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 : > 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 > >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 >--- > 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 05eb4e33a086..39573db0c8da 100644 >--- a/MdePkg/Include/Library/BaseLib.h >+++ b/MdePkg/Include/Library/BaseLib.h >@@ -4903,6 +4903,7 @@ MemoryFence ( > @retval 0 Indicates a return from SetJump(). > > **/ >+RETURNS_TWICE > UINTN > EFIAPI > SetJump ( >diff --git a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c >b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c >index 4c0dba55d52f..683527a36fda 100644 >--- a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c >+++ b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c >@@ -32,6 +32,7 @@ > @retval 0 Indicates a return from SetJump(). > > **/ >+RETURNS_TWICE > UINTN > EFIAPI > SetJump ( >diff --git a/MdePkg/Library/BaseLib/Ia32/SetJump.c >b/MdePkg/Library/BaseLib/Ia32/SetJump.c >index 304f3839b108..652d45d53ba7 100644 >--- a/MdePkg/Library/BaseLib/Ia32/SetJump.c >+++ b/MdePkg/Library/BaseLib/Ia32/SetJump.c >@@ -49,6 +49,7 @@ InternalAssertJumpBuffer ( > > **/ > _declspec (naked) >+RETURNS_TWICE > UINTN > EFIAPI > SetJump ( >-- >2.15.1