> On May 10, 2021, at 5:31 AM, Reshetova, Elena <elena.reshetova@xxxxxxxxx> wrote: > >>> On May 10, 2021, at 2:17 AM, Reshetova, Elena <elena.reshetova@xxxxxxxxx> >> wrote: >>> >>> What is the best way to create identifiers for the findings that certain smatch >>> pattern finds in the kernel? Let's say I have a new pattern that is able to find >>> different problematic places and report them in usual smatch way: errors and >>> warnings with file name, line number, function name, etc. >>> Now for our pattern in order to be sure that the reported issue exists/does not >>> exists, somebody needs to go and look at the code manually and make a call. >>> After this, it would be nice to mark this place as safe/concern in the report and be >>> able to transfer these results for kernel versions bumps (5.11->5.12, etc.) as soon >> as >>> the code in this function where finding was reported has not changed (and there >>> might be multiple findings per function). >> >> When I was doing a similar thing to integrate smatch into gerrit, I decided that >> line numbers are too unreliable a metric to really depend on. >> Instead for most cases just using the rest of the message seems to be good enough? >> Function name, whatever variable or the message there is and such is used to filter >> out >> known failures (two kinds: false positives and actual bugs that need to be fixed, but >> are yet unfixed so no point in alerting people to them again and again on unrelated >> review requests) >> >> Seems to be working well though I am sure might not be super perfect. > > I don't think this would be good enough for our case, since what we essentially looking > for with smatch is a set of function calls to a specific functions. So, I end up with > smth like: > > drivers/vme/boards/vme_vmivme7805.c:69 vmic_probe() warn: read using function > 'ioread32' to an int type local variable 'data', type is uint > drivers/vme/boards/vme_vmivme7805.c:74 vmic_probe() warn: read using function > 'ioread32' to an int type local variable 'data', type is uint > > The two above items are in the same function vmic_probe(), making the same function > call " ioread32" and writing down the result of this function call into the same var 'data'. > They just happen to do this in two different places in vmic_probe function. > So, essentially they are even exactly the same lines in this case and the only difference is > relative line position within the analyzed vmic_probe() function. So is your concern that one of those warnings might be a real bug while another one is not? Because if they both are correct or both are incorrect seemingly it does not matter (for filtering purposes) that you have two instead of one?
