x86 - clang / objtool status

7 views
Skip to first unread message

Thomas Gleixner

unread,
Jul 18, 2019, 4:40:14 PM7/18/19
to LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Josh Poimboeuf, x...@kernel.org
Folks,

after picking up Josh's objtool updates I gave clang a test ride again.

clan is built with https://github.com/ClangBuiltLinux/tc-build.git

That's using the llvm master branch. top commit is:

0c99d19470b ("[OPENMP]Fix sharing of threadprivate variables with TLS support.")

I merged

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core/urgent

into the tip of Linus tree to pick up the latest objtool changes.

Here are the results for different configs:

1) defconfig

objtool warnings:

drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x88: redundant UACCESS disable


2) debian distro config

objtool warnings:

drivers/gpu/drm/amd/amdgpu/atom.o: warning: objtool: atom_op_move() falls through to next function atom_op_and()
drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes() falls through to next function apply_tx_lanes()
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x86: redundant UACCESS disable
drivers/gpu/drm/radeon/evergreen_cs.o: warning: objtool: evergreen_cs_parse() falls through to next function evergreen_dma_cs_parse()

Build fails with:

clang-10: error: unknown argument: '-mpreferred-stack-boundary=4'
make[5]: *** [linux/scripts/Makefile.build:279: drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_resource.o] Error 1


3) allmodconfig:

objtool warnings:

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
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
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

Build fails with:

