Re: [PATCH] Semihosting fix for AArch64 when heapinfo is not provided by debugger

[Corinna Vinschen <vinschen at redhat dot com>]

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: signature.asc
Description: PGP signature