From b591845f4b482a2aec74f121f5f03da2ebefcef4 Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Fri, 26 Apr 2019 02:05:12 +0000 Subject: [PATCH] [analyzer] Fix crash when returning C++ objects from ObjC messages-to-nil. the assertion is in fact incorrect: there is a cornercase in Objective-C++ in which a C++ object is not constructed with a constructor, but merely zero-initialized. Namely, this happens when an Objective-C message is sent to a nil and it is supposed to return a C++ object. Differential Revision: https://reviews.llvm.org/D60988 llvm-svn: 359262 --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 9 ++++++++- clang/test/Analysis/nil-receiver.mm | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 clang/test/Analysis/nil-receiver.mm diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 4e1c73b..603be35 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2361,7 +2361,14 @@ RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B, // In C++17 aggregates may have base classes, handle those as well. // They appear before fields in the initializer list / compound value. if (const auto *CRD = dyn_cast(RD)) { - assert(CRD->isAggregate() && + // If the object was constructed with a constructor, its value is a + // LazyCompoundVal. If it's a raw CompoundVal, it means that we're + // performing aggregate initialization. The only exception from this + // rule is sending an Objective-C++ message that returns a C++ object + // to a nil receiver; in this case the semantics is to return a + // zero-initialized object even if it's a C++ object that doesn't have + // this sort of constructor; the CompoundVal is empty in this case. + assert((CRD->isAggregate() || (Ctx.getLangOpts().ObjC && VI == VE)) && "Non-aggregates are constructed with a constructor!"); for (const auto &B : CRD->bases()) { diff --git a/clang/test/Analysis/nil-receiver.mm b/clang/test/Analysis/nil-receiver.mm new file mode 100644 index 0000000..c462fce --- /dev/null +++ b/clang/test/Analysis/nil-receiver.mm @@ -0,0 +1,24 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \ +// RUN: -verify %s + +#define nil ((id)0) + +void clang_analyzer_eval(int); + +struct S { + int x; + S(); +}; + +@interface I +@property S s; +@end + +void foo() { + // This produces a zero-initialized structure. + // FIXME: This very fact does deserve the warning, because zero-initialized + // structures aren't always valid in C++. It's particularly bad when the + // object has a vtable. + S s = ((I *)nil).s; + clang_analyzer_eval(s.x == 0); // expected-warning{{TRUE}} +} -- 2.7.4