public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* Updating <sys/cdefs.h> in glibc and gnulib
@ 2023-02-21 10:14 Florian Weimer
  2023-02-26 19:02 ` Bruno Haible
  0 siblings, 1 reply; 3+ messages in thread
From: Florian Weimer @ 2023-02-21 10:14 UTC (permalink / raw)
  To: libc-alpha, bug-gnulib; +Cc: Sachin Monga

Why does gnulib bundle <sys/cdefs.h>?  We edit this file regularly in
glibc.  In the past, some gnulib-using programs supplied their own copy
of <sys/cdefs.h> instead, even when building against glibc.  This caused
build failures in the glibc headers because they (quite reasonably)
assumed that <sys/cdefs.h> defines the macros for that glibc version.

Does gnulib still override <sys/cdefs.h> unconditionally?

A version check will be difficult because sometimes, we have to backport
header fixes to older versions, and that may require adding additional
macros in <sys/cdefs.h>.

We could move glibc's internal definitions to a new header, reducing
<sys/cdefs.h> in scope, but presumably that means gnulib would just
starting bundling that other header, and we would have the same issue
once more.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Updating <sys/cdefs.h> in glibc and gnulib
  2023-02-21 10:14 Updating <sys/cdefs.h> in glibc and gnulib Florian Weimer
@ 2023-02-26 19:02 ` Bruno Haible
  2023-02-27 14:52   ` Florian Weimer
  0 siblings, 1 reply; 3+ messages in thread
From: Bruno Haible @ 2023-02-26 19:02 UTC (permalink / raw)
  To: libc-alpha, bug-gnulib; +Cc: Sachin Monga, Florian Weimer

Florian Weimer wrote:
> Does gnulib still override <sys/cdefs.h> unconditionally?

Gnulib does not override <sys/cdefs.h>, and never did.

That is, when a package that uses Gnulib does
  #include <sys/cdefs.h>
it will get the <sys/cdefs.h> of the system (from glibc, *BSD, Cygwin,
etc.).

Only when a package uses the compiler option "-I /usr/include/sys"
and
  #include <cdefs.h>
there would be a conflict between what Gnulib ships and the installed
file in /usr/include/sys. But nobody does that, because there would
already be a conflict between <time.h> and <sys/time.h>.

What Gnulib does is to ship a copy of glibc's <sys/cdefs.h> as <cdefs.h>,
not <sys/cdefs.h>. And it is used for a few compilation units only
(essentially regex, fnmatch, glob, and a few others).

> Why does gnulib bundle <sys/cdefs.h>?

Gnulib takes the source code of regex, fnmatch, glob, and a few other
functions from glibc and makes them portable to other platforms. Since
this source code contains references to __glibc_unlikely,
__attribute_warn_unused_result__, etc., Gnulib uses the copy of cdefs.h
to define these macros in the way the source code shared with glibc
expects it.

Some of these symbols are also defined on other platforms, sometimes
differently. Therefore Gnulib has to be careful to not override essential
symbols of these other platforms. It's not trivial, but there appears
to be enough room to navigate through the two constraints. [1]

> In the past, some gnulib-using programs supplied their own copy
> of <sys/cdefs.h> instead, even when building against glibc.  This caused
> build failures in the glibc headers because they (quite reasonably)
> assumed that <sys/cdefs.h> defines the macros for that glibc version.

You need to take this up with the respective packages. As I said above,
Gnulib overrides <cdefs.h>, not <sys/cdefs.h>.

There were some problems around the 'glob' code, but they were resolved
more than 1.5 years ago. This area is still a bit fragile, though.

> We could move glibc's internal definitions to a new header, reducing
> <sys/cdefs.h> in scope, but presumably that means gnulib would just
> starting bundling that other header, and we would have the same issue
> once more.

Yes, such a move would be pointless.

Bruno

[1] https://lists.gnu.org/archive/html/bug-gnulib/2023-01/msg00238.html




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Updating <sys/cdefs.h> in glibc and gnulib
  2023-02-26 19:02 ` Bruno Haible
@ 2023-02-27 14:52   ` Florian Weimer
  0 siblings, 0 replies; 3+ messages in thread
From: Florian Weimer @ 2023-02-27 14:52 UTC (permalink / raw)
  To: Bruno Haible; +Cc: libc-alpha, bug-gnulib, Sachin Monga, Siddhesh Poyarekar

* Bruno Haible:

> Florian Weimer wrote:
>> Does gnulib still override <sys/cdefs.h> unconditionally?
>
> Gnulib does not override <sys/cdefs.h>, and never did.

Thanks for looking into this.  gnulib's libc-config.h does this:

| #ifndef __attribute_nonnull__
| /* <sys/cdefs.h> either does not exist, or is too old for Gnulib.
|    Prepare to include <cdefs.h>, which is Gnulib's version of a
|    more-recent glibc <sys/cdefs.h>.  */
| …
| /* Include our copy of glibc <sys/cdefs.h>.  */
| # include <cdefs.h>

And as gnulib's <cdefs.h> uses the same _SYS_CDEFS_H header guard as
glibc's, that effectively replaces <sys/cdefs.h> with gnulib's
<cdefs.h>.

> That is, when a package that uses Gnulib does
>   #include <sys/cdefs.h>
> it will get the <sys/cdefs.h> of the system (from glibc, *BSD, Cygwin,
> etc.).

Apparently not if it includes libc-config.h first.

I think what happened is that the order of backporting

commit 0b5ca7c3e551e5502f3be3b06453324fe8604e82
Author: Paul Eggert <eggert@cs.ucla.edu>
Date:   Tue Sep 21 07:47:45 2021 -0700

    regex: copy back from Gnulib

(which brought in __attribute_nonnull__) and

commit a643f60c53876be0d57b4b7373770e6cb356fd13
Author: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date:   Wed Oct 20 18:12:41 2021 +0530

    Make sure that the fortified function conditionals are constant

was reversed on the glibc 2.34 branch, so the version check based on
__attribute_nonnull__ would signal that system <sys/cdefs.h> is too old.
But with the second commit for fortified functions, glibc 2.34 headers
started requiring other macros not present in gnulib's <cdefs.h> copy,
so some projects using copied gnulib sources would start to fail.

I backported the regex sync to glibc 2.34 in November, so this should
now be solved because we now define __attribute_nonnull__ even on the
2.34 branch.  I think only the 2.34 branch had this problem.

I think we should have backported the __attribute_nonnull__-defining
commit to glibc 2.34 earlier, when we noticed problems.  Updating the
gnulib-bundled copy only (which is what happened at first) wasn't the
best resolution.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-02-27 14:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-21 10:14 Updating <sys/cdefs.h> in glibc and gnulib Florian Weimer
2023-02-26 19:02 ` Bruno Haible
2023-02-27 14:52   ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).