linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Eduard Zingerman <eddyz87@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, linux-mm@kvack.org,
	 akpm@linux-foundation.org, adobriyan@gmail.com,
	shakeel.butt@linux.dev,  hannes@cmpxchg.org, ak@linux.intel.com,
	osandov@osandov.com, song@kernel.org,  jannh@google.com,
	linux-fsdevel@vger.kernel.org, willy@infradead.org
Subject: Re: [PATCH v6 bpf-next 10/10] selftests/bpf: add build ID tests
Date: Thu, 22 Aug 2024 15:55:56 -0700	[thread overview]
Message-ID: <CAEf4BzZULffF9_6Mz4dxm=owvSkyErt3kShZpxjFnCySzGDWNw@mail.gmail.com> (raw)
In-Reply-To: <e973f93d1dc2ebf54de285a7d83833ea6c47f2a2.camel@gmail.com>

On Thu, Aug 22, 2024 at 3:30 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2024-08-14 at 11:54 -0700, Andrii Nakryiko wrote:
> > Add a new set of tests validating behavior of capturing stack traces
> > with build ID. We extend uprobe_multi target binary with ability to
> > trigger uprobe (so that we can capture stack traces from it), but also
> > we allow to force build ID data to be either resident or non-resident in
> > memory (see also a comment about quirks of MADV_PAGEOUT).
> >
> > That way we can validate that in non-sleepable context we won't get
> > build ID (as expected), but with sleepable uprobes we will get that
> > build ID regardless of it being physically present in memory.
> >
> > Also, we add a small add-on linker script which reorders
> > .note.gnu.build-id section and puts it after (big) .text section,
> > putting build ID data outside of the very first page of ELF file. This
> > will test all the relaxations we did in build ID parsing logic in kernel
> > thanks to freader abstraction.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
>
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>
> [...]
>
> > diff --git a/tools/testing/selftests/bpf/uprobe_multi.c b/tools/testing/selftests/bpf/uprobe_multi.c
> > index 7ffa563ffeba..c7828b13e5ff 100644
> > --- a/tools/testing/selftests/bpf/uprobe_multi.c
> > +++ b/tools/testing/selftests/bpf/uprobe_multi.c
>
> [...]
>
> > +int __attribute__((weak)) trigger_uprobe(bool build_id_resident)
> > +{
> > +     int page_sz = sysconf(_SC_PAGESIZE);
> > +     void *addr;
> > +
> > +     /* page-align build ID start */
> > +     addr = (void *)((uintptr_t)&build_id_start & ~(page_sz - 1));
> > +
> > +     /* to guarantee MADV_PAGEOUT work reliably, we need to ensure that
> > +      * memory range is mapped into current process, so we unconditionally
> > +      * do MADV_POPULATE_READ, and then MADV_PAGEOUT, if necessary
> > +      */
> > +     madvise(addr, page_sz, MADV_POPULATE_READ);
>
> Nit: check error code?
>

Well, even if this errors out there is no one to notice and do
anything about it, given this is in a forked process. The idea,
though, is that if this doesn't work, we'll catch it as part of the
actual selftest.

> > +     if (!build_id_resident)
> > +             madvise(addr, page_sz, MADV_PAGEOUT);
> > +
> > +     (void)uprobe();
> > +
> > +     return 0;
> > +}
> > +
>
> [...]
>
> Silly question, unrelated to the patch-set itself.
> When I do ./test_progs -vvv -t build_id/sleepable five stack frames
> are printed:
>
> FRAME #00: BUILD ID = 46d2568fe293274105f9dad0cc73de54a176f368 OFFSET = 2c4156
> FRAME #01: BUILD ID = 46d2568fe293274105f9dad0cc73de54a176f368 OFFSET = 393aef
> FRAME #02: BUILD ID = 8f53abaad945a669f2bdcd25f471d80e077568ef OFFSET = 2a088
> FRAME #03: BUILD ID = 8f53abaad945a669f2bdcd25f471d80e077568ef OFFSET = 2a14b
> FRAME #04: BUILD ID = 46d2568fe293274105f9dad0cc73de54a176f368 OFFSET = 2c4095

In my QEMU I only get 3:

FRAME #00: BUILD ID = d370860567af6d28316d45726045f1c59bbfc416 OFFSET = 2c4156
FRAME #01: BUILD ID = d370860567af6d28316d45726045f1c59bbfc416 OFFSET = 393ac7
FRAME #02: BUILD ID = 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658 OFFSET = 27cd0

But see below, for my actual devserver there are 4 frames. My bet
would be that 568ef is libc. A bit confused why you get frame 04 from
uprobe_multi, but maybe that's how things work with musl or whatever?
Don't know. Check libc.so.

>
> The ...6f368 is build-id of the uprobe_multi.
> How do I check where ...568ef comes from?
> Also, why are there 5 frames when nesting level for uprobe() is 3?
>

Well, libc has some function calls before it gets to main. E.g., for
my local machine:

$ sudo bpftrace -e 'uprobe:./uprobe_multi:uprobe { print(ustack()); }'
Attaching 1 probe...

        uprobe+4
        trigger_uprobe+113
        main+176
        __libc_start_call_main+128

Note that you won't have trigger_uprobe in your stack trace until your
kernel has [0]

  [0] https://lore.kernel.org/linux-trace-kernel/20240729175223.23914-1-andrii@kernel.org/

  reply	other threads:[~2024-08-22 22:56 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-14 18:54 [PATCH v6 bpf-next 00/10] Harden and extend ELF build ID parsing logic Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 01/10] lib/buildid: harden " Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 02/10] lib/buildid: add single folio-based file reader abstraction Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 03/10] lib/buildid: take into account e_phoff when fetching program headers Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 04/10] lib/buildid: remove single-page limit for PHDR search Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 05/10] lib/buildid: rename build_id_parse() into build_id_parse_nofault() Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 06/10] lib/buildid: implement sleepable build_id_parse() API Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 07/10] lib/buildid: don't limit .note.gnu.build-id to the first page in ELF Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 08/10] bpf: decouple stack_map_get_build_id_offset() from perf_callchain_entry Andrii Nakryiko
2024-08-22 20:32   ` Eduard Zingerman
2024-08-14 18:54 ` [PATCH v6 bpf-next 09/10] bpf: wire up sleepable bpf_get_stack() and bpf_get_task_stack() helpers Andrii Nakryiko
2024-08-23 22:22   ` Eduard Zingerman
2024-08-26 16:19     ` Andrii Nakryiko
2024-08-14 18:54 ` [PATCH v6 bpf-next 10/10] selftests/bpf: add build ID tests Andrii Nakryiko
2024-08-22 22:30   ` Eduard Zingerman
2024-08-22 22:55     ` Andrii Nakryiko [this message]
2024-08-22 23:07       ` Eduard Zingerman
2024-08-23 23:22 ` [PATCH v6 bpf-next 00/10] Harden and extend ELF build ID parsing logic Eduard Zingerman
2024-08-25 19:35   ` Alexei Starovoitov
2024-08-26 21:30   ` Andrii Nakryiko

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='CAEf4BzZULffF9_6Mz4dxm=owvSkyErt3kShZpxjFnCySzGDWNw@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=adobriyan@gmail.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=eddyz87@gmail.com \
    --cc=hannes@cmpxchg.org \
    --cc=jannh@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=osandov@osandov.com \
    --cc=shakeel.butt@linux.dev \
    --cc=song@kernel.org \
    --cc=willy@infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).