linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linuxfoundation.org>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Nathan Chancellor <nathan@kernel.org>
Subject: Re: [GIT PULL v2] Kbuild updates for v5.15-rc1
Date: Fri, 3 Sep 2021 15:53:05 -0700	[thread overview]
Message-ID: <CAHk-=wgoX0pVqNMMOcrhq=nuOfoZB_3qihyHB3y1S8qo=MDs6w@mail.gmail.com> (raw)
In-Reply-To: <CAK7LNAQ0Q6CdXaD-dVGj_e3O3JYs_crpejWKpXHYQJYxyk-1VQ@mail.gmail.com>

On Thu, Sep 2, 2021 at 4:31 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> I fixed the warnings observed in the previous PR.

Ok, let's try it again.

>  - Add <linux/stdarg.h> to the kernel source instead of borrowing
>    <stdarg.h> from the compiler.

So I certainly agree with the reasoning, but this worries me a bit.

stdarg is truly intimately an internal compiler file, in ways that
stddef (to pick another example) isn't.

Yeah, yeah, offsetof() is "kind of compiler internal", and we end up
using __compiler_offsetof(), but in the absence of that we *can* just
do it by hand. So offsetof() really is one of those things where we
can just do our own version if some compiler is being difficult.

But va_start and friends absolutely *must* match the exact compiler version.

It does look like both gcc and clang have just standardized on using
__builtin_xyz for all the different stdarg things, and so I approve of
what that <linux/stdarg.h> ended up looking like.

But at the same time, it does make me go "ok, this is a big new
assumption that we've consciously avoided for a long time".

Nick is already on the cc here for other reasons, but let's add the
clang-built list and Nathan explicitly. Because this basically
codifies that

    typedef __builtin_va_list va_list;
    #define va_start(v, l)  __builtin_va_start(v, l)
    #define va_end(v)       __builtin_va_end(v)
    #define va_arg(v, T)    __builtin_va_arg(v, T)
    #define va_copy(d, s)   __builtin_va_copy(d, s)

being the way all the supported compilers work.

Did people talk to any gcc maintainers too? We don't have the same
kind of "gcc kernel people" list or contacts. The above builtins have
been the case for a long long time for gcc, so I don't think it's
wrong or likely to change, but I think it would be a good thing to
just make compiler people aware of how we're now relying on that
explicitly.

(Side note: Linux using the compiler <stdarg.h> goes so far back that
it very much predates all those nice builtins. I still have memories
of <stdarg.h> being a collection of nasty per-architecture messes back
in the bad old days. So I'm actually happy we can do this now, but
there most definitely was a time when we really really had to use the
compiler-provided stdarg.h).

                Linus

  reply	other threads:[~2021-09-03 22:53 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 23:30 [GIT PULL v2] Kbuild updates for v5.15-rc1 Masahiro Yamada
2021-09-03 22:53 ` Linus Torvalds [this message]
2021-09-03 23:04   ` Nathan Chancellor
2021-09-04  8:01     ` Florian Weimer
2021-09-04 13:19       ` Segher Boessenkool
2021-09-04 15:19         ` Florian Weimer
2021-09-04 16:19           ` Segher Boessenkool
2021-09-04 17:22           ` Linus Torvalds
2021-09-04 19:15             ` Segher Boessenkool
2021-09-04 19:58               ` Linus Torvalds
2021-09-06 15:46                 ` Segher Boessenkool
2021-09-06 16:30                   ` Linus Torvalds
2021-09-06 17:27                     ` Segher Boessenkool
2021-09-06 18:11                       ` Linus Torvalds
2021-09-06 18:27                         ` Florian Weimer
2021-09-06 19:48                           ` Segher Boessenkool
2021-09-06 20:14                             ` Jakub Jelinek
2021-09-06 21:08                               ` Linus Torvalds
2021-09-06 21:24                                 ` Linus Torvalds
2021-09-07 14:41                                   ` Segher Boessenkool
2021-09-06 21:52                                 ` Jakub Jelinek
2021-09-06 22:24                                   ` Linus Torvalds
2021-09-07 15:26                                     ` Segher Boessenkool
2021-09-07 14:52                                   ` Segher Boessenkool
2021-09-09  5:14                                     ` Masahiro Yamada
2021-09-07 14:29                                 ` Segher Boessenkool
2021-09-06  6:54             ` Florian Weimer
2021-09-06 16:02               ` Alexey Dobriyan
2021-09-08  3:09                 ` Masahiro Yamada
2021-09-03 23:12 ` pr-tracker-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgoX0pVqNMMOcrhq=nuOfoZB_3qihyHB3y1S8qo=MDs6w@mail.gmail.com' \
    --to=torvalds@linuxfoundation.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).