* [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).