On Fri, Jun 13, 2025 at 07:15:56PM +0000, marc.herbert@xxxxxxxxxxxxxxx wrote: > From: Marc Herbert <marc.herbert@xxxxxxxxxxxxxxx> > > Fixes undefined behavior that was spotted by Jonathan Cameron in > https://lore.kernel.org/linux-cxl/20250609170509.00003625@xxxxxxxxxx/ > > The possible consequences of the undefined behavior fixed here are fairly > well documented across the Internet but to save research time and avoid > doubts, I include a very short and simple demo below. I imagine kernel > compilation flags and various other conditions may not make the > consequences as bad as this example, however those conditions could change > and this type of code is still Undefined Behavior no matter what. > One of the best articles - there are many others: > https://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html > > Since commit b5ec6fd286dfa4 ("kbuild: Drop -Wdeclaration-after-statement"), > it's now possible to use C99 declarations; the kernel is not constrained > anymore to group all declarations at the top of a block like single-pass > compilers used to require. This allows combining declarations and > definitions in one place - like literally every other language and project > does - and trivially fix undefined behavior like this. This also reduces > variable scope and avoids misuse between declaration and definition like > uninitialized reads or writing to the wrong variable by mistake. C99 > declarations also allow using a lot more `const` (the default in some > languages) which avoids some misuse after legitimate use. > tl;dr: C99 declarations are not just a "codestyle" or "taste" issue; > they are an important (and not mandatory) feature. > > cc --version > cc (GCC) 15.1.1 20250425 > > for i in 0 1 2 g; do printf "gcc -O$i: "; gcc -O$i nullptrUB.c && > ./a.out; done > > gcc -O0: Segmentation fault (core dumped) > gcc -O1: ptr is zero > gcc -O2: ptr is NOT zero!!! > gcc -O3: ptr is NOT zero!!! > gcc -Og: ptr is zero > > clang --version > clang version 19.1.7 > > clang -O0: Segmentation fault (core dumped) > clang -O1: ptr is NOT zero!!! > clang -O2: ptr is NOT zero!!! > clang -O3: ptr is NOT zero!!! > clang -Og: ptr is NOT zero!!! > > int faux_device_destroy(int *ptr) > { > int i = *ptr; i++; > > // Because we dereferenced ptr, the compiler knows the pointer cannot > // be null (even when it is!) and can optimize this away. > if (!ptr) { > printf("ptr is zero\n"); > return 0; > } > > printf("ptr is NOT zero!!!\n"); > return 1; > } > > int main() > { > struct timespec t1, t2; > clock_gettime(CLOCK_MONOTONIC, &t1); > clock_gettime(CLOCK_MONOTONIC, &t2); > > // Use the clock to hide zero from the compiler > int * zeroptr = (int *)(t2.tv_sec - t1.tv_sec); > > return faux_device_destroy(zeroptr); > } > > Fixes: 35fa2d88ca94 ("driver core: add a faux bus for use when a simple device/bus is needed") > Signed-off-by: Marc Herbert <marc.herbert@xxxxxxxxxxxxxxx> Great writeup, but as Miguel says, this isn't needed at all, the kernel relies on the compiler to be sane :) thanks, greg k-h
