[PATCH] D25909: [analyzer] MacOSXApiChecker: Disallow dispatch_once predicates on heap and in ivars.

[Artem Dergachev via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp:94
+  else if (isa<UnknownSpaceRegion>(RS)) {
+    // FIXME: Presence of an IVar region has priority over this branch, because
+    // ObjC objects are on the heap even if the core doesn't realize this.
----------------
dcoughlin wrote:
> It is not clear to me that this FIXME is a good idea. I would remove it so someone doesn't spend a lot of time trying to address it.
> 
> Objective-C objects don't have the strong dis-aliasing guarantee that the analyzer assumes for heap base regions. In other words, two calls to [[Foo alloc] init] may yield exactly the same instance.  This is because, unlike malloc() and C++ global new, ObjC initializers can (and frequently do) return instances other than the passed-in, freshly-allocated self.
Hmm, that seems to be exactly the thing i'm looking for: heap-based regions that may alias.

The property of a region's staying on the heap has little to do with the property of being able to alias.

I've a feeling that we should have avoided using C++ inheritance in the memregion hierarchy, and instead went for a bunch of constraints. Eg., memory space is essentially a constraint (it may be unknown or get known later through exploring aliasing), region's value type is essentially a constraint (as seen during dynamic type propagation, it may be unknown, it may be partially known, we may get to know it better during the analysis by observing successful dynamic casts), extent is essentially a constraint (that we currently impose on SymbolExtent), offset of a symbolic region inside its true parent region is a constraint, and so on.

But that's too vague. I've no well-defined idea how to make this better at the moment.


https://reviews.llvm.org/D25909