On Tue, Jun 2, 2020 at 9:31 PM 'Bob Haarman' via Clang Built Linux <clang-built-linux@xxxxxxxxxxxxxxxx> wrote: > > 'jiffies' and 'jiffies_64' are meant to alias (two different symbols > that share the same address). Most architectures make the symbols alias > to the same address via linker script assignment in their > arch/<arch>/kernel/vmlinux.lds.S: > > jiffies = jiffies_64; > > which is effectively a definition of jiffies. > > jiffies and jiffies_64 are both forward declared for all arch's in: > include/linux/jiffies.h. > > jiffies_64 is defined in kernel/time/timer.c for all arch's. > > x86_64 was peculiar in that it wasn't doing the above linker script > assignment, but rather was: > 1. defining jiffies in arch/x86/kernel/time.c instead via linker script. > 2. overriding the symbol jiffies_64 from kernel/time/timer.c in > arch/x86/kernel/vmlinux.lds.s via 'jiffies_64 = jiffies;'. > > As Fangrui notes: > > In LLD, symbol assignments in linker scripts override definitions in > object files. GNU ld appears to have the same behavior. It would > probably make sense for LLD to error "duplicate symbol" but GNU ld > is unlikely to adopt for compatibility reasons. > > So we have an ODR violation (UB), which we seem to have gotten away > with thus far. Where it becomes harmful is when we: > > 1. Use -fno-semantic-interposition. > > As Fangrui notes: > > Clang after LLVM commit 5b22bcc2b70d > ("[X86][ELF] Prefer to lower MC_GlobalAddress operands to .Lfoo$local") > defaults to -fno-semantic-interposition similar semantics which help > -fpic/-fPIC code avoid GOT/PLT when the referenced symbol is defined > within the same translation unit. Unlike GCC > -fno-semantic-interposition, Clang emits such relocations referencing > local symbols for non-pic code as well. > > This causes references to jiffies to refer to '.Ljiffies$local' when > jiffies is defined in the same translation unit. Likewise, references > to jiffies_64 become references to '.Ljiffies_64$local' in translation > units that define jiffies_64. Because these differ from the names > used in the linker script, they will not be rewritten to alias one > another. > > Combined with ... > > 2. Full LTO effectively treats all source files as one translation > unit, causing these local references to be produced everywhere. When > the linker processes the linker script, there are no longer any > references to jiffies_64' anywhere to replace with 'jiffies'. And > thus '.Ljiffies$local' and '.Ljiffies_64$local' no longer alias > at all. > > In the process of porting patches enabling Full LTO from arm64 to > x86_64, we observe spooky bugs where the kernel appeared to boot, but > init doesn't get scheduled. > > Instead, we can avoid the ODR violation by matching other arch's by > defining jiffies only by linker script. For -fno-semantic-interposition > + Full LTO, there is no longer a global definition of jiffies for the > compiler to produce a local symbol which the linker script won't ensure > aliases to jiffies_64. > > Link: https://github.com/ClangBuiltLinux/linux/issues/852 > Fixes: 40747ffa5aa8 ("asmlinkage: Make jiffies visible") > Cc: stable@xxxxxxxxxxxxxxx > Reported-by: Nathan Chancellor <natechancellor@xxxxxxxxx> > Reported-by: Alistair Delva <adelva@xxxxxxxxxx> > Suggested-by: Fangrui Song <maskray@xxxxxxxxxx> > Debugged-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > Debugged-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx> > Signed-off-by: Bob Haarman <inglorion@xxxxxxxxxx> > Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx> > Reviewed-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx> > --- > v2: > * Changed commit message as requested by Josh Poimboeuf > (no code change) > Hi, I have tested v2 with my local patch-series together. Feel free to add my... Tested-by: Sedat Dilek <sedat.dilek@xxxxxxxxx> # build+boot on Debian/testing AMD64 with selfmade llvm-toolchain v10.0.1-rc1+ Thanks. Regards, - Sedat - > --- > arch/x86/kernel/time.c | 4 ---- > arch/x86/kernel/vmlinux.lds.S | 4 ++-- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c > index 371a6b348e44..e42faa792c07 100644 > --- a/arch/x86/kernel/time.c > +++ b/arch/x86/kernel/time.c > @@ -25,10 +25,6 @@ > #include <asm/hpet.h> > #include <asm/time.h> > > -#ifdef CONFIG_X86_64 > -__visible volatile unsigned long jiffies __cacheline_aligned_in_smp = INITIAL_JIFFIES; > -#endif > - > unsigned long profile_pc(struct pt_regs *regs) > { > unsigned long pc = instruction_pointer(regs); > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S > index 1bf7e312361f..7c35556c7827 100644 > --- a/arch/x86/kernel/vmlinux.lds.S > +++ b/arch/x86/kernel/vmlinux.lds.S > @@ -40,13 +40,13 @@ OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT) > #ifdef CONFIG_X86_32 > OUTPUT_ARCH(i386) > ENTRY(phys_startup_32) > -jiffies = jiffies_64; > #else > OUTPUT_ARCH(i386:x86-64) > ENTRY(phys_startup_64) > -jiffies_64 = jiffies; > #endif > > +jiffies = jiffies_64; > + > #if defined(CONFIG_X86_64) > /* > * On 64-bit, align RODATA to 2MB so we retain large page mappings for > -- > 2.27.0.rc2.251.g90737beb825-goog > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200602193100.229287-1-inglorion%40google.com.