public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Semihosting ctr0 typo (adr/ldr)?
@ 2021-02-16 22:56 iNvEr7
  2021-02-17 10:01 ` Corinna Vinschen
  0 siblings, 1 reply; 4+ messages in thread
From: iNvEr7 @ 2021-02-16 22:56 UTC (permalink / raw)
  To: newlib

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(-)

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Semihosting ctr0 typo (adr/ldr)?
  2021-02-16 22:56 [PATCH] Semihosting ctr0 typo (adr/ldr)? iNvEr7
@ 2021-02-17 10:01 ` Corinna Vinschen
  2021-02-17 11:22   ` Richard Earnshaw
  0 siblings, 1 reply; 4+ messages in thread
From: Corinna Vinschen @ 2021-02-17 10:01 UTC (permalink / raw)
  To: iNvEr7; +Cc: newlib, Richard Earnshaw

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Semihosting ctr0 typo (adr/ldr)?
  2021-02-17 10:01 ` Corinna Vinschen
@ 2021-02-17 11:22   ` Richard Earnshaw
  2021-02-17 12:02     ` iNvEr7
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Earnshaw @ 2021-02-17 11:22 UTC (permalink / raw)
  To: iNvEr7, newlib

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Semihosting ctr0 typo (adr/ldr)?
  2021-02-17 11:22   ` Richard Earnshaw
@ 2021-02-17 12:02     ` iNvEr7
  0 siblings, 0 replies; 4+ messages in thread
From: iNvEr7 @ 2021-02-17 12:02 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: newlib

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-17 12:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16 22:56 [PATCH] Semihosting ctr0 typo (adr/ldr)? iNvEr7
2021-02-17 10:01 ` Corinna Vinschen
2021-02-17 11:22   ` Richard Earnshaw
2021-02-17 12:02     ` iNvEr7

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).