[analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

[Szelethus]

Summary

https://bugs.llvm.org/show_bug.cgi?id=41590

For the following code snippet, UninitializedObjectChecker crashed:

struct MyAtomicInt {
  _Atomic(int) x;
  MyAtomicInt() {}
};

void entry() {
  MyAtomicInt b;
}

The problem was that _Atomic types were not regular records, unions, dereferencable or primitive, making the checker hit the llvm_unreachable at [[ https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp#L347 |lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:347 ]]. The solution is to regard these types as primitive as well. The test case shows that with this addition, not only are we able to get rid of the crash, but we can identify x as uninitialized.

[alexfh]

LG. Thanks for the fix!

[gribozavr]

Thanks for the fix!

test/Analysis/cxx-uninitialized-object.cpp

1145

c11atomicTest()

[jfb]

Have you checked what the C and C++ standards say about _Atomic / std::atomic initialization, and how they should actually be initialized by developers?

[Szelethus]

In D61106#1479336, @jfb wrote:

Have you checked what the C and C++ standards say about _Atomic / std::atomic initialization, and how they should actually be initialized by developers?

No, I have not, I'll admit. Did I make a mistake here? Can you tell me where I can read up on it maybe?

[Szelethus]

Upon further investigation, this still seems to be okay. This is not a bug-finding checker, but rather a tool to enforce the idiom of every non-trivial object should be fully initialized after construction. From what I've seen, ATOMIC_VAR_INIT should be used here, but it isn't.

Do you think that I still should be more forgiving with atomics?

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h

bool addFieldToUninits(FieldChainInfo LocalChain,
                          const MemRegion *PointeeR = nullptr);
 };
 
 /// Returns true if T is a primitive type. An object of a primitive type only
 /// needs to be analyzed as much as checking whether their value is undefined.
 inline bool isPrimitiveType(const QualType &T) {
   return T->isBuiltinType() || T->isEnumeralType() ||
          T->isMemberPointerType() || T->isBlockPointerType() ||
-         T->isFunctionType();
+         T->isFunctionType() || T->isAtomicType();
 }
 
 inline bool isDereferencableType(const QualType &T) {
   return T->isAnyPointerType() || T->isReferenceType();
 }
 
 // Template method definitions.
 

test/Analysis/cxx-uninitialized-object.cpp

struct CXX11MemberInitTest2 {
 
   CXX11MemberInitTest2() {}
 };
 
 void fCXX11MemberInitTest2() {
   // TODO: we'd expect the warning: {{2 uninitializeds field}}
   CXX11MemberInitTest2(); // no-warning
 }
+
+//===----------------------------------------------------------------------===//
+// _Atomic tests.
+//===----------------------------------------------------------------------===//
+
+struct MyAtomicInt {
+  _Atomic(int) x; // expected-note{{uninitialized field 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
+};
+
+void entry() {
+  MyAtomicInt b;
+}