On Mon, 11 Jul 2022 11:52:11 -0400 Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > On Mon, 11 Jul 2022 09:27:44 -0500 > "Zanussi, Tom" <tom.zanussi@xxxxxxxxxxxxxxx> wrote: > > > So I'm wondering if this means that that the original unnecessary bugfix > > was based on a bug in the clang static analyzer or if that would just be > > considered a false positive... > > Good question. I'd like to know this, as if it is the case, I want to > report that in my pull request to Linus. > > -- Steve I didn't use clang static analyzer before, but from its home page, 'False Positives' seems to exist, see https://clang-analyzer.llvm.org/: > Static analysis is not perfect. It can falsely flag bugs in a program > where the code behaves correctly. Because some code checks require more > analysis precision than others, the frequency of false positives can > vary widely between different checks. Our long-term goal is to have the > analyzer have a low false positive rate for most code on all checks. So I try the clang-14 which comes from https://github.com/llvm/llvm-project/releases/tag/llvmorg-14.0.0, then execute like: $ scan-build make -j16 Then I take a rough look at following warnings related to 'trace_events_hist.c' (serial number is manually added, no double free warning, maybe due to clang version): 1. kernel/trace/trace_events_hist.c:1540:6: warning: Branch condition evaluates to a garbage value [core.uninitialized.Branch] if (!attrs->keys_str) { ^~~~~~~~~~~~~~~~ 2. kernel/trace/trace_events_hist.c:1580:9: warning: Array access (via field 'field_var_str') results in a null pointer dereference [core.NullDereference] kfree(elt_data->field_var_str[i]); ^~~~~~~~~~~~~~~~~~~~~~~~~~ 3. kernel/trace/trace_events_hist.c:1898:3: warning: 1st function call argument is an uninitialized value [core.CallAndMessage] destroy_hist_field(hist_field->operands[i], level + 1); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 4. kernel/trace/trace_events_hist.c:2095:2: warning: 1st function call argument is an uninitialized value [core.CallAndMessage] kfree(ref_field->system); ^~~~~~~~~~~~~~~~~~~~~~~~ 5. kernel/trace/trace_events_hist.c:2099:2: warning: 1st function call argument is an uninitialized value [core.CallAndMessage] kfree(ref_field->name); ^~~~~~~~~~~~~~~~~~~~~~ 6. kernel/trace/trace_events_hist.c:2158:4: warning: Potential leak of memory pointed to by 'ref_field' [unix.Malloc] return NULL; Since I'm not very familiar with trace_events_hist.c, I roughly conclude that: 1. warning 1/3/6 are plausible but false-positive; 2. warning 2/4/5 seems positive although they don't cause practical problems because elt_data->field_var_str[i] / ref_field->system / ref_field->name can be 'NULL' on 'kfree'. Do we need to explicitly check 'NULL' there?
