mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: bot+bpf-ci@kernel.org
To: a.s.protopopov@gmail.com,ast@kernel.org,daniel@iogearbox.net,andrii@kernel.org,eddyz87@gmail.com,memxor@gmail.com,almagui@kernel.org,john.fastabend@gmail.com,qmo@kernel.org,brauner@kernel.org,kpsingh@kernel.org,yonghong.song@linux.dev,jolsa@kernel.org,emil@etsalapatis.com,ihor.solodrai@linux.dev,sashal@kernel.org,bpf@vger.kernel.org,linux-kernel@vger.kernel.org
Cc: a.s.protopopov@gmail.com,ast@kernel.org,andrii@kernel.org,daniel@iogearbox.net,martin.lau@kernel.org,eddyz87@gmail.com,yonghong.song@linux.dev,mason@kernel.org,ihor.solodrai@linux.dev
Subject: Re: [PATCH v2 bpf-next 11/18] bpf: Follow PHIs in bpf_scan_ir
Date: Mon,  9 Nov 2026 17:42:21 +0000 (UTC)	[thread overview]
Message-ID: <8ab81e05089b4f5ec1ecb1bea9e6dc5cc2953a67f16e9369db005c2f3f3ee242@mail.kernel.org> (raw)
In-Reply-To: <20261109170550.141443-12-a.s.protopopov@gmail.com>

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

> bpf: Follow PHIs in bpf_scan_ir

TL;DR: Nits only: in bpf_scan_ir.cpp, isErrnoRange() duplicates
errnoConstant()'s errno check, so the range is defined twice; also a comment
style nit.

> diff --git a/scripts/llvm-plugins/bpf_scan_ir.cpp b/scripts/llvm-plugins/bpf_scan_ir.cpp
> --- a/scripts/llvm-plugins/bpf_scan_ir.cpp
> +++ b/scripts/llvm-plugins/bpf_scan_ir.cpp

[ ... ]

> @@ -34,9 +36,40 @@ namespace {
>  
>  static constexpr int MaxErrno = 4095;
>  
> +// is Range in [-4095, 0) ?
> +static bool isErrnoRange(const ConstantRange &Range)
> +{
> +	unsigned Width = Range.getBitWidth();
> +
> +	if (Width != 32 && Width != 64)
> +		return false;
> +
> +	auto L = APInt(Width, 0) - APInt(Width, MaxErrno);
> +	auto R = APInt(Width, 0);
> +	return !Range.isEmptySet() && ConstantRange(L, R).contains(Range);
> +}
> +

this isn't a bug, but could singleErrno() and errnoConstant() share one
check?

isErrnoRange() has a single caller, singleErrno(), which only passes it a
range that getSingleElement() already showed to hold exactly one value.
For that input the width check, the isEmptySet() test and the wrapped
ConstantRange containment do the same thing as errnoConstant() below
(width of 32 or 64, then N >= -MaxErrno && N < 0), so the errno range is
now defined in two places, and the !Range.isEmptySet() test can never
fail.

For example, errnoConstant() could call
singleErrno(ConstantRange(C->getValue()), N), or singleErrno() could test
the sign-extended element against MaxErrno directly.

Also, the "// is Range in [-4095, 0) ?" comment is the only function-level
comment in the file using // rather than /* */, and it hardcodes -4095
where the code uses MaxErrno.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/42662799554

  reply	other threads:[~2026-11-09 17:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-09 17:05 [PATCH v2 bpf-next 00/18] Auto-detect BPF fmodret targets Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 01/18] bpf: Add BTF ID sets for fmod_ret targets Anton Protopopov
2026-11-09 17:42   ` bot+bpf-ci
2026-11-09 17:05 ` [PATCH v2 bpf-next 02/18] bpf: Add a btf_id_set8_entry helper Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 03/18] bpf: Build fmod_ret target sets from BTF decl tags Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 04/18] selftests/bpf: Declare bpf_fentry_test1 in tracing tests Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 05/18] bpf: Declare fmod_ret targets with BTF_FMODRET sets Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 06/18] selftests/bpf: Test fmod_ret BTF decl tags and attach permission Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 07/18] scripts: Add LLVM plugin infrastructure Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 08/18] bpf: Add the bpf_scan_ir LLVM plugin Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 09/18] bpf: Find fmod_ret candidates in bpf_scan_ir Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 10/18] bpf: Allow writes to private stack objects " Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 11/18] bpf: Follow PHIs " Anton Protopopov
2026-11-09 17:42   ` bot+bpf-ci [this message]
2026-11-09 17:05 ` [PATCH v2 bpf-next 12/18] bpf: Follow integer casts " Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 13/18] bpf: Check the return values of fmod_ret programs on found targets Anton Protopopov
2026-11-09 17:42   ` bot+bpf-ci
2026-11-09 17:05 ` [PATCH v2 bpf-next 14/18] bpf: Require the JIT for fmod_ret programs on found 64-bit targets Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 15/18] resolve_btfids: Tag fmod_ret candidates as targets Anton Protopopov
2026-11-09 17:42   ` bot+bpf-ci
2026-11-09 17:05 ` [PATCH v2 bpf-next 16/18] selftests/bpf: Test fmod_ret candidates found by bpf_scan_ir Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 17/18] selftests/bpf: Enable bpf_scan_ir in the config Anton Protopopov
2026-11-09 17:05 ` [PATCH v2 bpf-next 18/18] bpftool: List and show fmod_ret targets Anton Protopopov
2026-11-09 17:42   ` bot+bpf-ci

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=8ab81e05089b4f5ec1ecb1bea9e6dc5cc2953a67f16e9369db005c2f3f3ee242@mail.kernel.org \
    --to=bot+bpf-ci@kernel.org \
    --cc=a.s.protopopov@gmail.com \
    --cc=almagui@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=mason@kernel.org \
    --cc=memxor@gmail.com \
    --cc=qmo@kernel.org \
    --cc=sashal@kernel.org \
    --cc=yonghong.song@linux.dev \
    /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

all inboxes | Powered by JetHome®