ERROR: "__compiletime_assert_2801" [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined!
ERROR: "__compiletime_assert_2446" [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined!
ERROR: "__compiletime_assert_2452" [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined!
ERROR: "__compiletime_assert_2790" [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined!

This also emits a boatload of warnings like this:

linux/fs/nfs/dir.c:451:34: warning: variable 'wq' is uninitialized when used within its own initialization
[-Wuninitialized]
DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
^~
linux/include/linux/wait.h:74:63: note: expanded from macro 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
~~~~ ^~~~
linux/include/linux/wait.h:72:33: note: expanded from macro '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
({ init_waitqueue_head(&name); name; })

Thanks,

tglx

Nathan Chancellor

unread,
Jul 18, 2019, 4:58:43 PM7/18/19
to Thomas Gleixner, LKML, Nick Desaulniers, clang-built-linux, Josh Poimboeuf, x...@kernel.org
Hi Thomas,

I can't comment on the objtool stuff as it is a bit outside of my area
of expertise (probably going to be my next major learning project) but I
can comment on the other errors.

On Thu, Jul 18, 2019 at 10:40:09PM +0200, Thomas Gleixner wrote:
> Build fails with:
>
> clang-10: error: unknown argument: '-mpreferred-stack-boundary=4'
> make[5]: *** [linux/scripts/Makefile.build:279: drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_resource.o] Error 1

Arnd sent a patch for this which has been picked up:
https://lore.kernel.org/lkml/CADnq5_Mm=Fj4AkFtuo+W_295q8r6DY3Sumo7gTG-McUYY=Ce...@mail.gmail.com/

> 3) allmodconfig:
> Build fails with:
>
> ERROR: "__compiletime_assert_2801" [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined!
> ERROR: "__compiletime_assert_2446" [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined!
> ERROR: "__compiletime_assert_2452" [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined!
> ERROR: "__compiletime_assert_2790" [drivers/net/wireless/intel/iwlwifi/iwlwifi.ko] undefined!

Being tracked here:
https://github.com/ClangBuiltLinux/linux/issues/580

It is a clang bug but has a kernel side fix. Nick sent one but it sounds
like Intel has another one pending:

https://lore.kernel.org/lkml/20190712001708.170...@google.com/

https://lore.kernel.org/lkml/da053a97d771eff0ad8db37...@coelho.fi/

> This also emits a boatload of warnings like this:
>
> linux/fs/nfs/dir.c:451:34: warning: variable 'wq' is uninitialized when used within its own initialization
> [-Wuninitialized]
> DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
> ^~
> linux/include/linux/wait.h:74:63: note: expanded from macro 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
> struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
> ~~~~ ^~~~
> linux/include/linux/wait.h:72:33: note: expanded from macro '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
> ({ init_waitqueue_head(&name); name; })

Being tracked here:
https://github.com/ClangBuiltLinux/linux/issues/499

Has a kernel workaround patch posted but it should be fixed in clang:

https://lore.kernel.org/lkml/20190703081119...@arndb.de/

https://bugs.llvm.org/show_bug.cgi?id=42604

Thanks for continuing to test it and keeping us posted on the issues!
Nathan

Nick Desaulniers

unread,
Jul 18, 2019, 6:42:35 PM7/18/19
to Thomas Gleixner, LKML, Nathan Chancellor, clang-built-linux, Josh Poimboeuf, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Peter Zijlstra
On Thu, Jul 18, 2019 at 1:40 PM Thomas Gleixner <tg...@linutronix.de> wrote:
> after picking up Josh's objtool updates I gave clang a test ride again.

Thanks for testing and the reports; these are valuable and we
appreciate the help debugging them.

> 2) debian distro config

Is this checked into the tree, or where can I find it?

>
> objtool warnings:
>
> drivers/gpu/drm/amd/amdgpu/atom.o: warning: objtool: atom_op_move() falls through to next function atom_op_and()
> drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes() falls through to next function apply_tx_lanes()
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x86: redundant UACCESS disable
> drivers/gpu/drm/radeon/evergreen_cs.o: warning: objtool: evergreen_cs_parse() falls through to next function evergreen_dma_cs_parse()

fall through warnings look new to me, but Linaro's KernelCI is
currently screaming with tons of reports of -Wfallthrough throughout
the kernel. I assume they're related?

> 3) allmodconfig:
>
> objtool warnings:
>
> 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
> 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
> 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

Do you still have these object files laying around? Josh asked for
them in a new thread (from the previous thread), not sure if it's ok
to attach object files to emails to LKML? (html email is not allowed,
are binary attachments?)
--
Thanks,
~Nick Desaulniers

Thomas Gleixner

unread,
Jul 19, 2019, 2:39:05 AM7/19/19
to Nathan Chancellor, LKML, Nick Desaulniers, clang-built-linux, Josh Poimboeuf, x...@kernel.org, Arnd Bergmann
On Thu, 18 Jul 2019, Nathan Chancellor wrote:

> Hi Thomas,
>
> I can't comment on the objtool stuff as it is a bit outside of my area
> of expertise (probably going to be my next major learning project) but I
> can comment on the other errors.
>
> On Thu, Jul 18, 2019 at 10:40:09PM +0200, Thomas Gleixner wrote:
> > Build fails with:
> >
> > clang-10: error: unknown argument: '-mpreferred-stack-boundary=4'
> > make[5]: *** [linux/scripts/Makefile.build:279: drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_resource.o] Error 1
>
> Arnd sent a patch for this which has been picked up:
> https://lore.kernel.org/lkml/CADnq5_Mm=Fj4AkFtuo+W_295q8r6DY3Sumo7gTG-McUYY=Ce...@mail.gmail.com/

Which I applied and now I get:

ERROR: "__fixdfsi" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__eqdf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__truncdfsf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__nedf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__floatsidf" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdf3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__adddf3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__ledf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__subdf3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__muldf3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__fixunsdfsi" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__floatunsidf" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__extendsfdf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__gedf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__ltdf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__gtdf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__floatdidf" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
make[2]: *** [/home/tglx/work/kernel/linus/linux/scripts/Makefile.modpost:91: __modpost] Error 1

Thomas Gleixner

unread,
Jul 19, 2019, 2:44:27 AM7/19/19
to Nick Desaulniers, LKML, Nathan Chancellor, clang-built-linux, Josh Poimboeuf, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Peter Zijlstra
On Thu, 18 Jul 2019, Nick Desaulniers wrote:
> On Thu, Jul 18, 2019 at 1:40 PM Thomas Gleixner <tg...@linutronix.de> wrote:
> > after picking up Josh's objtool updates I gave clang a test ride again.
>
> Thanks for testing and the reports; these are valuable and we
> appreciate the help debugging them.
>
> > 2) debian distro config
>
> Is this checked into the tree, or where can I find it?

See below.

> > drivers/gpu/drm/radeon/evergreen_cs.o: warning: objtool: evergreen_cs_parse() falls through to next function evergreen_dma_cs_parse()
>
> fall through warnings look new to me, but Linaro's KernelCI is
> currently screaming with tons of reports of -Wfallthrough throughout
> the kernel. I assume they're related?

I don't think so. The compiler does not warn about a missing fallthrough.

> > 3) allmodconfig:
> >
> > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x56: redundant UACCESS disable
>
> Do you still have these object files laying around? Josh asked for
> them in a new thread (from the previous thread), not sure if it's ok
> to attach object files to emails to LKML? (html email is not allowed,
> are binary attachments?)

I've uploaded the configs and object files to:

https://tglx.de/~tglx/clang.tar.bz2

contains:

clang/
clang/debian/
clang/debian/atom.o
clang/debian/platform.o
clang/debian/i915_gem_execbuffer.o
clang/debian/.config
clang/debian/evergreen_cs.o
clang/allmod/
clang/allmod/ubsan.o
clang/allmod/i915_gem_execbuffer.o
clang/allmod/.config
clang/allmod/sata_dwc_460ex.o
clang/allmod/common.o
clang/allmod/signal.o
clang/allmod/ia32_signal.o

i.e. the .config and the offending object files for the debian and
allmodconfig builds.

Thanks,

tglx


Arnd Bergmann

unread,
Jul 19, 2019, 3:00:39 AM7/19/19
to Thomas Gleixner, Nathan Chancellor, LKML, Nick Desaulniers, clang-built-linux, Josh Poimboeuf, the arch/x86 maintainers
On Fri, Jul 19, 2019 at 8:39 AM Thomas Gleixner <tg...@linutronix.de> wrote:
>
> On Thu, 18 Jul 2019, Nathan Chancellor wrote:
>
> > Hi Thomas,
> >
> > I can't comment on the objtool stuff as it is a bit outside of my area
> > of expertise (probably going to be my next major learning project) but I
> > can comment on the other errors.
> >
> > On Thu, Jul 18, 2019 at 10:40:09PM +0200, Thomas Gleixner wrote:
> > > Build fails with:
> > >
> > > clang-10: error: unknown argument: '-mpreferred-stack-boundary=4'
> > > make[5]: *** [linux/scripts/Makefile.build:279: drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_resource.o] Error 1
> >
> > Arnd sent a patch for this which has been picked up:
> > https://lore.kernel.org/lkml/CADnq5_Mm=Fj4AkFtuo+W_295q8r6DY3Sumo7gTG-McUYY=Ce...@mail.gmail.com/
>
> Which I applied and now I get:
>
> ERROR: "__fixdfsi" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
> ERROR: "__eqdf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
> ERROR: "__truncdfsf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
> ERROR: "__nedf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!

I saw that earlier and use this local workaround that I still need to
submit, see
the bottom of this mail.

The amdgpu driver has a rather liberal use of floating point math in the kernel
that has caused other problems in the past as it is not portable to non-x86
architectures and breaks at least KCOV. Ideally we would try to get the
driver owners to rewrite that code to avoid floating point math, but that
does not seem likely.

It is also possible that we just need to pass the correct flags to clang to
make it actually use hardfloat mode.

Arnd

commit 3c12c0c7fceaf492d41e6bfc46f0000198f496df
Author: Arnd Bergmann <ar...@arndb.de>
Date: Thu Jul 11 16:09:18 2019 +0200

drm/amd/display: disable DRM_AMD_DC_DCN1_0 with clang

The DRM_AMD_DC_DCN1_0 code and several other parts of the display
code use x86 floating point math. When compiling with clang instead
of gcc, this causes link errors:

ERROR: "__subdf3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__gedf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__truncdfsf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__muldf3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdf3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__ledf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__fixdfsi" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__floatunsidf" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__adddf3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__extendsfdf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__fixunsdfsi" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__ltdf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__floatsidf" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__gtdf2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!

I don't really see a way to fix this, so disable the DCN when
building with clang instead until someone finds a way to fix it.

Signed-off-by: Arnd Bergmann <ar...@arndb.de>

diff --git a/drivers/gpu/drm/amd/display/Kconfig
b/drivers/gpu/drm/amd/display/Kconfig
index f954bf61af28..2cfbbf3b85dd 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -6,7 +6,7 @@ config DRM_AMD_DC
bool "AMD DC - Enable new display engine"
default y
select SND_HDA_COMPONENT if SND_HDA_CORE
- select DRM_AMD_DC_DCN1_0 if X86 && !(KCOV_INSTRUMENT_ALL &&
KCOV_ENABLE_COMPARISONS)
+ select DRM_AMD_DC_DCN1_0 if X86 && !(KCOV_INSTRUMENT_ALL &&
KCOV_ENABLE_COMPARISONS) && !CC_IS_CLANG
help
Choose this option if you want to use the new display engine
support for AMDGPU. This adds required support for Vega and

Nathan Chancellor

unread,
Jul 19, 2019, 3:03:28 AM7/19/19
to Thomas Gleixner, LKML, Nick Desaulniers, clang-built-linux, Josh Poimboeuf, x...@kernel.org, Arnd Bergmann, Sami Tolvanen
Ah right, allmodconfig...

https://github.com/ClangBuiltLinux/linux/issues/327

https://github.com/samitolvanen/linux/commit/2fd53310b7d7e9234c4018c45aefe056a0784e2b

This needs to find its way upstream again, I am not sure what's holding
it back at this point (probably real world testing), maybe Sami or Nick
will know.

Sedat Dilek

unread,
Jul 19, 2019, 7:37:54 AM7/19/19
to Thomas Gleixner, Josh Poimboeuf, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org
Hi Thomas,

Thanks for the feedback.

The last days I tried several iterations of Josh's "v0", v1 and v2.

First of all I find out that it is possible to download and apply the
series (here: v2) from patchwork (see [1]).
I highly appreciate to have this in Josh's Git [2].

BTW, what is/was the base for the patchset?
I tried next-20190712 as this was the base in Josh's Git.
Later next-20190717 and next-20190718.
Please clarify.

With v2 applied on top of next-20190718 and an adapted/customized
kernel-config based on an official Debian/experimental 5.0.2 kernel I
fell over exactly the same 4 objtool warnings.

drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool:
.altinstr_replacement+0x86: redundant UACCESS disable

drivers/gpu/drm/radeon/atom.o: warning: objtool: atom_op_move() falls
through to next function atom_op_and()
drivers/gpu/drm/radeon/evergreen_cs.o: warning: objtool:
evergreen_cs_parse() falls through to next function
evergreen_dma_cs_parse()
drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes()
falls through to next function apply_tx_lanes()

Kconfigs?
I highly appreciate to turn off AMDGPU if you do not need it.

Yes, I know all the patches mentioned in this thread as reading the
ClangBuiltLinux (CBL) mailing-list and following the CBL issue-tracker
(being a part of the project).
To get/attract more followers all these patches should be placed
somewhere (preferable Git) and/or be easily pickable.
Or at least a list or be tagged in patchwork.
I have no idea for an ideal patch-management in this topic.

I can boot into QEMU.
But on bare metal it looks like BPF and systemd-udevd/systemd-journald
do end in a maintenance mode.
I saved the dmesg-log.

Kernel-config, dmesg-log and systemd-log attached.

I have not tried with Debian/buster GCC v8 and/or dropping Josh's
objtool patchset.
I am building with a pre-release of Clang v9-pre and linking with LLD v9-pre.

As a recommendation:
apt.llvm.org offers binaries for Debian/buster.
I can send you my APT stuff if you like.

Regards,
- Sedat -

[0] https://lore.kernel.org/patchwork/project/lkml/list/?series=402470
[1] https://lore.kernel.org/patchwork/series/402470/mbox/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git/log/?h=objtool-many-fixes
dmesg_5.2.0-4-amd64-cbl-asmgoto.txt
config-5.2.0-4-amd64-cbl-asmgoto
systemctl-failed.txt

Sedat Dilek

unread,
Jul 19, 2019, 9:49:00 AM7/19/19
to Thomas Gleixner, Josh Poimboeuf, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org
> First of all I find out that it is possible to download and apply the
> series (here: v2) from patchwork (see [1]).
> I highly appreciate to have this in Josh's Git [2].
>

There it is.

- sed@ -

[1] https://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git/log/?h=objtool-many-fixes-v2

Sedat Dilek

unread,
Jul 22, 2019, 11:40:26 AM7/22/19
to Thomas Gleixner, Josh Poimboeuf, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org
next-20190722 has them.

Parallely, I opened an CBL issue #617
"drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool:
.altinstr_replacement+0x86: redundant UACCESS disable"

I hope Josh can look at this.

- Sedat -

[1] https://github.com/ClangBuiltLinux/linux/issues/617

Josh Poimboeuf

unread,
Jul 23, 2019, 10:43:29 PM7/23/19
to Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Peter Zijlstra, Linus Torvalds
On Thu, Jul 18, 2019 at 10:40:09PM +0200, Thomas Gleixner wrote:

> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x86: redundant UACCESS disable

Looking at this one, I think I agree with objtool.

PeterZ, Linus, I know y'all discussed this code a few months ago.

__copy_from_user() already does a CLAC in its error path. So isn't the
user_access_end() redundant for the __copy_from_user() error path?

Untested fix:

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 5fae0e50aad0..41dab9ea33cd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1628,6 +1628,7 @@ static int check_relocations(const struct drm_i915_gem_exec_object2 *entry)

static int eb_copy_relocations(const struct i915_execbuffer *eb)
{
+ struct drm_i915_gem_relocation_entry *relocs;
const unsigned int count = eb->buffer_count;
unsigned int i;
int err;
@@ -1635,7 +1636,6 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
for (i = 0; i < count; i++) {
const unsigned int nreloc = eb->exec[i].relocation_count;
struct drm_i915_gem_relocation_entry __user *urelocs;
- struct drm_i915_gem_relocation_entry *relocs;
unsigned long size;
unsigned long copied;

@@ -1663,14 +1663,8 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)

if (__copy_from_user((char *)relocs + copied,
(char __user *)urelocs + copied,
- len)) {
-end_user:
- user_access_end();
-end:
- kvfree(relocs);
- err = -EFAULT;
- goto err;
- }
+ len))
+ goto end;

copied += len;
} while (copied < size);
@@ -1699,10 +1693,14 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)

