Re: [PATCH v3 3/3] MdePkg: add NORETURN attribute to LongJump and InternalLongJump

["Gao, Liming" <liming.gao@intel.com>]

Michael:
  Just push the change on RETURNS_TWICE

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Michael Zimmermann
> Sent: Monday, January 8, 2018 2:22 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [PATCH v3 3/3] MdePkg: add NORETURN attribute to LongJump and InternalLongJump
> 
> Liming:
> It's not required to fix the bug which generates bad code. The NORETURN
> attribute just fixes the compiler warning mentioned in the commit message
> so I'm fine with separating this patch from the series.
> 
> And actually this "unreachable code" warning sounds like a more complicated
> problem because while the compiler is right and the code after the call to
> LongJump should never run there is a slight possibility for that to happen
> anyway - just imagine that because of memory corruption the PC/LR inside
> the jump buffer point to after the call of longjump.
> 
> On Mon, Jan 8, 2018 at 6:34 AM, Gao, Liming <liming.gao@intel.com> wrote:
> 
> > Michael:
> >   After more test, this patch will cause MdeModulePkg DxeCore link failure
> > on VS2015x86 tool chain. The warning message is caused by NORETURN
> > attribute in LongJump() API. Is this patch really required? Could we
> > separate it from the change RETURNS_TWICE? If yes, I suggest to commit
> > RETURNS_TWICE patch first, then work out the solution on NORETURN attribute
> > enabling.
> >
> > c:\edk2\mdemodulepkg\core\dxe\image\image.c(1863) : error C2220: warning
> > treated as error - no 'executable' file generated
> > c:\edk2\mdemodulepkg\core\dxe\image\image.c(1863) : warning C4702:
> > unreachable code
> > c:\edk2\mdemodulepkg\core\dxe\image\image.c(1864) : warning C4702:
> > unreachable code
> >
> > >-----Original Message-----
> > >From: M1cha [mailto:sigmaepsilon92@gmail.com]
> > >Sent: Thursday, December 28, 2017 3:29 AM
> > >To: edk2-devel@lists.01.org
> > >Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Kinney, Michael D
> > ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> > >Subject: [edk2] [PATCH v3 3/3] MdePkg: add NORETURN attribute to
> > >LongJump and InternalLongJump
> > >
> > >This fixes compiler warnings when using them in functions which
> > >should return a value but rely on LongJump to never return instead.
> > >
> > >Contributed-under: TianoCore Contribution Agreement 1.1
> > >Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
> > >---
> > >V3: fix VS compilation errors
> > >
> > > MdePkg/Include/Library/BaseLib.h             | 1 +
> > > MdePkg/Library/BaseLib/BaseLibInternals.h    | 1 +
> > > MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c | 1 +
> > > MdePkg/Library/BaseLib/Ia32/LongJump.c       | 1 +
> > > MdePkg/Library/BaseLib/LongJump.c            | 1 +
> > > 5 files changed, 5 insertions(+)
> > >
> > >diff --git a/MdePkg/Include/Library/BaseLib.h
> > >b/MdePkg/Include/Library/BaseLib.h
> > >index 10976032adaa..c6783f4624e2 100644
> > >--- a/MdePkg/Include/Library/BaseLib.h
> > >+++ b/MdePkg/Include/Library/BaseLib.h
> > >@@ -4927,6 +4927,7 @@ SetJump (
> > >                       restored and must be non-zero.
> > >
> > > **/
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > LongJump (
> > >diff --git a/MdePkg/Library/BaseLib/BaseLibInternals.h
> > >b/MdePkg/Library/BaseLib/BaseLibInternals.h
> > >index 9dca97a0dcc9..5cc83d2956e5 100644
> > >--- a/MdePkg/Library/BaseLib/BaseLibInternals.h
> > >+++ b/MdePkg/Library/BaseLib/BaseLibInternals.h
> > >@@ -441,6 +441,7 @@ InternalAssertJumpBuffer (
> > >   @param  Value         The value to return when the SetJump() context is
> > >restored.
> > >
> > > **/
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > InternalLongJump (
> > >diff --git a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> > >b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> > >index e309e8b57d7a..f48d7d17c4e4 100644
> > >--- a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> > >+++ b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> > >@@ -54,6 +54,7 @@ SetJump (
> > >   @param  Value         The value to return when the SetJump() context is
> > >restored.
> > >
> > > **/
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > InternalLongJump (
> > >diff --git a/MdePkg/Library/BaseLib/Ia32/LongJump.c
> > >b/MdePkg/Library/BaseLib/Ia32/LongJump.c
> > >index 73973a9cceae..8fc86f9efb69 100644
> > >--- a/MdePkg/Library/BaseLib/Ia32/LongJump.c
> > >+++ b/MdePkg/Library/BaseLib/Ia32/LongJump.c
> > >@@ -28,6 +28,7 @@
> > >
> > > **/
> > > __declspec (naked)
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > InternalLongJump (
> > >diff --git a/MdePkg/Library/BaseLib/LongJump.c
> > >b/MdePkg/Library/BaseLib/LongJump.c
> > >index 062be8f2daa1..fef764d6787e 100644
> > >--- a/MdePkg/Library/BaseLib/LongJump.c
> > >+++ b/MdePkg/Library/BaseLib/LongJump.c
> > >@@ -33,6 +33,7 @@
> > >                       restored and must be non-zero.
> > >
> > > **/
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > LongJump (
> > >--
> > >2.15.1
> >
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel