Semihosting ctr0 typo (adr/ldr)?
diff mbox series

Message ID 2SG25aQ-_EdUvM9w1kXfGjpV_I6OSbvGLA9jIuXTs51Rzc7ESf1s9rIDWF85eATSD7jtZle5D2asRAeEEwtJs5adLfOf_c-uOa3myGWT-Ys=@protonmail.com
State New
Headers show
Series
  • Semihosting ctr0 typo (adr/ldr)?
Related show

Commit Message

Corinna Vinschen via Newlib Feb. 16, 2021, 10:56 p.m. 
From c0d80dfc1b832d690263af93231041ae7bb3f68e Mon Sep 17 00:00:00 2001
From: iNvEr7 <iNvEr7@protonmail.com>

Date: Tue, 16 Feb 2021 14:53:52 -0800
Subject: [PATCH] fix semihosting typo

---
 libgloss/arm/crt0.S        | 2 +-
 newlib/libc/sys/arm/crt0.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--
2.29.2

Comments

Corinna Vinschen via Newlib Feb. 17, 2021, 10:01 a.m. | #1 
On Feb 16 22:56, iNvEr7 via Newlib wrote:
> 

> ---

>  libgloss/arm/crt0.S        | 2 +-

>  newlib/libc/sys/arm/crt0.S | 2 +-

>  2 files changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S

> index 8490bde2f..b8bbcee18 100644

> --- a/libgloss/arm/crt0.S

> +++ b/libgloss/arm/crt0.S

> @@ -283,7 +283,7 @@

>  #ifdef ARM_RDI_MONITOR

>  	/*  Issue Angel SWI to read stack info.  */

>  	movs	r0, #AngelSWI_Reason_HeapInfo

> -	adr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */

> +	ldr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */

>  #ifdef THUMB_VXM

>  	bkpt	AngelSWI

>  #elif defined(__thumb2__)

> diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S

> index 5e677a23c..3c5771229 100644

> --- a/newlib/libc/sys/arm/crt0.S

> +++ b/newlib/libc/sys/arm/crt0.S

> @@ -270,7 +270,7 @@

>  #ifdef ARM_RDI_MONITOR

>  	/*  Issue Angel SWI to read stack info.  */

>  	movs	r0, #AngelSWI_Reason_HeapInfo

> -	adr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */

> +	ldr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */

>  #ifdef THUMB_VXM

>  	bkpt	AngelSWI

>  #elif defined(__thumb2__)

> --

> 2.29.2


I'm a bit suprised that this problem hasn't been noticed for the
last 10 years.  I'm not familiar with the ARM stuff at all, so maybe
somebody from ARM can chime in here?


Thanks,
Corinna
Corinna Vinschen via Newlib Feb. 17, 2021, 11:22 a.m. | #2 
On 17/02/2021 10:01, Corinna Vinschen wrote:
> On Feb 16 22:56, iNvEr7 via Newlib wrote:

>>

>> ---

>>  libgloss/arm/crt0.S        | 2 +-

>>  newlib/libc/sys/arm/crt0.S | 2 +-

>>  2 files changed, 2 insertions(+), 2 deletions(-)

>>

>> diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S

>> index 8490bde2f..b8bbcee18 100644

>> --- a/libgloss/arm/crt0.S

>> +++ b/libgloss/arm/crt0.S

>> @@ -283,7 +283,7 @@

>>  #ifdef ARM_RDI_MONITOR

>>  	/*  Issue Angel SWI to read stack info.  */

>>  	movs	r0, #AngelSWI_Reason_HeapInfo

>> -	adr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */

>> +	ldr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */

>>  #ifdef THUMB_VXM

>>  	bkpt	AngelSWI

>>  #elif defined(__thumb2__)

>> diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S

>> index 5e677a23c..3c5771229 100644

>> --- a/newlib/libc/sys/arm/crt0.S

>> +++ b/newlib/libc/sys/arm/crt0.S

>> @@ -270,7 +270,7 @@

>>  #ifdef ARM_RDI_MONITOR

>>  	/*  Issue Angel SWI to read stack info.  */

>>  	movs	r0, #AngelSWI_Reason_HeapInfo

>> -	adr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */

>> +	ldr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */

>>  #ifdef THUMB_VXM

>>  	bkpt	AngelSWI

>>  #elif defined(__thumb2__)

>> --

>> 2.29.2

> 

> I'm a bit suprised that this problem hasn't been noticed for the

> last 10 years.  I'm not familiar with the ARM stuff at all, so maybe

> somebody from ARM can chime in here?

> 

> 

> Thanks,

> Corinna

> 


No, I don't think that is right, but it took some digging to be sure.

The specification for the HeapInfo syscall is here:

https://developer.arm.com/documentation/100863/0300/Semihosting-operations/SYS-HEAPINFO--0x16-?lang=en