return 0;

+end_user:
+ user_access_end();
+end:
+ kvfree(relocs);
+ err = -EFAULT;
err:
while (i--) {
- struct drm_i915_gem_relocation_entry *relocs =
- u64_to_ptr(typeof(*relocs), eb->exec[i].relocs_ptr);
+ relocs = u64_to_ptr(typeof(*relocs), eb->exec[i].relocs_ptr);
if (eb->exec[i].relocation_count)
kvfree(relocs);
}

Peter Zijlstra

unread,
Jul 24, 2019, 3:47:42 AM7/24/19
to Josh Poimboeuf, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds
On Tue, Jul 23, 2019 at 09:43:24PM -0500, Josh Poimboeuf wrote:
> On Thu, Jul 18, 2019 at 10:40:09PM +0200, Thomas Gleixner wrote:
>
> > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x86: redundant UACCESS disable
>
> Looking at this one, I think I agree with objtool.
>
> PeterZ, Linus, I know y'all discussed this code a few months ago.
>
> __copy_from_user() already does a CLAC in its error path. So isn't the
> user_access_end() redundant for the __copy_from_user() error path?

Hmm, is this a result of your c705cecc8431 ("objtool: Track original function across branches") ?

I'm thinking it might've 'overlooked' the CLAC in the error path before
(because it didn't have a related function) and now it sees it and
worries about it.

Then again, I'm not seeing this warning on my GCC builds; so what's
happening?

Josh Poimboeuf

unread,
Jul 24, 2019, 8:37:26 AM7/24/19
to Peter Zijlstra, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds
Yeah, I think so.

> Then again, I'm not seeing this warning on my GCC builds; so what's
> happening?

Good question...

--
Josh

Josh Poimboeuf

unread,
Jul 24, 2019, 8:55:30 AM7/24/19
to Peter Zijlstra, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds
On Wed, Jul 24, 2019 at 09:47:32AM +0200, Peter Zijlstra wrote:
According to the github issue[1] my patch doesn't fix the warning with
Clang. So questions remain:

a) what is objtool actually warning about?

