Re: Update on the glibc segfault issue on Alpha

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 03/01/23 10:29, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> diff --git a/csu/libc-start.c b/csu/libc-start.c
>> index 543560f36c..63a3eceaea 100644
>> --- a/csu/libc-start.c
>> +++ b/csu/libc-start.c
>> @@ -271,18 +271,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>>           So we can set up _dl_phdr and _dl_phnum even without any
>>           information from auxv.  */
>>
>> -      extern const ElfW(Ehdr) __ehdr_start
>> -# if BUILD_PIE_DEFAULT
>> -       __attribute__ ((visibility ("hidden")));
>> -# else
>> -       __attribute__ ((weak, visibility ("hidden")));
>> -      if (&__ehdr_start != NULL)
>> -# endif
>> -        {
>> -          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
>> -          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
>> -          GL(dl_phnum) = __ehdr_start.e_phnum;
>> -        }
>> +      extern const ElfW(Ehdr) __ehdr_start attribute_hidden;
>> +      assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
>> +      GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
>> +      GL(dl_phnum) = __ehdr_start.e_phnum;
> 
> There's a separate thread that e.ph_off is not actually correct in this
> context because it's a file offset that doesn't necessarily match the
> virtual memory offset:> 
>   [PATCH 0/1] __libc_start_main() now uses auxv to obtain phdr's address
>   [BZ #29864]
>   <https://sourceware.org/pipermail/libc-alpha/2022-December/143874.html>


Yes, but this is a fallback case where the kernel does not provide AT_PHDR
and AT_PHNUM.  As I added on the thread, I think it is better to use the
kernel provided value since they will always present and it leverages a
kernel fix for a similar issue [1].

> 
> I think this needs to be cleaned up so that the static and dynamic cases
> are aligned.  That is, we probably want to do the equivalent of
> 
>       GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
>       GL(dl_phnum) = __ehdr_start.e_phnum;
> 

I think we should use the __ehdr_start only as fallback.

> in common code.  Ideally, we don't use global variables for that because
> in both cases, we only briefly need these variables.

Right, this make sense.

> 
> Thanks,
> Florian
> 

[1] https://sourceware.org/pipermail/libc-alpha/2022-December/143942.html