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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
