-Wuninitialized false positive due to control dependency.

[nico]

Bugzilla Link	9122
Version	unspecified
OS	All
CC	@tkremenek

Extended Description

Spun off from http://llvm.org/bugs/show_bug.cgi?id=9063#c11 (see there for attachment):

Here's another example that looks fairly similar that I
still see with your patch.

Compile like this:
~/src/llvm-svn/Release+Asserts/bin/clang -arch i386 -c repro.ii
-Wuninitialized

Warnings:
In file included from
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:1:
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:662:3:
warning: use of uninitialized variable 'last_accessed' [-Wuninitialized]
double last_accessed;
^~~~~~~~~~~~~~~~~~~~
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:672:48:
note: variable 'last_accessed' is possibly uninitialized when used here
p->last_accessed = base::Time::FromDoubleT(last_accessed);
^~~~~~~~~~~~~
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:662:23:
note: add initialization to silence this warning
double last_accessed;
^
= 0.0
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:661:3:
warning: use of uninitialized variable 'last_modified' [-Wuninitialized]
double last_modified;
^~~~~~~~~~~~~~~~~~~~
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:671:48:
note: variable 'last_modified' is possibly uninitialized when used here
p->last_modified = base::Time::FromDoubleT(last_modified);
^~~~~~~~~~~~~
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:661:23:
note: add initialization to silence this warning
double last_modified;
^
= 0.0
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:663:3:
warning: use of uninitialized variable 'creation_time' [-Wuninitialized]
double creation_time;
^~~~~~~~~~~~~~~~~~~~
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:673:48:
note: variable 'creation_time' is possibly uninitialized when used here
p->creation_time = base::Time::FromDoubleT(creation_time);
^~~~~~~~~~~~~
/Volumes/MacintoshHD2/src/chrome-git/src/chrome/common/common_param_traits.cc:663:23:
note: add initialization to silence this warning
double creation_time;
^
= 0.0
3 warnings generated.

Relevant code:
bool ParamTraitsbase::PlatformFileInfo::Read(
const Message* m, void** iter, param_type* p) {
double last_modified;
double last_accessed;
double creation_time;
bool result =
ReadParam(m, iter, &p->size) &&
ReadParam(m, iter, &p->is_directory) &&
ReadParam(m, iter, &last_modified) &&
ReadParam(m, iter, &last_accessed) &&
ReadParam(m, iter, &creation_time);
if (result) {
p->last_modified = base::Time::FromDoubleT(last_modified);
p->last_accessed = base::Time::FromDoubleT(last_accessed);
p->creation_time = base::Time::FromDoubleT(creation_time);
}
return result;
}

This example is much harder because it involves tracking a control-dependency,
e.g.:

bool result = ...
...
if (result) {
...
}

The static analyzer handles this easily, but I'm not certain how much
control-dependencies like this we should attempt (if any) to model with
-Wuninitialized.

[nico]

assigned to @tkremenek

[nico]

It's a bit hard to tell how common this is now because issue 9117 adds quite a few compile errors to chrome. Once that's resolved I will update this bug. (I did see this a few times tho.)

[nico]

Of the 19 warnings that are left, 10 are this issue (and ~2 are not false positives but valid warnings).

[nico]

11, not 10. I guess this is this issue too:

int* f() {
int* a;
bool b = true;
switch (4) {
case 4:
a = 0;
break;
case 5:
b = false;
break;
default:
return 0;
}
if (b)
return a;
return 0;
}

(Or is this more like issue 9062?)

[tkremenek]

Of the 19 warnings that are left, 10 are this issue (and ~2 are not false
positives but valid warnings).

Of the ~2 issues that are valid, does GCC pick them up?

[tkremenek]

11, not 10. I guess this is this issue too:

int* f() {
int* a;
bool b = true;
switch (4) {
case 4:
a = 0;
break;
case 5:
b = false;
break;
default:
return 0;
}
if (b)
return a;
return 0;
}

(Or is this more like issue 9062?)

I highly doubt GCC actually reasons about this correctly, and by luck (or suppression) doesn't report the issue.