Properly model the copy semantics of NSString.stringWithUTF8String and NSString.stringWithString.

Summary:
Both `stringWithUTF8String` and `stringWithString` implements copy semantics that copies the content of their parameter into a newly allocated buffer. We modeled this as pointer assignment in the past, which means that once we write
```
NSString* foo() {
  char buf[...];
  ...
  return [NSString stringWithUTF8String:buf];
}
```
We are going to get a spurious stack variable address escape report because local pointer `buf` is assigned to the newly created string and the string gets returned.
This diff tries to address the issue by heap-allocating a buffer and `memcpy` the contents in `stringWithUTF8String` and `stringWithString`. But this change will create another problem: the allocated buffer will be reported as leaked by the backend, while in reality those buffers won't actually be leaked as they are allocated in a region that will be periodically autoreleased. To suppress spurious memory leak FPs, I added another attribute `Awont_leak` that will suppress the leakage report on any expressions that get tagged with it.

Reviewed By: jeremydubreil

Differential Revision: D5403084

fbshipit-source-id: df6de7f
master
Jia Chen 7 years ago committed by Facebook Github Bot
parent bd90e80859
commit a8897c2412

@ -19,4 +19,5 @@
+ (instancetype)localizedStringWithFormat:(NSString*)format, ...; + (instancetype)localizedStringWithFormat:(NSString*)format, ...;
- (instancetype)initWithFormat:(NSString*)format, ...; - (instancetype)initWithFormat:(NSString*)format, ...;
- (instancetype)initWithFormat:(NSString*)format arguments:(va_list)argList; - (instancetype)initWithFormat:(NSString*)format arguments:(va_list)argList;
- (void)dealloc;
@end @end

@ -10,22 +10,33 @@
#import "NSString.h" #import "NSString.h"
#import <stdlib.h> #import <stdlib.h>
void __get_array_length(const UInt8); size_t __get_array_length(const void* arr);
void __infer_assume(bool cond); void __infer_assume(bool cond);
void __set_wont_leak_attribute(const void* ptr);
@implementation NSString @implementation NSString
+ (instancetype)stringWithUTF8String:(const char*)bytes { + (instancetype)stringWithUTF8String:(const char*)bytes {
// using alloc as the documentation doesn't say it may return nil // using alloc as the documentation doesn't say it may return nil
NSString* s = [NSString alloc]; NSString* s = [NSString alloc];
s->value = bytes; int len;
len = __get_array_length(bytes);
s->value = (const char*)malloc(len);
// The newly allocated string will be autoreleased by the runtime
__set_wont_leak_attribute(s->value);
memcpy(s->value, bytes, len);
return s; return s;
} }
+ (instancetype)stringWithString:(NSString*)aString { + (instancetype)stringWithString:(NSString*)aString {
NSString* s = [NSString alloc]; NSString* s = [NSString alloc];
s->value = aString->value; int len;
len = __get_array_length(aString->value);
s->value = (const char*)malloc(len);
// The newly allocated string will be autoreleased by the runtime
__set_wont_leak_attribute(s->value);
memcpy(s->value, aString->value, len);
return s; return s;
} }
@ -70,4 +81,11 @@ void __infer_assume(bool cond);
} }
} }
- (void)dealloc {
if (self != nil && self->value != 0) {
free(self->value);
}
[super dealloc];
}
@end @end

@ -107,6 +107,8 @@ module type S = sig
val __set_untaint_attribute : t val __set_untaint_attribute : t
val __set_wont_leak_attribute : t
val __split_get_nth : t val __split_get_nth : t
val __throw : t val __throw : t

@ -123,6 +123,8 @@ let __set_unsubscribed_observer_attribute = create_procname "__set_unsubscribed_
let __set_untaint_attribute = create_procname "__set_untaint_attribute" let __set_untaint_attribute = create_procname "__set_untaint_attribute"
let __set_wont_leak_attribute = create_procname "__set_wont_leak_attribute"
let __split_get_nth = create_procname "__split_get_nth" let __split_get_nth = create_procname "__split_get_nth"
let __throw = create_procname "__throw" let __throw = create_procname "__throw"

