All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: lttng-dev <lttng-dev@lists.lttng.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: stack validation warning on lttng-modules bytecode interpreter
Date: Wed, 15 Jun 2016 14:38:46 -0500	[thread overview]
Message-ID: <20160615193846.qsa7vffhi7rn6x2s@treble> (raw)
In-Reply-To: <734452688.38065.1466018019445.JavaMail.zimbra@efficios.com>

On Wed, Jun 15, 2016 at 07:13:39PM +0000, Mathieu Desnoyers wrote:
> ----- On Jun 15, 2016, at 2:18 PM, Josh Poimboeuf jpoimboe@redhat.com wrote:
> 
> > On Wed, Jun 15, 2016 at 04:55:16PM +0000, Mathieu Desnoyers wrote:
> >> Hi Josh,
> >> 
> >> I notice that with gcc 6.1.1, kernel 4.6, with
> >> CONFIG_STACK_VALIDATION=y, building lttng-modules master
> >> at commit 6c09dd94 gives this warning:
> >> 
> >> lttng-modules/lttng-filter-interpreter.o: warning: objtool:
> >> lttng_filter_interpret_bytecode()+0x58: sibling call from callable instruction
> >> with changed frame pointer
> >> 
> >> this object implements a bytecode interpreter using an explicit
> >> jump table (see
> >> https://github.com/lttng/lttng-modules/blob/master/lttng-filter-interpreter.c)
> >> 
> >> If I define "INTERPRETER_USE_SWITCH" at the top of the file,
> >> thus using the switch-case fallback implementation, the
> >> warning vanishes.
> >> 
> >> We use an explicit jump table rather than a switch case whenever
> >> possible for performance reasons.
> >> 
> >> I notice that tools/objtool/builtin-check.c needs to be aware of
> >> switch-cases transformed into jump tables by the compiler. Are
> >> explicit jump tables supported by the stack validator ? Do we
> >> need to add annotation to our code ?
> > 
> > Hi Mathieu,
> > 
> > Unfortunately objtool doesn't know how to validate this type of jump
> > table.  So to avoid the warning you'll need to add an annotation to tell
> > objtool to ignore it:
> > 
> >  STACK_FRAME_NON_STANDARD(lttng_filter_interpret_bytecode);
> > 
> > We had to annotate __bpf_prog_run() in the kernel for the same reason.
> 
> Thanks for the tip! Unfortunately it does not seem to work.
> 
> objdump -t lttng/lttng-filter-interpreter.o output gives:
> 
> 0000000000000000 l    d  __func_stack_frame_non_standard        0000000000000000 __func_stack_frame_non_standard
> 0000000000000000 l     O __func_stack_frame_non_standard        0000000000000008 __func_stack_frame_non_standard_lttng_filter_interpret_bytecode
> 
> Running objtool check (built in O0) in gdb on lttng-filter-interpreter.o
> built with the STACK_FRAME_NON_STANDARD define, it appears that the
> following function:
> 
> static bool ignore_func(struct objtool_file *file, struct symbol *func)
> {
>         struct rela *rela;
>         struct instruction *insn;
> 
>         /* check for STACK_FRAME_NON_STANDARD */
>         if (file->whitelist && file->whitelist->rela)
>                 list_for_each_entry(rela, &file->whitelist->rela->rela_list, list)
>                         if (rela->sym->sec == func->sec &&
>                             rela->addend == func->offset)
>                                 return true;
> 
>         /* check if it has a context switching instruction */
>         func_for_each_insn(file, func, insn)
>                 if (insn->type == INSN_CONTEXT_SWITCH)
>                         return true;
> 
>         return false;
> }
> 
> For lttng_filter_interpret_bytecode, while in the first list
> iteration:
> 
> (gdb) print rela->sym->sec
> $18 = (struct section *) 0x7ffff7e20010
> (gdb) print func->sec
> $19 = (struct section *) 0x7ffff7e20010
> 
> But
> 
> (gdb) print rela->addend
> $20 = 0
> (gdb) print func->offset
> $21 = 928
> 
> So for some reason it never match the ignore_func.
> This happens both when I build lttng-modules as a kernel module,
> and when I build it into the kernel image.
> 
> Any idea why ?

Hm, no idea.  Can you send me the object file?

-- 
Josh

  reply	other threads:[~2016-06-15 19:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-15 16:55 stack validation warning on lttng-modules bytecode interpreter Mathieu Desnoyers
2016-06-15 18:18 ` Josh Poimboeuf
2016-06-15 19:13   ` Mathieu Desnoyers
2016-06-15 19:38     ` Josh Poimboeuf [this message]
2016-06-15 20:01       ` Mathieu Desnoyers
2016-06-15 20:24         ` Josh Poimboeuf
2016-06-15 20:28           ` Josh Poimboeuf
2016-06-15 20:42             ` Mathieu Desnoyers
2016-06-15 20:42             ` Mathieu Desnoyers
2016-06-15 19:38     ` Josh Poimboeuf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160615193846.qsa7vffhi7rn6x2s@treble \
    --to=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lttng-dev@lists.lttng.org \
    --cc=mathieu.desnoyers@efficios.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.