From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::242; helo=mail-io0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x242.google.com (mail-io0-x242.google.com [IPv6:2607:f8b0:4001:c06::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 98524208F7A5D for ; Fri, 22 Dec 2017 07:22:45 -0800 (PST) Received: by mail-io0-x242.google.com with SMTP id g70so12373124ioj.6 for ; Fri, 22 Dec 2017 07:27:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=q2Q1eQfnT4LZb+tibG4RgvxKrMB60VexRszbDHPRMlw=; b=KQznvyMhwJEYGUdywo8/RqxJ6r+53K5RvMhF2YggbCW0XAV9XTwMYP7KdmQ5tZku3P N6/SLI3jHB6c3RqWRtJrr+ua4P+aidqyAK/x+J2ASMnop8MQeBnt5YMJ59cMUY/sYw58 eM0YUfBRIBQqYIv26AfDpV1e9ZKEq/HwNh5r0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=q2Q1eQfnT4LZb+tibG4RgvxKrMB60VexRszbDHPRMlw=; b=HgxW4wrXTY1CVYT7zOTbiPAr6n1jDhqwMo9iURvs5u35nM3a3Ilc+KYEmGWOKSR3J4 L4fXkuBQtYGQ4EE9RZhl0jV0CQwm1DIiJbfvOALNonY7wbWZYDJqz4m/FeDfQQPo90BG B0nfRcR7VZBE90BaSgPe04szMLcrwe89jEyZlFL5cVbM6+/qpHcxikXKEJVlK+uYM8Zg 9/jy98NcFCNpOyEU6Ma9m0qjnswa2QFgrf6tM6B+hSKcjXpTAbr06wvJZL2DLC6Ybkvq A7AXrkZ57/aAMVomAmEaYDrm9pIrzS6g8tR0048++JZLH0Je5cpRnf4lbMqmtP6Z/Ajd atTg== X-Gm-Message-State: AKGB3mK9ej0kPGUbYUZKrHZCaDusu6EcKLAGzJtNQzD2jhBjXu5iU0Df NfY6cBfGq2BoKhCeMDa3upFhLYliXsZ3l3B6sbksGw== X-Google-Smtp-Source: ACJfBouesutRuAI3bVuuc+gZe8s3izLgRJky96o9q4tTv0lG9iw2q7deei6yzdgxGb+eRjgpXVvzicH5xivIMSPaJV4= X-Received: by 10.107.137.15 with SMTP id l15mr18417693iod.52.1513956454639; Fri, 22 Dec 2017 07:27:34 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.52.14 with HTTP; Fri, 22 Dec 2017 07:27:34 -0800 (PST) In-Reply-To: <20171222072336.23504-3-sigmaepsilon92@gmail.com> References: <20171222072336.23504-1-sigmaepsilon92@gmail.com> <20171222072336.23504-3-sigmaepsilon92@gmail.com> From: Ard Biesheuvel Date: Fri, 22 Dec 2017 15:27:34 +0000 Message-ID: To: M1cha Cc: "edk2-devel@lists.01.org" , Michael D Kinney , Liming Gao Subject: Re: [PATCH 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: Fri, 22 Dec 2017 15:22:46 -0000 Content-Type: text/plain; charset="UTF-8" On 22 December 2017 at 07:23, M1cha wrote: > 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 Thanks for fixing this. Note that this affects all architectures targeted by Clang/GCC, not just 32-bit ARM. Reviewed-by: Ard Biesheuvel > --- > MdePkg/Include/Library/BaseLib.h | 1 + > 1 file changed, 1 insertion(+) > > 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 > ); > -- > 2.15.1 >