objtool warning breaks build for fs/dlm/lock.o

objtool warning breaks build for fs/dlm/lock.o

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]

Hi,

During randconfig testing with clang-10 I came across a number
of additional objtool warnings, I'll send another mail about those
when I have collected more information and some object files.

This one sticks out however, as objtool returns an error code that
stops the build:

$ make V=1 2>&1 | tail
clang-10 -Wp,-MMD,fs/dlm/.lock.o.d  -nostdinc -isystem
/usr/lib/llvm-10/lib/clang/10.0.1/include
-I/git/arm-soc/arch/x86/include -I./arch/x86/include/generated
-I/git/arm-soc/include -I./include
-I/git/arm-soc/arch/x86/include/uapi
-I./arch/x86/include/generated/uapi -I/git/arm-soc/include/uapi
-I./include/generated/uapi -include
/git/arm-soc/include/linux/kconfig.h -include
/git/arm-soc/include/linux/compiler_types.h -D__KERNEL__ -Wall -Wundef
-Werror=strict-prototypes -fno-strict-aliasing -fno-common
-fshort-wchar -fno-PIE -Werror=implicit-function-declaration
-Werror=implicit-int -std=gnu89 --target=x86_64-linux
--prefix=/home/arnd/cross/x86_64/gcc-10.0.20200413-nolibc/x86_64-linux/bin/
--gcc-toolchain=/home/arnd/cross/x86_64/gcc-10.0.20200413-nolibc/x86_64-linux
-no-integrated-as -Werror=unknown-warning-option -mno-sse -mno-mmx
-mno-sse2 -mno-3dnow -mno-avx -m64 -mno-80387 -mstack-alignment=8
-mtune=generic -mno-red-zone -mcmodel=kernel -DCONFIG_X86_X32_ABI
-Wno-sign-compare -fno-asynchronous-unwind-tables
-fno-delete-null-pointer-checks -O2 -Wframe-larger-than=2048
-fno-stack-protector -fno-omit-frame-pointer
-fno-optimize-sibling-calls -mno-omit-leaf-frame-pointer
-Wdeclaration-after-statement -Wvla -fno-strict-overflow
-fno-merge-all-constants -fno-stack-check -Werror=date-time
-Werror=incompatible-pointer-types -fmacro-prefix-map=/git/arm-soc/=
-Werror  -I /git/arm-soc/fs/dlm -I ./fs/dlm
-DKBUILD_MODFILE='"fs/dlm/dlm"' -DKBUILD_BASENAME='"lock"'
-DKBUILD_MODNAME='"dlm"' -c -o fs/dlm/lock.o
/git/arm-soc/fs/dlm/lock.c
   ./tools/objtool/objtool check  --uaccess fs/dlm/lock.o
fs/dlm/lock.o: warning: objtool: __receive_convert_reply()+0x1e5:
can't find jump dest instruction at .text+0xcaa7
make[4]: *** [/git/arm-soc/scripts/Makefile.build:273: fs/dlm/lock.o] Error 255

Attached object file and .config.

      Arnd

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38681 bytes --]

