Messages in this thread | | | From | Linus Torvalds <> | Date | Wed, 29 Jun 2022 14:39:45 -0700 | Subject | Re: mainline build failure due to 281d0c962752 ("fortify: Add Clang support") |
| |
On Wed, Jun 29, 2022 at 2:21 PM Nick Desaulniers <ndesaulniers@google.com> 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
| |