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

On Tue, Jul 2, 2024 at 10:30 AM Warner Losh <imp@bsdimp.com> wrote:

> 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?
>

 https://reviews.freebsd.org/D45836

Warner