and it says that the parameter is the /address of a pointer/ to the
4-word block.  In other words, it is not the address of the block itself.

It's odd that it is defined this way, but I think the code is doing what
the specification says.  Certainly the comments in the code seem to
reflect this.

R.
Corinna Vinschen via Newlib Feb. 17, 2021, 12:02 p.m. | #3 
On Wednesday, February 17, 2021 3:22 AM, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote:

> On 17/02/2021 10:01, Corinna Vinschen wrote:

>

> > On Feb 16 22:56, iNvEr7 via Newlib wrote:

> >

> > > libgloss/arm/crt0.S | 2 +-

> > > newlib/libc/sys/arm/crt0.S | 2 +-

> > > 2 files changed, 2 insertions(+), 2 deletions(-)

> > > diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S

> > > index 8490bde2f..b8bbcee18 100644

> > > --- a/libgloss/arm/crt0.S

> > > +++ b/libgloss/arm/crt0.S

> > > @@ -283,7 +283,7 @@

> > > #ifdef ARM_RDI_MONITOR

> > > /* Issue Angel SWI to read stack info. */

> > > movs r0, #AngelSWI_Reason_HeapInfo

> > >

> > > -   adr r1, .LC0 /* Point at ptr to 4 words to receive data. */

> > >

> > > -   ldr r1, .LC0 /* Point at ptr to 4 words to receive data. /

> > >     #ifdef THUMB_VXM

> > >     bkpt AngelSWI

> > >     #elif defined(thumb2)

> > >     diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S

> > >     index 5e677a23c..3c5771229 100644

> > >     --- a/newlib/libc/sys/arm/crt0.S

> > >     +++ b/newlib/libc/sys/arm/crt0.S

> > >     @@ -270,7 +270,7 @@

> > >     #ifdef ARM_RDI_MONITOR

> > >     / Issue Angel SWI to read stack info. */movs r0, #AngelSWI_Reason_HeapInfo

> > >

> > >

> > > -   adr r1, .LC0 /* Point at ptr to 4 words to receive data. */

> > >

> > > -   ldr r1, .LC0 /* Point at ptr to 4 words to receive data. */

> > >     #ifdef THUMB_VXM

> > >     bkpt AngelSWI

> > >     #elif defined(thumb2)

> > >     --

> > >     2.29.2

> > >

> >

> > I'm a bit suprised that this problem hasn't been noticed for the

> > last 10 years. I'm not familiar with the ARM stuff at all, so maybe

> > somebody from ARM can chime in here?

> > Thanks,

> > Corinna

>

> No, I don't think that is right, but it took some digging to be sure.

>

> The specification for the HeapInfo syscall is here:

>

> https://developer.arm.com/documentation/100863/0300/Semihosting-operations/SYS-HEAPINFO--0x16-?lang=en

>

> and it says that the parameter is the /address of a pointer/ to the

> 4-word block. In other words, it is not the address of the block itself.

>

> It's odd that it is defined this way, but I think the code is doing what

> the specification says. Certainly the comments in the code seem to

> reflect this.

>

> R.


Thanks for clarifying this. It is indeed an odd definition. I encountered a crash when running a newlib-linked bare-metal binary on QEMU with semihosting enabled. I narrowed down the crash to the heapinfo semihosting call in the crt0. Now we know it's the QEMU implementation's problem, not newlib. It seems they are the one misinterpreted the ARM spec. I will open an issue to QEMU instead.

Patch
diff mbox series 

diff --git a/libgloss/arm/crt0.S b/libgloss/arm/crt0.S
index 8490bde2f..b8bbcee18 100644
--- a/libgloss/arm/crt0.S
+++ b/libgloss/arm/crt0.S
@@ -283,7 +283,7 @@ 
 #ifdef ARM_RDI_MONITOR
 	/*  Issue Angel SWI to read stack info.  */
 	movs	r0, #AngelSWI_Reason_HeapInfo
-	adr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */
+	ldr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */
 #ifdef THUMB_VXM
 	bkpt	AngelSWI
 #elif defined(__thumb2__)
diff --git a/newlib/libc/sys/arm/crt0.S b/newlib/libc/sys/arm/crt0.S
index 5e677a23c..3c5771229 100644
--- a/newlib/libc/sys/arm/crt0.S
+++ b/newlib/libc/sys/arm/crt0.S
@@ -270,7 +270,7 @@ 
 #ifdef ARM_RDI_MONITOR
 	/*  Issue Angel SWI to read stack info.  */
 	movs	r0, #AngelSWI_Reason_HeapInfo
-	adr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */
+	ldr	r1, .LC0	/*  Point at ptr to 4 words to receive data.  */
 #ifdef THUMB_VXM
 	bkpt	AngelSWI
 #elif defined(__thumb2__)