* [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger
@ 2017-10-11 12:00 Alexander Fedotov
2017-10-13 11:05 ` Corinna Vinschen
0 siblings, 1 reply; 7+ messages in thread
From: Alexander Fedotov @ 2017-10-11 12:00 UTC (permalink / raw)
To: newlib
[-- Attachment #1: Type: text/plain, Size: 112 bytes --]
Use the same approach as in ARM port for case when Debugger not
providing necessary info for semihosting.
Alex
[-- Attachment #2: 0001-Fixed-semihosting-for-AArch64-when-heapinfo-paramete.patch --]
[-- Type: text/x-patch, Size: 4207 bytes --]
From 4f2ac350345f1126fa491dc0a335a9e1da92817e Mon Sep 17 00:00:00 2001
From: Alexander Fedotov <alfedotov@gmail.com>
Date: Wed, 11 Oct 2017 14:52:20 +0300
Subject: [PATCH] Fixed semihosting for AArch64 when heapinfo parameters are
not provided by debugger
---
libgloss/aarch64/crt0.S | 78 ++++++++++++++++++++++++++++++---------------
libgloss/aarch64/syscalls.c | 7 +++-
2 files changed, 59 insertions(+), 26 deletions(-)
diff --git a/libgloss/aarch64/crt0.S b/libgloss/aarch64/crt0.S
index f670e03..f831be1 100644
--- a/libgloss/aarch64/crt0.S
+++ b/libgloss/aarch64/crt0.S
@@ -102,33 +102,44 @@
ldr x0, .LC0 /* point at returned values */
ldr x1, [x0, #8] /* get heap_limit */
+ /* Set __heap_limit. */
+#ifdef __ILP32__
+ /* Sanity check on the __heap_limit. */
+ tst x1, #0xffffffff00000000
+ bne .Linsanepar
+#endif
+ cmp x1, xzr
+ beq .LC4
+ adrp x2, __heap_limit
+ add x2, x2, #:lo12:__heap_limit
+ str x1, [x2]
+.LC4:
+
+ ldr x1, [x0] /* get heap_base */
#ifdef __ILP32__
/* Sanity check on the heap base. */
- ldr x0, [x0] /* get heap_base */
- tst x0, #0xffffffff00000000
- beq 1f
- /* Exit with 1 if the heap base is not within the 32-bit address
- space. */
- mov x0, ADP_Stopped_ApplicationExit & 0xff
- movk x0, ADP_Stopped_ApplicationExit >> 16, lsl #16
- adrp x1, HeapBase /* Reuse to construct the parameter block. */
- add x1, x1, #:lo12:HeapBase
- str x0, [x1]
- mov x0, 1
- str x0, [x1, #8]
- mov w0, #AngelSVC_Reason_ReportException
- AngelSVCAsm AngelSVC
-1:
- /* For the sake of safety, set the stack base to the top end of
- the 32-bit address space if the returned value from the
- Angel API call is larger than or equal to 4 GiB. */
tst x1, #0xffffffff00000000
- csinv w1, w1, wzr, eq
+ bne .Linsanepar
#endif
-#else
- /* Set up the stack pointer to a fixed value. */
- ldr x1, .Lstack
+ cmp x1, xzr
+ bne .LC5
+ /* If the heap base value [x0, #0] is 0 then the heap base is actually
+ at the end of program data (i.e. __end__) */
+ ldr x1, .LC3
+ str x1, [x0, #0]
+.LC5:
+ ldr x1, [x0, #16] /* get stack_base */
+
+#ifdef __ILP32__
+ /* Sanity check on the stack_base. */
+ tst x1, #0xffffffff00000000
+ bne .Linsanepar
+#endif
+ cmp x1, xzr
+ bne .LC6
#endif
+ ldr x1, .Lstack /* Set up the stack pointer to a fixed value */
+.LC6:
/* Ensure quad-word stack alignment. */
and x0, x1, #~15
@@ -234,6 +245,22 @@
b FUNCTION (exit) /* Cannot return. */
+#if defined (ARM_RDI_MONITOR) && defined (__ILP32__)
+.Linsanepar:
+ /* Exit with 1 if the parameter is not within the 32-bit address
+ space. */
+ mov x1, ADP_Stopped_ApplicationExit & 0xff
+ movk x1, ADP_Stopped_ApplicationExit >> 16, lsl #16
+ adrp x0, HeapBase /* Reuse to construct the parameter block. */
+ add x0, x0, #:lo12:HeapBase
+ str x1, [x0]
+ mov x1, 1
+ str x1, [x0, #8]
+ mov w1, #AngelSVC_Reason_ReportException
+ AngelSVCAsm AngelSVC
+ b .
+#endif
+
/* Function initializing exception vector table, flatmap, etc.
Declared as weak symbol so that user can override this definition
by linking in their own version of the function. */
@@ -245,12 +272,13 @@ FUNCTION (_cpu_init_hook):
#ifdef ARM_RDI_MONITOR
.LC0:
GEN_DWORD HeapBase
-#else
+.LC3:
+ GEN_DWORD __end__
+#endif
.Lstack:
GEN_DWORD __stack
-
.weak __stack
-#endif
+
.LC1:
GEN_DWORD __bss_start__
.LC2:
diff --git a/libgloss/aarch64/syscalls.c b/libgloss/aarch64/syscalls.c
index af206a1..4de4e0d 100644
--- a/libgloss/aarch64/syscalls.c
+++ b/libgloss/aarch64/syscalls.c
@@ -625,6 +625,9 @@ _getpid (int n __attribute__ ((unused)))
return 1;
}
+/* Heap limit returned from SYS_HEAPINFO Angel semihost call. */
+ulong __heap_limit __attribute__ ((aligned (8))) = 0xcafedead;
+
caddr_t
_sbrk (int incr)
{
@@ -637,7 +640,9 @@ _sbrk (int incr)
prev_heap_end = heap_end;
- if (heap_end + incr > stack_ptr)
+ if ((heap_end + incr > stack_ptr)
+ /* Honour heap limit if it's valid. */
+ || ((__heap_limit != 0xcafedead) && (heap_end + incr > __heap_limit)))
{
/* Some of the libstdc++-v3 tests rely upon detecting
out of memory errors, so do not abort here. */
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger
2017-10-11 12:00 [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger Alexander Fedotov
@ 2017-10-13 11:05 ` Corinna Vinschen
2017-10-20 9:19 ` Alexander Fedotov
0 siblings, 1 reply; 7+ messages in thread
From: Corinna Vinschen @ 2017-10-13 11:05 UTC (permalink / raw)
To: Alexander Fedotov; +Cc: newlib
[-- Attachment #1: Type: text/plain, Size: 277 bytes --]
On Oct 11 14:57, Alexander Fedotov wrote:
> Use the same approach as in ARM port for case when Debugger not
> providing necessary info for semihosting.
I'd like to see an ACK from some arm folks.
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger
2017-10-13 11:05 ` Corinna Vinschen
@ 2017-10-20 9:19 ` Alexander Fedotov
2017-10-23 18:57 ` Tamar Christina
0 siblings, 1 reply; 7+ messages in thread
From: Alexander Fedotov @ 2017-10-20 9:19 UTC (permalink / raw)
To: newlib
Any comments/thoughts on this patch ?
It would be great to have this feature in newlib release version.
Alex
On Fri, 13 Oct 2017 at 12:16, Corinna Vinschen <vinschen@redhat.com> wrote:
> On Oct 11 14:57, Alexander Fedotov wrote:
> > Use the same approach as in ARM port for case when Debugger not
> > providing necessary info for semihosting.
>
> I'd like to see an ACK from some arm folks.
>
>
> Thanks,
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat
>
--
Best regards,
AF
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger
2017-10-20 9:19 ` Alexander Fedotov
@ 2017-10-23 18:57 ` Tamar Christina
2017-10-24 15:20 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 7+ messages in thread
From: Tamar Christina @ 2017-10-23 18:57 UTC (permalink / raw)
To: Alexander Fedotov, newlib; +Cc: nd
Hi Alex,
In principle nothing against the patch. Just a comment
> diff --git a/libgloss/aarch64/crt0.S b/libgloss/aarch64/crt0.S
> index f670e03..f831be1 100644
> --- a/libgloss/aarch64/crt0.S
> +++ b/libgloss/aarch64/crt0.S
> @@ -102,33 +102,44 @@
> ldr x0, .LC0 /* point at returned values */
> ldr x1, [x0, #8] /* get heap_limit */
>
> + /* Set __heap_limit. */
> +#ifdef __ILP32__
> + /* Sanity check on the __heap_limit. */
> + tst x1, #0xffffffff00000000
> + bne .Linsanepar
> +#endif
if x1 is 0 it won't pass the first test in ILP32 and takes the branch,
I assume you intended it to fall through in that case? So I think this test
should be re-ordered.
> + cmp x1, xzr
> + beq .LC4
> + adrp x2, __heap_limit
> + add x2, x2, #:lo12:__heap_limit
> + str x1, [x2]
> +.LC4:
Thanks,
Tamar
> -----Original Message-----
> From: newlib-owner@sourceware.org [mailto:newlib-
> owner@sourceware.org] On Behalf Of Alexander Fedotov
> Sent: 19 October 2017 22:41
> To: newlib@sourceware.org
> Subject: Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not
> provided by debugger
>
> Any comments/thoughts on this patch ?
> It would be great to have this feature in newlib release version.
>
> Alex
>
> On Fri, 13 Oct 2017 at 12:16, Corinna Vinschen <vinschen@redhat.com>
> wrote:
>
> > On Oct 11 14:57, Alexander Fedotov wrote:
> > > Use the same approach as in ARM port for case when Debugger not
> > > providing necessary info for semihosting.
> >
> > I'd like to see an ACK from some arm folks.
> >
> >
> > Thanks,
> > Corinna
> >
> > --
> > Corinna Vinschen
> > Cygwin Maintainer
> > Red Hat
> >
> --
> Best regards,
> AF
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger
2017-10-23 18:57 ` Tamar Christina
@ 2017-10-24 15:20 ` Richard Earnshaw (lists)
2017-10-24 15:28 ` Tamar Christina
0 siblings, 1 reply; 7+ messages in thread
From: Richard Earnshaw (lists) @ 2017-10-24 15:20 UTC (permalink / raw)
To: Tamar Christina, Alexander Fedotov, newlib; +Cc: nd
On 23/10/17 18:23, Tamar Christina wrote:
> Hi Alex,
>
> In principle nothing against the patch. Just a comment
>
>> diff --git a/libgloss/aarch64/crt0.S b/libgloss/aarch64/crt0.S
>> index f670e03..f831be1 100644
>> --- a/libgloss/aarch64/crt0.S
>> +++ b/libgloss/aarch64/crt0.S
>> @@ -102,33 +102,44 @@
>> ldr x0, .LC0 /* point at returned values */
>> ldr x1, [x0, #8] /* get heap_limit */
>>
>> + /* Set __heap_limit. */
>> +#ifdef __ILP32__
>> + /* Sanity check on the __heap_limit. */
>> + tst x1, #0xffffffff00000000
>> + bne .Linsanepar
>> +#endif
>
> if x1 is 0 it won't pass the first test in ILP32 and takes the branch,
> I assume you intended it to fall through in that case? So I think this test
> should be re-ordered.
Huh? zero & anything is zero, so the Z bit will be set and the branch
will not be taken.
>
>> + cmp x1, xzr
>> + beq .LC4
>> + adrp x2, __heap_limit
>> + add x2, x2, #:lo12:__heap_limit
>> + str x1, [x2]
>> +.LC4:
>
> Thanks,
> Tamar
>
Looks basically OK to me.
R.
>
>
>> -----Original Message-----
>> From: newlib-owner@sourceware.org [mailto:newlib-
>> owner@sourceware.org] On Behalf Of Alexander Fedotov
>> Sent: 19 October 2017 22:41
>> To: newlib@sourceware.org
>> Subject: Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not
>> provided by debugger
>>
>> Any comments/thoughts on this patch ?
>> It would be great to have this feature in newlib release version.
>>
>> Alex
>>
>> On Fri, 13 Oct 2017 at 12:16, Corinna Vinschen <vinschen@redhat.com>
>> wrote:
>>
>>> On Oct 11 14:57, Alexander Fedotov wrote:
>>>> Use the same approach as in ARM port for case when Debugger not
>>>> providing necessary info for semihosting.
>>>
>>> I'd like to see an ACK from some arm folks.
>>>
>>>
>>> Thanks,
>>> Corinna
>>>
>>> --
>>> Corinna Vinschen
>>> Cygwin Maintainer
>>> Red Hat
>>>
>> --
>> Best regards,
>> AF
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger
2017-10-24 15:20 ` Richard Earnshaw (lists)
@ 2017-10-24 15:28 ` Tamar Christina
2017-10-25 15:49 ` Corinna Vinschen
0 siblings, 1 reply; 7+ messages in thread
From: Tamar Christina @ 2017-10-24 15:28 UTC (permalink / raw)
To: Richard Earnshaw, Alexander Fedotov, newlib; +Cc: nd
> -----Original Message-----
> From: Richard Earnshaw (lists) [mailto:Richard.Earnshaw@arm.com]
> Sent: 24 October 2017 16:20
> To: Tamar Christina; Alexander Fedotov; newlib@sourceware.org
> Cc: nd
> Subject: Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not
> provided by debugger
>
> On 23/10/17 18:23, Tamar Christina wrote:
> > Hi Alex,
> >
> > In principle nothing against the patch. Just a comment
> >
> >> diff --git a/libgloss/aarch64/crt0.S b/libgloss/aarch64/crt0.S index
> >> f670e03..f831be1 100644
> >> --- a/libgloss/aarch64/crt0.S
> >> +++ b/libgloss/aarch64/crt0.S
> >> @@ -102,33 +102,44 @@
> >> ldr x0, .LC0 /* point at returned values */
> >> ldr x1, [x0, #8] /* get heap_limit */
> >>
> >> + /* Set __heap_limit. */
> >> +#ifdef __ILP32__
> >> + /* Sanity check on the __heap_limit. */
> >> + tst x1, #0xffffffff00000000
> >> + bne .Linsanepar
> >> +#endif
> >
> > if x1 is 0 it won't pass the first test in ILP32 and takes the branch,
> > I assume you intended it to fall through in that case? So I think this
> > test should be re-ordered.
>
> Huh? zero & anything is zero, so the Z bit will be set and the branch will not
> be taken.
>
Yes, sorry, I flipped the mask. Disregard my earlier comment.
>
> >
> >> + cmp x1, xzr
> >> + beq .LC4
> >> + adrp x2, __heap_limit
> >> + add x2, x2, #:lo12:__heap_limit
> >> + str x1, [x2]
> >> +.LC4:
> >
> > Thanks,
> > Tamar
> >
>
> Looks basically OK to me.
>
> R.
>
> >
> >
> >> -----Original Message-----
> >> From: newlib-owner@sourceware.org [mailto:newlib-
> >> owner@sourceware.org] On Behalf Of Alexander Fedotov
> >> Sent: 19 October 2017 22:41
> >> To: newlib@sourceware.org
> >> Subject: Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not
> >> provided by debugger
> >>
> >> Any comments/thoughts on this patch ?
> >> It would be great to have this feature in newlib release version.
> >>
> >> Alex
> >>
> >> On Fri, 13 Oct 2017 at 12:16, Corinna Vinschen <vinschen@redhat.com>
> >> wrote:
> >>
> >>> On Oct 11 14:57, Alexander Fedotov wrote:
> >>>> Use the same approach as in ARM port for case when Debugger not
> >>>> providing necessary info for semihosting.
> >>>
> >>> I'd like to see an ACK from some arm folks.
> >>>
> >>>
> >>> Thanks,
> >>> Corinna
> >>>
> >>> --
> >>> Corinna Vinschen
> >>> Cygwin Maintainer
> >>> Red Hat
> >>>
> >> --
> >> Best regards,
> >> AF
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger
2017-10-24 15:28 ` Tamar Christina
@ 2017-10-25 15:49 ` Corinna Vinschen
0 siblings, 0 replies; 7+ messages in thread
From: Corinna Vinschen @ 2017-10-25 15:49 UTC (permalink / raw)
To: newlib
[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]
On Oct 24 15:20, Tamar Christina wrote:
>
>
> > -----Original Message-----
> > From: Richard Earnshaw (lists) [mailto:Richard.Earnshaw@arm.com]
> > Sent: 24 October 2017 16:20
> > To: Tamar Christina; Alexander Fedotov; newlib@sourceware.org
> > Cc: nd
> > Subject: Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not
> > provided by debugger
> >
> > On 23/10/17 18:23, Tamar Christina wrote:
> > > Hi Alex,
> > >
> > > In principle nothing against the patch. Just a comment
> > >
> > >> diff --git a/libgloss/aarch64/crt0.S b/libgloss/aarch64/crt0.S index
> > >> f670e03..f831be1 100644
> > >> --- a/libgloss/aarch64/crt0.S
> > >> +++ b/libgloss/aarch64/crt0.S
> > >> @@ -102,33 +102,44 @@
> > >> ldr x0, .LC0 /* point at returned values */
> > >> ldr x1, [x0, #8] /* get heap_limit */
> > >>
> > >> + /* Set __heap_limit. */
> > >> +#ifdef __ILP32__
> > >> + /* Sanity check on the __heap_limit. */
> > >> + tst x1, #0xffffffff00000000
> > >> + bne .Linsanepar
> > >> +#endif
> > >
> > > if x1 is 0 it won't pass the first test in ILP32 and takes the branch,
> > > I assume you intended it to fall through in that case? So I think this
> > > test should be re-ordered.
> >
> > Huh? zero & anything is zero, so the Z bit will be set and the branch will not
> > be taken.
> >
>
> Yes, sorry, I flipped the mask. Disregard my earlier comment.
>
> >
> > >
> > >> + cmp x1, xzr
> > >> + beq .LC4
> > >> + adrp x2, __heap_limit
> > >> + add x2, x2, #:lo12:__heap_limit
> > >> + str x1, [x2]
> > >> +.LC4:
> > >
> > > Thanks,
> > > Tamar
> > >
> >
> > Looks basically OK to me.
> >
> > R.
Thanks, guys. Pushed.
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-24 15:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-11 12:00 [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger Alexander Fedotov
2017-10-13 11:05 ` Corinna Vinschen
2017-10-20 9:19 ` Alexander Fedotov
2017-10-23 18:57 ` Tamar Christina
2017-10-24 15:20 ` Richard Earnshaw (lists)
2017-10-24 15:28 ` Tamar Christina
2017-10-25 15:49 ` Corinna Vinschen
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).