Page MenuHomeFreeBSD

PVS Static Analysis
Needs ReviewPublic

Authored by wblock on Feb 10 2016, 6:34 PM.
Tags
None
Referenced Files
F80160930: D5245.diff
Thu, Mar 28, 5:49 PM
Unknown Object (File)
Mon, Mar 18, 10:22 AM
Unknown Object (File)
Sun, Mar 3, 5:04 AM
Unknown Object (File)
Feb 6 2024, 8:38 PM
Unknown Object (File)
Jan 12 2024, 8:10 PM
Unknown Object (File)
Jan 3 2024, 5:46 AM
Unknown Object (File)
Dec 25 2023, 4:22 AM
Unknown Object (File)
Dec 20 2023, 12:20 AM
This revision needs review, but there are no reviewers specified.

Details

Summary

These are the static analysis results provided by Program Verification Systems. See https://lists.freebsd.org/pipermail/freebsd-doc/2016-February/026371.html

The review was run against r295235 (determined by des@, see https://lists.freebsd.org/pipermail/freebsd-doc/2016-February/026410.html).

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

I've fixed all the usb issues in sys/dev/usb . The others are false positives. Thank you!

kp added inline comments.
PVS-Studio-log-freebsd.txt
899

This looks like it might be a problem, added to my TODO list.

901

Doesn't look like a bug, we don't always allocate new rule, (but if we do it's never NULL because M_WAITOK), so it makes sense to NULL-check it.

902

This check is just wrong. Yes, we use *m before checking it, but that's because pf_test6() can change it.

903

Also seems to be wrong, we can only hit the NULL check from places where newpa is allocated.
We can probably get rid of the NULL check, especially considering that free() checks for NULL anyway.

904

size2 is never NULL, so we can get rid of the NULL check (perhaps adding an assert at the top of the function.

rmacklem added inline comments.
PVS-Studio-log-freebsd.txt
667

All of the "tl" ones are bogus. The NFSM_BUILD() and NFSM_DISSECT() macros
set tl to point to a block of memory the size of the argument. It is then incremented
through the block, sometimes skipping some bytes.

681

The case where it is used it is valid and non-null.

682

bp is only used when n > 0 and it is always valid and non-null when n > 0.

728

It is initialized before it is used.

PVS-Studio-log-freebsd.txt
862

This is not an issue, but the code has been cleaned up such that the warning will not show up anymore.
This is done in https://svnweb.freebsd.org/changeset/base/295709.

PVS-Studio-log-freebsd.txt
846

False positive. If net != NULL then stcb =! NULL.

847

False positive.

848

False positive.

849
850
852

False positive.

853

False positive.

854

False positive.

855

False positive.

857

False positive.

858

False positive.

859

False positive.

860

False positive.

861

False positive.

866

False positive.

867

False positive.

869

False positive.

870

False positive.

871

False positive.

872

False positive.

873

False positive.

874

False positive.

875

False positive.

876

False positive.

PVS-Studio-log-freebsd.txt
863

False positive.

864

False positive.

865

False positive.

868

False positive.

markj added inline comments.
PVS-Studio-log-freebsd.txt
37

The comment below tries to explain it: the check is bogus and is intended to prevent the compiler from making the dtrace_getarg() call a tail call. So this is a false positive.

I wonder if that trick is really reliable though. The check is only false if undefined behaviour has been invoked.

38

False positive. panic() doesn't return.

39

False positive; nspec > 0 => spec != NULL.

40

False positive; the code is assuming that enab->dten_ndesc > 0 implies that enab->dten_desc != NULL.

41

It does seem excessive, but I don't see why it might be wrong. It's also an assertion condition.

marius added inline comments.
PVS-Studio-log-freebsd.txt
269

False positive

558

False positive

PVS-Studio-log-freebsd.txt
883

The bug reported can not happen, since the function is not really working as expected.
This is fixed in https://svnweb.freebsd.org/changeset/base/295771.

884

The bug reported can not happen, since the function is not really working as expected.
This is fixed in https://svnweb.freebsd.org/changeset/base/295771.

PVS-Studio-log-freebsd.txt
29
PVS-Studio-log-freebsd.txt
30

False positive. We always have a bp for a DELETE ccb, but the code is twisty enough that the static analyzer missed it.

32

False positive. serial_buf is always != NULL when have_serial is true, which we check in this loop.

PVS-Studio-log-freebsd.txt
732

It is initialized before used.

734

It is initialized before being used.

735

The if and else do the same thing, which is silly but not broken. I believe this
was inherited from the old NFS client, so I left the code that way, although I
did notice this.

736

This one I don't understand. "iv" is declared local to the function and is used
within the function, so I have no idea why it thinks there is a scope issue?

737

False positive, since the code checks for nfsd_master_proc != NULL
and then procp == nfsd_master_proc --> procp cannot be NULL.

768

It is initialized before being used.

787

It is initialized before being used.

667->788 are all false positives (no bugs identified), although #735 is silly and the
code should probably be cleaned up to take the if/else out someday.

PVS-Studio-log-freebsd.txt
788

Also a false positive. The only time nd == NULL is when the function is
called with CLOPS_RENEW. This code block isn't executed for CLOPS_RENEW.

wblock changed the visibility from "committers (Project)" to "All Users".Feb 19 2016, 4:08 PM
eadler changed the visibility from "All Users" to "Public (No Login Required)".Feb 19 2016, 4:18 PM
se added inline comments.
PVS-Studio-log-freebsd.txt
359

Does no harm, but I'll fix it to simplify the loop condition where it occurs.

PVS-Studio-log-freebsd.txt
588

False positive: baddr != NULL is tested and only possible for vbp != NULL

589

Looks like a false positive: The value read into cp cannot be NULL unless there was a malloc error during driver attach (which had made the attach fail).

PVS-Studio-log-freebsd.txt
801

This is false positive, but moving "structsize = sizeof(elf_kinfo_proc_t);" before the check would fix this warning.
Not yet fixed / silenced this warning.

802

This is false positive, p used as dummy parameter at this call.

trasz added inline comments.
PVS-Studio-log-freebsd.txt
289

Hm, no idea what might be wrong here. False positive, perhaps? I'd appreciate if someone took a look at this.

PVS-Studio-log-freebsd.txt
932

This one looks like a valid bug: ufs_dir_dd_ino() should set *dd_vp to NULL before returning in all cases.

This revision was automatically updated to reflect the committed changes.

\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (436): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points:

I would have coded this differently, using structs, making the code self documenting. This is OK as coded.

\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (912): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points:

This is OK.

All these are OK.

\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (994): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (436): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (912): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (877): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (549): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (438): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (437): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (895): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (891): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (433): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.

More false positives. The scanner failed to detect htons(), htonl(), ntohs() and ntohl().

\sys\contrib\ipfilter\netinet\ip_sync.c (358): error V523: The 'then' statement is equivalent to the 'else' statement.
\sys\contrib\ipfilter\netinet\ip_sync.c (330): error V523: The 'then' statement is equivalent to the 'else' statement.