[-- Attachment #3: lock.o.gz --]
[-- Type: application/gzip, Size: 31418 bytes --]

Re: objtool warning breaks build for fs/dlm/lock.o

[Josh Poimboeuf]

On Wed, May 06, 2020 at 04:07:25PM +0200, Arnd Bergmann wrote:
> Hi,
> 
> During randconfig testing with clang-10 I came across a number
> of additional objtool warnings, I'll send another mail about those
> when I have collected more information and some object files.
> 
> This one sticks out however, as objtool returns an error code that
> stops the build:

> fs/dlm/lock.o: warning: objtool: __receive_convert_reply()+0x1e5: can't find jump dest instruction at .text+0xcaa7

Thanks for sending the patch for this one.  Objtool always gets confused
by new compiler versions, I really think we need to revert
 
  644592d32837 ("objtool: Fail the kernel build on fatal errors")

because objtool is never going to be reliable enough such that we can be
confident that failing the build is the right thing to do.

-- 
Josh

Re: objtool warning breaks build for fs/dlm/lock.o

[Arnd Bergmann]

On Fri, May 8, 2020 at 1:29 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Wed, May 06, 2020 at 04:07:25PM +0200, Arnd Bergmann wrote:
> > Hi,
> >
> > During randconfig testing with clang-10 I came across a number
> > of additional objtool warnings, I'll send another mail about those
> > when I have collected more information and some object files.
> >
> > This one sticks out however, as objtool returns an error code that
> > stops the build:
>
> > fs/dlm/lock.o: warning: objtool: __receive_convert_reply()+0x1e5: can't find jump dest instruction at .text+0xcaa7
>
> Thanks for sending the patch for this one.  Objtool always gets confused
> by new compiler versions, I really think we need to revert
>
>   644592d32837 ("objtool: Fail the kernel build on fatal errors")
>
> because objtool is never going to be reliable enough such that we can be
> confident that failing the build is the right thing to do.

Right, makes sense. It would be nice though to have a way of intentionally
turning all objtool warnings into errors. I do my randconfig tests
with '-Werror'
at the moment in order to catch all new warnings, but this does not catch
objtool errors at the moment. For now, this is probably the right thing to do,
as there are a couple of warnings that I have no patches for, but at some point
I would prefer to trap immediately when a new warning pops up.

     Arnd

Re: objtool warning breaks build for fs/dlm/lock.o

[Peter Zijlstra]

On Thu, May 07, 2020 at 06:29:41PM -0500, Josh Poimboeuf wrote:
> On Wed, May 06, 2020 at 04:07:25PM +0200, Arnd Bergmann wrote:
> > Hi,
> > 
> > During randconfig testing with clang-10 I came across a number
> > of additional objtool warnings, I'll send another mail about those
> > when I have collected more information and some object files.
> > 
> > This one sticks out however, as objtool returns an error code that
> > stops the build:
> 
> > fs/dlm/lock.o: warning: objtool: __receive_convert_reply()+0x1e5: can't find jump dest instruction at .text+0xcaa7
> 
> Thanks for sending the patch for this one.  Objtool always gets confused
> by new compiler versions, I really think we need to revert
>  
>   644592d32837 ("objtool: Fail the kernel build on fatal errors")
> 
> because objtool is never going to be reliable enough such that we can be
> confident that failing the build is the right thing to do.

So on the one hand I agree. We're forever playing catch up with the
compilers, and as I said earlier, objtool generating an error has the
very unfortunate effect of the actual .o file getting deleted by the
build system, which is all sorts of annoying when you then want to
figure out what objtool got upset about.

So, yes, we probably should revert that.

OTOH, if we don't break the build, people are going to continue to
ignore objtool warnings/errors when they writes new code.

Re: objtool warning breaks build for fs/dlm/lock.o

[Peter Zijlstra]

On Fri, May 08, 2020 at 11:27:39AM +0200, Arnd Bergmann wrote:

> Right, makes sense. It would be nice though to have a way of intentionally
> turning all objtool warnings into errors. I do my randconfig tests
> with '-Werror'
> at the moment in order to catch all new warnings, but this does not catch
> objtool errors at the moment. For now, this is probably the right thing to do,
> as there are a couple of warnings that I have no patches for, but at some point
> I would prefer to trap immediately when a new warning pops up.

Completely untested, and I'm 100% unsure of the Makefile change, but
something like so, then?

---

 scripts/Makefile.build        | 2 ++
 tools/objtool/builtin-check.c | 3 ++-
 tools/objtool/builtin.h       | 2 +-
 tools/objtool/check.c         | 8 +-------
 4 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 9fcbfac15d1d..c656f114ce30 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -201,6 +201,8 @@ objtool_args = $(if $(CONFIG_UNWINDER_ORC),orc generate,check)
 
 objtool_args += $(if $(part-of-module), --module,)
 
+objtool_args += $(if $(findstring $(KBUILD_CFLAGS), "-Werror "), --error,)
+
 ifndef CONFIG_FRAME_POINTER
 objtool_args += --no-fp
 endif
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index be42b716166b..3d10ae8b0b25 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -18,7 +18,7 @@
 #include "builtin.h"
 #include "check.h"
 
-bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux;
+bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux, error;
 
 static const char * const check_usage[] = {
 	"objtool check [<options>] file.o",
@@ -35,6 +35,7 @@ const struct option check_options[] = {
 	OPT_BOOLEAN('s', "stats", &stats, "print statistics"),
 	OPT_BOOLEAN('d', "duplicate", &validate_dup, "duplicate validation for vmlinux.o"),
 	OPT_BOOLEAN('l', "vmlinux", &vmlinux, "vmlinux.o validation"),
+	OPT_BOOLEAN('e', "error", &error, "Return error on warnings"),
 	OPT_END(),
 };
 
diff --git a/tools/objtool/builtin.h b/tools/objtool/builtin.h
index 85c979caa367..90c43c6c1e35 100644
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -8,7 +8,7 @@
 #include <subcmd/parse-options.h>
 
 extern const struct option check_options[];
-extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux;
+extern bool no_fp, no_unreachable, retpoline, module, backtrace, uaccess, stats, validate_dup, vmlinux, error;
 
 extern int cmd_check(int argc, const char **argv);
 extern int cmd_orc(int argc, const char **argv);
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 831ffa9e68c4..eed24c584d71 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2793,14 +2793,8 @@ int check(const char *_objname, bool orc)
 	}
 
 out:
-	if (ret < 0) {
-		/*
-		 *  Fatal error.  The binary is corrupt or otherwise broken in
-		 *  some way, or objtool itself is broken.  Fail the kernel
-		 *  build.
-		 */
+	if (error && ret < 0)
 		return ret;
-	}
 
 	return 0;
 }

