linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Song Liu <songliubraving@fb.com>,
	Howard Chu <howardchu95@gmail.com>,
	ndrea Righi <andrea.righi@linux.dev>,
	peterz@infradead.org, mingo@redhat.com, mark.rutland@arm.com,
	alexander.shishkin@linux.intel.com, jolsa@kernel.org,
	irogers@google.com, adrian.hunter@intel.com,
	kan.liang@linux.intel.com, linux-perf-users@vger.kernel.org,
	linux-kernel@vger.kernel.org, james.clark@linaro.org,
	alan.maguire@oracle.com
Subject: Re: [PATCH v2 0/2] perf trace: Fix support for the new BPF feature in clang 12
Date: Tue, 15 Oct 2024 16:35:16 -0300	[thread overview]
Message-ID: <Zw7D9HXBanPLUO4G@x1> (raw)
In-Reply-To: <Zw61TUe1V97dKWer@google.com>

On Tue, Oct 15, 2024 at 11:32:45AM -0700, Namhyung Kim wrote:
> On Thu, Oct 10, 2024 at 07:14:00PM -0700, Howard Chu wrote:
> > Changes in v2:
> > - Resolved a clang-16 build error pointed out by Namhyung Kim
> >   <namhyung@kernel.org>

> > The new augmentation feature in perf trace, along with the protocol
> > change (from payload to payload->value), breaks the clang 12 build.

> > perf trace actually builds for any clang version newer than clang 16.
> > However, as pointed out by Namhyung Kim <namhyung@kernel.org> and Ian
> > Rogers <irogers@google.com>, clang 16, which was released in 2023, is
> > still too new for most users. Additionally, as James Clark
> > <james.clark@linaro.org> noted, some commonly used distributions do not
> > yet support clang 16. Therefore, breaking BPF features between clang 12
> > and clang 15 is not a good approach.

> > This patch series rewrites the BPF program in a way that allows it to
> > pass the BPF verifier, even when the BPF bytecode is generated by older
> > versions of clang.

> > However, I have only tested it till clang 14, as older versions are not
> > supported by my distribution.
 
> > Howard Chu (2):
> >   perf build: Change the clang check back to 12.0.1
> >   perf trace: Rewrite BPF code to pass the verifier
 
> Tested with clang 16.  And I think it's better to change the order of
> the commits so it can fix the problem first and then check the version.

So, I tested it on a RHEL8 system and it gets built with clang 17 but
then fails to load, the verifier complains about lack of bounds checking
for the index of the syscall array, with or without this last patch from
Howard.

I also simplified it to a more minimal version withour renaming
variables, so that we see what exactly fixed the problem, its available
at the perf-tools/tmp.perf-tools branch, I've talked about it with
Howard over chat.

Song Liu reproduced the problem (unsure with what clang and kernel
versions) and couldn't find a way to fix it using the usual tricks to
coax clang to keep the bounds checking for the verifier to get
satisfied.

More generally I'll use virtme-ng[1] to test with a wider range of
kernels, not just clang versions.

- Arnaldo

[1] https://kernel-recipes.org/en/2024/virtme-ng/

  reply	other threads:[~2024-10-15 19:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-11  2:14 [PATCH v2 0/2] perf trace: Fix support for the new BPF feature in clang 12 Howard Chu
2024-10-11  2:14 ` [PATCH v2 1/2] perf build: Change the clang check back to 12.0.1 Howard Chu
2024-10-11  2:14 ` [PATCH v2 2/2] perf trace: Rewrite BPF code to pass the verifier Howard Chu
2024-10-11  8:18 ` [PATCH v2 0/2] perf trace: Fix support for the new BPF feature in clang 12 James Clark
2024-10-15 18:32 ` Namhyung Kim
2024-10-15 19:35   ` Arnaldo Carvalho de Melo [this message]
2024-10-15 19:58     ` Arnaldo Carvalho de Melo
2024-10-15 20:37       ` Arnaldo Carvalho de Melo
2024-10-15 21:37         ` Song Liu
     [not found]           ` <CA+JHD905Xtbb2OYqm3mGbh3C1dKOd-avnC=01=uJfTVEnmA1zQ@mail.gmail.com>
2024-10-15 23:20             ` Song Liu
2024-10-16  2:06         ` Namhyung Kim
2024-10-16 14:22           ` Arnaldo Carvalho de Melo
2024-10-22 17:04             ` Namhyung Kim
2024-10-22 18:33               ` Arnaldo Carvalho de Melo
2024-10-22 23:52                 ` Namhyung Kim
2024-10-23 14:39                   ` Arnaldo Carvalho de Melo
2024-10-23 21:57                     ` Namhyung Kim

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=Zw7D9HXBanPLUO4G@x1 \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alan.maguire@oracle.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andrea.righi@linux.dev \
    --cc=howardchu95@gmail.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=songliubraving@fb.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 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).