Re: continuing egcs-1.1 problems

[Gary Thomas <gdt at linuxppc dot org>]

On 02-Nov-98 Elgin Lee wrote:
>>>>>> In <Pine.LNX.3.96.981031152626.20552A-100000@keeshond.aerodyne.com> 
>>>>>>  Fred Bacon <bacon@aerodyne.com> wrote:
> 
>> The trampoline patch fixes a couple of problems with verifying the size of
>> the stack and keeping the stack aligned.  My best guess is that this is
>> the critical patch, but I haven't confirmed it.  Someone with a little
>> patience needs to rebuild egcs with these patches one at a time to see
>> which one really fixes the problem.  Frankly, I've built egcs five times
>> in the last 36 hours, and I'm sick of the sight of it. :-)
> 
> I think that this is the critical path.  In particular,
> 
> --- egcs-1.0/gcc/config/rs6000/tramp.asm~     Mon Aug 11 17:57:23 1997
> +++ egcs-1.0/gcc/config/rs6000/tramp.asm      Fri Feb 27 06:11:10 1998
> @@ -85,7 +85,7 @@
>          lwz  r7,.Ltramp(r11)         /* trampoline addres -4 */
>  
>       li      r8,__trampoline_size    /* verify that the trampoline is big enough */
> -     cmpw    cr1,r8,r4
> +     cmpw    cr1,r4,r8
>       srwi    r4,r4,2                 /* # words to move */
>       addi    r9,r3,-4                /* adjust pointer for lwzu */
>       mtctr   r4
> 
> is definitely needed.  When the test program enters this path,
> r8 (__trampoline_size) is 40 but r4 is 48--hence the branch
> 
>       blt     cr1,.Labort
> 
> jumps to the abort code.
> 
> I'm hazy on the workings of the code, but on the surface this looks
> like a comparison-reversal bug that is fixed by the patch.  (But why
> does this work on the RS6000/AIX?  Perhaps because r4 and r8 are equal
> there on the RS6000/AIX?)
> 
> The core dump stems from the abort() call that's made in __trampoline_setup.
> See gcc/config/rs6000/tramp.asm.  If the trampoline is not considered
> large enough, that routine will call abort()--and that's precisely what's
> happening.  You can see for yourself by putting a breakpoint at
> __trampoline_setup and then re-running your test program.
> 
> Note that the default (nss) implementation in glibc uses a nested
> function, which requires a trampoline.  See the definition of
> nss_lookup_function in nss/nsswitch.c in the glibc source--it
> dynamically uses the nested function do_open() to handle the opening of
> the appropriate shared library for the host lookup.
> 
> The libresolv.so implementation, on the other hand, does not use a
> nested function in this case.  That's the reason that the program runs
> fine when linked explicitly with -lresolv.
> 
> Without this patch, I suspect that some other trampolines (and hence,
> other nested functions in glibc and elsewhere) would likewize fail.
> 
> On the surface, it looks like you should indeed include the particular
> patch portion above in the egcs build.  I don't know if the other
> portion of the trampoline patch (which causes rs6000_trampoline_size
> to return 48 rather than 40) is also necessary.
> 
> --Elgin

Two quick comments about this (since I'm away from home this week):

1. I believe that the compare *is* backwards, thus the patch.
2. I also believe that there should be a second patch which causes 
   GCC to always leave 48 bytes for the trampoline.  This is because
   a) the SYSV4 ABI states that the stack should always be 16 byte 
   aligned and b) GCC relies on thisto simplify some instruction
   sequences. 

------------------------------------------------------------------------
Gary Thomas                              |
email: gdt@linuxppc.org                  | "Fine wine is a necessity of
   ... opinions expressed here are mine  |        life for me"
       and no one else would claim them! |
                                         |      Thomas Jefferson
------------------------------------------------------------------------