[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1127
+        // only consist of ObjC objects, and escapes of ObjC objects
+        // aren't so important (eg., retain count checker ignores them).
+        if (isa<CXXStdInitializerListExpr>(Ex) ||
----------------
dcoughlin wrote:
> Note that we do have other ObjC checkers that rely on escaping of ObjC objects, such as the ObjCLoopChecker and ObjCDeallocChecker. I think having the TODO is great, but I'd like you to remove the the bit about "escapes of ObjC objects aren't so important".
Hmm, should have double-checked. Will fix.


================
Comment at: test/Analysis/objc-boxing.m:66
+  BoxableStruct bs;
+  bs.str = strdup("dynamic string"); // The duped string shall be owned by val.
+  NSValue *val = @(bs); // no-warning
----------------
dcoughlin wrote:
> In this case the duped string is not owned by `val`. NSValue doesn't take ownership of the string, so this *will* leak and we should warn about it.
I mean, the pointer to the raw string is stored inside the `NSValue`, and can be used or freed from there. The caller can free this string by looking into the `val`, even though `val` itself won't release the pointer (i guess i messed up the comment again). From MallocChecker's perspective, this is an escape and no-warning. If we free the string in this function, it'd most likely cause use-after-free in the caller.

I tested that the string is indeed not strduped during boxing:

**`$ cat test.m`**
```
#import <Foundation/Foundation.h>

typedef struct __attribute__((objc_boxable)) {
  const char *str;
} BoxableStruct;

int main() {
  BoxableStruct bs;
  bs.str = strdup("dynamic string");
  NSLog(@"%p\n", bs.str);

  NSValue *val = @(bs);

  BoxableStruct bs2;
  [val getValue:&bs2];
  NSLog(@"%p\n", bs2.str);

  return 0;
}
```
**`$ clang test.m -framework Foundation`**
**`$ ./a.out`**
```
2017-10-02 17:56:00.004 a.out[17933:1083757] 0x7ffd23407380
2017-10-02 17:56:00.004 a.out[17933:1083757] 0x7ffd23407380
```
So it's possible to retrieve the exact same pointer from the boxed value. So if `val` is returned to the caller, like in the test, it shouldn't be freed.

If the `NSValue` itself dies and never escapes, then of course it's a leak, but in order to see that we'd need to model contents of `NSValue`.


https://reviews.llvm.org/D35216