Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1490449 > unrolled thread

new objtool warnings again...

Back to article view | Back to linux.kernel

Contents

  new objtool warnings again... Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-23 22:40 +0200
    Re: new objtool warnings again... Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-23 23:10 +0200
      Re: new objtool warnings again... Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-23 23:20 +0200
        Re: new objtool warnings again... Arnaldo Carvalho de Melo <acme@redhat.com> - 2016-09-26 18:50 +0200
          Re: new objtool warnings again... Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-27 20:00 +0200
      Re: new objtool warnings again... Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-23 23:20 +0200
    Re: new objtool warnings again... Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-23 23:20 +0200
      [PATCH] x86/alternatives: add stack frame dependency to  alternative_call_2() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-23 23:50 +0200
        Re: [PATCH] x86/alternatives: add stack frame dependency to alternative_call_2() Linus Torvalds <torvalds@linux-foundation.org> - 2016-09-24 01:30 +0200
        [tip:x86/asm] x86/alternatives: Add stack frame dependency to  alternative_call_2() tip-bot for Josh Poimboeuf <tipbot@zytor.com> - 2016-09-24 18:30 +0200
    Re: new objtool warnings again... Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-23 23:20 +0200

#1490449 — new objtool warnings again...

Josh,

 the current F24 toolchain causes

    kernel/signal.o: warning: objtool: .altinstr_replacement+0x54:
call without frame pointer save/setup

during a regular allmodconfig build.

Doing an objdump says:

...
  54:   e8 00 00 00 00          callq  59 <.altinstr_replacement+0x59>
                        55: R_X86_64_PC32       copy_user_generic_string-0x4
  59:   e8 00 00 00 00          callq  5e <.altinstr_replacement+0x5e>
                        5a: R_X86_64_PC32
copy_user_enhanced_fast_string-0x4
...

so it seems to come from the alternative_call_2() in copy_user_generic().

It's somewhere in copy_siginfo_to_user(), so I assume it's just the

        if (from->si_code < 0)
                return __copy_to_user(to, from, sizeof(siginfo_t))
                        ? -EFAULT : 0;

case.  Looking at the code generation, it looks like the frame pointer
generation in that function has been moved down past this code, so the
objtool warning seems to be correct, but this indicates that gcc has
decided that we don't need a frame for that alternative_call_2()
thing.

So this code is clearly missing the magic to tell gcc that the asm
needs a frame pointer.

What was that magic again? Mind sending a patch?

                Linus

[toc] | [next] | [standalone]

#1490454

On Fri, Sep 23, 2016 at 1:33 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So this code is clearly missing the magic to tell gcc that the asm
> needs a frame pointer.

Independently of that, the objtool build seems racy or somehow
fragile. I've now twice gotten into a situation where I end up getting

  cat: /home/torvalds/v2.6/linux/tools/objtool/.fixdep.o.d: No such
file or directory
  make[4]: *** [/home/torvalds/v2.6/linux/tools/objtool/fixdep.o] Error 1
  make[3]: *** [/home/torvalds/v2.6/linux/tools/objtool/fixdep-in.o] Error 2
  make[2]: *** [fixdep] Error 2
  make[1]: *** [objtool] Error 2
  make: *** [tools/objtool] Error 2

with just the right timings, and then ccache ends up remembering that
as a build failure and causing that to be "sticky" even across "git
clean -dqfx" builds (and the "ccache -C" clears it).

Adding Michal to the cc, in case he can see what the problem is.

                 Linus

[toc] | [prev] | [next] | [standalone]

#1490457

On Fri, Sep 23, 2016 at 2:16 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> I just started seeing this problem today.  I suspect it's a ccache
> issue, since it only showed up after ccache was updated.

Ahh, I didn't even notice that ccache got updated, but yeah, that makes sense.

            Linus

[toc] | [prev] | [next] | [standalone]

#1491434

Em Fri, Sep 23, 2016 at 02:19:04PM -0700, Linus Torvalds escreveu:
> On Fri, Sep 23, 2016 at 2:16 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > I just started seeing this problem today.  I suspect it's a ccache
> > issue, since it only showed up after ccache was updated.
> 
> Ahh, I didn't even notice that ccache got updated, but yeah, that makes sense.

