Thread (37 messages) 37 messages, 10 authors, 2018-03-22

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2018-03-18 23:36:13
Also in: linux-input, lkml

On Sun, Mar 18, 2018 at 3:59 PM, Rasmus Villemoes
[off-list ref] wrote:
OK, I missed where this was made about side effects of x and y
We never made it explicit, since all we really cared about in the end
is the constantness.

But yes:
but I suppose the idea was to use

  no_side_effects(x) && no_side_effects(y) ? trivial_max(x, y) :
old_max(x, y)
Exactly. Except with __builtin_choose_expr(), because we need the end
result to be seen as a integer constant expression, so that we can
then use it as an array size. So it needs that early parse-time
resolution.
quoted
We only really use __builtin_constant_p() as a (bad) approximation of
that in this case, since it's the best we can do.
I don't think you should parenthesize bad, rather capitalize it. ({x++;
0;}) is constant in the eyes of __builtin_constant_p, but not
side-effect free.
Hmm. Yeah, but probably don't much care for the kernel.

For example, we do things like this:

   #define __swab64(x)                             \
        (__builtin_constant_p((__u64)(x)) ?     \
        ___constant_swab64(x) :                 \
        __fswab64(x))

where that "___constant_swab64()" very much uses the same argument
over and over.

And we do that for related reasons - we really want to do the constant
folding at build time for some cases, and this was the only sane way
to do it. Eg code like

        return (addr & htonl(0xff000000)) == htonl(0x7f000000);

wants to do the right thing, and long ago gcc didn't have builtins for
byte swapping, so we had to just do nasty horribly macros that DTRT
for constants.

But since the kernel is standalone, we don't need to really worry
about the *generic* case, we just need to worry about our own macros,
and if somebody does that example you show I guess we'll just have to
shun them ;)

Of course, our own macros are often macros from hell, exactly because
they often contain a lot of type-checking and/or type-(size)-based
polymorphism. But then we actually *want* gcc to simplify things, and
avoid side effects, just have potentially very complex expressions.

But we basically never have that kind of intentionally evil macros, so
we are willing to live with a bad substitute.

But yes, it would be better to have some more control over things, and
actually have a way to say "if X is a constant integer expression, do
transformation Y, otherwise call function y()".

Actually sparse started out with the idea in the background that it
could become not just a checker, but a "transformation tool". Partly
because of long gone historical issues (ie gcc people used to be very
anti-plugin due to licensing issues).

Of course, a more integrated and powerful preprocessor language is
almost always what we *really* wanted, but traditionally "powerful
preprocessor" has always meant "completely incomprehensible and badly
integrated preprocessor".

"cpp" may be a horrid language, but it's there and it's fast (when
integrated with the front-end, like everybody does now)

But sadly, cpp is really bad for the above kind of "if argument is
constant" kind of tricks. I suspect we'd use it a lot otherwise.
quoted
So the real use would be to say "can we use the simple direct macro
that just happens to also fold into a nice integer constant
expression" for __builtin_choose_expr().

I tried to find something like that, but it really doesn't exist, even
though I would actually have expected it to be a somewhat common
concern for macro writers: write a macro that works in any arbitrary
environment.
Yeah, I think the closest is a five year old suggestion
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57612) to add a
__builtin_assert_no_side_effects, that could be used in macros that for
some reason cannot be implemented without evaluating some argument
multiple times. It would be more useful to have the predicate version,
which one could always turn into a build bug version. But we have
neither, unfortunately.
Yeah, and since we're in the situation that *new* gcc versions work
for us anyway, and we only have issues with older gcc's (that sadly
people still use), even if there was a new cool feature we couldn't
use it.

Oh well. Thanks for the background.

            Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help