Messages in this thread | | | Date | Tue, 17 Mar 2020 12:46:05 +0100 | From | Jakub Jelinek <> | Subject | Re: [PATCH] x86: fix early boot crash on gcc-10 |
| |
On Mon, Mar 16, 2020 at 10:12:51PM +0000, Sergei Trofimovich wrote: > In case you are still interested in preprocessed files and results I've collected > all the bits in a single tarball: > https://dev.gentoo.org/~slyfox/bugs/linux-gcc-10-boot-2020-03-14.tar.gz > Same available in separate files in: > https://dev.gentoo.org/~slyfox/bugs/linux-gcc-10-boot-2020-03-14/
Thanks, that is helpful. So, a few comments.
One thing I've noticed in the command line is that --param=allow-store-data-races=0 got dropped. That is fine, the parameter is gone, but it has been replaced in https://gcc.gnu.org/PR92046 by the -fno-allow-store-data-races option. Like the param which defaulted to 0 and has been enabled only with -Ofast this option is also -fno-allow-store-data-races by default unless -Ofast, but if kernel wanted to be explicit or make sure not to introduce them even with -Ofast, I'd say it should: # Tell gcc to never replace conditional load with a non-conditional one KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0) +KBUILD_CFLAGS += $(call cc-option,-fno-allow-store-data-races) in the toplevel Makefile.
The actual change that causes cpu_startup_entry to be tail called in GCC 10 was my https://gcc.gnu.org/PR59813 https://gcc.gnu.org/PR89060 https://gcc.gnu.org/r10-230-gab87ac8d53f3d903bfc9eeb0f0b5e7eed1f38cbc optimization. Previously, GCC punted just because it saw some earlier call which passed address of some automatic variable to some other function and escaped it that way (and could be possibly used during the function considered for tail call, thus making the tail call not possible as with tail call the frame is gone then). Now, GCC tries to use information about scopes to determine that eventhough some automatic variables can escape that way, if they aren't in the scope anymore during the last call, they shouldn't be problematic. There are two variables that prevented the tail call optimization in the past it seems, int cpuid; in smp_callin function which is inlined, then its address escapes: __dynamic_pr_debug(&__UNIQUE_ID_ddebug114, "smpboot" ": " "Stack at about %p\n", &cpuid); and then cpuid goes out of scope. Similarly, the u64 canary; variable in boot_init_stack_canary which is also inlined. The address escapes in get_random_bytes(&canary, sizeof(canary)); and later on is used and eventually goes out of scope. Finally, there is the cpu_startup_entry call at the end of function.
Regarding the reason why GCC doesn't tailcall noreturn functions, that is a deliberate decision, often that results in shorter code (there is no need to restore stack etc. before the call and because it is noreturn, anything after the call can be thrown away as unreachable) and more importantly, results in more useful backtraces when something calls abort etc.
Hope this helps.
Jakub
| |