b) why doesn't objtool detect the case I found?


I can look at it later, post-meetings.


[1] https://github.com/ClangBuiltLinux/linux/issues/617

--
Josh

Peter Zijlstra

unread,
Jul 24, 2019, 9:35:22 AM7/24/19
to Josh Poimboeuf, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds
On Wed, Jul 24, 2019 at 07:55:25AM -0500, Josh Poimboeuf wrote:
> On Wed, Jul 24, 2019 at 09:47:32AM +0200, Peter Zijlstra wrote:
> > On Tue, Jul 23, 2019 at 09:43:24PM -0500, Josh Poimboeuf wrote:
> > > On Thu, Jul 18, 2019 at 10:40:09PM +0200, Thomas Gleixner wrote:
> > >
> > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x86: redundant UACCESS disable
> > >
> > > Looking at this one, I think I agree with objtool.
> > >
> > > PeterZ, Linus, I know y'all discussed this code a few months ago.
> > >
> > > __copy_from_user() already does a CLAC in its error path. So isn't the
> > > user_access_end() redundant for the __copy_from_user() error path?
> >
> > Hmm, is this a result of your c705cecc8431 ("objtool: Track original function across branches") ?
> >
> > I'm thinking it might've 'overlooked' the CLAC in the error path before
> > (because it didn't have a related function) and now it sees it and
> > worries about it.
> >
> > Then again, I'm not seeing this warning on my GCC builds; so what's
> > happening?
>
> According to the github issue[1] my patch doesn't fix the warning with
> Clang. So questions remain:

I was thinking your patch resulted in the warning due to the exception
code gaining a ->func. But then that doesn't make sense either, because
all that lives in copy_user_64.S which is a completely different
translation unit.

> a) what is objtool actually warning about?

