This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by Szelethus on Apr 24 2019, 5:25 PM.

Details

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.

Diff Detail

Event Timeline

Szelethus created this revision.Apr 24 2019, 5:25 PM
Szelethus edited the summary of this revision. (Show Details)Apr 24 2019, 5:32 PM
alexfh accepted this revision.Apr 24 2019, 5:40 PM
alexfh added a subscriber: alexfh.

LG. Thanks for the fix!

This revision is now accepted and ready to land.Apr 24 2019, 5:40 PM
gribozavr accepted this revision.Apr 24 2019, 11:32 PM

Thanks for the fix!

test/Analysis/cxx-uninitialized-object.cpp
1145

c11atomicTest()

This revision was automatically updated to reflect the committed changes.
jfb added a comment.Apr 25 2019, 1:34 PM

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

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?

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?