* [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off'
@ 2025-08-20 6:25 Chenghao Duan
2025-08-20 6:52 ` Pu Lehui
0 siblings, 1 reply; 7+ messages in thread
From: Chenghao Duan @ 2025-08-20 6:25 UTC (permalink / raw)
To: ast, bjorn, pulehui, puranjay, paul.walmsley, palmer, aou
Cc: daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, alex, bpf,
linux-riscv, linux-kernel, duanchenghao
In __arch_prepare_bpf_trampoline(), retval_off is only meaningful when
save_ret is true, so the current logic is correct. However, in the
original logic, retval_off is only initialized under certain
conditions, which may cause a build warning.
So initialize retval_off unconditionally to fix it.
Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
---
arch/riscv/net/bpf_jit_comp64.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 10e01ff06312..49bbda8372b0 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1079,10 +1079,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
stack_size += 16;
save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
- if (save_ret) {
+ if (save_ret)
stack_size += 16; /* Save both A5 (BPF R0) and A0 */
- retval_off = stack_size;
- }
+ retval_off = stack_size;
stack_size += nr_arg_slots * 8;
args_off = stack_size;
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off'
2025-08-20 6:25 [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off' Chenghao Duan
@ 2025-08-20 6:52 ` Pu Lehui
2025-08-20 9:26 ` Chenghao Duan
0 siblings, 1 reply; 7+ messages in thread
From: Pu Lehui @ 2025-08-20 6:52 UTC (permalink / raw)
To: Chenghao Duan, ast, bjorn, puranjay, paul.walmsley, palmer, aou
Cc: daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, alex, bpf,
linux-riscv, linux-kernel
On 2025/8/20 14:25, Chenghao Duan wrote:
> In __arch_prepare_bpf_trampoline(), retval_off is only meaningful when
> save_ret is true, so the current logic is correct. However, in the
lgtm, and same for `ip_off`, pls patch it together.
> original logic, retval_off is only initialized under certain
> conditions, which may cause a build warning.
>
> So initialize retval_off unconditionally to fix it.
>
> Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
> ---
> arch/riscv/net/bpf_jit_comp64.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 10e01ff06312..49bbda8372b0 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -1079,10 +1079,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> stack_size += 16;
>
> save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> - if (save_ret) {
> + if (save_ret)
> stack_size += 16; /* Save both A5 (BPF R0) and A0 */
> - retval_off = stack_size;
> - }
> + retval_off = stack_size;
>
> stack_size += nr_arg_slots * 8;
> args_off = stack_size;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off'
2025-08-20 6:52 ` Pu Lehui
@ 2025-08-20 9:26 ` Chenghao Duan
2025-08-20 10:10 ` Pu Lehui
0 siblings, 1 reply; 7+ messages in thread
From: Chenghao Duan @ 2025-08-20 9:26 UTC (permalink / raw)
To: Pu Lehui
Cc: ast, bjorn, puranjay, paul.walmsley, palmer, aou, daniel, andrii,
martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, alex, bpf, linux-riscv, linux-kernel
On Wed, Aug 20, 2025 at 02:52:01PM +0800, Pu Lehui wrote:
>
>
> On 2025/8/20 14:25, Chenghao Duan wrote:
> > In __arch_prepare_bpf_trampoline(), retval_off is only meaningful when
> > save_ret is true, so the current logic is correct. However, in the
>
> lgtm, and same for `ip_off`, pls patch it together.
I also checked at the time that ip_off is only initialized and assigned
when flags & BPF_TRAMP_F_IP_ARG is true. However, I noticed that the use
of ip_off also requires this condition, so the compiler did not issue a
warning.
Chenghao
>
> > original logic, retval_off is only initialized under certain
> > conditions, which may cause a build warning.
> >
> > So initialize retval_off unconditionally to fix it.
> >
> > Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
> > ---
> > arch/riscv/net/bpf_jit_comp64.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> > index 10e01ff06312..49bbda8372b0 100644
> > --- a/arch/riscv/net/bpf_jit_comp64.c
> > +++ b/arch/riscv/net/bpf_jit_comp64.c
> > @@ -1079,10 +1079,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> > stack_size += 16;
> > save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> > - if (save_ret) {
> > + if (save_ret)
> > stack_size += 16; /* Save both A5 (BPF R0) and A0 */
> > - retval_off = stack_size;
> > - }
> > + retval_off = stack_size;
> > stack_size += nr_arg_slots * 8;
> > args_off = stack_size;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off'
2025-08-20 9:26 ` Chenghao Duan
@ 2025-08-20 10:10 ` Pu Lehui
2025-08-20 10:35 ` Chenghao Duan
0 siblings, 1 reply; 7+ messages in thread
From: Pu Lehui @ 2025-08-20 10:10 UTC (permalink / raw)
To: Chenghao Duan
Cc: ast, bjorn, puranjay, paul.walmsley, palmer, aou, daniel, andrii,
martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, alex, bpf, linux-riscv, linux-kernel
On 2025/8/20 17:26, Chenghao Duan wrote:
> On Wed, Aug 20, 2025 at 02:52:01PM +0800, Pu Lehui wrote:
>>
>>
>> On 2025/8/20 14:25, Chenghao Duan wrote:
>>> In __arch_prepare_bpf_trampoline(), retval_off is only meaningful when
>>> save_ret is true, so the current logic is correct. However, in the
>>
>> lgtm, and same for `ip_off`, pls patch it together.
>
> I also checked at the time that ip_off is only initialized and assigned
> when flags & BPF_TRAMP_F_IP_ARG is true. However, I noticed that the use
> of ip_off also requires this condition, so the compiler did not issue a
> warning.
>
> Chenghao
>
>>
>>> original logic, retval_off is only initialized under certain
Can you show how to replay this warning? I guess the warning path is as
follow. Compiler didn't know fmod_ret prog need BPF_TRAMP_F_CALL_ORIG.
```
if (fmod_ret->nr_links) {
...
emit_sd(RV_REG_FP, -retval_off, RV_REG_ZERO, ctx);
}
```
>>> conditions, which may cause a build warning.
>>>
>>> So initialize retval_off unconditionally to fix it.
>>>
>>> Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
>>> ---
>>> arch/riscv/net/bpf_jit_comp64.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
>>> index 10e01ff06312..49bbda8372b0 100644
>>> --- a/arch/riscv/net/bpf_jit_comp64.c
>>> +++ b/arch/riscv/net/bpf_jit_comp64.c
>>> @@ -1079,10 +1079,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
>>> stack_size += 16;
>>> save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
>>> - if (save_ret) {
>>> + if (save_ret)
>>> stack_size += 16; /* Save both A5 (BPF R0) and A0 */
>>> - retval_off = stack_size;
>>> - }
>>> + retval_off = stack_size;
>>> stack_size += nr_arg_slots * 8;
>>> args_off = stack_size;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off'
2025-08-20 10:10 ` Pu Lehui
@ 2025-08-20 10:35 ` Chenghao Duan
2025-08-21 1:58 ` Pu Lehui
0 siblings, 1 reply; 7+ messages in thread
From: Chenghao Duan @ 2025-08-20 10:35 UTC (permalink / raw)
To: Pu Lehui
Cc: ast, bjorn, puranjay, paul.walmsley, palmer, aou, daniel, andrii,
martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, alex, bpf, linux-riscv, linux-kernel
On Wed, Aug 20, 2025 at 06:10:07PM +0800, Pu Lehui wrote:
>
>
> On 2025/8/20 17:26, Chenghao Duan wrote:
> > On Wed, Aug 20, 2025 at 02:52:01PM +0800, Pu Lehui wrote:
> > >
> > >
> > > On 2025/8/20 14:25, Chenghao Duan wrote:
> > > > In __arch_prepare_bpf_trampoline(), retval_off is only meaningful when
> > > > save_ret is true, so the current logic is correct. However, in the
> > >
> > > lgtm, and same for `ip_off`, pls patch it together.
> >
> > I also checked at the time that ip_off is only initialized and assigned
> > when flags & BPF_TRAMP_F_IP_ARG is true. However, I noticed that the use
> > of ip_off also requires this condition, so the compiler did not issue a
> > warning.
> >
> > Chenghao
> >
> > >
> > > > original logic, retval_off is only initialized under certain
>
> Can you show how to replay this warning? I guess the warning path is as
> follow. Compiler didn't know fmod_ret prog need BPF_TRAMP_F_CALL_ORIG.
>
> ```
> if (fmod_ret->nr_links) {
> ...
> emit_sd(RV_REG_FP, -retval_off, RV_REG_ZERO, ctx);
> }
> ```
>
Exactly, the compiler sees the unconditional use of retval_off.
Chenghao
> > > > conditions, which may cause a build warning.
> > > >
> > > > So initialize retval_off unconditionally to fix it.
> > > >
> > > > Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
> > > > ---
> > > > arch/riscv/net/bpf_jit_comp64.c | 5 ++---
> > > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> > > > index 10e01ff06312..49bbda8372b0 100644
> > > > --- a/arch/riscv/net/bpf_jit_comp64.c
> > > > +++ b/arch/riscv/net/bpf_jit_comp64.c
> > > > @@ -1079,10 +1079,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> > > > stack_size += 16;
> > > > save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> > > > - if (save_ret) {
> > > > + if (save_ret)
> > > > stack_size += 16; /* Save both A5 (BPF R0) and A0 */
> > > > - retval_off = stack_size;
> > > > - }
> > > > + retval_off = stack_size;
> > > > stack_size += nr_arg_slots * 8;
> > > > args_off = stack_size;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off'
2025-08-20 10:35 ` Chenghao Duan
@ 2025-08-21 1:58 ` Pu Lehui
2025-08-21 2:55 ` Chenghao Duan
0 siblings, 1 reply; 7+ messages in thread
From: Pu Lehui @ 2025-08-21 1:58 UTC (permalink / raw)
To: Chenghao Duan
Cc: ast, bjorn, puranjay, paul.walmsley, palmer, aou, daniel, andrii,
martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, alex, bpf, linux-riscv, linux-kernel
On 2025/8/20 18:35, Chenghao Duan wrote:
> On Wed, Aug 20, 2025 at 06:10:07PM +0800, Pu Lehui wrote:
>>
>>
>> On 2025/8/20 17:26, Chenghao Duan wrote:
>>> On Wed, Aug 20, 2025 at 02:52:01PM +0800, Pu Lehui wrote:
>>>>
>>>>
>>>> On 2025/8/20 14:25, Chenghao Duan wrote:
>>>>> In __arch_prepare_bpf_trampoline(), retval_off is only meaningful when
>>>>> save_ret is true, so the current logic is correct. However, in the
OK, I think we should make commit msg more explicit. Such like the
follow. wdyt?
`However, in the fmod_ret logic, the compiler is not aware that the
flags of the fmod_ret prog have set BPF_TRAMP_F_CALL_ORIG, resulting in
an uninitialized symbol compilation warning.`
>>>>
>>>> lgtm, and same for `ip_off`, pls patch it together.
>>>
>>> I also checked at the time that ip_off is only initialized and assigned
>>> when flags & BPF_TRAMP_F_IP_ARG is true. However, I noticed that the use
>>> of ip_off also requires this condition, so the compiler did not issue a
>>> warning.
>>>
>>> Chenghao
>>>
>>>>
>>>>> original logic, retval_off is only initialized under certain
>>
>> Can you show how to replay this warning? I guess the warning path is as
>> follow. Compiler didn't know fmod_ret prog need BPF_TRAMP_F_CALL_ORIG.
>>
>> ```
>> if (fmod_ret->nr_links) {
>> ...
>> emit_sd(RV_REG_FP, -retval_off, RV_REG_ZERO, ctx);
>> }
>> ```
>>
>
> Exactly, the compiler sees the unconditional use of retval_off.
>
> Chenghao
>
>>>>> conditions, which may cause a build warning.
>>>>>
>>>>> So initialize retval_off unconditionally to fix it.
>>>>>
>>>>> Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
>>>>> ---
>>>>> arch/riscv/net/bpf_jit_comp64.c | 5 ++---
>>>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
>>>>> index 10e01ff06312..49bbda8372b0 100644
>>>>> --- a/arch/riscv/net/bpf_jit_comp64.c
>>>>> +++ b/arch/riscv/net/bpf_jit_comp64.c
>>>>> @@ -1079,10 +1079,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
>>>>> stack_size += 16;
>>>>> save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
>>>>> - if (save_ret) {
>>>>> + if (save_ret)
>>>>> stack_size += 16; /* Save both A5 (BPF R0) and A0 */
>>>>> - retval_off = stack_size;
>>>>> - }
>>>>> + retval_off = stack_size;
>>>>> stack_size += nr_arg_slots * 8;
>>>>> args_off = stack_size;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off'
2025-08-21 1:58 ` Pu Lehui
@ 2025-08-21 2:55 ` Chenghao Duan
0 siblings, 0 replies; 7+ messages in thread
From: Chenghao Duan @ 2025-08-21 2:55 UTC (permalink / raw)
To: Pu Lehui
Cc: ast, bjorn, puranjay, paul.walmsley, palmer, aou, daniel, andrii,
martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh,
sdf, haoluo, jolsa, alex, bpf, linux-riscv, linux-kernel
On Thu, Aug 21, 2025 at 09:58:20AM +0800, Pu Lehui wrote:
>
>
> On 2025/8/20 18:35, Chenghao Duan wrote:
> > On Wed, Aug 20, 2025 at 06:10:07PM +0800, Pu Lehui wrote:
> > >
> > >
> > > On 2025/8/20 17:26, Chenghao Duan wrote:
> > > > On Wed, Aug 20, 2025 at 02:52:01PM +0800, Pu Lehui wrote:
> > > > >
> > > > >
> > > > > On 2025/8/20 14:25, Chenghao Duan wrote:
> > > > > > In __arch_prepare_bpf_trampoline(), retval_off is only meaningful when
> > > > > > save_ret is true, so the current logic is correct. However, in the
>
> OK, I think we should make commit msg more explicit. Such like the follow.
> wdyt?
>
> `However, in the fmod_ret logic, the compiler is not aware that the flags of
> the fmod_ret prog have set BPF_TRAMP_F_CALL_ORIG, resulting in an
> uninitialized symbol compilation warning.`
>
Good idea
> > > > >
> > > > > lgtm, and same for `ip_off`, pls patch it together.
> > > >
> > > > I also checked at the time that ip_off is only initialized and assigned
> > > > when flags & BPF_TRAMP_F_IP_ARG is true. However, I noticed that the use
> > > > of ip_off also requires this condition, so the compiler did not issue a
> > > > warning.
> > > >
> > > > Chenghao
> > > >
> > > > >
> > > > > > original logic, retval_off is only initialized under certain
> > >
> > > Can you show how to replay this warning? I guess the warning path is as
> > > follow. Compiler didn't know fmod_ret prog need BPF_TRAMP_F_CALL_ORIG.
> > >
> > > ```
> > > if (fmod_ret->nr_links) {
> > > ...
> > > emit_sd(RV_REG_FP, -retval_off, RV_REG_ZERO, ctx);
> > > }
> > > ```
> > >
> >
> > Exactly, the compiler sees the unconditional use of retval_off.
> >
> > Chenghao
> >
> > > > > > conditions, which may cause a build warning.
> > > > > >
> > > > > > So initialize retval_off unconditionally to fix it.
> > > > > >
> > > > > > Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
> > > > > > ---
> > > > > > arch/riscv/net/bpf_jit_comp64.c | 5 ++---
> > > > > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> > > > > > index 10e01ff06312..49bbda8372b0 100644
> > > > > > --- a/arch/riscv/net/bpf_jit_comp64.c
> > > > > > +++ b/arch/riscv/net/bpf_jit_comp64.c
> > > > > > @@ -1079,10 +1079,9 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> > > > > > stack_size += 16;
> > > > > > save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
> > > > > > - if (save_ret) {
> > > > > > + if (save_ret)
> > > > > > stack_size += 16; /* Save both A5 (BPF R0) and A0 */
> > > > > > - retval_off = stack_size;
> > > > > > - }
> > > > > > + retval_off = stack_size;
> > > > > > stack_size += nr_arg_slots * 8;
> > > > > > args_off = stack_size;
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-08-21 2:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 6:25 [PATCH] riscv: bpf: Fix uninitialized symbol 'retval_off' Chenghao Duan
2025-08-20 6:52 ` Pu Lehui
2025-08-20 9:26 ` Chenghao Duan
2025-08-20 10:10 ` Pu Lehui
2025-08-20 10:35 ` Chenghao Duan
2025-08-21 1:58 ` Pu Lehui
2025-08-21 2:55 ` Chenghao Duan
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).