On 10/9/20 1:18 PM, Paul E. McKenney wrote: > On Fri, Oct 09, 2020 at 12:47:36PM -0700, [email protected] wrote: >> From: Tom Rix <[email protected]> >> >> clang static analysis reports this problem: >> >> rcutorture.c:1999:2: warning: Called function pointer >> is null (null dereference) >> cur_ops->sync(); /* Later readers see above write. */ >> ^~~~~~~~~~~~~~~ >> >> This is a false positive triggered by an earlier, later ignored >> NULL check of sync() op. By inspection of the rcu_torture_ops, >> the sync() op is never uninitialized. So this earlier check is >> not needed. > You lost me on this one. This check is at the very beginning of > rcu_torture_fwd_prog_nr(). Or are you saying that clang is seeing an > earlier check in one of rcu_torture_fwd_prog_nr()'s callers? If so, > where exactly is this check? > > In any case, the check is needed because all three functions are invoked > if there is a self-propagating RCU callback that ensures that there is > always an RCU grace period outstanding. > > Ah. Is clang doing local analysis and assuming that because there was > a NULL check earlier, then the pointer might be NULL later? That does > not seem to me to be a sound check. > > So please let me know exactly what is causing clang to emit this > diagnostic. It might or might not be worth fixing this, but either way > I need to understand the situation so as to be able to understand the > set of feasible fixes. > > Thanx, Paul
In rcu_prog_nr() there is check for for sync. if ( ... cur_op->sync ... do something This flags in clang's static analyzer as 'could be null' later in the function, in a reachable block it is used cur_ops->sync() I agree this is not a good check that's why i said is was a false positive. However when looking closer at how cur_ops is set, it is never uninitialized. So the check is not needed. This is not a fix, the code works fine. It is a small optimization. Tom > >> Signed-off-by: Tom Rix <[email protected]> >> --- >> kernel/rcu/rcutorture.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c >> index beba9e7963c8..6efc03a1d623 100644 >> --- a/kernel/rcu/rcutorture.c >> +++ b/kernel/rcu/rcutorture.c >> @@ -1989,7 +1989,7 @@ static void rcu_torture_fwd_prog_nr(struct rcu_fwd >> *rfp, >> unsigned long stopat; >> static DEFINE_TORTURE_RANDOM(trs); >> >> - if (cur_ops->call && cur_ops->sync && cur_ops->cb_barrier) { >> + if (cur_ops->call && cur_ops->cb_barrier) { >> init_rcu_head_on_stack(&fcs.rh); >> selfpropcb = true; >> } >> -- >> 2.18.1 >>
