On Mon, May 13, 2024 at 03:12:53PM -0700, Linus Torvalds wrote: > On Sun, 12 May 2024 at 20:50, Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > > > > Lukas Wunner (1): > > X.509: Introduce scope-based x509_certificate allocation [...] > Having random kernel code add random "assume()" lines is absolutely > not what we should do. Particularly not in some random code sequence > where it absolutely does not matter ONE WHIT. > > Now, I've pulled this, but I killed that "assume()" hackery in my merge. Thanks, appreciated. This way of handling it spares me from having to resubmit the patch without assume(). (The patch is prep work for upcoming PCI device authentication.) > > However, this patch still has two outstanding build defects which > > have not been addressed: > > > > https://lore.kernel.org/all/202404240904.Qi3nM37B-lkp@xxxxxxxxx/ > > This one just seems to be a sanity check for "you shouldn't check > kmalloc() for ERR_PTR", so it's a validation test that then doesn't > like the new test in that 'assume()'. I've been in touch with Julia (+cc) to silence this coccinelle false-positive. But now that the assume() is gone, the coccinelle warning won't appear anyway: https://lore.kernel.org/all/alpine.DEB.2.22.394.2405062136410.3284@hadrien/ > And the second one: > > > https://lore.kernel.org/all/202404252210.KJE6Uw1h-lkp@xxxxxxxxx/ > > looks *very* much like the cases we've seen with clang in particular > where clang goes "this code isn't reachable, so I'll just drop > everything on the floor", and then it just becomes a fallthrough to > whatever else code happens to come next. Most of the time that's just > more (unrelated) code in the same function, but sometimes it causes > that "falls through to next function" instead, entirely randomly > depending on how the code was laid out. Curiously, this particular 0-day report is for gcc 13.2.0 though, not clang. The assume() macro had no effect with clang when I tested it. So the unnecessary IS_ERR() check persisted despite the macro when compiling with clang. Only gcc honors it. Probably another reason why you would hate the macro. :) clang supports __builtin_assume(). In theory that should have the same effect as __builtin_unreachable() on gcc (albeit with inverse boolean semantics). In practice it had no effect. (Tested with clang 15.0.6.) https://clang.llvm.org/docs/LanguageExtensions.html#builtin-assume So with clang there doesn't seem to be a working way to tell the compiler about assumptions it can make. And with gcc it's apparently "hit and miss" depending on the exact gcc version and code. :( > I suspect that because I removed the whole 'assume()' hackery, neither > of the above issues will now happen, and the code nwo works. Yes. I guess this effort was over the top, so apologies for the noise! Thanks, Lukas