@ -121,6 +121,7 @@ type t =
| Aobserver (** denotes an object registered as an observers to a notification center *) | Aobserver (** denotes an object registered as an observers to a notification center *)
| Aunsubscribed_observer | Aunsubscribed_observer
(** denotes an object unsubscribed from observers of a notification center *) (** denotes an object unsubscribed from observers of a notification center *)
| Awont_leak (** value do not participate in memory leak analysis *)
[@@deriving compare] [@@deriving compare]
let equal = [%compare.equal : t] let equal = [%compare.equal : t]
@ -158,6 +159,7 @@ type category =
| ACundef | ACundef
| ACretval | ACretval
| ACobserver | ACobserver
| ACwontleak
[@@deriving compare] [@@deriving compare]
let equal_category = [%compare.equal : category] let equal_category = [%compare.equal : category]
@ -182,9 +184,13 @@ let to_category att =
-> ACundef -> ACundef
| Aobserver | Aunsubscribed_observer | Aobserver | Aunsubscribed_observer
-> ACobserver -> ACobserver
| Awont_leak
-> ACwontleak
let is_undef = function Aundef _ -> true | _ -> false let is_undef = function Aundef _ -> true | _ -> false
let is_wont_leak = function Awont_leak -> true | _ -> false
(** convert the attribute to a string *) (** convert the attribute to a string *)
let to_string pe = function let to_string pe = function
| Aresource ra | Aresource ra
@ -254,6 +260,8 @@ let to_string pe = function
-> "OBSERVER" -> "OBSERVER"
| Aunsubscribed_observer | Aunsubscribed_observer
-> "UNSUBSCRIBED_OBSERVER" -> "UNSUBSCRIBED_OBSERVER"
| Awont_leak
-> "WONT_LEAK"
(** dump an attribute *) (** dump an attribute *)
let d_attribute (a: t) = L.add_print_action (L.PTattribute, Obj.repr a) let d_attribute (a: t) = L.add_print_action (L.PTattribute, Obj.repr a)

@ -96,6 +96,7 @@ type t =
| Aobserver (** denotes an object registered as an observers to a notification center *) | Aobserver (** denotes an object registered as an observers to a notification center *)
| Aunsubscribed_observer | Aunsubscribed_observer
(** denotes an object unsubscribed from observers of a notification center *) (** denotes an object unsubscribed from observers of a notification center *)
| Awont_leak (** value do not participate in memory leak analysis *)
[@@deriving compare] [@@deriving compare]
val equal : t -> t -> bool val equal : t -> t -> bool
@ -117,6 +118,7 @@ type category =
| ACundef | ACundef
| ACretval | ACretval
| ACobserver | ACobserver
| ACwontleak
[@@deriving compare] [@@deriving compare]
val equal_category : category -> category -> bool val equal_category : category -> category -> bool
@ -126,6 +128,8 @@ val to_category : t -> category
val is_undef : t -> bool val is_undef : t -> bool
val is_wont_leak : t -> bool
val to_string : Pp.env -> t -> string val to_string : Pp.env -> t -> string
(** convert the attribute to a string *) (** convert the attribute to a string *)

@ -107,6 +107,8 @@ let get_observer tenv prop exp = get tenv prop exp ACobserver
let get_retval tenv prop exp = get tenv prop exp ACretval let get_retval tenv prop exp = get tenv prop exp ACretval
let get_wontleak tenv prop exp = get tenv prop exp ACwontleak
let has_dangling_uninit tenv prop exp = let has_dangling_uninit tenv prop exp =
let la = get_for_exp tenv prop exp in let la = get_for_exp tenv prop exp in
List.exists List.exists

