glibc 2.27: less than two weeks till release

glibc 2.27: less than two weeks till release

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 2592 bytes --]

Hi,

We have less than two weeks till the planned release date.  By this date
in the freeze calendar we should have been ready to start architecture
testing for 2.27, but, unfortunately, with pending complicated patches
that affect all architectures we cannot really start the testing until
these patches have either been accepted or postponed to 2.28.
Nevertheless, architecture maintainers are encouraged to do preliminary
testing.

Here is a recap of pending issues.

* Release blockers listed in the release page[1]:

** Reported failure on Intel hardware involving steam/mono runtimes[2],
likely a regression caused by recent setjmp changes[3].

There is going to be an ABI change if we have to revert these changes,
hampering the pre-release testing.
H.J., is there any progress with this?

** Month names in alternative grammatical case[4]

This has already been reviewed by Carlos.
Rafal, is there anything that has to be done before these series is committed?

** Review / update collation data from Unicode / ISO 14651[5]

Given that the bug is almost 6 year old and no patch has been submitted
for review yet, does it have any chance for a review and a proper testing
before the release?
Mike, why do you consider this change as a blocker for 2.27?

* Features listed as desirable in the release page:

** C11 threads[6]

This has been waiting for review for many weeks, last week Carlos said
he will review this.
It's unfortunate that we still have complicated all-arch patch series
like this still pending review for 2.27.

** Istvan Kurucsai's malloc security patches[7]

This series has been under review for many weeks.
Florian, is it still targeted for 2.27?

** RISC-V port v4[8]

There is a separate summary[9] on this topic.

** Don't include libio.h from the installed stdio.h[10]

This is still pending review for 2.27.
Florian, anyone else, is there any progress with this?

[1] https://sourceware.org/glibc/wiki/Release/2.27
[2] https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html
[3] https://sourceware.org/ml/libc-alpha/2018-01/msg00268.html
[4] https://sourceware.org/ml/libc-alpha/2018-01/msg00258.html
[5] https://sourceware.org/bugzilla/show_bug.cgi?id=14095
[6] https://sourceware.org/ml/libc-alpha/2017-09/msg00871.html
[7] https://sourceware.org/ml/libc-alpha/2017-11/msg00229.html
[8] https://sourceware.org/ml/libc-alpha/2018-01/msg00475.html
[9] https://sourceware.org/ml/libc-alpha/2018-01/msg00379.html
[10] https://sourceware.org/ml/libc-alpha/2018-01/msg00248.html


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

TODO: Alternative month names (was: Re: glibc 2.27: less than two weeks till release)

[Rafal Luzynski]

18.01.2018 18:56 "Dmitry V. Levin" <ldv@altlinux.org> wrote:
> [...]
> ** Month names in alternative grammatical case[4]
>
> This has already been reviewed by Carlos.
> Rafal, is there anything that has to be done before these series is committed?
> [...]
> [4] https://sourceware.org/ml/libc-alpha/2018-01/msg00258.html
> [...]

I posted the 12th version of the patches for the final review [5]
because of some changes in the documentation and test cases.
Carlos answered that he'd verify the compatibility of statically
built binaries before giving the final OK [6] but he did not yet.
In the meantime I did a simple test and stated that the old static
library is unable to load any new binary locale data. [7][8] My test
was very cursory, that means I did not test what actually failed
and whether the problem is really impossible to resolve, I only guess
this is because of some sanity tests which verify the size of the
binary data and expect it to be equal to the size defined by their
version of glibc.  Florian agreed that even if it is not compatible
the problem is not blocking. [9] Joseph Myers said that this
incompatibility is acceptable and this already had happened in the
past but needs to be mentioned in NEWS. [10][11] This NEWS section
does not exist yet, though.

Rical Jasan provided his own review. [12] He has many documentation
remarks but I applied locally only one (reword "month name" to
"month names") because I am not sure that other changes should be
applied.  OTOH, if anybody wants to update the documentation after
I push the patches to master you are most welcome. That may be
even easier than telling me what I should fix locally, waiting
for another post etc.  Rical also suggests that instead of
_NL_ABALTMON_* pattern I should use __ABALTMON_* and conditionally
#ifdef __USE_GNU #define ABALTMON_*.  Again, I am not sure if
this is the right moment.  Of course, we indeed can add these
symbols in the future.  See also: [13] Note: _NL_WABALTMON_* should
remain, same as _NL_WMON_* and _NL_WABMON_*.

Please note that in order for these patches to be really useful
we still need the actual data to be updated and I'd like to contact
translators before I push the locale data changes.  So far I have
contacted only pl_PL translator and got the permission.  I think
it does not make sense to contact the translators before the patches
are pushed.

Regards,

Rafal