CLAC with AC already clear. Either we do double CLAC at the end, or we
do CLAC without having done STAC first.

The issue isn't BAD(tm), as AC clear is the safe state, but it typically
indicates confused code flow.

> b) why doesn't objtool detect the case I found?

With GCC you mean? Yes, that is really really weird.

Let me go stare at objdump output for this file (which doesn't build
with:

make O=defconfig-build/ drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o
)

Josh Poimboeuf

unread,
Jul 24, 2019, 10:05:43 AM7/24/19
to Peter Zijlstra, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds
On Wed, Jul 24, 2019 at 03:35:16PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 24, 2019 at 07:55:25AM -0500, Josh Poimboeuf wrote:
> > On Wed, Jul 24, 2019 at 09:47:32AM +0200, Peter Zijlstra wrote:
> > > On Tue, Jul 23, 2019 at 09:43:24PM -0500, Josh Poimboeuf wrote:
> > > > On Thu, Jul 18, 2019 at 10:40:09PM +0200, Thomas Gleixner wrote:
> > > >
> > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x86: redundant UACCESS disable
> > > >
> > > > Looking at this one, I think I agree with objtool.
> > > >
> > > > PeterZ, Linus, I know y'all discussed this code a few months ago.
> > > >
> > > > __copy_from_user() already does a CLAC in its error path. So isn't the
> > > > user_access_end() redundant for the __copy_from_user() error path?
> > >
> > > Hmm, is this a result of your c705cecc8431 ("objtool: Track original function across branches") ?
> > >
> > > I'm thinking it might've 'overlooked' the CLAC in the error path before
> > > (because it didn't have a related function) and now it sees it and
> > > worries about it.
> > >
> > > Then again, I'm not seeing this warning on my GCC builds; so what's
> > > happening?
> >
> > According to the github issue[1] my patch doesn't fix the warning with
> > Clang. So questions remain:
>
> I was thinking your patch resulted in the warning due to the exception
> code gaining a ->func.

I had the same thought.

> But then that doesn't make sense either, because all that lives in
> copy_user_64.S which is a completely different translation unit.

Hm? __copy_from_user() uses raw_copy_from_user() to do the STAC/CLAC in
a header file for the __builtin_constant_p() case.

> > a) what is objtool actually warning about?
>
> CLAC with AC already clear. Either we do double CLAC at the end, or we
> do CLAC without having done STAC first.
>
> The issue isn't BAD(tm), as AC clear is the safe state, but it typically
> indicates confused code flow.

But as I said my patch didn't fix the Clang warning. Or is there
another redundant UACCESS disable you know about?

> > b) why doesn't objtool detect the case I found?
>
> With GCC you mean? Yes, that is really really weird.

With both compilers...

> Let me go stare at objdump output for this file (which doesn't build
> with:
>
> make O=defconfig-build/ drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o
> )

--
Josh

Peter Zijlstra

