[PATCH] sh: fix __HAVE_SHARED__ condition in crti.S.

[Carmelo AMOROSO]

Paul Mundt wrote:
> On Thu, Aug 28, 2008 at 07:58:15AM +0200, Carmelo AMOROSO wrote:
>> Paul Mundt wrote:
>>> On Tue, Aug 26, 2008 at 03:53:18PM +0200, Carmelo AMOROSO wrote:
>>>> Takashi Yoshii wrote:
>>>>> For SH, init/fini function prologue is defined in 
>>>>> libc/sysdeps/linux/sh/crti.S
>>>>> as follows.
>>>>>
>>>>> | (frame entry)
>>>>> |#ifndef __HAVE_SHARED__
>>>>> | (GOT pointer initialization)
>>>>> |#endif
>>>>>
>>>>> I think this should be "ifdef", but "ifndef".
>>>>> Or, are there any reason that GOT should be used (only)in NON-shared 
>>>>> case ?
>>>>>
>>>> Well, this change has been committed by lethal at
>>>> http://www.uclibc.org/cgi-bin/viewcvs.cgi/trunk/uClibc/libc/sysdeps/linux/sh/crti.S?rev=16426&view=diff&r1=16426&r2=16425&p1=trunk/uClibc/libc/sysdeps/linux/sh/crti.S&p2=/trunk/uClibc/libc/sysdeps/linux/sh/crti.S
>>>>
>>>> so, unless there something I'm not understanding too, I suppose it was a 
>>>> typo by Paul (aka lethal).
>>>> Paul, my you give some lights on this change ?
>>>>
>>> I had to think about this briefly. It was part of the SH-2 work done by
>>> Mark Shinwell (added to Cc). The disabling of GOT related code in the
>>> non-shared case was intentional, but I don't immediately recall what the
>>> rationale was.
>>>
>> yes, but as Yoshii spotted, the code now seems doing just the opposite.
>>
> Good point, it seems I'm unable to read in that case. :-)
> 
> Inverting it seems to be the reasonable thing to do. I'll give it a test
> and check in Yoshii-san's patch if nothing breaks.
> 
Hi Paul,
I did not success to create a test that could fail.
application ctor/dtor defined by gcc attribute ((__contructor__)) on 
((__destructor__)) are correctly invoked.
Indeed, if I put the ctor/dtor in a separate object file and I build it 
as a PIC object, then the compiler will create the proper 
_GLOBAL_OFFSET_TABLE_ entry and will produce the proper code to load and 
use r12.
Yes, glibc _init function does it, but I'm thinking that it is useless.
I cannot see a scenario in which this may fail. Are we sure we need this 
code at all? or we simply have taken the code as is from glibc in the past ?

I agree that the patch is logically correct, but we might add extra 
useless code.

Indeed, never before, I had problem using uclibc on sh4 due to this 
missing code.

Cheers,
Carmelo