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

Devin Coughlin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 2 10:08:06 PDT 2017


dcoughlin accepted this revision.
dcoughlin added inline comments.
This revision is now accepted and ready to land.


================
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
----------------
NoQ wrote:
> 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`.
You're absolutely right about this. The documentation for NSValue even states: "Important: The NSValue object doesn’t copy the contents of the string, but the pointer itself. If you create an NSValue object with an allocated data item, don’t free the data’s memory while the NSValue object exists."

 Sorry! My mistake.


https://reviews.llvm.org/D35216





More information about the cfe-commits mailing list