On 30/09/2020 21:36, Maarten Hoes wrote:
[2]
https://drive.google.com/file/d/1kbX_uo_jRqQkny1Qf6XYT1d72nUC8PO0/view?usp=sharing
The last discussion I find about running Clang Static Analyzer on the
LibreOffice code base is the mail thread
<https://lists.freedesktop.org/archives/libreoffice/2015-October/thread.html#70456>
"Report from llvms static analysis tool" from five years ago. Back then
we had a list of 1385 issues, now we are at almost the same number of
1348 issues.
From a quick look at the list, I see instances of all of: clearly true
positives, clearly false positives, and unclear findings.
* For example, "Use-after-move"
2020-09-30-155842-506845-1/report-2cdbb5.html#EndPath is clearly a true
positive (and one that e.g. Coverity Scan apparently doesn't find), see
comment at
<https://gerrit.libreoffice.org/c/core/+/78201/5#message-1b8efe85f0adf7e32bc1143393ac66c0a18ae22f>
"tdf#126959 impress table border resize handle support".
* For example, "Use-after-free"
2020-09-30-155842-506845-1/report-366f11.html#EndPath (presumably along
with many, many other of the 827 Use-after-free) is clearly a false
positive: While VclPtr::disposeAndClear holds
::rtl::Reference<reference_type> aTmp(m_rInnerRef);
the call to
m_rInnerRef.clear();
cannot decrement m_rInnerRef's ref count to zero, so the following
if (aTmp.get()) {
will /not/ access freed memory. We would need some mechanism to filter
out such identified false positives, with whatever mechanism would be
suitable: an annotation in the source code, a modification of the
-analyzer-... command line options passed to clang, etc. However, that
filtering should be done in an auditable way, so that we can later
discover that we are filtering false positives relating to a certain
location in the code, and can learn the rationale why those were
considered false positives. (Something that can be a pain with the way
we use Coverity Scan, see below.)
* For example, "Result of operation is garbage or undefined"
2020-09-30-155842-506845-1/report-78b0b7.html#EndPath is an unclear
finding: SfxSplitWindow::MoveWindow does not check the
SfxSplitWindow::GetWindowPos return value and unconditionally uses nL
(which is uninitialized when GetWindowPos returned false). It can
either be the case that GetWindowPos cannot return false here, so it
would be a false positive (and the code in SfxSplitWindow::MoveWindow
should arguably assert that the return value of GetWindowPos is true),
or it indeed is a true positive. It is hard to find out whether this
issue had already been found by Coverity Scan and dismissed as a false
positive through its Web UI.
_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice
