This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCHv4] powerpc: Fix float128 IFUNC relocations [BZ #21707]


Carlos O'Donell <carlos@redhat.com> writes:

> On 07/17/2017 10:05 AM, Tulio Magno Quites Machado Filho wrote:
>> Changes since version 3:
>> 
>>  - Moved definition of ARCH_APPLY_IREL and ARCH_SETUP_IREL to
>>    libc-start.h.
>>  - Added a comment to elf/rtld.c explaining when thread-local variables
>>    are available.
>>  - Improved the test to read a TCB variable.
>
> This very is good. No ifdef's in the generic libc main startup code, which
> is the way it should be. Tests are good too.
>
> OK to checkin with additional comments added.
>
> Thanks for working through this.
>
> In Fedora Rawhide / Fedora 27 I went with your v1 patch to get our mass
> rebuild working. However, before we release F27 I'll update to this version
> and drop the intermediate v1 patch.

Ack!

>> diff --git a/sysdeps/generic/libc-start.h b/sysdeps/generic/libc-start.h
>> new file mode 100644
>> index 0000000..dfb1c75
>> --- /dev/null
>> +++ b/sysdeps/generic/libc-start.h
>> @@ -0,0 +1,26 @@
>
> Needs a one line header comment explaining what this file is.
>
> "Generic definitions for libc main startup."

Done.

>> +/* Copyright (C) 2017 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +#ifndef _LIBC_START_H
>> +#define _LIBC_START_H
>> +
>> +#ifndef SHARED
>
> Should have a comment:
>
> /* By default we perform STT_GNU_IFUNC resolution *before* TLS
>    initialization, and this means you cannot, without machine
>    knowledge, access TLS from a IFUNC resolver.  */

Done.

I'm going to push this patch with the changes requested.

Thanks!

-- 
Tulio Magno


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