Re: objtool warning breaks build for fs/dlm/lock.o

[Josh Poimboeuf]

On Fri, May 08, 2020 at 11:55:09AM +0200, Peter Zijlstra wrote:
> On Fri, May 08, 2020 at 11:27:39AM +0200, Arnd Bergmann wrote:
> 
> > Right, makes sense. It would be nice though to have a way of intentionally
> > turning all objtool warnings into errors. I do my randconfig tests
> > with '-Werror'
> > at the moment in order to catch all new warnings, but this does not catch
> > objtool errors at the moment. For now, this is probably the right thing to do,
> > as there are a couple of warnings that I have no patches for, but at some point
> > I would prefer to trap immediately when a new warning pops up.
> 
> Completely untested, and I'm 100% unsure of the Makefile change, but
> something like so, then?

Mostly looks good, but it only errors out on fatal errors, right?  For
-Werror (and what Arnd is asking about) should it also return error on
"warnings > 0"?

-- 
Josh

Re: objtool warning breaks build for fs/dlm/lock.o

[Josh Poimboeuf]

On Fri, May 08, 2020 at 11:45:18AM +0200, Peter Zijlstra wrote:
> On Thu, May 07, 2020 at 06:29:41PM -0500, Josh Poimboeuf wrote:
> > On Wed, May 06, 2020 at 04:07:25PM +0200, Arnd Bergmann wrote:
> > > Hi,
> > > 
> > > During randconfig testing with clang-10 I came across a number
> > > of additional objtool warnings, I'll send another mail about those
> > > when I have collected more information and some object files.
> > > 
> > > This one sticks out however, as objtool returns an error code that
> > > stops the build:
> > 
> > > fs/dlm/lock.o: warning: objtool: __receive_convert_reply()+0x1e5: can't find jump dest instruction at .text+0xcaa7
> > 
> > Thanks for sending the patch for this one.  Objtool always gets confused
> > by new compiler versions, I really think we need to revert
> >  
> >   644592d32837 ("objtool: Fail the kernel build on fatal errors")
> > 
> > because objtool is never going to be reliable enough such that we can be
> > confident that failing the build is the right thing to do.
> 
> So on the one hand I agree. We're forever playing catch up with the
> compilers, and as I said earlier, objtool generating an error has the
> very unfortunate effect of the actual .o file getting deleted by the
> build system, which is all sorts of annoying when you then want to
> figure out what objtool got upset about.
> 
> So, yes, we probably should revert that.
> 
> OTOH, if we don't break the build, people are going to continue to
> ignore objtool warnings/errors when they writes new code.

True, but they can already do that anyway since right now it only fails
on fatal warnings.

-- 
Josh

Re: objtool warning breaks build for fs/dlm/lock.o

[Peter Zijlstra]

On Fri, May 08, 2020 at 11:17:50AM -0500, Josh Poimboeuf wrote:
> On Fri, May 08, 2020 at 11:55:09AM +0200, Peter Zijlstra wrote:
> > On Fri, May 08, 2020 at 11:27:39AM +0200, Arnd Bergmann wrote:
> > 
> > > Right, makes sense. It would be nice though to have a way of intentionally
> > > turning all objtool warnings into errors. I do my randconfig tests
> > > with '-Werror'
> > > at the moment in order to catch all new warnings, but this does not catch
> > > objtool errors at the moment. For now, this is probably the right thing to do,
> > > as there are a couple of warnings that I have no patches for, but at some point
> > > I would prefer to trap immediately when a new warning pops up.
> > 
> > Completely untested, and I'm 100% unsure of the Makefile change, but
> > something like so, then?
> 
> Mostly looks good, but it only errors out on fatal errors, right?  For
> -Werror (and what Arnd is asking about) should it also return error on
> "warnings > 0"?

Oh, good point. Extra knob or just: if (error && ret) return -1 ?

Re: objtool warning breaks build for fs/dlm/lock.o

[Josh Poimboeuf]

On Fri, May 08, 2020 at 06:26:04PM +0200, Peter Zijlstra wrote:
> On Fri, May 08, 2020 at 11:17:50AM -0500, Josh Poimboeuf wrote:
> > On Fri, May 08, 2020 at 11:55:09AM +0200, Peter Zijlstra wrote:
> > > On Fri, May 08, 2020 at 11:27:39AM +0200, Arnd Bergmann wrote:
> > > 
> > > > Right, makes sense. It would be nice though to have a way of intentionally
> > > > turning all objtool warnings into errors. I do my randconfig tests
> > > > with '-Werror'
> > > > at the moment in order to catch all new warnings, but this does not catch
> > > > objtool errors at the moment. For now, this is probably the right thing to do,
> > > > as there are a couple of warnings that I have no patches for, but at some point
> > > > I would prefer to trap immediately when a new warning pops up.
> > > 
> > > Completely untested, and I'm 100% unsure of the Makefile change, but
> > > something like so, then?
> > 
> > Mostly looks good, but it only errors out on fatal errors, right?  For
> > -Werror (and what Arnd is asking about) should it also return error on
> > "warnings > 0"?
> 
> Oh, good point. Extra knob or just: if (error && ret) return -1 ?

I think a single knob (hooked up to -Werror) is good enough for now.

-- 
Josh

Re: objtool warning breaks build for fs/dlm/lock.o

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]

On Fri, May 8, 2020 at 1:29 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Wed, May 06, 2020 at 04:07:25PM +0200, Arnd Bergmann wrote:
> > Hi,
> >
> > During randconfig testing with clang-10 I came across a number
> > of additional objtool warnings, I'll send another mail about those
> > when I have collected more information and some object files.
> >
> > This one sticks out however, as objtool returns an error code that
> > stops the build:
>
> > fs/dlm/lock.o: warning: objtool: __receive_convert_reply()+0x1e5: can't find jump dest instruction at .text+0xcaa7
>
> Thanks for sending the patch for this one.  Objtool always gets confused
> by new compiler versions, I really think we need to revert
>
>   644592d32837 ("objtool: Fail the kernel build on fatal errors")
>
> because objtool is never going to be reliable enough such that we can be
> confident that failing the build is the right thing to do.

I'm now struggling with a clang -fintegrated-as related failure:

arch/x86/kernel/ftrace_64.o: warning: objtool: missing symbol for insn
at offset 0x16
make[4]: *** [/git/arm-soc/scripts/Makefile.build:355:
arch/x86/kernel/ftrace_64.o] Error 255

Using this as a local workaround, but I'd like to find out if this is a bug
in clang or in objtool:

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index e77261db2391..b4a0f3dd321d 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -38,6 +38,9 @@ OBJECT_FILES_NON_STANDARD_paravirt_patch.o            := y
 ifdef CONFIG_FRAME_POINTER
 OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o             := y
 endif
