Re: mainline build failure due to 281d0c962752 ("fortify: Add Clang support")

[Linus Torvalds]

On Wed, Jun 29, 2022 at 2:21 PM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> For hahas I tried:
>
> ```
> diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c

Yeah, no.

I think the proper fix is to just say

     depends on !CLANG

in the UBSAN Kconfig.

The whole point of UBSAN is to *detect* undefined behavior, but continue.

If we wanted to crash on undefined behavior, we'd just leave it alone,
without any compiler option.

Think of it this way: you have a user (corporate-speak: "customer")
that is seeing behavior you can't figure out. You enable a lot of
these debug options to see "are they triggering something I can't
trigger?".

But you want to get the *report* about that, you don't want to
actually break the users load.  You want the user to be able to do
what they always did, knowing that at least you're not making their
problem any worse. Sure, it's slower, but it should just continue as
well as it ever did.

Whatever UBSAN then detects may OR MAY NOT be the actual cause of the
user report. So no, it's not ok to say "Ahh, I found it, I will now
print out the report and crash".

So at no point is it ok to say "oh, that's undefined, so we can do
anything we want". NOT AT ALL. That's actually true in general, but
it's _particularly_ true when you build with UBSAN, since now you're
actively trying to get reports about undefined behavior, and crashing
will destroy that.

It will destroy that particularly for a kernel where crashing in an
unusual place that the developer doesn't see quite possibly means "odd
device driver". In this case it would be the particular frame buffer
the user uses - crashing there probably means that there is no output
to help debug things.

But this is actually true even for non-kernel uses: if you are running
an app as a developer, and you're looking for problems, having one
place with undefined behavior in no way means that you don't care
about any other undefined places. So crashing is literally *never* the
right thing to do.

So no, the whole "it's still undefined, so we can't return" argument
is completely broken.

                Linus