[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

Umann Kristóf via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 26 07:57:26 PDT 2018


Szelethus marked 3 inline comments as done.
Szelethus added a comment.

By the way, thank you all for taking the time to review my code!



================
Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:306
+
+  const RecordDecl *RD =
+      R->getValueType()->getAs<RecordType>()->getDecl()->getDefinition();
----------------
a.sidorin wrote:
> What will happen if we analyze a member which is a pointer to a structure type and the structure is of incomplete type?
For that field `isPointerOrReferenceUninit` will be called. If it happens not to be null, unknown or undef, the CSA core will construct a symbolic region for it, and the checker always assumes that symbolic regions are initialized, meaning that we'll never call `isNonUnionUninit` for a field of incomplete type.

However, I did not cover this with tests, thanks for pointing it out!


================
Comment at: test/Analysis/ctor-uninitialized-member.cpp:554
+
+void f23p15() {
+  void *vptr = malloc(sizeof(int));
----------------
a.sidorin wrote:
> Could you please explain what is the logic of test naming?
The test files follow the strict structure that for each test case we'll define a structure type and then call its constructor(s) in a function right after it (functions are used to avoid zero initializations).

To be honest, there is no real logic behind the naming of the functions, it is only to keep the ODR. I used numbers in an increasing order, however I later added there cases, so in between `f23` and `f24` I used `f23p5` and so on.

I figured that the strict structure of the test files would avoid confusion. Do you find it distracting?


https://reviews.llvm.org/D45532





More information about the cfe-commits mailing list