netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kees Cook <keescook@chromium.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	"Tobin C. Harding" <me@tobin.cc>,
	Steven Rostedt <rostedt@goodmis.org>,
	Jonathan Corbet <corbet@lwn.net>, Chris Mason <clm@fb.com>,
	Josef Bacik <jbacik@fb.com>, David Sterba <dsterba@suse.com>,
	"David S. Miller" <davem@davemloft.net>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Masahiro Yamada <yamada.masahiro@socio
Subject: Re: [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max()
Date: Sat, 10 Mar 2018 20:08:55 +0100	[thread overview]
Message-ID: <CANiq72mTC6oo5WnomzOOh31FO_a9hiUebPmensLOcDAfi0sQ-A@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFw+guiiJ3OavrP-RpFV9NRAF4t=oj3-GDfSyuZ9zBRfdg@mail.gmail.com>

On Sat, Mar 10, 2018 at 6:51 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So in *historical* context - when a compiler didn't do variable length
> arrays at all - the original semantics of C "constant expressions"
> actually make a ton of sense.
>
> You can basically think of a constant expression as something that can
> be (historically) handled by the front-end without any real
> complexity, and no optimization phases - just evaluating a simple
> parse tree with purely compile-time constants.
>
> So there's a good and perfectly valid reason for why C limits certain
> expressions to just a very particular subset. It's not just array
> sizes, it's  case statements etc too. And those are very much part of
> the C standard.
>
> So an error message like
>
>    warning: ISO C90 requires array sizes to be constant-expressions
>
> would be technically correct and useful from a portability angle. It
> tells you when you're doing something non-portable, and should be
> automatically enabled with "-ansi -pedantic", for example.
>
> So what's misleading is actually the name of the warning and the
> message, not that it happens. The warning isn't about "variable
> length", it's literally about the rules for what a
> "constant-expression" is.
>
> And in C, the expression (2,3) has a constant _value_ (namely 3), but
> it isn't a constant-expression as specified by the language.
>
> Now, the thing is that once you actually do variable length arrays,
> those old front-end rules make no sense any more (outside of the "give
> portability warnings" thing).
>
> Because once you do variable length arrays, you obviously _parse_
> everything just fine, and you're doing to evaluate much more complex
> expressions than some limited constant-expression rule.
>
> And at that point it would make a whole lot more sense to add a *new*
> warning that basically says "I have to generate code for a
> variable-sized array", if you actually talk about VLA's.
>
> But that's clearly not what gcc actually did.
>
> So the problem really is that -Wvla doesn't actually warn about VLA's,
> but about something technically completely different.
>

I *think* I followed your reasoning. For gcc, -Wvla is the "I have to
generate code for a variable-sized array" one; but in this case, the
array size is the actual issue that you would have liked to be warned
about; since people writing:

    int a[(2,3)];

did not really mean to declare a VLA. Therefore, you say warning them
about the "warning: ISO C90 requires array sizes to be
constant-expressions" (let's call it -Wpedantic-array-sizes) would be
more helpful here instead of saying stuff about VLAs.

In my case, I was just expecting gcc to give us both warnings and
that's it, instead of trying to be smart and give only the
-Wpedantic-array-sizes one (which is the one I was wondering in my
previous email about why it was missing). I think it would be clear
enough if both warnings are shown are the same time. And it makes
sense, since if you write that line in ISO C90 it means there really
are 2 things going wrong in the end (fishy syntax while in ISO C90
mode and, due to that, VLA code generated as well), no?

Thanks for taking the time to write about the historical context, by the way!

Miguel

> And that's why those stupid syntactic issues with min/max matter. It's
> not whether the end result is a compile-time constant or not, it's
> about completely different issues, like whether there is a
> comma-expression in it.
>
>               Linus

  reply	other threads:[~2018-03-10 19:08 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-09 20:05 [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max() Kees Cook
2018-03-09 21:10 ` Linus Torvalds
2018-03-09 21:47   ` Kees Cook
2018-03-11 22:46   ` Tobin C. Harding
2018-03-13 13:31   ` David Laight
2018-03-10  0:07 ` Andrew Morton
2018-03-10  0:28   ` Linus Torvalds
2018-03-10  0:32     ` Andrew Morton
2018-03-10  0:38       ` Linus Torvalds
2018-03-10  1:30         ` Kees Cook
2018-03-10  1:31           ` Kees Cook
2018-03-10  2:37             ` Linus Torvalds
     [not found]             ` <20180310023907.798690563@goodmis.org>
2018-03-10  3:10               ` [PATCH 3/3] tracing: Rewrite filter logic to be simpler and faster Steven Rostedt
2018-03-10  3:15                 ` Steven Rostedt
2018-03-10  3:22                   ` Steven Rostedt
2018-03-12 22:55           ` [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max() Andrew Morton
2018-03-12 23:57             ` Linus Torvalds
2018-03-13  4:28               ` Kees Cook
2018-03-13 21:02                 ` Andrew Morton
2018-03-13 22:14                   ` Kees Cook
2018-03-14 11:35                     ` David Laight
2018-03-10  3:11   ` Randy Dunlap
2018-03-10  6:10     ` Miguel Ojeda
2018-03-10  7:03       ` Miguel Ojeda
2018-03-10 16:04         ` Linus Torvalds
2018-03-10 15:33       ` Kees Cook
2018-03-10 16:11         ` Linus Torvalds
2018-03-10 16:30         ` Linus Torvalds
2018-03-10 17:34           ` Miguel Ojeda
2018-03-10 17:51             ` Linus Torvalds
2018-03-10 19:08               ` Miguel Ojeda [this message]
2018-03-11 11:05               ` Ingo Molnar
2018-03-11 18:23                 ` Linus Torvalds

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=CANiq72mTC6oo5WnomzOOh31FO_a9hiUebPmensLOcDAfi0sQ-A@mail.gmail.com \
    --to=miguel.ojeda.sandonis@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=clm@fb.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=dsterba@suse.com \
    --cc=gustavo@embeddedor.com \
    --cc=jbacik@fb.com \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=me@tobin.cc \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=yamada.masahiro@socio \
    --cc=yoshfuji@linux-ipv6.org \
    /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).