On Tue, 19 Mar 2024 10:10:00 -0400
Steven Rostedt <rost...@goodmis.org> wrote:
> On Tue, 19 Mar 2024 10:19:09 +0300
> Dan Carpenter <dan.carpen...@linaro.org> wrote:
>
> > Hello Masami Hiramatsu (Google),
> >
> > Commit 25f00e40ce79 ("tracing/probes: Support $argN in return probe
> > (kprobe and fprobe)") from Mar 4, 2024 (linux-next), leads to the
> > following Smatch static checker warning:
> >
> > kernel/trace/trace_probe.c:856 store_trace_entry_data()
> > error: uninitialized symbol 'val'.
> >
> > kernel/trace/trace_probe.c
> > 846 return;
> > 847
> > 848 for (i = 0; i < earg->size; i++) {
> > 849 struct fetch_insn *code = &earg->code[i];
> > 850
> > 851 switch (code->op) {
> > 852 case FETCH_OP_ARG:
> > 853 val = regs_get_kernel_argument(regs,
> > code->param);
> > 854 break;
> > 855 case FETCH_OP_ST_EDATA:
> > --> 856 *(unsigned long *)((unsigned long)edata +
> > code->offset) = val;
> >
> > Probably the earg->code[i] always has FETCH_OP_ARG before
> > FETCH_OP_ST_EDATA but Smatch isn't smart enough to figure that out...
>
> Looks that way:
>
> case FETCH_OP_END:
> earg->code[i].op = FETCH_OP_ARG;
> earg->code[i].param = argnum;
> earg->code[i + 1].op = FETCH_OP_ST_EDATA;
> earg->code[i + 1].offset = offset;
> return offset;
>
> But probably should still initialize val to zero or have a WARN_ON() if
> that doesn't happen.
OK, let's val = 0 in the store_trace_entry_data(), but WARN_ON() in this loop
is a bit strange. I think we should have a verifiler.
Thank you,
>
> -- Steve
>
>
> >
> > 857 break;
> > 858 case FETCH_OP_END:
> > 859 goto end;
> > 860 default:
> > 861 break;
> > 862 }
> > 863 }
> > 864 end:
> > 865 return;
> > 866 }
> >
> > regards,
> > dan carpenter
>
--
Masami Hiramatsu (Google) <mhira...@kernel.org>