[5] https://sourceware.org/ml/libc-alpha/2018-01/msg00433.html
[6] https://sourceware.org/ml/libc-alpha/2018-01/msg00494.html
[7] https://sourceware.org/ml/libc-alpha/2018-01/msg00496.html
[8] https://sourceware.org/ml/libc-alpha/2018-01/msg00592.html
[9] https://sourceware.org/ml/libc-alpha/2018-01/msg00594.html
[10] https://sourceware.org/ml/libc-alpha/2018-01/msg00463.html
[11] https://sourceware.org/bugzilla/show_bug.cgi?id=19084
[12] https://sourceware.org/ml/libc-alpha/2018-01/msg00497.html
[13] http://austingroupbugs.net/view.php?id=1166

Re: TODO: Alternative month names (was: Re: glibc 2.27: less than two weeks till release)

[Zack Weinberg]

On Thu, Jan 18, 2018 at 4:56 PM, Rafal Luzynski
<digitalfreak@lingonborough.com> wrote:
> Please note that in order for these patches to be really useful
> we still need the actual data to be updated and I'd like to contact
> translators before I push the locale data changes.  So far I have
> contacted only pl_PL translator and got the permission.  I think
> it does not make sense to contact the translators before the patches
> are pushed.

We can easily update the locale data in point releases of 2.27.
Please do not consider the availability of locale data a blocker for
the code changes.

zw

Re: TODO: Alternative month names (was: Re: glibc 2.27: less than two weeks till release)

[Rafal Luzynski]

18.01.2018 23:04 Zack Weinberg <zackw@panix.com> wrote:
>
> On Thu, Jan 18, 2018 at 4:56 PM, Rafal Luzynski
> <digitalfreak@lingonborough.com> wrote:
> > Please note that in order for these patches to be really useful
> > we still need the actual data to be updated and I'd like to contact
> > translators before I push the locale data changes. So far I have
> > contacted only pl_PL translator and got the permission. I think
> > it does not make sense to contact the translators before the patches
> > are pushed.
>
> We can easily update the locale data in point releases of 2.27.
> Please do not consider the availability of locale data a blocker for
> the code changes.

No, I meant the opposite: I consider code changes as a blocker for
the locale data update.  Once the code is accepted and pushed it will
be easy for me to update the data.  But not in the reverse order.

Regards,

Rafal

Re: TODO: Alternative month names