@ -65,6 +65,9 @@ val get_taint : Tenv.t -> 'a Prop.t -> Exp.t -> Sil.atom option
val get_undef : Tenv.t -> 'a Prop.t -> Exp.t -> Sil.atom option val get_undef : Tenv.t -> 'a Prop.t -> Exp.t -> Sil.atom option
(** Get the undef attribute associated to the expression, if any *) (** Get the undef attribute associated to the expression, if any *)
val get_wontleak : Tenv.t -> 'a Prop.t -> Exp.t -> Sil.atom option
(** Get the wontleak attribute associated to the expression, if any *)
val has_dangling_uninit : Tenv.t -> 'a Prop.t -> Exp.t -> bool val has_dangling_uninit : Tenv.t -> 'a Prop.t -> Exp.t -> bool
(** Test for existence of an Adangling DAuninit attribute associated to the exp *) (** Test for existence of an Adangling DAuninit attribute associated to the exp *)

@ -636,6 +636,10 @@ let execute___set_unlocked_attribute ({Builtin.pdesc; loc} as builtin_args) : Bu
in in
execute___set_attr (PredSymb.Aresource ra) builtin_args execute___set_attr (PredSymb.Aresource ra) builtin_args
(** Set the attibute of the value as wont leak*)
let execute___set_wont_leak_attribute builtin_args : Builtin.ret_typ =
execute___set_attr PredSymb.Awont_leak builtin_args
(** Set the attibute of the value as tainted *) (** Set the attibute of the value as tainted *)
let execute___set_taint_attribute {Builtin.tenv; pdesc; args; prop_; path} : Builtin.ret_typ = let execute___set_taint_attribute {Builtin.tenv; pdesc; args; prop_; path} : Builtin.ret_typ =
match args with match args with
@ -1189,6 +1193,9 @@ let __set_unsubscribed_observer_attribute =
let __set_untaint_attribute = let __set_untaint_attribute =
Builtin.register BuiltinDecl.__set_untaint_attribute execute___set_untaint_attribute Builtin.register BuiltinDecl.__set_untaint_attribute execute___set_untaint_attribute
let __set_wont_leak_attribute =
Builtin.register BuiltinDecl.__set_wont_leak_attribute execute___set_wont_leak_attribute
(* splits a string given a separator and returns the nth string *) (* splits a string given a separator and returns the nth string *)
let __split_get_nth = Builtin.register BuiltinDecl.__split_get_nth execute___split_get_nth let __split_get_nth = Builtin.register BuiltinDecl.__split_get_nth execute___split_get_nth