unread,
Jul 24, 2019, 10:10:51 AM7/24/19
to Josh Poimboeuf, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds
0000 0000000000000240 <eb_copy_relocations.isra.34>:
0000 240: 41 57 push %r15
0002 242: 41 56 push %r14
0004 244: 41 55 push %r13
0006 246: 41 54 push %r12
0008 248: 55 push %rbp
0009 249: 53 push %rbx
000a 24a: 48 83 ec 20 sub $0x20,%rsp
000e 24e: 85 f6 test %esi,%esi
0010 250: 74 39 je 28b <eb_copy_relocations.isra.34+0x4b>
0012 252: 89 74 24 14 mov %esi,0x14(%rsp)
0016 256: 45 31 f6 xor %r14d,%r14d
0019 259: 48 c7 04 24 00 00 00 movq $0x0,(%rsp)
0020 260: 00
0021 261: 48 89 7c 24 08 mov %rdi,0x8(%rsp)
0026 266: 48 8b 44 24 08 mov 0x8(%rsp),%rax
002b 26b: 48 8b 34 24 mov (%rsp),%rsi
002f 26f: 48 03 30 add (%rax),%rsi
0032 272: 44 8b 46 04 mov 0x4(%rsi),%r8d
0036 276: 45 85 c0 test %r8d,%r8d
0039 279: 75 23 jne 29e <eb_copy_relocations.isra.34+0x5e>
003b 27b: 41 83 c6 01 add $0x1,%r14d
003f 27f: 48 83 04 24 38 addq $0x38,(%rsp)
0044 284: 44 3b 74 24 14 cmp 0x14(%rsp),%r14d
0049 289: 75 db jne 266 <eb_copy_relocations.isra.34+0x26>
004b 28b: 31 db xor %ebx,%ebx
004d 28d: 48 83 c4 20 add $0x20,%rsp
0051 291: 89 d8 mov %ebx,%eax
0053 293: 5b pop %rbx
0054 294: 5d pop %rbp
0055 295: 41 5c pop %r12
0057 297: 41 5d pop %r13
0059 299: 41 5e pop %r14
005b 29b: 41 5f pop %r15
005d 29d: c3 retq

(<- from +39)

005e 29e: 48 83 c6 08 add $0x8,%rsi
0062 2a2: 44 89 c7 mov %r8d,%edi
0065 2a5: e8 26 ff ff ff callq 1d0 <check_relocations.isra.32>
006a 2aa: 85 c0 test %eax,%eax
006c 2ac: 0f 85 35 01 00 00 jne 3e7 <eb_copy_relocations.isra.34+0x1a7>
0072 2b2: 48 8b 44 24 08 mov 0x8(%rsp),%rax
0077 2b7: 48 8b 0c 24 mov (%rsp),%rcx
007b 2bb: ba ff ff ff ff mov $0xffffffff,%edx
0080 2c0: be c0 0c 00 00 mov $0xcc0,%esi
0085 2c5: 48 8b 00 mov (%rax),%rax
0088 2c8: 4c 8b 6c 08 08 mov 0x8(%rax,%rcx,1),%r13
008d 2cd: 44 89 c0 mov %r8d,%eax
0090 2d0: 49 89 c4 mov %rax,%r12
0093 2d3: 48 89 44 24 18 mov %rax,0x18(%rsp)
0098 2d8: 49 c1 e4 05 shl $0x5,%r12
009c 2dc: 4c 89 e7 mov %r12,%rdi
009f 2df: e8 00 00 00 00 callq 2e4 <eb_copy_relocations.isra.34+0xa4>
00a0 2e0: R_X86_64_PLT32 kvmalloc_node-0x4
00a4 2e4: 49 89 c7 mov %rax,%r15
00a7 2e7: 48 85 c0 test %rax,%rax
00aa 2ea: 0f 84 e8 00 00 00 je 3d8 <eb_copy_relocations.isra.34+0x198>
00b0 2f0: 31 ed xor %ebp,%ebp
00b2 2f2: eb 08 jmp 2fc <eb_copy_relocations.isra.34+0xbc>

(<- from +e0)

00b4 2f4: 48 01 dd add %rbx,%rbp
00b7 2f7: 49 39 ec cmp %rbp,%r12
00ba 2fa: 76 73 jbe 36f <eb_copy_relocations.isra.34+0x12f>

(<- from +b2)

00bc 2fc: 4c 89 e3 mov %r12,%rbx
00bf 2ff: b8 00 00 00 80 mov $0x80000000,%eax
00c4 304: 49 8d 3c 2f lea (%r15,%rbp,1),%rdi
00c8 308: 48 29 eb sub %rbp,%rbx
00cb 30b: 49 8d 74 2d 00 lea 0x0(%r13,%rbp,1),%rsi
00d0 310: 48 39 c3 cmp %rax,%rbx
00d3 313: 48 0f 47 d8 cmova %rax,%rbx
00d7 317: 89 da mov %ebx,%edx
00d9 319: e8 00 00 00 00 callq 31e <eb_copy_relocations.isra.34+0xde>
00da 31a: R_X86_64_PLT32 copy_user_generic_unrolled-0x4
00de 31e: 85 c0 test %eax,%eax
00e0 320: 74 d2 je 2f4 <eb_copy_relocations.isra.34+0xb4>
00e2 322: 4c 89 f8 mov %r15,%rax
00e5 325: 4c 8b 7c 24 08 mov 0x8(%rsp),%r15
00ea 32a: 90 nop
00eb 32b: 90 nop
00ec 32c: 90 nop

^^^ CLAC

And that most certainly should trigger...

Let me gdb that objtool thing.

