[Arm] Remaining difference in crt0 code

Message ID CAN8C2CoYZcb1VHS1JmZ_5Bv5Q0=LUMBBE-ax7=Twvy=1qEL+0A@mail.gmail.com
State New
Headers show
Series
  • [Arm] Remaining difference in crt0 code
Related show

Commit Message

Alexander Fedotov April 12, 2019, 8:42 p.m.
Hello Richard

There is still some difference in crt0.S code in libgloss and libc
respectively. Could you help me figure out what version is correct so
we can get rid of this.

Differences are following:

 #ifdef ARM_RDP_MONITOR
     /*  Issue Demon SWI to read stack info.  */
@@ -105,13 +118,20 @@
     bkpt    AngelSWI
 #elif defined(__thumb2__)
     /*  We are in thumb mode for startup on armv7 architectures.  */
-    AngelSWIAsm    AngelSWI
+    AngelSWIAsm (AngelSWI)
 #else
     /*  We are always in ARM mode for startup on pre armv7 archs.  */
-    AngelSWIAsm    AngelSWI_ARM
+    AngelSWIAsm (AngelSWI_ARM)
 #endif
     ldr    r0, .LC0    /*  Point at values read.  */

+    /* Set __heap_limit.  */
+    ldr     r1, [r0, #4]
+    cmp     r1, #0
+    beq     .LC33
+    ldr     r2, =__heap_limit
+    str     r1, [r2]
+.LC33:
     ldr     r1, [r0, #0]
     cmp     r1, #0
     bne     .LC32
@@ -279,13 +299,10 @@
     movs    r1, r0
 #else
     movs    r0, #AngelSWI_Reason_GetCmdLine
-    adr    r1, .LC30    /* Space for command line.  */
-#ifdef THUMB_VXM
-    bkpt    AngelSWI
-#else
-     AngelSWIAsm    AngelSWI
-#endif
+    ldr    r1, .LC30    /* Space for command line.  */
+    AngelSWIAsm (AngelSWI)
     ldr    r1, .LC30
+    ldr    r1, [r1]
 #endif
     /*  Parse string at r1.  */
     movs    r0, #0        /* Count of arguments so far.  */
@@ -309,7 +326,7 @@
     beq    .LC10

 /* See whether we are scanning a string.  */
-    cmp    r3, #'\"'
+    cmp    r3, #'"'
 #ifdef __thumb__
     beq    .LC20
     cmp    r3, #'\''
@@ -396,8 +413,17 @@
        for _fini to be called at program exit.  */
     movs    r4, r0
     movs    r5, r1
+#ifdef _LITE_EXIT
+    /* Make reference to atexit weak to avoid unconditionally pulling in
+       support code.  Refer to comments in __atexit.c for more details.  */
+    .weak    FUNCTION(atexit)
+    ldr    r0, .Latexit
+    cmp    r0, #0
+    beq    .Lweak_atexit
+#endif
     ldr    r0, .Lfini
     bl    FUNCTION (atexit)
+.Lweak_atexit:
     bl    FUNCTION (_init)
     movs    r0, r4
     movs    r1, r5
@@ -469,13 +495,19 @@
 .LC2:
     .word    __bss_end__
 #ifdef __USES_INITFINI__
+#ifdef _LITE_EXIT
+.Latexit:
+    .word    FUNCTION(atexit)
+
+    /* Weak reference _fini in case of lite exit.  */
+    .weak    FUNCTION(_fini)
+#endif
 .Lfini:
     .word    FUNCTION(_fini)
 #endif
 #ifdef ARM_RDI_MONITOR
 .LC30:
-    .word    CommandLine
-    .word    255
+    .word    AngelSWIArgs
 .LC31:
     .word    __end__

@@ -488,6 +520,9 @@
 __stack_base__:    .word    0
 StackLimit:    .word    0
 CommandLine:    .space    256,0    /*  Maximum length of 255 chars handled.  */
+AngelSWIArgs:
+    .word    CommandLine
+    .word    255
 #endif

 #ifdef __pe__

Best regards,
Alex

Comments

Richard Earnshaw (lists) April 15, 2019, 9:14 a.m. | #1
On 12/04/2019 21:42, Alexander Fedotov wrote:
> Hello Richard

> 

> There is still some difference in crt0.S code in libgloss and libc

> respectively. Could you help me figure out what version is correct so

> we can get rid of this.

> 


I don't think we can clean this up file at a time.  Trying to do so will
likely create inconsistencies in the sources as some files will be
patched and others not.

"git blame" and "git show" will be your friends here... For example,
taking the first hunk:

> Differences are following:

> 

> --- newlib/libc/sys/arm/crt0.S    2019-04-12 23:28:32.237488800 +0300

> +++ libgloss/arm/crt0.S    2019-04-12 23:28:32.237488800 +0300

> @@ -79,6 +79,19 @@

>      .fnstart

>  #endif

> 

> +    /* __ARM_ARCH_PROFILE is defined from GCC 4.8 onwards, however

> __ARM_ARCH_7A

> +    has been defined since 4.2 onwards, which is when v7-a support was added

> +    and hence 'A' profile support was added in the compiler.  Allow for this

> +    file to be built with older compilers.  We only call this for A profile

> +    cores.  */

> +#if defined (__ARM_ARCH_7A__) || (__ARM_ARCH_PROFILE == 'A')

> +/*  The init hook does not use the stack and is called before the

> stack has been set up.  */

> +#ifdef ARM_RDI_MONITOR

> +    bl    _rdimon_hw_init_hook

> +    .weak    FUNCTION (_rdimon_hw_init_hook)

> +#endif

> +#endif

> +


"git blame" shows that this hunk is derived from two commits 99be2bc4ff
and then shortly afterwards 639951dda7.  "git show" can then be used to
view these individual patches and you can also use the date information
to identify the post to the newlib mailing list, which may contain
additional information about the patch.

Unwinding all of this is going to be a bit tedious, I fear, so I really
appreciate any assistance you can give.

R.
Alexander Fedotov April 15, 2019, 10:17 a.m. | #2
Ok, thanks.

On Mon, Apr 15, 2019 at 12:14 PM Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
>

> On 12/04/2019 21:42, Alexander Fedotov wrote:

> > Hello Richard

> >

> > There is still some difference in crt0.S code in libgloss and libc

> > respectively. Could you help me figure out what version is correct so

> > we can get rid of this.

> >

>

> I don't think we can clean this up file at a time.  Trying to do so will

> likely create inconsistencies in the sources as some files will be

> patched and others not.

>

> "git blame" and "git show" will be your friends here... For example,

> taking the first hunk:

>

> > Differences are following:

> >

> > --- newlib/libc/sys/arm/crt0.S    2019-04-12 23:28:32.237488800 +0300

> > +++ libgloss/arm/crt0.S    2019-04-12 23:28:32.237488800 +0300

> > @@ -79,6 +79,19 @@

> >      .fnstart

> >  #endif

> >

> > +    /* __ARM_ARCH_PROFILE is defined from GCC 4.8 onwards, however

> > __ARM_ARCH_7A

> > +    has been defined since 4.2 onwards, which is when v7-a support was added

> > +    and hence 'A' profile support was added in the compiler.  Allow for this

> > +    file to be built with older compilers.  We only call this for A profile

> > +    cores.  */

> > +#if defined (__ARM_ARCH_7A__) || (__ARM_ARCH_PROFILE == 'A')

> > +/*  The init hook does not use the stack and is called before the

> > stack has been set up.  */

> > +#ifdef ARM_RDI_MONITOR

> > +    bl    _rdimon_hw_init_hook

> > +    .weak    FUNCTION (_rdimon_hw_init_hook)

> > +#endif

> > +#endif

> > +

>

> "git blame" shows that this hunk is derived from two commits 99be2bc4ff

> and then shortly afterwards 639951dda7.  "git show" can then be used to

> view these individual patches and you can also use the date information

> to identify the post to the newlib mailing list, which may contain

> additional information about the patch.

>

> Unwinding all of this is going to be a bit tedious, I fear, so I really

> appreciate any assistance you can give.

>

> R.
Alexander Fedotov April 15, 2019, 2:09 p.m. | #3
Well, at first glance all the differences are related to semihosting v2.
I think Tamar can help me with that (if he doesn't mind). Especially I
do see his comment in
https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=commit;h=6d6a623e7d8eb9e521bdbd73a7eafdd482678cea

"This patch does this, I have not updated newlib/libc/* as that is
quite out of date already. A proper sync is needed in order to get
things back in sync.
A different patch for this would be best."
I can say time " The time has come" :)

Moreover Tamar has a commit rights so it can be done much faster.

Alex

On Mon, Apr 15, 2019 at 1:17 PM Alexander Fedotov <alfedotov@gmail.com> wrote:
>

> Ok, thanks.

>

> On Mon, Apr 15, 2019 at 12:14 PM Richard Earnshaw (lists)

> <Richard.Earnshaw@arm.com> wrote:

> >

> > On 12/04/2019 21:42, Alexander Fedotov wrote:

> > > Hello Richard

> > >

> > > There is still some difference in crt0.S code in libgloss and libc

> > > respectively. Could you help me figure out what version is correct so

> > > we can get rid of this.

> > >

> >

> > I don't think we can clean this up file at a time.  Trying to do so will

> > likely create inconsistencies in the sources as some files will be

> > patched and others not.

> >

> > "git blame" and "git show" will be your friends here... For example,

> > taking the first hunk:

> >

> > > Differences are following:

> > >

> > > --- newlib/libc/sys/arm/crt0.S    2019-04-12 23:28:32.237488800 +0300

> > > +++ libgloss/arm/crt0.S    2019-04-12 23:28:32.237488800 +0300

> > > @@ -79,6 +79,19 @@

> > >      .fnstart

> > >  #endif

> > >

> > > +    /* __ARM_ARCH_PROFILE is defined from GCC 4.8 onwards, however

> > > __ARM_ARCH_7A

> > > +    has been defined since 4.2 onwards, which is when v7-a support was added

> > > +    and hence 'A' profile support was added in the compiler.  Allow for this

> > > +    file to be built with older compilers.  We only call this for A profile

> > > +    cores.  */

> > > +#if defined (__ARM_ARCH_7A__) || (__ARM_ARCH_PROFILE == 'A')

> > > +/*  The init hook does not use the stack and is called before the

> > > stack has been set up.  */

> > > +#ifdef ARM_RDI_MONITOR

> > > +    bl    _rdimon_hw_init_hook

> > > +    .weak    FUNCTION (_rdimon_hw_init_hook)

> > > +#endif

> > > +#endif

> > > +

> >

> > "git blame" shows that this hunk is derived from two commits 99be2bc4ff

> > and then shortly afterwards 639951dda7.  "git show" can then be used to

> > view these individual patches and you can also use the date information

> > to identify the post to the newlib mailing list, which may contain

> > additional information about the patch.

> >

> > Unwinding all of this is going to be a bit tedious, I fear, so I really

> > appreciate any assistance you can give.

> >

> > R.

Patch

--- newlib/libc/sys/arm/crt0.S    2019-04-12 23:28:32.237488800 +0300
+++ libgloss/arm/crt0.S    2019-04-12 23:28:32.237488800 +0300
@@ -79,6 +79,19 @@ 
     .fnstart
 #endif

+    /* __ARM_ARCH_PROFILE is defined from GCC 4.8 onwards, however
__ARM_ARCH_7A
+    has been defined since 4.2 onwards, which is when v7-a support was added
+    and hence 'A' profile support was added in the compiler.  Allow for this
+    file to be built with older compilers.  We only call this for A profile
+    cores.  */
+#if defined (__ARM_ARCH_7A__) || (__ARM_ARCH_PROFILE == 'A')
+/*  The init hook does not use the stack and is called before the
stack has been set up.  */
+#ifdef ARM_RDI_MONITOR
+    bl    _rdimon_hw_init_hook
+    .weak    FUNCTION (_rdimon_hw_init_hook)
+#endif
+#endif
+
 /* Start by setting up a stack.  */