yeah, that explains the problems on my side as well, I "fixed" it by
uninstalling ccache, will get the older version..


- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1492080

On Mon, Sep 26, 2016 at 01:49:14PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Sep 23, 2016 at 02:19:04PM -0700, Linus Torvalds escreveu:
> > On Fri, Sep 23, 2016 at 2:16 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > >
> > > I just started seeing this problem today.  I suspect it's a ccache
> > > issue, since it only showed up after ccache was updated.
> > 
> > Ahh, I didn't even notice that ccache got updated, but yeah, that makes sense.
> 
> yeah, that explains the problems on my side as well, I "fixed" it by
> uninstalling ccache, will get the older version..

FYI, here's the upstream ccache github issue:

  https://github.com/ccache/ccache/issues/133

and possibly related:

  https://github.com/ccache/ccache/issues/134

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1490460

On Fri, Sep 23, 2016 at 02:06:03PM -0700, Linus Torvalds wrote:
> On Fri, Sep 23, 2016 at 1:33 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > So this code is clearly missing the magic to tell gcc that the asm
> > needs a frame pointer.
> 
> Independently of that, the objtool build seems racy or somehow
> fragile. I've now twice gotten into a situation where I end up getting
> 
>   cat: /home/torvalds/v2.6/linux/tools/objtool/.fixdep.o.d: No such
> file or directory
>   make[4]: *** [/home/torvalds/v2.6/linux/tools/objtool/fixdep.o] Error 1
>   make[3]: *** [/home/torvalds/v2.6/linux/tools/objtool/fixdep-in.o] Error 2
>   make[2]: *** [fixdep] Error 2
>   make[1]: *** [objtool] Error 2
>   make: *** [tools/objtool] Error 2
> 
> with just the right timings, and then ccache ends up remembering that
> as a build failure and causing that to be "sticky" even across "git
> clean -dqfx" builds (and the "ccache -C" clears it).
> 
> Adding Michal to the cc, in case he can see what the problem is.

I just started seeing this problem today.  I suspect it's a ccache
issue, since it only showed up after ccache was updated.  I "fixed" it
with:

  yum downgrade ccache

I'll open a bug...

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1490456

On Fri, Sep 23, 2016 at 2:14 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> Is this with your latest pushed master branch?  I have F24, but I don't
> see the warning.

It is with the latest branch, but I was wrong - it doesn't show up for
"allmodconfig", it only shows up for my fairly-minimal config on this
laptop.

> In any case, I'll come up with a patch for you to test.

I can send you my odd config in case you want it, but I'm assuming you
just went "ahh, I know what's up" and don't even need it.

            Linus

[toc] | [prev] | [next] | [standalone]

#1490479 — [PATCH] x86/alternatives: add stack frame dependency to alternative_call_2()

Linus reported the following objtool warning:

  kernel/signal.o: warning: objtool: .altinstr_replacement+0x54: call without frame pointer save/setup

The warning is valid.  It's caused by the fact that gcc placed the call
instruction in alternative_call_2()'s inline asm before the frame
pointer setup, which breaks frame pointer convention and can result in a
bad stack trace.

Force a stack frame to be created before the call instruction by listing
the stack pointer as an output operand in the inline asm statement.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/include/asm/alternative.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index e77a644..1b02038 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -217,10 +217,14 @@ static inline int alternatives_text_reserved(void *start, void *end)
  */
 #define alternative_call_2(oldfunc, newfunc1, feature1, newfunc2, feature2,   \
 			   output, input...)				      \