Peter Zijlstra

unread,
Jul 24, 2019, 12:48:28 PM7/24/19
to Josh Poimboeuf, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds
On Wed, Jul 24, 2019 at 04:10:40PM +0200, Peter Zijlstra wrote:
> And that most certainly should trigger...
>
> Let me gdb that objtool thing.

---
Subject: objtool: Improve UACCESS coverage

A clang build reported an (obvious) double CLAC while a GCC build did
not; it turns out we only re-visit instructions if the first visit was
with AC=0. If OTOH the first visit was with AC=1, we completely ignore
any subsequent visit, even when it has AC=0.

Fix this by using a visited mask, instead of boolean and (explicitly)
mark the AC state.

$ ./objtool check -b --no-fp --retpoline --uaccess ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o
../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x22: redundant UACCESS disable
../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xea: (alt)
../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0xffffffffffffffff: (branch)
../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xd9: (alt)
../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xb2: (branch)
../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0x39: (branch)
../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0x0: <=== (func)

Reported-by: Josh Poimboeuf <jpoi...@redhat.com>
Reported-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
---
tools/objtool/check.c | 7 ++++---
tools/objtool/check.h | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 5f26620f13f5..176f2f084060 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1946,6 +1946,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
struct alternative *alt;
struct instruction *insn, *next_insn;
struct section *sec;
+ u8 visited;
int ret;

insn = first;
@@ -1972,12 +1973,12 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
return 1;
}

+ visited = 1 << state.uaccess;
if (insn->visited) {
if (!insn->hint && !insn_state_match(insn, &state))
return 1;

- /* If we were here with AC=0, but now have AC=1, go again */
- if (insn->state.uaccess || !state.uaccess)
+ if (insn->visited & visited)
return 0;
}

@@ -2024,7 +2025,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
} else
insn->state = state;

- insn->visited = true;
+ insn->visited |= visited;

