Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: LoongArch: Add BPF JIT support
To:     "Colin King (gmail)" <colin.i.king@gmail.com>
References: <bcf97046-e336-712a-ac68-7fd194f2953e@gmail.com>
Cc:     Huacai Chen <chenhuacai@loongson.cn>, loongarch@lists.linux.dev,
        "bpf@vger.kernel.org >> bpf" <bpf@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
From:   Tiezhu Yang <yangtiezhu@loongson.cn>
Message-ID: <9ce766dd-4bd9-d4a0-6da5-a29af7d9aa28@loongson.cn>
Date:   Tue, 25 Jul 2023 08:52:02 +0800
User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <bcf97046-e336-712a-ac68-7fd194f2953e@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Colin,

On 07/24/2023 08:27 PM, Colin King (gmail) wrote:
> Hi,
>
> Static analysis with clang scan build on arch/loongarch/net/bpf_jit.h
> has detected a potential issue with the following commit:
>
> commit 5dc615520c4dfb358245680f1904bad61116648e
> Author: Tiezhu Yang <yangtiezhu@loongson.cn>
> Date:   Wed Oct 12 16:36:20 2022 +0800
>
>     LoongArch: Add BPF JIT support
>
> This issue is as follows:
>
> arch/loongarch/net/bpf_jit.h:153:23: warning: Logical disjunction always
> evaluates to true: imm_51_31 != 0 || imm_51_31 != 0x1fffff.
> [incorrectLogicOperator]
>    if (imm_51_31 != 0 || imm_51_31 != 0x1fffff) {

Thanks for your report.

>
>
> The statement seems to be always true. I suspect it should it be instead:
>
>    if (imm_51_31 != 0 && imm_51_31 != 0x1fffff) {

Yes, you are right. It is same with

if (!(imm_51_31 == 0 || imm_51_31 == 0x1fffff)) {

As the code comment says, the initial aim is to reduce one instruction
in some corner cases, if bit[51:31] is all 0 or all 1, no need to call
lu32id, that is to say, it should call lu32id only if bit[51:31] is not
all 0 and not all 1. The current code always call lu32id, the result is
right but the logic is unexpected and wrong.

I will send a patch to fix it as soon as possible.

Thanks,
Tiezhu