[clang frontend] Translate BinaryOperators in ifs equally whether they are part of a ExprWithCleanups or not

Summary: I noticed when looking into a false positive of strongSelf Not Checked, that there were some inconsistencies in the translation of if statements with an and, with an extra redundant join only if using a method in the condition that returned an object. So I could repro the problem and investigate and found the place of the inconsistency in the translation. This diff fixes it without changing things too much.

Reviewed By: jvillard

Differential Revision: D19518368

fbshipit-source-id: 47a6a778c
master
Dulma Churchill 5 years ago committed by Facebook Github Bot
parent 5a1641d319
commit 430727519b

@ -1651,7 +1651,8 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
* she need to change both the codes here and the `match` in * she need to change both the codes here and the `match` in
* binaryOperator_trans_with_cond *) * binaryOperator_trans_with_cond *)
match cond with match cond with
| BinaryOperator (si, ss, ei, boi) -> | BinaryOperator (si, ss, ei, boi)
| ExprWithCleanups (_, [BinaryOperator (si, ss, ei, boi)], _, _) ->
binaryOperator_trans trans_state boi si ei ss binaryOperator_trans trans_state boi si ei ss
| _ -> | _ ->
instruction trans_state cond instruction trans_state cond
@ -1719,7 +1720,8 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
L.(debug Capture Verbose) "Translating Condition for If-then-else/Loop/Conditional Operator @\n" ; L.(debug Capture Verbose) "Translating Condition for If-then-else/Loop/Conditional Operator @\n" ;
let open Clang_ast_t in let open Clang_ast_t in
match cond with match cond with
| BinaryOperator (_, [s1; s2], _, boi) -> ( | BinaryOperator (_, [s1; s2], _, boi)
| ExprWithCleanups (_, [BinaryOperator (_, [s1; s2], _, boi)], _, _) -> (
match boi.boi_kind with match boi.boi_kind with
| `LAnd -> | `LAnd ->
short_circuit (if negate_cond then Binop.LOr else Binop.LAnd) s1 s2 short_circuit (if negate_cond then Binop.LOr else Binop.LAnd) s1 s2

@ -40,9 +40,9 @@ codetoanalyze/objc/errors/memory_leaks_benchmark/retain_cycle2.m, strongcycle2,
codetoanalyze/objc/errors/npe/UpdateDict.m, add_nil_in_dict, 10, NULL_DEREFERENCE, B1, ERROR, [start of procedure add_nil_in_dict(),Skipping dictionaryWithObjectsAndKeys:: method has no implementation] codetoanalyze/objc/errors/npe/UpdateDict.m, add_nil_in_dict, 10, NULL_DEREFERENCE, B1, ERROR, [start of procedure add_nil_in_dict(),Skipping dictionaryWithObjectsAndKeys:: method has no implementation]
codetoanalyze/objc/errors/npe/UpdateDict.m, add_nil_to_array, 4, NULL_DEREFERENCE, B1, ERROR, [start of procedure add_nil_to_array()] codetoanalyze/objc/errors/npe/UpdateDict.m, add_nil_to_array, 4, NULL_DEREFERENCE, B1, ERROR, [start of procedure add_nil_to_array()]
codetoanalyze/objc/errors/npe/UpdateDict.m, insert_nil_in_array, 4, NULL_DEREFERENCE, B1, ERROR, [start of procedure insert_nil_in_array()] codetoanalyze/objc/errors/npe/UpdateDict.m, insert_nil_in_array, 4, NULL_DEREFERENCE, B1, ERROR, [start of procedure insert_nil_in_array()]
codetoanalyze/objc/errors/npe/UpdateDict.m, nullable_NSDictionary_objectForKey, 4, NULL_DEREFERENCE, B5, ERROR, [start of procedure nullable_NSDictionary_objectForKey(),Taking true branch,Condition is true,Taking true branch] codetoanalyze/objc/errors/npe/UpdateDict.m, nullable_NSDictionary_objectForKey, 4, NULL_DEREFERENCE, B5, ERROR, [start of procedure nullable_NSDictionary_objectForKey(),Taking true branch,Taking true branch]
codetoanalyze/objc/errors/npe/UpdateDict.m, nullable_NSDictionary_objectForKeyedSubscript, 5, NULL_DEREFERENCE, B5, ERROR, [start of procedure nullable_NSDictionary_objectForKeyedSubscript(),Taking true branch,Condition is true,Taking true branch] codetoanalyze/objc/errors/npe/UpdateDict.m, nullable_NSDictionary_objectForKeyedSubscript, 5, NULL_DEREFERENCE, B5, ERROR, [start of procedure nullable_NSDictionary_objectForKeyedSubscript(),Taking true branch,Taking true branch]
codetoanalyze/objc/errors/npe/UpdateDict.m, nullable_NSMapTable_objectForKey, 4, NULL_DEREFERENCE, B5, ERROR, [start of procedure nullable_NSMapTable_objectForKey(),Taking true branch,Condition is true,Taking true branch] codetoanalyze/objc/errors/npe/UpdateDict.m, nullable_NSMapTable_objectForKey, 4, NULL_DEREFERENCE, B5, ERROR, [start of procedure nullable_NSMapTable_objectForKey(),Taking true branch,Taking true branch]
codetoanalyze/objc/errors/npe/UpdateDict.m, update_array_with_null, 5, NULL_DEREFERENCE, B1, ERROR, [start of procedure update_array_with_null()] codetoanalyze/objc/errors/npe/UpdateDict.m, update_array_with_null, 5, NULL_DEREFERENCE, B1, ERROR, [start of procedure update_array_with_null()]
codetoanalyze/objc/errors/npe/UpdateDict.m, update_dict_with_key_null, 10, NULL_DEREFERENCE, B1, ERROR, [start of procedure update_dict_with_key_null(),Skipping dictionaryWithObjectsAndKeys:: method has no implementation] codetoanalyze/objc/errors/npe/UpdateDict.m, update_dict_with_key_null, 10, NULL_DEREFERENCE, B1, ERROR, [start of procedure update_dict_with_key_null(),Skipping dictionaryWithObjectsAndKeys:: method has no implementation]
codetoanalyze/objc/errors/npe/nil_in_array_literal.m, Arr::insertNilBad, 3, NULL_DEREFERENCE, B1, ERROR, [start of procedure insertNilBad,start of procedure initA,Taking true branch,return from a call to A::initA] codetoanalyze/objc/errors/npe/nil_in_array_literal.m, Arr::insertNilBad, 3, NULL_DEREFERENCE, B1, ERROR, [start of procedure insertNilBad,start of procedure initA,Taking true branch,return from a call to A::initA]

@ -8,6 +8,7 @@ TESTS_DIR = ../../../..
CLANG_OPTIONS = -c $(OBJC_CLANG_OPTIONS) -fobjc-arc CLANG_OPTIONS = -c $(OBJC_CLANG_OPTIONS) -fobjc-arc
SOURCES = \ SOURCES = \
../if_and_arc/If_and_arc.m \
../predefined_expr/PredefinedExprExample.m \ ../predefined_expr/PredefinedExprExample.m \
../types/attributes.m \ ../types/attributes.m \
../types/void_call.m \ ../types/void_call.m \

@ -0,0 +1,27 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
#include <Foundation/NSObject.h>
id foo();
bool foo_bool();
int call_bool_function_in_and_cond_in_if() {
int* p;
if (p && foo_bool()) {
return 5;
} else
return 4;
}
int call_object_function_in_and_cond_in_if() {
int* p;
if (p && foo()) {
return 5;
} else
return 4;
}

@ -0,0 +1,93 @@
/* @generated */
digraph cfg {
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_1" [label="1: Start call_bool_function_in_and_cond_in_if\nFormals: \nLocals: p:int* \n " color=yellow style=filled]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_1" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_5" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_1" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_6" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_2" [label="2: Exit call_bool_function_in_and_cond_in_if \n " color=yellow style=filled]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_3" [label="3: + \n " ]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_3" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_4" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_4" [label="4: between_join_and_exit \n " shape="box"]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_4" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_2" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_5" [label="5: Prune (true branch, if) \n n$0=*&p:int* [line 15, column 7]\n PRUNE(n$0, true); [line 15, column 7]\n " shape="invhouse"]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_5" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_7" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_6" [label="6: Prune (false branch, if) \n n$0=*&p:int* [line 15, column 7]\n PRUNE(!n$0, false); [line 15, column 7]\n " shape="invhouse"]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_6" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_11" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_7" [label="7: Call _fun_foo_bool \n n$1=_fun_foo_bool() [line 15, column 12]\n " shape="box"]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_7" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_8" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_7" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_9" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_8" [label="8: Prune (true branch, if) \n PRUNE(n$1, true); [line 15, column 12]\n " shape="invhouse"]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_8" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_10" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_9" [label="9: Prune (false branch, if) \n PRUNE(!n$1, false); [line 15, column 12]\n " shape="invhouse"]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_9" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_11" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_10" [label="10: Return Stmt \n *&return:int=5 [line 16, column 5]\n " shape="box"]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_10" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_2" ;
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_11" [label="11: Return Stmt \n *&return:int=4 [line 18, column 5]\n " shape="box"]
"call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_11" -> "call_bool_function_in_and_cond_in_if.acba4fbddb9f2790bb3c01ace140ce3f_2" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_1" [label="1: Start call_object_function_in_and_cond_in_if\nFormals: \nLocals: p:int* \n " color=yellow style=filled]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_1" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_5" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_1" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_6" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_2" [label="2: Exit call_object_function_in_and_cond_in_if \n " color=yellow style=filled]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_3" [label="3: + \n " ]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_3" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_4" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_4" [label="4: between_join_and_exit \n " shape="box"]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_4" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_2" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_5" [label="5: Prune (true branch, if) \n n$0=*&p:int* [line 23, column 7]\n PRUNE(n$0, true); [line 23, column 7]\n " shape="invhouse"]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_5" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_7" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_6" [label="6: Prune (false branch, if) \n n$0=*&p:int* [line 23, column 7]\n PRUNE(!n$0, false); [line 23, column 7]\n " shape="invhouse"]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_6" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_11" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_7" [label="7: Call _fun_foo \n n$1=_fun_foo() [line 23, column 12]\n " shape="box"]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_7" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_8" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_7" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_9" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_8" [label="8: Prune (true branch, if) \n PRUNE(n$1, true); [line 23, column 12]\n " shape="invhouse"]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_8" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_10" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_9" [label="9: Prune (false branch, if) \n PRUNE(!n$1, false); [line 23, column 12]\n " shape="invhouse"]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_9" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_11" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_10" [label="10: Return Stmt \n *&return:int=5 [line 24, column 5]\n " shape="box"]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_10" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_2" ;
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_11" [label="11: Return Stmt \n *&return:int=4 [line 26, column 5]\n " shape="box"]
"call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_11" -> "call_object_function_in_and_cond_in_if.1ada6e5c400b531e58154167f56912e3_2" ;
}

@ -15,6 +15,11 @@
- (void)use_self_in_block_test_nullable:(int)x - (void)use_self_in_block_test_nullable:(int)x
and:(_Nullable SelfInBlockTest*)test; and:(_Nullable SelfInBlockTest*)test;
@end
@interface A
@property(nonatomic, strong) id image;
@end @end
@ -146,6 +151,17 @@ void m2(_Nullable SelfInBlockTest* obj) {}
}; };
} }
- (void)strongSelfNotCheck5_good:(A*)a {
__weak __typeof(self) weakSelf = self;
int (^my_block)() = ^() {
__strong __typeof(weakSelf) strongSelf = weakSelf;
if (strongSelf && a.image) {
int x = strongSelf->x; // no bug here
}
return 0;
};
}
- (void)wekSelfMultiple_bad { - (void)wekSelfMultiple_bad {
__weak __typeof(self) weakSelf = self; __weak __typeof(self) weakSelf = self;
int (^my_block)(BOOL) = ^(BOOL isTapped) { int (^my_block)(BOOL) = ^(BOOL isTapped) {

@ -3,4 +3,4 @@ codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strong
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 6, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null] codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 6, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 8, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null] codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 8, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 10, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null] codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 10, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null,Using &strongSelf not checked for null]
codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::wekSelfMultiple_bad_9, 1, MULTIPLE_WEAKSELF, no_bucket, ERROR, [Using &weakSelf,Using &weakSelf] codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::wekSelfMultiple_bad_10, 1, MULTIPLE_WEAKSELF, no_bucket, ERROR, [Using &weakSelf,Using &weakSelf]

Loading…
Cancel
Save