Re: [linux-safety] [PATCH] scripts: Report 'suspicious' comments

Thanks for the details. I will update the script and submit a revised
version of the patch to address your comments.

Thanks

On Thu, Sep 10, 2020 at 2:42 AM Lukas Bulwahn <[email protected]> wrote:
>
>
>
> On Wed, 9 Sep 2020, Mohammed Billoo wrote:
>
> > > - Second, we then need to look into true and false positives here.
> > > BUG appears in intended error messages, in DEBUG, in functions, such as
> > > BUG, BUG_ON. So we need to collect the context around BUG and see to
> > > improve the pattern to at least reduce the false positives.
> > >
> > I thought this was addressed in the Perl script. Did you find
> > instances where this isn't the case (IIRC, BUG(), BUG_ON(), and
> > anything else that is outside of a comment were ignored)?
> >
>
> Yes, I did when scanning through the instances.
>
> Let me consider these 10 examples here:
>
> grep "BUG" commentcheck_on_v5.9-rc4 | grep "arch/x86" | head -n 10
>
> ./arch/x86/entry/entry_32.S contains BUG on line 155
> ./arch/x86/include/asm/cpufeatures.h contains BUG on line 381
> ./arch/x86/kernel/cpu/cpuid-deps.c contains BUG on line 90
> ./arch/x86/kernel/kprobes/core.c contains BUG on line 655
> ./arch/x86/kernel/nmi_selftest.c contains BUG on line 165
> ./arch/x86/kernel/paravirt.c contains BUG on line 128
> ./arch/x86/kernel/traps.c contains BUG on line 225
> ./arch/x86/kernel/traps.c contains BUG on line 314
> ./arch/x86/kvm/mmu/mmu.c contains BUG on line 1321
> ./arch/x86/kvm/mmu/mmu.c contains BUG on line 1326
>
>
> Case:
>
> ./arch/x86/kernel/nmi_selftest.c contains BUG on line 165
>
> printk("BUG: %3d unexpected failures (out of %3d) - debugging disabled! |\n",
>         unexpected_testcase_failures, testcase_total);
>
>
> Here BUG is within an intended printk message.
>
>
> Case:
>
> ./arch/x86/kernel/traps.c contains BUG on line 314
>
>
>         printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n",
>                  (void *)fault_address, current->stack,
>                  (char *)current->stack + THREAD_SIZE - 1);
>
> Here BUG is within an intended printk message.
>
>
> Case:
>
> ./arch/x86/kvm/mmu/mmu.c contains BUG on line 1321
>
> pr_err("%s: %p 0->BUG\n", __func__, spte);
>
> Here BUG is within an intended pr_err message.
>
>
> Case:
>
> ./arch/x86/kvm/mmu/mmu.c contains BUG on line 1326
>
> pr_err("%s:  %p 1->BUG\n", __func__, spte);
>
> Here BUG is within an intended pr_err message.
>
>
> Summary:
>
> 10 examples looked at, 4 cases where it is part of an error message.
>
> At least in six cases, it needs more thought why it might not be
> relevant :)
>
> > > - Third, we then need have suitable ways to aggregate the findings to know
> > > which places are known to have many such findings and hence deserve some
> > > kind of special care.
> > >
> > Do you think CodeChecker would be suitable for this?
> >
>
> Well, I think we can record and track findings with CodeChecker if the
> output from the tool has a suitable format to parse back.
>
> The aggregations I have in mind are a bit orthogonal, though.
>
> For example:
>
> How "bad" is my architecture?
>
> grep "\./arch" commentcheck_on_v5.9-rc4  | sed 's!^.*arch/\([^/]*\)/.*$!\1!' | sort | uniq -c | sort -nr
>
>     159 powerpc
>     113 x86
>     109 arm
>      48 parisc
>      42 mips
>      28 alpha
>      24 microblaze
>      20 sh
>      18 ia64
>      16 m68k
>      15 sparc
>      15 openrisc
>      13 arc
>      10 xtensa
>      10 um
>      10 arm64
>       7 s390
>       7 riscv
>       5 nds32
>       3 hexagon
>       3 h8300
>       1 nios2
>       1 csky
>       1 c6x
>
> We want to provide a tool that can guide a user through this data your
> script collects. Some way to show this data in a reasonable aggregation
> can help (not that I am suggesting that this aggregation above is
> reasonable, but it is a first example I could quickly hack together, but
> maybe it is even reasonable?).
>
> > > And then, we need a lot of help to make proper evaluations what to do.
> > Can you elaborate on this? I don't understand what you mean here.
> >
>
> The statistics above shows there are 113 issues in x86; now, if you would
> ever want to use that architecture for anything that matters to you, I
> guess you need to understand all those 113 issues. That needs a lot of
> help from others that understand the code and the issue, right?
>
> Collecting data is a good first step, but there is still a long way ahead.
>
> Lukas



-- 
Mohammed A Billoo
Founder
MAB Labs, LLC
www.mab-labs.com
201-338-2022