Re: [PATCH 00/22] x86, objtool: several fixes/improvements

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnd Bergmann <arnd@arndb.de>, Jann Horn <jannh@google.com>,
	Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH 00/22] x86, objtool: several fixes/improvements
Date: Tue, 16 Jul 2019 18:17:18 -0500	[thread overview]
Message-ID: <20190716231718.flutou25wemgsfju@treble> (raw)
In-Reply-To: <CAKwvOdnXt=_NVjK7+RjuxeyESytO6ra769i4qjSwt1Gd1G22dA@mail.gmail.com>

On Mon, Jul 15, 2019 at 02:45:39PM -0700, Nick Desaulniers wrote:
> For a defconfig, that's the only issue I see.
> (Note that I just landed https://reviews.llvm.org/rL366130 for fixing
> up bugs from loop unrolling loops containing asm goto with Clang, so
> anyone else testing w/ clang will see fewer objtool warnings with that
> patch applied.  A follow up is being worked on in
> https://reviews.llvm.org/D64101).
> 
> For allmodconfig:
> arch/x86/ia32/ia32_signal.o: warning: objtool:
> ia32_setup_rt_frame()+0x247: call to memset() with UACCESS enabled
> mm/kasan/common.o: warning: objtool: kasan_report()+0x52: call to
> __stack_chk_fail() with UACCESS enabled
> arch/x86/kernel/signal.o: warning: objtool:
> x32_setup_rt_frame()+0x255: call to memset() with UACCESS enabled
> arch/x86/kernel/signal.o: warning: objtool: __setup_rt_frame()+0x254:
> call to memset() with UACCESS enabled
> drivers/ata/sata_dwc_460ex.o: warning: objtool:
> sata_dwc_bmdma_start_by_tag()+0x3a0: can't find switch jump table
> lib/ubsan.o: warning: objtool: __ubsan_handle_type_mismatch()+0x88:
> call to memset() with UACCESS enabled
> lib/ubsan.o: warning: objtool: ubsan_type_mismatch_common()+0x610:
> call to __stack_chk_fail() with UACCESS enabled
> lib/ubsan.o: warning: objtool: __ubsan_handle_type_mismatch_v1()+0x88:
> call to memset() with UACCESS enabled
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool:
> .altinstr_replacement+0x56: redundant UACCESS disable
> 
> Without your series, I see them anyways, so I don't consider them
> regressions added by this series.  Let's follow up on these maybe in a
> new thread?  (Shall I send you these object files?)

Yes, maybe open a new thread and be sure to copy PeterZ.  He loves those
warnings ;-)  Object files are definitely needed.

> So for the series:
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>

Thanks!

> 
> > >
> > >    I haven't dug into it yet.
> > >
> > > 2) There's also an issue in clang where a large switch table had a bunch
> > >    of unused (bad) entries.  It's not a code correctness issue, but
> > >    hopefully it can get fixed in clang anyway.  See patch 20/22 for more
> > >    details.
> 
> Thanks for the report, let's follow up on steps for me to reproduce.

Just to clarify, there are two clang issues.  Both of them were reported
originally by Arnd, IIRC.

1) The one described above and in patch 20, where the switch table is
   mostly unused entries.  Not a real bug, but it's a bit sloppy and
   wasteful, and objtool doesn't know how to interpret it.

2) The bug with the noreturn call site having a different stack size
   depending on which code path was taken.

> > > These patches are also at:
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git objtool-many-fixes
> 
> Are these the same patches? Some of the commit messages look different, like:
> https://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git/commit/?h=objtool-many-fixes&id=3e39561c52c4f0062207d604c972148b7b60c341

Oops, those extra 3 commits weren't supposed to be there.  That's future
work.  I dropped them from the branch.

-- 
Josh