[edk2-devel] [PATCH v2 0/1] RISCV: Fix InternalLongJump

[edk2-devel] [PATCH v2 0/1] RISCV: Fix InternalLongJump

[Andrei Warkentin]

I was playing around with building code with -Os and ran into
weird crashes, that I ended up chasing down to an obviously
incorrect InternalLongJump implementation, which has never
been correctly passing its 2nd parameter down as the return
value of SetJump.

You can find the pull at https://github.com/tianocore/edk2/pull/4836

V2: get rid of openssl-related change due to dirty build tree.

Andrei Warkentin (1):
  RISCV: Fix InternalLongJump to return correct value

 MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108820): https://edk2.groups.io/g/devel/message/108820
Mute This Topic: https://groups.io/mt/101452204/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/1] RISCV: Fix InternalLongJump to return correct value

[Andrei Warkentin]

InternalLongJump was not returning the 2nd parameter passed
to LongJmp (Value) as the return value from SetJmp.

Seen with code compiled with -Os, where an LongJmp (Buffer, -1)
somehow translated to SetJmp returning 0...

Cc: Yong Li <yong.li@intel.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Tuan Phan <tphan@ventanamicro.com>
Cc: Daniel Schaefer <git@danielschaefer.me>
Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
---
 MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S b/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
index 34486eabba4c..e97a7d0727b8 100644
--- a/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
+++ b/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
@@ -3,6 +3,7 @@
 // Set/Long jump for RISC-V
 //
 // Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
+// Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
 //
 // SPDX-License-Identifier: BSD-2-Clause-Patent
 //
@@ -47,9 +48,5 @@ InternalLongJump:
     REG_L s10, 11*SZREG(a0)
     REG_L s11, 12*SZREG(a0)
     REG_L sp,  13*SZREG(a0)
-
-    add   a0, s0, 0
-    add   a1, s1, 0
-    add   a2, s2, 0
-    add   a3, s3, 0
+    mv    a0, a1
     ret
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108821): https://edk2.groups.io/g/devel/message/108821
Mute This Topic: https://groups.io/mt/101452205/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/1] RISCV: Fix InternalLongJump to return correct value

[Sunil V L]

On Tue, Sep 19, 2023 at 03:16:59AM -0500, Andrei Warkentin wrote:
> InternalLongJump was not returning the 2nd parameter passed
> to LongJmp (Value) as the return value from SetJmp.
> 
> Seen with code compiled with -Os, where an LongJmp (Buffer, -1)
> somehow translated to SetJmp returning 0...
> 
> Cc: Yong Li <yong.li@intel.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> Cc: Tuan Phan <tphan@ventanamicro.com>
> Cc: Daniel Schaefer <git@danielschaefer.me>
> Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
> ---
>  MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S b/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
> index 34486eabba4c..e97a7d0727b8 100644
> --- a/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
> +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
> @@ -3,6 +3,7 @@
>  // Set/Long jump for RISC-V
>  //
>  // Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +// Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
>  //
>  // SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
> @@ -47,9 +48,5 @@ InternalLongJump:
>      REG_L s10, 11*SZREG(a0)
>      REG_L s11, 12*SZREG(a0)
>      REG_L sp,  13*SZREG(a0)
> -
> -    add   a0, s0, 0
> -    add   a1, s1, 0
> -    add   a2, s2, 0
> -    add   a3, s3, 0
> +    mv    a0, a1
>      ret

Thanks!, Andrei for finding and fixing this issue.

Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108826): https://edk2.groups.io/g/devel/message/108826
Mute This Topic: https://groups.io/mt/101452205/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/1] RISCV: Fix InternalLongJump to return correct value

[Sunil V L]

On Tue, Sep 19, 2023 at 03:16:59AM -0500, Andrei Warkentin wrote:
> InternalLongJump was not returning the 2nd parameter passed
> to LongJmp (Value) as the return value from SetJmp.
> 
> Seen with code compiled with -Os, where an LongJmp (Buffer, -1)
> somehow translated to SetJmp returning 0...
> 
> Cc: Yong Li <yong.li@intel.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> Cc: Tuan Phan <tphan@ventanamicro.com>
> Cc: Daniel Schaefer <git@danielschaefer.me>
> Signed-off-by: Andrei Warkentin <andrei.warkentin@intel.com>
> ---

Merged as #4851.

Thanks,
Sunil
>  MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S b/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
> index 34486eabba4c..e97a7d0727b8 100644
> --- a/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
> +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVSetJumpLongJump.S
> @@ -3,6 +3,7 @@
>  // Set/Long jump for RISC-V
>  //
>  // Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> +// Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
>  //
>  // SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
> @@ -47,9 +48,5 @@ InternalLongJump:
>      REG_L s10, 11*SZREG(a0)
>      REG_L s11, 12*SZREG(a0)
>      REG_L sp,  13*SZREG(a0)
> -
> -    add   a0, s0, 0
> -    add   a1, s1, 0
> -    add   a2, s2, 0
> -    add   a3, s3, 0
> +    mv    a0, a1
>      ret
> -- 
> 2.34.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108934): https://edk2.groups.io/g/devel/message/108934
Mute This Topic: https://groups.io/mt/101452205/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-