Re: Update on the glibc segfault issue on Alpha
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
Reply to: