scan-build does not understand TAILQ_REMOVE from sys/queue.h

[Quuxplusone]

Bugzilla Link	PR42780
Status	CONFIRMED
Importance	P normal
Reported by	Jim Harris (james.r.harris@intel.com)
Reported on	2019-07-26 08:45:02 -0700
Last modified on	2019-07-29 11:36:57 -0700
Version	8.0
Hardware	PC Linux
CC	dcoughlin@apple.com, llvm-bugs@lists.llvm.org, noqnoqneo@gmail.com
Fixed by commit(s)
Attachments	scantest.c (572 bytes, text/x-csrc)
Blocks
Blocked by
See also

Created attachment 22294
Test program to reproduce false positive

scan-build 8.0 has introduced false positives related to TAILQ_REMOVE from
sys/queue.h.  These false positives were not seen with previous versions of
scan-build.  This was found when trying to upgrade scan-build on CI systems for
SPDK (Storage Performance Development Kit).

https://bugs.llvm.org/show_bug.cgi?id=15297 seems to point to the same issue,
but it was marked as resolved and fixed 6 years ago.  This still seems like a
regression since behavior has changed with scan-build 8.0.

See attachment for a test program that reproduces the false positive.

Thanks,

Jim Harris
SPDK Core Maintainer

[Quuxplusone]

Attached scantest.c (572 bytes, text/x-csrc): Test program to reproduce false positive

[Quuxplusone]

This doesn't look like a regression; i can reproduce the problem on 8.0 (as
well as on 7.1).

In this specific example the problem is that we ignore the global initializer,
even though we're in main(). This would have been the valid behavior in other
cases, but it's usually incorrect in main(). I guess we should fix that. If i
turn the queue into a local variable the false positive disappears.

The bigger problem, however, is that even if it's not main(), there's no way
for us to infer that the doubly linked list is *initially* well-formed. There's
also no way for you to communicate it back to us with various forms of
assertions or annotations (partially because such assertions are impossible to
write, partially because even the simple ones, such as
assert(TAILQ_EMPTY(&head)), will most likely be unsupported due to how aliasing
isn't working correctly when discovered through comparisons).

So the action items here are:
- Maybe add explicit modeling for TAILQ etc. because they are fairly popular on
various security-critical codebases.
- Trust global initializers when we're in main().
- Improve pointer aliasing tests support.

[Quuxplusone]

Whoops, misunderstood; i though you're saying that it's a regression from 8.0. Anyway, i can still reproduce it on 7.0.

[Quuxplusone]

Hi Artem,

Sorry - I should have specified more details around saying this was a "regression".

We have a number of test systems using Fedora 29 which package clang 7.0. We did not see these problems on F29. Fedora 30 packages clang 8.0 which started showing these types of errors.

-Jim