@ -1232,6 +1232,11 @@ let check_junk ?original_prop pname tenv prop =
let res = ref None in let res = ref None in
let do_entry e = let do_entry e =
check_observer_is_unsubscribed_deallocation tenv prop e ; check_observer_is_unsubscribed_deallocation tenv prop e ;
match Attribute.get_wontleak tenv prop e with
| Some Apred ((Awont_leak as a), _)
-> L.d_strln "WONT_LEAK" ;
res := Some a
| _ ->
match Attribute.get_resource tenv prop e with match Attribute.get_resource tenv prop e with
| Some Apred ((Aresource {ra_kind= Racquire} as a), _) | Some Apred ((Aresource {ra_kind= Racquire} as a), _)
-> L.d_str "ATTRIBUTE: " ; -> L.d_str "ATTRIBUTE: " ;
@ -1241,7 +1246,8 @@ let check_junk ?original_prop pname tenv prop =
| _ -> | _ ->
match Attribute.get_undef tenv prop e with match Attribute.get_undef tenv prop e with
| Some Apred ((Aundef _ as a), _) | Some Apred ((Aundef _ as a), _)
-> res := Some a -> L.d_strln "UNDEF" ;
res := Some a
| _ | _
-> () -> ()
in in
@ -1285,6 +1291,8 @@ let check_junk ?original_prop pname tenv prop =
in in
let ignore_resource, exn = let ignore_resource, exn =
match (alloc_attribute, resource) with match (alloc_attribute, resource) with
| Some PredSymb.Awont_leak, Rmemory _
-> (true, exn_leak)
| Some _, Rmemory Mobjc when hpred_in_cycle hpred | Some _, Rmemory Mobjc when hpred_in_cycle hpred
-> (* When there is a cycle in objc we ignore it -> (* When there is a cycle in objc we ignore it
only if it's empty or it has weak or unsafe_unretained fields. only if it's empty or it has weak or unsafe_unretained fields.

@ -91,7 +91,6 @@ codetoanalyze/objc/shared/memory_leaks_benchmark/TollBridgeExample.m, TollBridge
codetoanalyze/objc/shared/memory_leaks_benchmark/TollBridgeExample.m, TollBridgeExample_bridge, 2, MEMORY_LEAK, [start of procedure bridge] codetoanalyze/objc/shared/memory_leaks_benchmark/TollBridgeExample.m, TollBridgeExample_bridge, 2, MEMORY_LEAK, [start of procedure bridge]
codetoanalyze/objc/shared/memory_leaks_benchmark/TollBridgeExample.m, bridgeDictionaryNoLeak, 1, UNINITIALIZED_VALUE, [start of procedure bridgeDictionaryNoLeak()] codetoanalyze/objc/shared/memory_leaks_benchmark/TollBridgeExample.m, bridgeDictionaryNoLeak, 1, UNINITIALIZED_VALUE, [start of procedure bridgeDictionaryNoLeak()]
codetoanalyze/objc/errors/memory_leaks_benchmark/NSData_models_tests.m, NSData_models_tests_macForIV:, 2, MEMORY_LEAK, [start of procedure macForIV:] codetoanalyze/objc/errors/memory_leaks_benchmark/NSData_models_tests.m, NSData_models_tests_macForIV:, 2, MEMORY_LEAK, [start of procedure macForIV:]
codetoanalyze/objc/errors/memory_leaks_benchmark/NSString_models_tests.m, createURLQueryStringBodyEscaping, 6, PRECONDITION_NOT_MET, [start of procedure createURLQueryStringBodyEscaping(),Condition is true]
codetoanalyze/objc/errors/memory_leaks_benchmark/NSString_models_tests.m, createURLQueryStringBodyEscaping, 11, UNINITIALIZED_VALUE, [start of procedure createURLQueryStringBodyEscaping(),Condition is false] codetoanalyze/objc/errors/memory_leaks_benchmark/NSString_models_tests.m, createURLQueryStringBodyEscaping, 11, UNINITIALIZED_VALUE, [start of procedure createURLQueryStringBodyEscaping(),Condition is false]
codetoanalyze/objc/errors/memory_leaks_benchmark/RetainReleaseExampleBucketing.m, RetainReleaseTest, 0, RETURN_VALUE_IGNORED, [start of procedure RetainReleaseTest()] codetoanalyze/objc/errors/memory_leaks_benchmark/RetainReleaseExampleBucketing.m, RetainReleaseTest, 0, RETURN_VALUE_IGNORED, [start of procedure RetainReleaseTest()]
codetoanalyze/objc/errors/npe/Fraction.m, test_virtual_call, 7, NULL_DEREFERENCE, [start of procedure test_virtual_call(),start of procedure setNumerator:,return from a call to Fraction_setNumerator:,start of procedure getNumerator,return from a call to Fraction_getNumerator,Condition is true] codetoanalyze/objc/errors/npe/Fraction.m, test_virtual_call, 7, NULL_DEREFERENCE, [start of procedure test_virtual_call(),start of procedure setNumerator:,return from a call to Fraction_setNumerator:,start of procedure getNumerator,return from a call to Fraction_getNumerator,Condition is true]

@ -30,6 +30,17 @@ NSString* createURLQueryStringBodyEscaping(NSDictionary* parameters,
return resultString; return resultString;
} }
NSString* stringWithUTF8StringNoLeakOk(const char* buf) {
return [NSString stringWithUTF8String:buf];
}
NSString* stringWithStringNoLeakOk(const char* buf) {
return [NSString stringWithString:buf];
}
- (void)NSAssertNoLeakOk {
NSAssert(true, @"This string does not create any memory leak");
}
- (NSString*)hexStringValue { - (NSString*)hexStringValue {
size_t hexLen = 2 * 10 * sizeof(char); size_t hexLen = 2 * 10 * sizeof(char);
char* outString = (char*)malloc(hexLen + 1); char* outString = (char*)malloc(hexLen + 1);

Loading…
Cancel
Save