[Carlos O'Donell]

On 01/18/2018 01:56 PM, Rafal Luzynski wrote:
> 18.01.2018 18:56 "Dmitry V. Levin" <ldv@altlinux.org> wrote:
>> [...]
>> ** Month names in alternative grammatical case[4]
>>
>> This has already been reviewed by Carlos.
>> Rafal, is there anything that has to be done before these series is committed?
>> [...]
>> [4] https://sourceware.org/ml/libc-alpha/2018-01/msg00258.html
>> [...]
> 
> I posted the 12th version of the patches for the final review [5]
> because of some changes in the documentation and test cases.
> Carlos answered that he'd verify the compatibility of statically
> built binaries before giving the final OK [6] but he did not yet.
> In the meantime I did a simple test and stated that the old static
> library is unable to load any new binary locale data. [7][8] My test
> was very cursory, that means I did not test what actually failed
> and whether the problem is really impossible to resolve, I only guess
> this is because of some sanity tests which verify the size of the
> binary data and expect it to be equal to the size defined by their
> version of glibc.  Florian agreed that even if it is not compatible
> the problem is not blocking. [9] Joseph Myers said that this
> incompatibility is acceptable and this already had happened in the
> past but needs to be mentioned in NEWS. [10][11] This NEWS section
> does not exist yet, though.
> 
> Rical Jasan provided his own review. [12] He has many documentation
> remarks but I applied locally only one (reword "month name" to
> "month names") because I am not sure that other changes should be
> applied.  OTOH, if anybody wants to update the documentation after
> I push the patches to master you are most welcome. That may be
> even easier than telling me what I should fix locally, waiting
> for another post etc.  Rical also suggests that instead of
> _NL_ABALTMON_* pattern I should use __ABALTMON_* and conditionally
> #ifdef __USE_GNU #define ABALTMON_*.  Again, I am not sure if
> this is the right moment.  Of course, we indeed can add these
> symbols in the future.  See also: [13] Note: _NL_WABALTMON_* should
> remain, same as _NL_WMON_* and _NL_WABMON_*.
> 
> Please note that in order for these patches to be really useful
> we still need the actual data to be updated and I'd like to contact
> translators before I push the locale data changes.  So far I have
> contacted only pl_PL translator and got the permission.  I think
> it does not make sense to contact the translators before the patches
> are pushed.

My apologies, I've been a bit busy. I will be doing a final review of
these patches tomorrow morning EST.

-- 
Cheers,
Carlos.

Re: glibc 2.27: less than two weeks till release

[Carlos O'Donell]

On 01/18/2018 09:56 AM, Dmitry V. Levin wrote:
> ** C11 threads[6]
> 
> This has been waiting for review for many weeks, last week Carlos said
> he will review this.
> It's unfortunate that we still have complicated all-arch patch series
> like this still pending review for 2.27.

My apologies for the delay, but basically metldown/spectre is probably
consuming *everyone's* free cycles on figuring out how the mitigation
will work. This has been the quietest freeze period I have ever seen
in my history working on the project.

Adhemerval, Would you be opposed to me reviewing this and committing
once the branch opens? Making it a feature of 2.28? I'd like to avoid
this commit so late in the freeze?


Cheers,
Carlos.

Re: TODO: Alternative month names

[Rafal Luzynski]

19.01.2018 09:51 Carlos O'Donell <carlos@redhat.com> wrote:
> [...]
> My apologies, I've been a bit busy. I will be doing a final review of
> these patches tomorrow morning EST.

Thank you, Carlos.  BTW, I have read Florian's post [1] again and
as far as I understand he currently withdraws all his objections
regarding the compatibility of the static binaries.  So the
questionable points, as far as I know, are:

* Should we add ABALTMON_* instead of _NL_ABALTMON_* already?
  My suggestion is no.
* Should I change anything else in the code?  I think that no.
* Should I change anything else in the documentation?  My suggestion
  is no, see the next point.
* Should someone else change anything in the documentation? My
  suggestion is yes (mention the incompatibility of old static
  binaries with the new locale data in NEWS, review again) but
  the easiest way is if I push the changes ASAP and someone else
  fixes the documentation on top of this.  Rical Jasan has already
  volunteered to provide more documentation fixes.

Of course I am open to any other opinion.

Regards,

Rafal


[1] https://sourceware.org/ml/libc-alpha/2018-01/msg00594.html

Re: glibc 2.27: less than two weeks till release

[Romain Naour]

Hi,

Le 18/01/2018 à 18:56, Dmitry V. Levin a écrit :
> Hi,
> 
> We have less than two weeks till the planned release date.  By this date
> in the freeze calendar we should have been ready to start architecture
> testing for 2.27, but, unfortunately, with pending complicated patches
> that affect all architectures we cannot really start the testing until
> these patches have either been accepted or postponed to 2.28.
> Nevertheless, architecture maintainers are encouraged to do preliminary
> testing.

I did a preliminary build test with current glibc master
(glibc-2.26.9000-1140-g4612268a0ad8e3409d8ce2314dd2dd8ee0af5269) for several
architecture using gcc 7.2, binutils 2.29.1 and kernel headers 4.9.

I would like report some build issue here before the release.

1) microblaze (glibc):
glibc-glibc-2.26.9000-1140-g4612268a0ad8e3409d8ce2314dd2dd8ee0af5269/build/io/xstatconv.o
../sysdeps/unix/sysv/linux/copy_file_range.c: In function 'copy_file_range':
../sysdeps/unix/sysv/linux/copy_file_range.c:44:10: error: implicit declaration
of function 'copy_file_range_compat'; did you mean 'copy_file_range'?
[-Werror=implicit-function-declaration]
   return copy_file_range_compat (infd, pinoff, outfd, poutoff, length, flags);
          ^~~~~~~~~~~~~~~~~~~~~~
          copy_file_range
cc1: all warnings being treated as errors

Seems a copy paste error introduced by:
https://sourceware.org/git/?p=glibc.git;a=commit;h=bad7a0c81f501fbbcc79af9eaa4b8254441c4a1f

2) aarch64 (gcc):

libsanitizer/sanitizer_common/sanitizer_linux.cc:1265:35: error: ‘struct
mcontext_t’ has no member named ‘__reserved’; did you mean ‘__glibc_reserved1’?
   u8 *aux = ucontext->uc_mcontext.__reserved;
                                   ^~~~~~~~~~
                                   __glibc_reserved1

Build issue introduced by:
https://sourceware.org/git/?p=glibc.git;a=commit;h=4fa9b3bfe6759c82beb4b043a54a3598ca467289

	* sysdeps/unix/sysv/linux/aarch64/ucontext_i.sym (oEXTENSION): Use
	__glibc_reserved1 instead of __reserved.

Even with the current gcc master, we can't build an aarch64 toolchain with the
upcoming glibc 2.27 since ucontext->uc_mcontext.__reserved is still used.

Best regards,
Romain

