This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: continuing egcs-1.1 problems


At 20:36 02.11.98 , Gary Thomas wrote:
>
>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.

I would really appreciate if someone could finally and reliably state
what's the intention of the code:

a. abort if r4>__trampoline_size
b. abort if r4>=__trampoline_size
c. abort if r4<=__trampoline_size
d. abort if r4<__trampoline_size

??? 
What's the correct one? Tell me and I will produce a patch with proper
commenting which solves this once and forever!

>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. 

Hmm, someone (I think Michael Meissner) stated the exact opposite to me,
your patch would violate the ABI. Who's correct?

Franz.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]