Re: [PATCH 1/4] localename: -Wtautological-pointer-compare
On 2023-01-15 14:03, Bruno Haible wrote: My confusion arose partly because I am accustomed to languages where the distinction between null and non-null pointers is checked statically ... Oh, now I understand. May I guess the language: Haskell, OCaml, TypeScript, Rust? These days OCaml and maybe Rust, though the language that first comes to my mind is one I designed in the 1970s and never got off the ground: B Pascal. Unfortunately, this is an excellent example for a portability problem: The division yields a SIGFPE on x86, x86_64, alpha, m68k, and s390/s390x CPU, but not on other architectures. I suppose I should have written that my assumption was that the code is executed in a context where the divisor must be positive. (This is pretty common when calculating indexes from sizes.) In other contexts the extra checking may be necessary, or at least helpful.
Re: [PATCH v2] Define alignof_slot using _Alignof when using C11 or newer
On Sun, Jan 15, 2023 at 11:52 AM Paul Eggert wrote: > On 2023-01-14 17:51, Paul Eggert wrote: > > > We already have two copies of that stuff elsewhere (in lib/stdalign.in.h > > and m4/stdalign.m4), for reasons that currently escape me. > > To try to address that in an upward-compatible way, I installed the > attached. This shrinks the number of copies of that code down to one. > The basic idea is to prefer C23 semantics in apps and the rest of Gnulib > by using the new alignasof module, whereas the existing stdalign module > is now for C11/C17 compatibility and is now deprecated. Your patch fixed the issue I did not try regenerating the gnulib in the concerned project but copied the change manually to test. AFAICT c++ looks to be not affected >
Re: RFC: git-commit based mtime-reproducible tarballs
Paul Eggert wrote:
> some users want to "trust but verify" and a reproducible
> tarball is easier to audit than a non-reproducible one, so for these
> users it can be a win to omit the irrelevant data from the tarball.
Reproducibility can be implemented in different ways:
- by omitting irrelevant data from the tarball,
- by having a customized comparison program 'diff', such that
"diff --ignore-irrelevant-metadata contents1 contents2"
would ignore the irrelevant parts.
> when I do an 'ls
> -l' of a source directory that I got from a distribution tarball, it's
> useful to see the last time the contents of each source file was changed
> upstream.
OK, now we're discussing different ways to make a tarball reproducible.
That's nice, because Simon's proposal was to make all timestamps equal,
and that puts me off.
In binutils-2.40.tar.bz2 all files are from 2023-01-14.
In android-studio-2021.3.1.17-linux.tar.gz all files are from 2010-01-01.
It gives me as a user no idea whether this tarball is 13 years old,
2 years old, or from yesterday.
I much prefer Paul's approach, since it still conveys meaningful
timestamps:
> For TZDB, where users have long wanted reproducibility, I use something
> like this in a Makefile recipe for each source file $$file:
>
> time=`git log -1 --format='tformat:%ct' $$file` &&
> touch -cmd @$$time $$file
That's good for the files that are under version control.
> 2. What about platform-independent files that are automatically created
> from source files from the repository, and that are shipped in the
> release tarball?
For these, you could unpack the tarball, see in which order the timestamps
are, and then assign artificial timestamps, in the same order but exactly
2 seconds apart. For example, if the tarball contains
under version control:
hello.c 2023-01-14 13:28:14
configure.ac2023-01-01 14:03:07
and not under version control:
configure 2023-01-15 04:09:10
config.h.in 2023-01-15 04:05:19
then you would determine the
max_timestamp_under_vc = max { 2023-01-14 13:28:14, 2023-01-01 14:03:07 }
= 2023-01-14 13:28:14
and then, since config.h.in is older than configure:
touch -m (max_timestamp_under_vc + 2 seconds) config.h.in
touch -m (max_timestamp_under_vc + 4 seconds) configure
You can do this without knowing the Makefile rules or scripts which created
config.h.in and configure.
The increment of 2 seconds is, of course, for VFAT file systems, which have
only 2 seconds of resolution for file modification times.
> GNUTARFLAGS uses delete=atime,delete=ctime so that atime and
> ctime do not leak into the tarball and make it less reproducible
I agree, it's pointless to have atime and ctime in a tarball.
Bruno
Re: [PATCH 1/4] localename: -Wtautological-pointer-compare
Hi Paul, > My confusion arose partly because I am accustomed to languages where the > distinction between null and non-null pointers is checked statically and > reliably, and I keep forgetting that with C, GCC and Clang are only poor > approximations to that Oh, now I understand. May I guess the language: Haskell, OCaml, TypeScript, Rust? > - though I hope the approximations are slowly getting better. Still it will take a lot of time. The following steps need to happen: 1. Standards need to define a notation for declaring a non-null type value, non-null argument, or non-null return value. (Partially done.) 2. Compilers need to diagnose places where a non-null declaration could be added, like they do for function attributes __pure__ and __const__. 3. Developers need to add such declarations to their .h files. Then only such static checking can happen, without producing floods of diagnostics that developers would discard. > Also, in my experience the debugger doesn't always point to the exact > line of the abort(). For example, if there are two abort() calls in the > same function they are routinely coalesced. True :( I should have mentioned to compile with "-g -O0", not just "-g". > To give a different example: I wouldn't bother with the following code > (where M and N are int arguments to a function): > > if (n == 0) >abort (); > if (n == -1 && m < -INT_MAX) >abort (); > return m / n; > > and would instead write this: > > return m / n; > > as the user and debugging experiences are similar and the shorter form > simplifies code maintenance. Unfortunately, this is an excellent example for a portability problem: The division yields a SIGFPE on x86, x86_64, alpha, m68k, and s390/s390x CPU, but not on other architectures. > Sure, the longer form is safer for oddball > platforms, but it's not worth the aggravation. Some distros feel differently. I have such code in GNU gettext, and optimize away the entry checks on platforms where I know it yields a SIGFPE. And some distros disabled these optimizations, i.e. enabled the entry checks always... Bruno
Re: [PATCH v2] Define alignof_slot using _Alignof when using C11 or newer
Oh, that patch had a misspelling "alignasf"; fixed by installing the attached further patch. From 784f8c776223ba3234c7673664f1fd2a9629bcda Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 15 Jan 2023 11:53:40 -0800 Subject: [PATCH] sys_socket: fix typo Fix typo in previous change. --- modules/sys_socket | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/sys_socket b/modules/sys_socket index ebb5d7d4a6..57ea690270 100644 --- a/modules/sys_socket +++ b/modules/sys_socket @@ -8,7 +8,7 @@ m4/sys_socket_h.m4 m4/sockpfaf.m4 Depends-on: -alignasf +alignasof errno extern-inline gen-header -- 2.37.2
Re: [PATCH v2] Define alignof_slot using _Alignof when using C11 or newer
On 2023-01-14 17:51, Paul Eggert wrote: We already have two copies of that stuff elsewhere (in lib/stdalign.in.h and m4/stdalign.m4), for reasons that currently escape me. To try to address that in an upward-compatible way, I installed the attached. This shrinks the number of copies of that code down to one. The basic idea is to prefer C23 semantics in apps and the rest of Gnulib by using the new alignasof module, whereas the existing stdalign module is now for C11/C17 compatibility and is now deprecated.From c593e834e1c17daf5c151ec2bdadbccc65b9efd4 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 15 Jan 2023 11:48:43 -0800 Subject: [PATCH] alignasof: new module This splits off support for the C23 keywords alignas and alignof, from the now-deprecated stdalign module. The latter now merely provides C11 support. * MODULES.html.sh, NEWS, doc/gnulib.texi: * doc/posix-headers/stdalign.texi: Document the change. * lib/stdalign.in.h: Remove most of the definitions (which are now supplied by the alignasof module), leaving only __alignas_is_defined and __alignof_is_defined. * modules/alignasof, modules/alignasof-tests: New files. * m4/stdalign.m4 (gl_ALIGNASOF): New macro, with most of the contents of the old gl_STDALIGN_H. Do not define __alignas_is_defined or __alignof_is_defined. (gl_STDALIGN_H): Rely on gl_ALIGNASOF for most of the work. * modules/alignalloc, modules/alignof, modules/argp: * modules/crypto/md4-buffer, modules/crypto/md5-buffer: * modules/crypto/sha1-buffer, modules/crypto/sha256-buffer: * modules/crypto/sha512-buffer, modules/crypto/sm3-buffer: * modules/fts, modules/rawmemchr, modules/relocatable-prog-wrapper: * modules/stddef-tests, modules/sys_socket: Depend on alignasof, not stdalign. * modules/stdalign: Deprecate. Depend on alignasof. * modules/stdalign-tests: Move most contents to the new module alignasof-tests, and depend on that. --- ChangeLog| 27 +++ MODULES.html.sh | 1 + NEWS | 2 + doc/gnulib.texi | 7 +- doc/posix-headers/stdalign.texi | 31 +--- lib/stdalign.in.h| 109 ++ m4/stdalign.m4 | 127 +-- modules/alignalloc | 2 +- modules/alignasof| 18 + modules/alignasof-tests | 13 modules/alignof | 2 +- modules/argp | 2 +- modules/crypto/md4-buffer| 2 +- modules/crypto/md5-buffer| 2 +- modules/crypto/sha1-buffer | 2 +- modules/crypto/sha256-buffer | 2 +- modules/crypto/sha512-buffer | 2 +- modules/crypto/sm3-buffer| 2 +- modules/fts | 2 +- modules/rawmemchr| 2 +- modules/relocatable-prog-wrapper | 2 +- modules/stdalign | 7 +- modules/stdalign-tests | 7 +- modules/stddef-tests | 2 +- modules/sys_socket | 2 +- 25 files changed, 198 insertions(+), 179 deletions(-) create mode 100644 modules/alignasof create mode 100644 modules/alignasof-tests diff --git a/ChangeLog b/ChangeLog index 0dedc0c7d6..4755f19360 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,30 @@ +2023-01-15 Paul Eggert + + alignasof: new module + This splits off support for the C23 keywords alignas and alignof, + from the now-deprecated stdalign module. The latter now merely + provides C11 support. + * MODULES.html.sh, NEWS, doc/gnulib.texi: + * doc/posix-headers/stdalign.texi: Document the change. + * lib/stdalign.in.h: Remove most of the definitions (which are now + supplied by the alignasof module), leaving only + __alignas_is_defined and __alignof_is_defined. + * modules/alignasof, modules/alignasof-tests: New files. + * m4/stdalign.m4 (gl_ALIGNASOF): New macro, with + most of the contents of the old gl_STDALIGN_H. + Do not define __alignas_is_defined or __alignof_is_defined. + (gl_STDALIGN_H): Rely on gl_ALIGNASOF for most of the work. + * modules/alignalloc, modules/alignof, modules/argp: + * modules/crypto/md4-buffer, modules/crypto/md5-buffer: + * modules/crypto/sha1-buffer, modules/crypto/sha256-buffer: + * modules/crypto/sha512-buffer, modules/crypto/sm3-buffer: + * modules/fts, modules/rawmemchr, modules/relocatable-prog-wrapper: + * modules/stddef-tests, modules/sys_socket: + Depend on alignasof, not stdalign. + * modules/stdalign: Deprecate. Depend on alignasof. + * modules/stdalign-tests: Move most contents to the new module + alignasof-tests, and depend on that. + 2023-01-15 Bruno Haible fpending: Fix compilation error with NDK ≥ r14b and Android API < 23. diff --git a/MODULES.html.sh b/MODULES.html.sh index 185e390440..937e9c02d8 100755 --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2383,6 +2383,7 @@ func_all_modules () func_echo "$element" func_begin_table + func_module alignasof func_module stdckdint
Re: RFC: git-commit based mtime-reproducible tarballs
On 2023-01-15 05:21, Bruno Haible wrote: Reproducibility is about verifying that an artifact A was generated from a source S. Quite true. However, there's something else going on: when I do an 'ls -l' of a source directory that I got from a distribution tarball, it's useful to see the last time the contents of each source file was changed upstream. When sources are in a Git repository, I've found the commit timestamp to be a good representation for that. For TZDB, where users have long wanted reproducibility, I use something like this in a Makefile recipe for each source file $$file: time=`git log -1 --format='tformat:%ct' $$file` && touch -cmd @$$time $$file Here are three problems I ran into with this approach, and the solutions that TZDB uses: 1. As you mentioned, what if you're building a release from sources that have not yet been committed? In this case TZDB's Makefile recipe warns but goes ahead with the timestamp that the working file already has. 2. What about platform-independent files that are automatically created from source files from the repository, and that are shipped in the release tarball? In this case, the TZDB Makefile arranges for each such file to have a timestamp one second later than the maximum of timestamps of files that the file depends on. This step is the biggest hassle, since it means I need to repeat in the Makefile the logic that 'make' already uses when calculating dependencies. 3. What about tarball metadata other than last-modified time? Here, TZDB uses the following GNU Tar options: GNUTARFLAGS= --format=pax --pax-option='delete=atime,delete=ctime' \ --numeric-owner --owner=0 --group=0 \ --mode=go+u,go-w --sort=name The need for most of this should be obvious, if one wants the tarball to be reproducible. However, some details are less obvious. GNUTARFLAGS specifies pax format because the default GNU Tar format becomes unportable after 2242-03-16 12:56:32 UTC due to the 33-bit limitation of ustar. And GNUTARFLAGS uses delete=atime,delete=ctime so that atime and ctime do not leak into the tarball and make it less reproducible; since mtime values are always a multiple of 1 second (given steps 1 and 2) this means the tarball will be ustar-compatible until 2242, giving users *plenty* of time to prepare for pax format timestamps. There is an argument that we need not have a fancy GNUTARFLAGS like this, because I'm signing the tarballs and users have to trust me anyway. Still, some users want to "trust but verify" and a reproducible tarball is easier to audit than a non-reproducible one, so for these users it can be a win to omit the irrelevant data from the tarball.
Re: RFC: git-commit based mtime-reproducible tarballs
Hi Simon, > > This attempts to make > > reproducible tarballs by sorting the files and passing the > > "--mtime=" option to tar. ... > Having the same mtime on all files in a tarball First question: What is the point of doing that? Reproducibility is about verifying that an artifact A was generated from a source S. When I, as a GNU maintainer or uploader, create a tarball and upload it to ftp.gnu.org, that tarball is the source S. Because that's what I sign with my GPG key. The commits in the git repo aren't the source, and even the git checkout on my disk aren't the source — because I am free to unpack and repack the tarball as I like, before I upload it to ftp.gnu.org. When someone runs a complex build on possibly untrusted servers in the cloud, then it makes sense to view the tarball as an artifact A and the git repository as the source S. (If the git repository is hosted elsewhere. If the git repository is being hosted on the same untrusted servers, it is not sufficient.) As a consequence, please make such modifications dependent on an option or environment variable (maybe SOURCE_DATE_EPOCH [1]?); don't activate them for everyone. > 1) Having the same mtime on all files in a tarball may cause problems Definitely. HP-UX 'make' attempts to rebuilds a file Y that depends on a file X, if Y and X have the same timestamp (mtime). It is long known that you have to have actually different timestamps for some files. Bruno [1] https://reproducible-builds.org/docs/source-date-epoch/
Re: fpending Android bug
Bruno Haible writes: > Ah, now that makes sense: When they moved 'struct __sFILE' out of > into , they also removed its named fields. > > In other words, looking at the timeline of the Android API level 19 support: > - They added this support in NDK r10e, > - In NDK r13b the elements of a FILE were accessible, > - In NDK r14b the elements of a FILE were suddenly not accessible any more. > > I'm applying your patch: > > > 2023-01-15 Bruno Haible > > fpending: Fix compilation error with NDK ≥ r14b and Android API < 23. > Report and patch by Po Lu . > * lib/fpending.c (__fpending) [__ANDROID__]: Use the fp_ macro. > > diff --git a/lib/fpending.c b/lib/fpending.c > index afa840b851..e57155e586 100644 > --- a/lib/fpending.c > +++ b/lib/fpending.c > @@ -41,7 +41,7 @@ __fpending (FILE *fp) >return fp->_IO_write_ptr - fp->_IO_write_base; > #elif defined __sferror || defined __DragonFly__ || defined __ANDROID__ >/* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin < 1.7.34, Minix > 3, Android */ > - return fp->_p - fp->_bf._base; > + return fp_->_p - fp_->_bf._base; > #elif defined __EMX__/* emx+gcc */ >return fp->_ptr - fp->_buffer; > #elif defined __minix/* Minix */ Thanks, Bruno.
RFC: git-commit based mtime-reproducible tarballs
Hi. Quoting the recent binutils announcement:
> As an experiment these tarballs were made with the new "-r "
> option supported by the src-release.sh script. This attempts to make
> reproducible tarballs by sorting the files and passing the
> "--mtime=" option to tar. The date used for these tarballs was
> obtained by running:
>
> git log -1 --format=%cd --date=format:%F bfd/version.m4
This got me thinking about git-version-gen and GNUmakefile, and I came
up with the patch below to use the most recent commit as the timestamp
for all files in the tarball. What do you think?
There are some concerns about this:
1) Having the same mtime on all files in a tarball may cause problems
for some projects that have fragile dependency-systems. While I think
all dependency checks really should be using >= timestamp tests, I
wouldn't rule out that some use > timestamp tests, which would cause
(sometimes unwanted) rebuilding of some files. Are there
dependency-constructs where the same mtime for all files in a tarball is
just a bad idea, with no better approach available?
2) The use of TAR_OPTIONS in GNUmakefile is complex and somewhat hard to
debug. I can't find any cleaner way to provide options to tar for 'make
dist' though. Automake defines $(AMTAR) but looks like an internal
symbol which also isn't used (bug?), instead $(am__tar) is used and
defined as am__tar = $${TAR-tar} chof - "$$tardir". So we can override
TAR in Makefile.am but it looks like a user-variable that we shouldn't
override. So pending support for a AMTAR (or AM_TAR?) variable in
Makefile.am that actually works, I guess we are stuck with the
TAR_OPTIONS approach. We could do 'TAR = env TAR_OPTIONS_=... tar' in
Makefile.am but it looks like the wrong approach.
3) The Makefile.am snippet in git-version-gen is difficult to maintain,
can't we put such snippets in a gnulib-owned file and suggest use of
'include gl/top-gl-Makefile.am-include.mk' instead? The same applies to
gen-ChangeLog rule. The logic would have to be a bit more complex to
support per-project modifications to these rules though.
Two small bugs that are possible to fix but not important before we know
if mtime-reproducible tarballs is useful or not:
4) If there is no .version file when you type 'make dist' my patch below
would fail to provide --mtime=... to tar. So it fails if you didn't do
'make' before 'make dist' after ./bootstrap + ./configure in a clean
checkout.
5) It is also a bit fragile that it assume 'git log -1' works without
checking for errors before invoking touch.
/Simon
diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
index a72057bf2c..0a98cb12dd 100755
--- a/build-aux/git-version-gen
+++ b/build-aux/git-version-gen
@@ -66,6 +66,7 @@ scriptversion=2022-07-09.08; # UTC
# BUILT_SOURCES = $(top_srcdir)/.version
# $(top_srcdir)/.version:
# echo '$(VERSION)' > $@-t
+# touch -m -d @$(shell git log -1 --format=%cd --date=unix) $@-t
# mv $@-t $@
# dist-hook:
# echo '$(VERSION)' > $(distdir)/.tarball-version
diff --git a/top/GNUmakefile b/top/GNUmakefile
index 07b331fe53..f0dd41b5b4 100644
--- a/top/GNUmakefile
+++ b/top/GNUmakefile
@@ -25,8 +25,14 @@
_gl-Makefile := $(wildcard [M]akefile)
ifneq ($(_gl-Makefile),)
+_gl-.version := $(wildcard .version)
+ifneq ($(_gl-.version),)
+_tar_mtime := --mtime=.version
+endif
+
# Make tar archive easier to reproduce.
-export TAR_OPTIONS = --owner=0 --group=0 --numeric-owner --sort=name
+export TAR_OPTIONS = --owner=0 --group=0 --numeric-owner --sort=name \
+ $(_tar_mtime)
# Allow the user to add to this in the Makefile.
ALL_RECURSIVE_TARGETS =
signature.asc
Description: PGP signature
Re: fpending Android bug
Po Lu wrote:
> It doesn't compile because ->_p and ->_base are undefined in FILE *.
> ...
> I'm using the NDK r25b; however, bits/struct_file.h in the NDK r25b only
> includes:
>
> __BEGIN_DECLS
>
> /** The opaque structure implementing `FILE`. Do not make any assumptions
> about its content. */
> struct __sFILE {
> #if defined(__LP64__)
> char __private[152];
> #else
> char __private[84];
> #endif
> } __attribute__((aligned(sizeof(void*;
>
> __END_DECLS
>
> even when __ANDROID_API__ is 19.
Ah, now that makes sense: When they moved 'struct __sFILE' out of
into , they also removed its named fields.
In other words, looking at the timeline of the Android API level 19 support:
- They added this support in NDK r10e,
- In NDK r13b the elements of a FILE were accessible,
- In NDK r14b the elements of a FILE were suddenly not accessible any more.
I'm applying your patch:
2023-01-15 Bruno Haible
fpending: Fix compilation error with NDK ≥ r14b and Android API < 23.
Report and patch by Po Lu .
* lib/fpending.c (__fpending) [__ANDROID__]: Use the fp_ macro.
diff --git a/lib/fpending.c b/lib/fpending.c
index afa840b851..e57155e586 100644
--- a/lib/fpending.c
+++ b/lib/fpending.c
@@ -41,7 +41,7 @@ __fpending (FILE *fp)
return fp->_IO_write_ptr - fp->_IO_write_base;
#elif defined __sferror || defined __DragonFly__ || defined __ANDROID__
/* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin < 1.7.34, Minix 3,
Android */
- return fp->_p - fp->_bf._base;
+ return fp_->_p - fp_->_bf._base;
#elif defined __EMX__/* emx+gcc */
return fp->_ptr - fp->_buffer;
#elif defined __minix/* Minix */
