[analyzer] Checker for uninitialized C++ objects fails in some trivial situations

[Quuxplusone]

Bugzilla Link	PR37965
Status	NEW
Importance	P enhancement
Reported by	andi@mozilla.com
Reported on	2018-06-27 09:45:52 -0700
Last modified on	2018-08-13 03:52:44 -0700
Version	unspecified
Hardware	PC All
CC	dkszelethus@gmail.com, klimek@google.com, sylvestre@debian.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Hello,

We've ran scan-build wit cxx-uninitialized-object enabled, and among other
issues we also got this kinda trivial issue:

http://sylvestre.ledru.info/reports/fx-scan-build/report-nsCOMPtr.h-nsGetServiceByContractIDWithError-1-24.html#EndPath

It's obvious that |mErrorPtr| it's initialised in the initialisers list in the
ctor: nsGetServiceByContractIDWithError(const char* aContractID, nsresult*
aErrorPtr), please see https://dxr.mozilla.org/mozilla-
central/source/xpcom/base/nsCOMPtr.h?q=nsGetServiceByContractIDWithError%28const+char%2A+aContractID%2C+nsresult%2A+aErrorPtr%29&redirect_type=single#265

Maybe it only support primitive type initialisation, but even so, this is a
pointer and shouldn't matter this much.

[Quuxplusone]

Hello!

Thank you so much for testing this checker, really appreciated!

From what I can see, the checker behaved as it should. The note message says
'uninitialized pointee', not 'uninitialized pointer'. Issues like this could
arrive from similar codes:

int i; // say that this isn't zero initialized

struct A {
  int *iptr;
  A(int *iptr) : iptr(iptr) {}
};

A i(&a);

This is exactly what happened in the code.

>Maybe it only support primitive type initialisation, but even so, this is a
>pointer and shouldn't matter this much.

Do you think that this checker shouldn't report issues like this?

[Quuxplusone]

Thanks for the fast reply! This kind of checker is more than welcome for us since using it can mitigate some security issues.

Regarding the checker, and it's actual result, I see your point here and I can confirm it's valid, still I don't know if we want to do this deep in the analysis, since the deeper we go, we can have false-positives. I think for our case It would be enough to know that the pointer is initialised or not. I argument this statement based on the RAII architecture where is constructor is delegated to initialise al of it's members. Indeed in this case the pointee is not initialised but I guess giving the result of the analysis inside of the ctor is a bit confusing.

To answer to your last question, I'm not sure what to say about to report or not, since indeed the value that it adds knowing this exists, but maybe we can add some configuration flags for this?

[Quuxplusone]

I've seen that this checker is still in alpha, are you going to continue the work on it and promote it to a more stable version?

[Quuxplusone]

Sorry for the late reply.

Absolutely! It's top-prio for me at the moment. I've just added a new flag that makes dereferencing optional, which is disabled by default for the time being, partially based on your suggestion. There are a number of changes I'd like to see implemented first, but I think it isn't far from moving out of alpha :)