+{									      \
+	register void *__sp asm(_ASM_SP);				      \
 	asm volatile (ALTERNATIVE_2("call %P[old]", "call %P[new1]", feature1,\
 		"call %P[new2]", feature2)				      \
-		: output : [old] "i" (oldfunc), [new1] "i" (newfunc1),	      \
-		[new2] "i" (newfunc2), ## input)
+		: output, "+r" (__sp)					      \
+		: [old] "i" (oldfunc), [new1] "i" (newfunc1),		      \
+		  [new2] "i" (newfunc2), ## input);			      \
+}
 
 /*
  * use this macro(s) if you need more than one output parameter
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1490500 — Re: [PATCH] x86/alternatives: add stack frame dependency to alternative_call_2()

On Fri, Sep 23, 2016 at 2:49 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>

Works for me. So:

  Reported-and-tested-by: Linus Torvalds <torvalds@linux-foundation.org>

but I don't think I'll apply this to my tree now, since it's largely
harmless, and we're already going to do an rc8 and I worry that it
will end up showing something else. But it probably would be good to
queue for the x86 tree for 4.9

                Linus

[toc] | [prev] | [next] | [standalone]

#1490675 — [tip:x86/asm] x86/alternatives: Add stack frame dependency to alternative_call_2()

Commit-ID:  317c2ce77d8ab73c24f4fb9c75e5bb441fbe3e30
Gitweb:     http://git.kernel.org/tip/317c2ce77d8ab73c24f4fb9c75e5bb441fbe3e30
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Fri, 23 Sep 2016 16:49:39 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 24 Sep 2016 09:30:03 +0200

x86/alternatives: Add stack frame dependency to alternative_call_2()

Linus reported the following objtool warning:

  kernel/signal.o: warning: objtool: .altinstr_replacement+0x54: call without frame pointer save/setup

The warning is valid.  It's caused by the fact that gcc placed the call
instruction in alternative_call_2()'s inline asm before the frame
pointer setup, which breaks frame pointer convention and can result in a
bad stack trace.

Force a stack frame to be created before the call instruction by listing
the stack pointer as an output operand in the inline asm statement.

Reported-and-tested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20160923214939.j5o7c67nhepzmh3t@treble
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/alternative.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index e77a644..1b02038 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -217,10 +217,14 @@ static inline int alternatives_text_reserved(void *start, void *end)
  */
 #define alternative_call_2(oldfunc, newfunc1, feature1, newfunc2, feature2,   \
 			   output, input...)				      \
+{									      \
+	register void *__sp asm(_ASM_SP);				      \
 	asm volatile (ALTERNATIVE_2("call %P[old]", "call %P[new1]", feature1,\
 		"call %P[new2]", feature2)				      \
-		: output : [old] "i" (oldfunc), [new1] "i" (newfunc1),	      \
-		[new2] "i" (newfunc2), ## input)
+		: output, "+r" (__sp)					      \
+		: [old] "i" (oldfunc), [new1] "i" (newfunc1),		      \
+		  [new2] "i" (newfunc2), ## input);			      \
+}
 
 /*
  * use this macro(s) if you need more than one output parameter

[toc] | [prev] | [next] | [standalone]

#1490462

On Fri, Sep 23, 2016 at 01:33:45PM -0700, Linus Torvalds wrote:
> Josh,
> 
>  the current F24 toolchain causes
> 
>     kernel/signal.o: warning: objtool: .altinstr_replacement+0x54:
> call without frame pointer save/setup
> 
> during a regular allmodconfig build.
> 
> Doing an objdump says:
> 
> ...
>   54:   e8 00 00 00 00          callq  59 <.altinstr_replacement+0x59>
>                         55: R_X86_64_PC32       copy_user_generic_string-0x4
>   59:   e8 00 00 00 00          callq  5e <.altinstr_replacement+0x5e>
>                         5a: R_X86_64_PC32
> copy_user_enhanced_fast_string-0x4
> ...
> 
> so it seems to come from the alternative_call_2() in copy_user_generic().
> 
> It's somewhere in copy_siginfo_to_user(), so I assume it's just the
> 
>         if (from->si_code < 0)
>                 return __copy_to_user(to, from, sizeof(siginfo_t))
>                         ? -EFAULT : 0;
> 
> case.  Looking at the code generation, it looks like the frame pointer
> generation in that function has been moved down past this code, so the
> objtool warning seems to be correct, but this indicates that gcc has
> decided that we don't need a frame for that alternative_call_2()
> thing.
> 
> So this code is clearly missing the magic to tell gcc that the asm
> needs a frame pointer.
> 
> What was that magic again? Mind sending a patch?

Is this with your latest pushed master branch?  I have F24, but I don't
see the warning.

In any case, I'll come up with a patch for you to test.

-- 
Josh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web