+ifdef CONFIG_CC_IS_CLANG
+AFLAGS_ftrace_64.o                                     += -fno-integrated-as
+endif

 # If instrumentation of this dir is enabled, boot hangs during first second.
 # Probably could be more selective here, but note that files related to irqs,

Attaching the broken object file fore reference.

     Arnd

[-- Attachment #2: ftrace_64.o --]
[-- Type: application/x-object, Size: 1040 bytes --]

Re: objtool warning breaks build for fs/dlm/lock.o

[Josh Poimboeuf]

On Mon, May 18, 2020 at 09:57:50PM +0200, Arnd Bergmann wrote:
> On Fri, May 8, 2020 at 1:29 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > On Wed, May 06, 2020 at 04:07:25PM +0200, Arnd Bergmann wrote:
> > > Hi,
> > >
> > > During randconfig testing with clang-10 I came across a number
> > > of additional objtool warnings, I'll send another mail about those
> > > when I have collected more information and some object files.
> > >
> > > This one sticks out however, as objtool returns an error code that
> > > stops the build:
> >
> > > fs/dlm/lock.o: warning: objtool: __receive_convert_reply()+0x1e5: can't find jump dest instruction at .text+0xcaa7
> >
> > Thanks for sending the patch for this one.  Objtool always gets confused
> > by new compiler versions, I really think we need to revert
> >
> >   644592d32837 ("objtool: Fail the kernel build on fatal errors")
> >
> > because objtool is never going to be reliable enough such that we can be
> > confident that failing the build is the right thing to do.
> 
> I'm now struggling with a clang -fintegrated-as related failure:
> 
> arch/x86/kernel/ftrace_64.o: warning: objtool: missing symbol for insn at offset 0x16
> make[4]: *** [/git/arm-soc/scripts/Makefile.build:355:
> arch/x86/kernel/ftrace_64.o] Error 255
> 
> Using this as a local workaround, but I'd like to find out if this is a bug
> in clang or in objtool:

It seems like an objtool bug, though at first glance I don't know where
exactly.  It could be a problem with the rb-tree stuff Peter added
recently.

That instruction should be part of the __fentry__ function:

     4: 0000000000000000   165 FUNC    GLOBAL DEFAULT    3 __fentry__

So find_symbol_containing() should associate it with __fentry__.

-- 
Josh

Re: objtool warning breaks build for fs/dlm/lock.o

[Josh Poimboeuf]

On Mon, May 18, 2020 at 05:53:28PM -0500, Josh Poimboeuf wrote:
> > I'm now struggling with a clang -fintegrated-as related failure:
> > 
> > arch/x86/kernel/ftrace_64.o: warning: objtool: missing symbol for insn at offset 0x16
> > make[4]: *** [/git/arm-soc/scripts/Makefile.build:355:
> > arch/x86/kernel/ftrace_64.o] Error 255
> > 
> > Using this as a local workaround, but I'd like to find out if this is a bug
> > in clang or in objtool:
> 
> It seems like an objtool bug, though at first glance I don't know where
> exactly.  It could be a problem with the rb-tree stuff Peter added
> recently.
> 
> That instruction should be part of the __fentry__ function:
> 
>      4: 0000000000000000   165 FUNC    GLOBAL DEFAULT    3 __fentry__
> 
> So find_symbol_containing() should associate it with __fentry__.

Peter, is find_symbol_containing() broken by aliased functions and/or
overlapping symbols?

Symbol table '.symtab' contains 7 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    5 __ksym_marker___fentry__
     2: 000000000000000e     0 NOTYPE  LOCAL  DEFAULT    3 fgraph_trace
     3: 000000000000000f     0 NOTYPE  LOCAL  DEFAULT    3 trace
     4: 0000000000000000   165 FUNC    GLOBAL DEFAULT    3 __fentry__
     5: 000000000000000e     0 NOTYPE  GLOBAL DEFAULT    3 ftrace_stub
     6: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND ftrace_trace_function

Notice the NOTYPEs are inside the FUNC.

-- 
Josh