Re: git: 67d1a1cd9e77 - main - cdefs.h: Remove support for pre gcc 4.0

Hey John,

On Tue, Jul 2, 2024 at 9:44 AM John Baldwin <jhb@freebsd.org> wrote:

> On 7/1/24 4:09 PM, Warner Losh wrote:
> > On Mon, Jul 1, 2024, 3:53 PM John Baldwin <jhb@freebsd.org> wrote:
> >
> >> On 6/20/24 7:41 PM, Warner Losh wrote:
> >>> The branch main has been updated by imp:
> >>>
> >>> URL:
> >>
> https://cgit.FreeBSD.org/src/commit/?id=67d1a1cd9e772e2ef94003579f4fbc271d38be7d
> >>>
> >>> commit 67d1a1cd9e772e2ef94003579f4fbc271d38be7d
> >>> Author:     Warner Losh <imp@FreeBSD.org>
> >>> AuthorDate: 2024-06-20 23:02:56 +0000
> >>> Commit:     Warner Losh <imp@FreeBSD.org>
> >>> CommitDate: 2024-06-21 02:41:08 +0000
> >>>
> >>>       cdefs.h: Remove support for pre gcc 4.0
> >>>
> >>>       All supported compilers support the gcc 3 attribute extensions.
> >> Remove
> >>>       the #else clauses for this. Also, latter-day pcc compilers also
> >> define
> >>>       __GNUC__, so there's not need to also test for __PCC__.
> >>>
> >>>       Reviewed by:            brooks
> >>>       Differential Revision:  https://reviews.freebsd.org/D45654
> >>>       Sponsored by:           Netflix
> >>> ---
> >>>    sys/sys/cdefs.h | 42 ++++--------------------------------------
> >>>    1 file changed, 4 insertions(+), 38 deletions(-)
> >>>
> >>> diff --git a/sys/sys/cdefs.h b/sys/sys/cdefs.h
> >>> index 88019819eb35..a6ecdca5d8b9 100644
> >>> --- a/sys/sys/cdefs.h
> >>> +++ b/sys/sys/cdefs.h
> >>> @@ -408,15 +389,10 @@
> >>>     * assign pointer x to a local variable, to check that its type is
> >>>     * compatible with member m.
> >>>     */
> >>> -#if __GNUC_PREREQ__(3, 1)
> >>>    #define     __containerof(x, s, m) ({
> >>       \
> >>>        const volatile __typeof(((s *)0)->m) *__x = (x);
> \
> >>>        __DEQUALIFY(s *, (const volatile char *)__x - __offsetof(s,
> m));\
> >>>    })
> >>> -#else
> >>> -#define      __containerof(x, s, m)
> >>        \
> >>> -     __DEQUALIFY(s *, (const volatile char *)(x) - __offsetof(s, m))
> >>> -#endif
> >>>
> >>>    /*
> >>>     * Compiler-dependent macros to declare that functions take
> printf-like
> >>> @@ -434,14 +410,8 @@
> >>>    #define     __strftimelike(fmtarg, firstvararg) \
> >>>            __attribute__((__format__ (__strftime__, fmtarg,
> firstvararg)))
> >>>
> >>> -/* Compiler-dependent macros that rely on FreeBSD-specific extensions.
> >> */
> >>> -#if defined(__FreeBSD_cc_version) && __FreeBSD_cc_version >= 300001
> && \
> >>> -    defined(__GNUC__)
> >>>    #define     __printf0like(fmtarg, firstvararg) \
> >>>            __attribute__((__format__ (__printf0__, fmtarg,
> firstvararg)))
> >>> -#else
> >>> -#define      __printf0like(fmtarg, firstvararg)
> >>> -#endif
> >>
> >> Does this still work with external GCC?  I didn't think printf0 was
> >> supported
> >> by external GCC (or maybe I had to readd it in the port and that's what
> I
> >> remember).  Ah, yes, printf0 is a local patch in the devel/freebsd-gccX
> >> ports, but is not available in stock GCC (e.g. lang/gcc does not support
> >> it).
> >>
> >
> > Ah. That would explain why it just worked for me. That's what I tested
> > with. Clang also seemed happy with it. But that was the in tree clang. Is
> > there a similar issue? Gnuc is defined for both.
>
> So we don't support building the base system with lang/gcc, only
> devel/freebsd-gccX (which has a local patch to add printf0 support).
> The only question might be, do we support using __printf0like for things
> that aren't in the base system that could be built with lang/gcc.  If so,
> we might need to guard this somehow.  I'm not sure though that we care
> about random software not in base using a FreeBSD-specific keyword from
> <sys/cdefs.h>.
>

Yes. The question is "do we use __printf0like  in our headers" since we
definitely
can't build FreeBSD itself w/o at least some of the extensions for other
things...
and the answer is "yes". err.h uses it, for example, as does setproctitle
in stdlib.h

The interesting thing for me is that gcc13 will produce no warnings if I
include errr.h
because -Wsystem-header is off. With it on, warnings crop up too. This is
why my testing
didn't see it...

It looks like clang has it as a builtin for all versions we care about, so
I'll create a phab to add some
of this back. Maybe we should upstream what we have, for this and
freebsd_printf since that's also
in clang and one of the small number of patches we have for the lang/gcc*
family?

Warner