> 
> Here is a recap of pending issues.
> 
> * Release blockers listed in the release page[1]:
> 
> ** Reported failure on Intel hardware involving steam/mono runtimes[2],
> likely a regression caused by recent setjmp changes[3].
> 
> There is going to be an ABI change if we have to revert these changes,
> hampering the pre-release testing.
> H.J., is there any progress with this?
> 
> ** Month names in alternative grammatical case[4]
> 
> This has already been reviewed by Carlos.
> Rafal, is there anything that has to be done before these series is committed?
> 
> ** Review / update collation data from Unicode / ISO 14651[5]
> 
> Given that the bug is almost 6 year old and no patch has been submitted
> for review yet, does it have any chance for a review and a proper testing
> before the release?
> Mike, why do you consider this change as a blocker for 2.27?
> 
> * Features listed as desirable in the release page:
> 
> ** C11 threads[6]
> 
> This has been waiting for review for many weeks, last week Carlos said
> he will review this.
> It's unfortunate that we still have complicated all-arch patch series
> like this still pending review for 2.27.
> 
> ** Istvan Kurucsai's malloc security patches[7]
> 
> This series has been under review for many weeks.
> Florian, is it still targeted for 2.27?
> 
> ** RISC-V port v4[8]
> 
> There is a separate summary[9] on this topic.
> 
> ** Don't include libio.h from the installed stdio.h[10]
> 
> This is still pending review for 2.27.
> Florian, anyone else, is there any progress with this?
> 
> [1] https://sourceware.org/glibc/wiki/Release/2.27
> [2] https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html
> [3] https://sourceware.org/ml/libc-alpha/2018-01/msg00268.html
> [4] https://sourceware.org/ml/libc-alpha/2018-01/msg00258.html
> [5] https://sourceware.org/bugzilla/show_bug.cgi?id=14095
> [6] https://sourceware.org/ml/libc-alpha/2017-09/msg00871.html
> [7] https://sourceware.org/ml/libc-alpha/2017-11/msg00229.html
> [8] https://sourceware.org/ml/libc-alpha/2018-01/msg00475.html
> [9] https://sourceware.org/ml/libc-alpha/2018-01/msg00379.html
> [10] https://sourceware.org/ml/libc-alpha/2018-01/msg00248.html
> 
> 

Re: glibc 2.27: less than two weeks till release

[Szabolcs Nagy]

On 21/01/18 22:01, Romain Naour wrote:
> 2) aarch64 (gcc):
> 
> libsanitizer/sanitizer_common/sanitizer_linux.cc:1265:35: error: ‘struct
> mcontext_t’ has no member named ‘__reserved’; did you mean ‘__glibc_reserved1’?
>    u8 *aux = ucontext->uc_mcontext.__reserved;
>                                    ^~~~~~~~~~
>                                    __glibc_reserved1
> 
> Build issue introduced by:
> https://sourceware.org/git/?p=glibc.git;a=commit;h=4fa9b3bfe6759c82beb4b043a54a3598ca467289
> 
> 	* sysdeps/unix/sysv/linux/aarch64/ucontext_i.sym (oEXTENSION): Use
> 	__glibc_reserved1 instead of __reserved.
> 
> Even with the current gcc master, we can't build an aarch64 toolchain with the
> upcoming glibc 2.27 since ucontext->uc_mcontext.__reserved is still used.

indeed that commit seems faulty:

on aarch64 a list of cpu states (e.g fp registers) can only be
accessed via parsing the __reserved member (there are no macros
or functions defined for this, the user has to manually cast).

and __reserved does not violate the namespace rules, so i don't
think we need macro hackery to use different name based on
_GNU_SOURCE.

so i think this api should not be changed, i'll revert this part
of that patch.

thanks for catching it.

Re: glibc 2.27: less than two weeks till release

[Adhemerval Zanella]

On 22/01/2018 09:11, Szabolcs Nagy wrote:
> On 21/01/18 22:01, Romain Naour wrote:
>> 2) aarch64 (gcc):
>>
>> libsanitizer/sanitizer_common/sanitizer_linux.cc:1265:35: error: ‘struct
>> mcontext_t’ has no member named ‘__reserved’; did you mean ‘__glibc_reserved1’?
>>    u8 *aux = ucontext->uc_mcontext.__reserved;
>>                                    ^~~~~~~~~~
>>                                    __glibc_reserved1
>>
>> Build issue introduced by:
>> https://sourceware.org/git/?p=glibc.git;a=commit;h=4fa9b3bfe6759c82beb4b043a54a3598ca467289
>>
>> 	* sysdeps/unix/sysv/linux/aarch64/ucontext_i.sym (oEXTENSION): Use
>> 	__glibc_reserved1 instead of __reserved.
>>
>> Even with the current gcc master, we can't build an aarch64 toolchain with the
>> upcoming glibc 2.27 since ucontext->uc_mcontext.__reserved is still used.
> 
> indeed that commit seems faulty:
> 
> on aarch64 a list of cpu states (e.g fp registers) can only be
> accessed via parsing the __reserved member (there are no macros
> or functions defined for this, the user has to manually cast).
> 
> and __reserved does not violate the namespace rules, so i don't
> think we need macro hackery to use different name based on
> _GNU_SOURCE.
> 
> so i think this api should not be changed, i'll revert this part
> of that patch.
> 
> thanks for catching it.
> 

