lkml.org 
[lkml]   [2020]   [Jun]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2] x86_64: fix jiffies ODR violation
    On Tue, Jun 2, 2020 at 9:31 PM 'Bob Haarman' via Clang Built Linux
    <clang-built-linux@googlegroups.com> 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@vger.kernel.org
    > Reported-by: Nathan Chancellor <natechancellor@gmail.com>
    > Reported-by: Alistair Delva <adelva@google.com>
    > Suggested-by: Fangrui Song <maskray@google.com>
    > Debugged-by: Nick Desaulniers <ndesaulniers@google.com>
    > Debugged-by: Sami Tolvanen <samitolvanen@google.com>
    > Signed-off-by: Bob Haarman <inglorion@google.com>
    > Reviewed-by: Andi Kleen <ak@linux.intel.com>
    > Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
    > ---
    > 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@gmail.com> # 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@googlegroups.com.
    > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200602193100.229287-1-inglorion%40google.com.

    \
     
     \ /
      Last update: 2020-06-04 09:31    [W:5.606 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site