if (!insn->ignore_alts) {
bool skip_orig = false;
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index b881fafcf55d..6d875ca6fce0 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -33,8 +33,9 @@ struct instruction {
unsigned int len;
enum insn_type type;
unsigned long immediate;
- bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
+ bool alt_group, dead_end, ignore, hint, save, restore, ignore_alts;
bool retpoline_safe;
+ u8 visited;
struct symbol *call_dest;
struct instruction *jump_dest;
struct instruction *first_jump_src;

Peter Zijlstra

unread,
Jul 24, 2019, 12:52:58 PM7/24/19
to Josh Poimboeuf, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds, ch...@chris-wilson.co.uk
On Tue, Jul 23, 2019 at 09:43:24PM -0500, Josh Poimboeuf wrote:
> On Thu, Jul 18, 2019 at 10:40:09PM +0200, Thomas Gleixner wrote:
>
> > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x86: redundant UACCESS disable
>
> Looking at this one, I think I agree with objtool.
>
> PeterZ, Linus, I know y'all discussed this code a few months ago.
>
> __copy_from_user() already does a CLAC in its error path. So isn't the
> user_access_end() redundant for the __copy_from_user() error path?
>
> Untested fix:

Run this past i915 people, but with the objtool patch I just posted it
reproduces with GCC and this patch makes it go away.

Acked-by: Peter Zijlstra (Intel) <pet...@infradead.org>

Nathan Chancellor

unread,
Jul 24, 2019, 12:54:49 PM7/24/19
to Peter Zijlstra, Josh Poimboeuf, Thomas Gleixner, LKML, Nick Desaulniers, clang-built-linux, x...@kernel.org, Arnd Bergmann, Sedat Dilek, Linus Torvalds
On Wed, Jul 24, 2019 at 06:48:21PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 24, 2019 at 04:10:40PM +0200, Peter Zijlstra wrote:
> > And that most certainly should trigger...
> >
> > Let me gdb that objtool thing.
>
> ---
> Subject: objtool: Improve UACCESS coverage
>
> A clang build reported an (obvious) double CLAC while a GCC build did
> not; it turns out we only re-visit instructions if the first visit was
> with AC=0. If OTOH the first visit was with AC=1, we completely ignore
> any subsequent visit, even when it has AC=0.
>
> Fix this by using a visited mask, instead of boolean and (explicitly)
> mark the AC state.
>
> $ ./objtool check -b --no-fp --retpoline --uaccess ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x22: redundant UACCESS disable
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xea: (alt)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0xffffffffffffffff: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xd9: (alt)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xb2: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0x39: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0x0: <=== (func)
>
> Reported-by: Josh Poimboeuf <jpoi...@redhat.com>
> Reported-by: Thomas Gleixner <tg...@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>

This doesn't regress clang and I see the warning on GCC now too.

Tested-by: Nathan Chancellor <natecha...@gmail.com>

Nick Desaulniers

unread,
Jul 24, 2019, 12:55:33 PM7/24/19
to Peter Zijlstra, Josh Poimboeuf, Thomas Gleixner, LKML, Nathan Chancellor, clang-built-linux, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Arnd Bergmann, Sedat Dilek, Linus Torvalds
On Wed, Jul 24, 2019 at 9:48 AM Peter Zijlstra <pet...@infradead.org> wrote:
>
> On Wed, Jul 24, 2019 at 04:10:40PM +0200, Peter Zijlstra wrote:
> > And that most certainly should trigger...
> >
> > Let me gdb that objtool thing.
>
> ---
> Subject: objtool: Improve UACCESS coverage
>
> A clang build reported an (obvious) double CLAC while a GCC build did
> not; it turns out we only re-visit instructions if the first visit was
> with AC=0. If OTOH the first visit was with AC=1, we completely ignore
> any subsequent visit, even when it has AC=0.
>
> Fix this by using a visited mask, instead of boolean and (explicitly)
> mark the AC state.
>
> $ ./objtool check -b --no-fp --retpoline --uaccess ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x22: redundant UACCESS disable
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xea: (alt)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0xffffffffffffffff: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xd9: (alt)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xb2: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0x39: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0x0: <=== (func)
>
> Reported-by: Josh Poimboeuf <jpoi...@redhat.com>
> Reported-by: Thomas Gleixner <tg...@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>

Tested-by: Nick Desaulniers <ndesau...@google.com>
--
Thanks,
~Nick Desaulniers

Nick Desaulniers

unread,
Jul 24, 2019, 12:58:08 PM7/24/19
to Thomas Gleixner, Nathan Chancellor, LKML, clang-built-linux, Josh Poimboeuf, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Arnd Bergmann
Thanks for reminding me to send a v2: https://lkml.org/lkml/2019/7/22/1196
Sounds like it was picked up yesterday (but I'm not sure the
maintainer pushed it publicly anywhere yet).
--
Thanks,
~Nick Desaulniers

Nick Desaulniers

unread,
Jul 24, 2019, 1:23:06 PM7/24/19
to Josh Poimboeuf, Thomas Gleixner, LKML, Nathan Chancellor, clang-built-linux, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Arnd Bergmann, Sedat Dilek, Peter Zijlstra, Linus Torvalds
Thanks for this patch.
Tested-by: Nick Desaulniers <ndesau...@google.com>
Reported-by: Sedat Dilek <sedat...@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/617
Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the
execobjects array")

--
Thanks,
~Nick Desaulniers

Sedat Dilek

unread,
Jul 24, 2019, 2:31:07 PM7/24/19
to Peter Zijlstra, Josh Poimboeuf, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Linus Torvalds
On Wed, Jul 24, 2019 at 6:48 PM Peter Zijlstra <pet...@infradead.org> wrote:
>
> On Wed, Jul 24, 2019 at 04:10:40PM +0200, Peter Zijlstra wrote:
> > And that most certainly should trigger...
> >
> > Let me gdb that objtool thing.
>
> ---
> Subject: objtool: Improve UACCESS coverage
>
> A clang build reported an (obvious) double CLAC while a GCC build did
> not; it turns out we only re-visit instructions if the first visit was
> with AC=0. If OTOH the first visit was with AC=1, we completely ignore
> any subsequent visit, even when it has AC=0.
>
> Fix this by using a visited mask, instead of boolean and (explicitly)
> mark the AC state.
>
> $ ./objtool check -b --no-fp --retpoline --uaccess ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x22: redundant UACCESS disable
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xea: (alt)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0xffffffffffffffff: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xd9: (alt)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0xb2: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0x39: (branch)
> ../../defconfig-build/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: eb_copy_relocations.isra.34()+0x0: <=== (func)
>
> Reported-by: Josh Poimboeuf <jpoi...@redhat.com>
> Reported-by: Thomas Gleixner <tg...@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>

Thanks Peter Z. and Josh P.!

Reported-by: Sedat Dilek <sedat...@gmail.com>
Tested-by: Sedat Dilek <sedat...@gmail.com>

[1] https://github.com/ClangBuiltLinux/linux/issues/617

Sedat Dilek

unread,
Jul 24, 2019, 2:32:39 PM7/24/19
to Peter Zijlstra, Josh Poimboeuf, Thomas Gleixner, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org, Arnd Bergmann, Linus Torvalds
Please, add this reference:

Link: https://github.com/ClangBuiltLinux/linux/issues/617

Sedat Dilek

unread,
Jul 25, 2019, 2:17:28 AM7/25/19
to Thomas Gleixner, Josh Poimboeuf, LKML, Nick Desaulniers, Nathan Chancellor, clang-built-linux, x...@kernel.org
The objtool warning is fixed by [1].

Now, I see only 3 objtool warnings (all "falls through to next function"):

drivers/gpu/drm/radeon/atom.o: warning: objtool: atom_op_move() falls
through to next function atom_op_and()
drivers/gpu/drm/radeon/evergreen_cs.o: warning: objtool:
evergreen_cs_parse() falls through to next function
evergreen_dma_cs_parse()
drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes()
falls through to next function apply_tx_lanes()

The other i915 call-trace I have seen was independent of $COMPILER and
the objtool warning (details see [2] and [3]).

I was able to boot into my system.

- Sedat -

[1] https://lore.kernel.org/patchwork/series/403494/mbox/
[2] https://lore.kernel.org/lkml/2019072114...@tigerII.localdomain/
[3] https://github.com/ClangBuiltLinux/linux/issues/617#issuecomment-514906560
Reply all
Reply to author
Forward
0 new messages