I noted it sometime ago and added a fix on my ILP32 sanitizer support on
compiler-rt [1] (still in review):

+// glibc starting at 2.26 does not typedef sigcontext to mcontext_t,
+// but rather defines its own with internal reserved member with
+// different naming.  The mcontext_t definition below is based on
+// Linux UAPI for sicontex_t.
+struct __sanitizer_mcontext_t {
+  unsigned long long int fault_address;
+  unsigned long long int regs[31];
+  unsigned long long int pc;
+  unsigned long long int sp;
+  unsigned long long int pstate;
+  unsigned char reserved[4096] __attribute__ ((__aligned__ (16))); // NOLINT
+};
+
 static bool Aarch64GetESR(ucontext_t *ucontext, u64 *esr) {
   static const u32 kEsrMagic = 0x45535201;
-  u8 *aux = ucontext->uc_mcontext.__reserved;
+  __sanitizer_mcontext_t *mctx = reinterpret_cast<__sanitizer_mcontext_t *>
+    (&ucontext->uc_mcontext);
+  u8 *aux = mctx->reserved;
   while (true) {
     _aarch64_ctx *ctx = (_aarch64_ctx *)aux;
     if (ctx->size == 0) break;


I am not sure if we should fix on glibc or sanitizer side.


[1] https://reviews.llvm.org/D40134

Re: glibc 2.27: less than two weeks till release

[Szabolcs Nagy]

On 22/01/18 11:53, Adhemerval Zanella wrote:
> On 22/01/2018 09:11, Szabolcs Nagy wrote:
>> On 21/01/18 22:01, Romain Naour wrote:
>>> 2) aarch64 (gcc):
>>>
>>> libsanitizer/sanitizer_common/sanitizer_linux.cc:1265:35: error: ‘struct
>>> mcontext_t’ has no member named ‘__reserved’; did you mean ‘__glibc_reserved1’?
>>>    u8 *aux = ucontext->uc_mcontext.__reserved;
>>>                                    ^~~~~~~~~~
>>>                                    __glibc_reserved1
>>>
>>> Build issue introduced by:
>>> https://sourceware.org/git/?p=glibc.git;a=commit;h=4fa9b3bfe6759c82beb4b043a54a3598ca467289
>>>
>>> 	* sysdeps/unix/sysv/linux/aarch64/ucontext_i.sym (oEXTENSION): Use
>>> 	__glibc_reserved1 instead of __reserved.
>>>
>>> Even with the current gcc master, we can't build an aarch64 toolchain with the
>>> upcoming glibc 2.27 since ucontext->uc_mcontext.__reserved is still used.
>>
>> indeed that commit seems faulty:
>>
>> on aarch64 a list of cpu states (e.g fp registers) can only be
>> accessed via parsing the __reserved member (there are no macros
>> or functions defined for this, the user has to manually cast).
>>
>> and __reserved does not violate the namespace rules, so i don't
>> think we need macro hackery to use different name based on
>> _GNU_SOURCE.
>>
>> so i think this api should not be changed, i'll revert this part
>> of that patch.
>>
>> thanks for catching it.
>>
> 
> I noted it sometime ago and added a fix on my ILP32 sanitizer support on
> compiler-rt [1] (still in review):
> 
> +// glibc starting at 2.26 does not typedef sigcontext to mcontext_t,
> +// but rather defines its own with internal reserved member with
> +// different naming.  The mcontext_t definition below is based on
> +// Linux UAPI for sicontex_t.
> +struct __sanitizer_mcontext_t {
> +  unsigned long long int fault_address;
> +  unsigned long long int regs[31];
> +  unsigned long long int pc;
> +  unsigned long long int sp;
> +  unsigned long long int pstate;
> +  unsigned char reserved[4096] __attribute__ ((__aligned__ (16))); // NOLINT
> +};
> +
>  static bool Aarch64GetESR(ucontext_t *ucontext, u64 *esr) {
>    static const u32 kEsrMagic = 0x45535201;
> -  u8 *aux = ucontext->uc_mcontext.__reserved;
> +  __sanitizer_mcontext_t *mctx = reinterpret_cast<__sanitizer_mcontext_t *>
> +    (&ucontext->uc_mcontext);
> +  u8 *aux = mctx->reserved;

casting to struct sigcontext* is fine here (that's the
linux asm/sigcontext.h api), however no existing code
would do that since mcontext_t used to be a typedef of
struct sigcontext.

>    while (true) {
>      _aarch64_ctx *ctx = (_aarch64_ctx *)aux;
>      if (ctx->size == 0) break;
> 
> 
> I am not sure if we should fix on glibc or sanitizer side.
> 

i think glibc should keep the sigcontext and mcontext
fields the same for now (except in posix mode the
namespace violations should be fixed up), there are
likely other users of it than the sanitizers.

> 
> [1] https://reviews.llvm.org/D40134
> 

Re: glibc 2.27: less than two weeks till release

[Joseph Myers]

On Sun, 21 Jan 2018, Romain Naour wrote:

> I did a preliminary build test with current glibc master
> (glibc-2.26.9000-1140-g4612268a0ad8e3409d8ce2314dd2dd8ee0af5269) for several
> architecture using gcc 7.2, binutils 2.29.1 and kernel headers 4.9.
> 
> I would like report some build issue here before the release.
> 
> 1) microblaze (glibc):
> glibc-glibc-2.26.9000-1140-g4612268a0ad8e3409d8ce2314dd2dd8ee0af5269/build/io/xstatconv.o
> ../sysdeps/unix/sysv/linux/copy_file_range.c: In function 'copy_file_range':
> ../sysdeps/unix/sysv/linux/copy_file_range.c:44:10: error: implicit declaration
> of function 'copy_file_range_compat'; did you mean 'copy_file_range'?
> [-Werror=implicit-function-declaration]
>    return copy_file_range_compat (infd, pinoff, outfd, poutoff, length, flags);
>           ^~~~~~~~~~~~~~~~~~~~~~
>           copy_file_range
> cc1: all warnings being treated as errors
> 
> Seems a copy paste error introduced by:
> https://sourceware.org/git/?p=glibc.git;a=commit;h=bad7a0c81f501fbbcc79af9eaa4b8254441c4a1f

This code should only be active if __NR_copy_file_range is not defined, 
but copy_file_range_compat should be defined by the include of 
io/copy_file_range-compat.c unless __ASSUME_COPY_FILE_RANGE is defined.  
Are you configuring with a --enable-kernel option?  It doesn't make sense 
to have a --enable-kernel option resulting in __ASSUME_COPY_FILE_RANGE 
being defined unless __NR_copy_file_range is also defined.

It looks like copy_file_range was only added for microblaze in 4.10 
(commit 7181e5590e5ba898804aef3ee6be7f27606e6f8b).  Which means the 
definition of __ASSUME_COPY_FILE_RANGE is incorrect for microblaze - needs 
an override in microblaze/kernel-features.h - and should be checked more 
generally for all glibc architectures to determine the kernel versions in 
which they had both the syscall table entry and the asm/unistd.h 
definition.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: glibc 2.27: less than two weeks till release

[Joseph Myers]

On Mon, 22 Jan 2018, Szabolcs Nagy wrote:

> on aarch64 a list of cpu states (e.g fp registers) can only be
> accessed via parsing the __reserved member (there are no macros
> or functions defined for this, the user has to manually cast).

If there are semantics beyond simply being reserved, there needs to be a 
prominent comment explaining how there are already such semantics in 
current kernel etc. versions, since any field that is just reserved should 
be named using the __glibc_reserved convention, not __reserved, __unused, 
__pad etc. (and __reserved without such an explanation thus looks like a 
field that should be renamed to conform to normal glibc coding style).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: glibc 2.27: less than two weeks till release

[Florian Weimer]

On 01/22/2018 04:48 PM, Joseph Myers wrote:
> On Mon, 22 Jan 2018, Szabolcs Nagy wrote:
> 
>> on aarch64 a list of cpu states (e.g fp registers) can only be
>> accessed via parsing the __reserved member (there are no macros
>> or functions defined for this, the user has to manually cast).
> 
> If there are semantics beyond simply being reserved, there needs to be a
> prominent comment explaining how there are already such semantics in
> current kernel etc. versions, since any field that is just reserved should
> be named using the __glibc_reserved convention, not __reserved, __unused,
> __pad etc. (and __reserved without such an explanation thus looks like a
> field that should be renamed to conform to normal glibc coding style).

It is reserved space for dynamic allocations, similar to the __space 
field in struct scratch_buffer, and the context data contains offsets 
relative to the start of the field.  It is in active use today and not 
intended for future expansion.

Thanks,
Florian

Re: glibc 2.27: less than two weeks till release

[Romain Naour]

Hi Joseph,

Le 22/01/2018 à 16:45, Joseph Myers a écrit :
> On Sun, 21 Jan 2018, Romain Naour wrote:
> 
>> I did a preliminary build test with current glibc master
>> (glibc-2.26.9000-1140-g4612268a0ad8e3409d8ce2314dd2dd8ee0af5269) for several
>> architecture using gcc 7.2, binutils 2.29.1 and kernel headers 4.9.
>>
>> I would like report some build issue here before the release.
>>
>> 1) microblaze (glibc):
>> glibc-glibc-2.26.9000-1140-g4612268a0ad8e3409d8ce2314dd2dd8ee0af5269/build/io/xstatconv.o
>> ../sysdeps/unix/sysv/linux/copy_file_range.c: In function 'copy_file_range':
>> ../sysdeps/unix/sysv/linux/copy_file_range.c:44:10: error: implicit declaration
>> of function 'copy_file_range_compat'; did you mean 'copy_file_range'?
>> [-Werror=implicit-function-declaration]
>>    return copy_file_range_compat (infd, pinoff, outfd, poutoff, length, flags);
>>           ^~~~~~~~~~~~~~~~~~~~~~
>>           copy_file_range
>> cc1: all warnings being treated as errors
>>
>> Seems a copy paste error introduced by:
>> https://sourceware.org/git/?p=glibc.git;a=commit;h=bad7a0c81f501fbbcc79af9eaa4b8254441c4a1f
> 
> This code should only be active if __NR_copy_file_range is not defined, 
> but copy_file_range_compat should be defined by the include of 
> io/copy_file_range-compat.c unless __ASSUME_COPY_FILE_RANGE is defined.  
> Are you configuring with a --enable-kernel option?  It doesn't make sense 
> to have a --enable-kernel option resulting in __ASSUME_COPY_FILE_RANGE 
> being defined unless __NR_copy_file_range is also defined.

The Buildroot packaging is using --enable-kernel option since 2 years now for
all architectures, see [1].

[1]
https://git.buildroot.net/buildroot/commit/?id=fd5bcd0eda8fb21f639c34a09b212e6f9b066a04

> 
> It looks like copy_file_range was only added for microblaze in 4.10 
> (commit 7181e5590e5ba898804aef3ee6be7f27606e6f8b).  Which means the 
> definition of __ASSUME_COPY_FILE_RANGE is incorrect for microblaze - needs 
> an override in microblaze/kernel-features.h - and should be checked more 
> generally for all glibc architectures to determine the kernel versions in 
> which they had both the syscall table entry and the asm/unistd.h 
> definition.
> 
So, __ASSUME_COPY_FILE_RANGE needs to be undefined for toolchain built with
kernel-headers < 4.10, right ?

Best regards,
Romain

Re: glibc 2.27: less than two weeks till release

[Joseph Myers]

On Wed, 24 Jan 2018, Romain Naour wrote:

> > It looks like copy_file_range was only added for microblaze in 4.10 
> > (commit 7181e5590e5ba898804aef3ee6be7f27606e6f8b).  Which means the 
> > definition of __ASSUME_COPY_FILE_RANGE is incorrect for microblaze - needs 
> > an override in microblaze/kernel-features.h - and should be checked more 
> > generally for all glibc architectures to determine the kernel versions in 
> > which they had both the syscall table entry and the asm/unistd.h 
> > definition.
> > 
> So, __ASSUME_COPY_FILE_RANGE needs to be undefined for toolchain built with
> kernel-headers < 4.10, right ?

The microblaze/kernel-features.h needs to undefine 
__ASSUME_COPY_FILE_RANGE under such a condition, yes, much like it handles 
various other syscalls added later than on other architectures - but it 
would be appropriate to make a check for all glibc architectures to see if 
there are any others for which the default (>= 4.5) definition of 
__ASSUME_COPY_FILE_RANGE is wrong.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: glibc 2.27: less than two weeks till release

[Romain Naour]

Hi Joseph,

Le 24/01/2018 à 23:02, Joseph Myers a écrit :
> On Wed, 24 Jan 2018, Romain Naour wrote:
> 
>>> It looks like copy_file_range was only added for microblaze in 4.10 
>>> (commit 7181e5590e5ba898804aef3ee6be7f27606e6f8b).  Which means the 
>>> definition of __ASSUME_COPY_FILE_RANGE is incorrect for microblaze - needs 
>>> an override in microblaze/kernel-features.h - and should be checked more 
>>> generally for all glibc architectures to determine the kernel versions in 
>>> which they had both the syscall table entry and the asm/unistd.h 
>>> definition.
>>>
>> So, __ASSUME_COPY_FILE_RANGE needs to be undefined for toolchain built with
>> kernel-headers < 4.10, right ?
> 
> The microblaze/kernel-features.h needs to undefine 
> __ASSUME_COPY_FILE_RANGE under such a condition, yes, much like it handles 
> various other syscalls added later than on other architectures - but it 
> would be appropriate to make a check for all glibc architectures to see if 
> there are any others for which the default (>= 4.5) definition of 
> __ASSUME_COPY_FILE_RANGE is wrong.
> 

For now, I only tested with kernel headers 4.9 on several architectures (arm,
aarch64, x86, x86-64, powerpc, powerpc64, nios2, sh, sparc).

copy_file_range has been added for xtensa in 4.9, sh in 4.8, aarch64 in 4.7, so
the default (>= 4.5) definition of __ASSUME_COPY_FILE_RANGE is wrong for them.

Best regards,
Romain

Re: glibc 2.27: less than two weeks till release

[Joseph Myers]

On Sun, 28 Jan 2018, Romain Naour wrote:

> > The microblaze/kernel-features.h needs to undefine 
> > __ASSUME_COPY_FILE_RANGE under such a condition, yes, much like it handles 
> > various other syscalls added later than on other architectures - but it 
> > would be appropriate to make a check for all glibc architectures to see if 
> > there are any others for which the default (>= 4.5) definition of 
> > __ASSUME_COPY_FILE_RANGE is wrong.
> > 
> 
> For now, I only tested with kernel headers 4.9 on several architectures (arm,
> aarch64, x86, x86-64, powerpc, powerpc64, nios2, sh, sparc).
> 
> copy_file_range has been added for xtensa in 4.9, sh in 4.8, aarch64 in 4.7, so
> the default (>= 4.5) definition of __ASSUME_COPY_FILE_RANGE is wrong for them.

That aarch64 change looks like it was just for the compat syscall table 
(so possibly relevant for the arm port), not for the normal one?

(glibc doesn't support xtensa; there is/was a port, but it's never been 
upstreamed.)

It seems alpha added mlock2 and copy_file_range only in 4.13 (kernel 
commit a720830613eaa25eb5bc9b76705a88a36296709a - which has a load of 
other syscalls as well, but not ones that currently have __ASSUME_* 
macros).  There may well be other such issues; a proper review is 
certainly needed.  Things were correct (based on my review) as of glibc 
commit 6d08b0223aa3b3152ea7e9beb2b8f2a151b43109; it's __ASSUME_EXECVEAT, 
__ASSUME_MLOCK2 and __ASSUME_COPY_FILE_RANGE that need careful attention 
to when each architecture actually added the syscall to both asm/unistd.h 
and the syscall table.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: glibc 2.27: less than two weeks till release

[Romain Naour]

Hi Joseph,

Le 29/01/2018 à 17:28, Joseph Myers a écrit :
> On Sun, 28 Jan 2018, Romain Naour wrote:
> 
>>> The microblaze/kernel-features.h needs to undefine 
>>> __ASSUME_COPY_FILE_RANGE under such a condition, yes, much like it handles 
>>> various other syscalls added later than on other architectures - but it 
>>> would be appropriate to make a check for all glibc architectures to see if 
>>> there are any others for which the default (>= 4.5) definition of 
>>> __ASSUME_COPY_FILE_RANGE is wrong.
>>>
>>
>> For now, I only tested with kernel headers 4.9 on several architectures (arm,
>> aarch64, x86, x86-64, powerpc, powerpc64, nios2, sh, sparc).
>>
>> copy_file_range has been added for xtensa in 4.9, sh in 4.8, aarch64 in 4.7, so
>> the default (>= 4.5) definition of __ASSUME_COPY_FILE_RANGE is wrong for them.
> 
> That aarch64 change looks like it was just for the compat syscall table 
> (so possibly relevant for the arm port), not for the normal one?

I'm not sure it worth the effort to keep the support for kernel headers between
4.4 and 4.9 (i.e: kernel not maintained anymore). Buildroot has already removed
4.5 to 4.8 kernel headers selection.

> 
> (glibc doesn't support xtensa; there is/was a port, but it's never been 
> upstreamed.)

Right.

> 
> It seems alpha added mlock2 and copy_file_range only in 4.13 (kernel 
> commit a720830613eaa25eb5bc9b76705a88a36296709a - which has a load of 
> other syscalls as well, but not ones that currently have __ASSUME_* 
> macros).  There may well be other such issues; a proper review is 
> certainly needed.  Things were correct (based on my review) as of glibc 
> commit 6d08b0223aa3b3152ea7e9beb2b8f2a151b43109; it's __ASSUME_EXECVEAT, 
> __ASSUME_MLOCK2 and __ASSUME_COPY_FILE_RANGE that need careful attention 
> to when each architecture actually added the syscall to both asm/unistd.h 
> and the syscall table.
> 

I can't test with alpha architecture, but for other architectures supported by
Buildroot all syscall are correctly defined.

Best regards,
Romain

Re: glibc 2.27: less than two weeks till release

[Joseph Myers]

On Mon, 29 Jan 2018, Romain Naour wrote:

> I'm not sure it worth the effort to keep the support for kernel headers between
> 4.4 and 4.9 (i.e: kernel not maintained anymore). Buildroot has already removed
> 4.5 to 4.8 kernel headers selection.

--enable-kernel is about the kernel supported at runtime, not about kernel 
headers; it just so happened that in this case the mistake broke the 
build with certain kernel headers.  The oldest kernel version supported by 
the glibc sources should always be an LTS version, but that doesn't stop 
people using --enable-kernel with a more recent non-LTS version.

-- 
Joseph S. Myers
joseph@codesourcery.com