From 5adab3cb61ea3f0c9582c163bd9ddfa70df18f1f Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Tue, 7 Jun 2016 05:13:54 -0700 Subject: [PATCH] Fix StmtExpr Summary: The extra dereference in stmtexpr was wrong. When a dereference is needed, we have a cast. This was causing one dereference too many, and creating wrong results. Reviewed By: akotulski Differential Revision: D3393294 fbshipit-source-id: 7a1ec8e --- infer/src/clang/cTrans.ml | 26 ++---- .../c/errors/memory_leaks/test.c | 7 ++ .../c/frontend/nestedoperators/gnuexpr.c.dot | 4 +- .../subtyping/cast_with_enforce.cpp.dot | 4 +- .../objc/frontend/block/dispatch.dot | 83 ++++++++++++++----- .../objc/frontend/block/dispatch.m | 20 ++++- .../endtoend/objc/BlockDispatchTest.java | 11 +-- 7 files changed, 105 insertions(+), 50 deletions(-) diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index 695adc9ff..890e3a392 100644 --- a/infer/src/clang/cTrans.ml +++ b/infer/src/clang/cTrans.ml @@ -1441,27 +1441,15 @@ struct { empty_res_trans with root_nodes = top_nodes; leaf_nodes = succ_nodes } | _ -> assert false - and stmtExpr_trans trans_state stmt_info stmt_list = - let context = trans_state.context in + and stmtExpr_trans trans_state stmt_list = let stmt = - extract_stmt_from_singleton stmt_list - "ERROR: StmtExpr should have only one statement.\n" in + extract_stmt_from_singleton stmt_list "ERROR: StmtExpr should have only one statement.\n" in let res_trans_stmt = instruction trans_state stmt in let exps' = IList.rev res_trans_stmt.exps in match exps' with - | (last, typ) :: _ -> - (* The StmtExpr contains a single CompoundStmt node, which it evaluates and *) - (* takes the value of the last subexpression. *) - (* Exp returned by StmtExpr is always a RValue. - So we need to assign to a temp and return the temp. *) - let id = Ident.create_fresh Ident.knormal in - let loc = CLocation.get_sil_location stmt_info context in - let instr' = Sil.Letderef (id, last, typ, loc) in - { res_trans_stmt with - instrs = res_trans_stmt.instrs @ [instr']; - exps = [(Sil.Var id, typ)]; - } - | _ -> assert false + | last_exp :: _ -> + { res_trans_stmt with exps = [last_exp]; } + | [] -> res_trans_stmt and loop_instruction trans_state loop_kind stmt_info = let outer_continuation = trans_state.continuation in @@ -2369,8 +2357,8 @@ struct "FATAL: Passing from CaseStmt outside of SwitchStmt, terminating.\n"; assert false - | StmtExpr(stmt_info, stmt_list, _) -> - stmtExpr_trans trans_state stmt_info stmt_list + | StmtExpr(_, stmt_list, _) -> + stmtExpr_trans trans_state stmt_list | ForStmt(stmt_info, [init; decl_stmt; cond; incr; body]) -> forStmt_trans trans_state init decl_stmt cond incr body stmt_info diff --git a/infer/tests/codetoanalyze/c/errors/memory_leaks/test.c b/infer/tests/codetoanalyze/c/errors/memory_leaks/test.c index df69d18a7..44149a6ed 100644 --- a/infer/tests/codetoanalyze/c/errors/memory_leaks/test.c +++ b/infer/tests/codetoanalyze/c/errors/memory_leaks/test.c @@ -52,3 +52,10 @@ void conditional_last_instruction() { free(p); } } + +int* compound_return_no_leak() { + return ({ + int* p = malloc(sizeof(int)); + p; + }); +} diff --git a/infer/tests/codetoanalyze/c/frontend/nestedoperators/gnuexpr.c.dot b/infer/tests/codetoanalyze/c/frontend/nestedoperators/gnuexpr.c.dot index 6fa4fdb3b..ee9c7240a 100644 --- a/infer/tests/codetoanalyze/c/frontend/nestedoperators/gnuexpr.c.dot +++ b/infer/tests/codetoanalyze/c/frontend/nestedoperators/gnuexpr.c.dot @@ -1,6 +1,6 @@ /* @generated */ digraph iCFG { -8 [label="8: Return Stmt \n n$2=*&p:int * [line 22]\n n$3=*n$2:int [line 22]\n *&x:int =n$3 [line 22]\n *&y:int =1 [line 23]\n n$0=*&x:int [line 24]\n n$1=*&y:int [line 24]\n n$4=*(n$0 + n$1):int [line 21]\n *&return:int =n$4 [line 21]\n " shape="box"] +8 [label="8: Return Stmt \n n$2=*&p:int * [line 22]\n n$3=*n$2:int [line 22]\n *&x:int =n$3 [line 22]\n *&y:int =1 [line 23]\n n$0=*&x:int [line 24]\n n$1=*&y:int [line 24]\n *&return:int =(n$0 + n$1) [line 21]\n " shape="box"] 8 -> 7 ; @@ -15,7 +15,7 @@ digraph iCFG { 5 -> 4 ; -4 [label="4: BinaryOperatorStmt: Assign \n *&X:int =4 [line 14]\n n$0=*&X:int [line 15]\n n$1=*n$0:int [line 13]\n *&y:int =n$1 [line 13]\n " shape="box"] +4 [label="4: BinaryOperatorStmt: Assign \n *&X:int =4 [line 14]\n n$0=*&X:int [line 15]\n *&y:int =n$0 [line 13]\n " shape="box"] 4 -> 3 ; diff --git a/infer/tests/codetoanalyze/cpp/errors/subtyping/cast_with_enforce.cpp.dot b/infer/tests/codetoanalyze/cpp/errors/subtyping/cast_with_enforce.cpp.dot index 93fe4d236..d9b93c0f9 100644 --- a/infer/tests/codetoanalyze/cpp/errors/subtyping/cast_with_enforce.cpp.dot +++ b/infer/tests/codetoanalyze/cpp/errors/subtyping/cast_with_enforce.cpp.dot @@ -23,7 +23,7 @@ digraph iCFG { 104 -> 103 ; -103 [label="103: DeclStmt \n n$11=_fun___cast(&base:class Base *,sizeof(class Derived ( sub )(cast)):void ) [line 47]\n *&derived:class Derived *=n$11 [line 47]\n " shape="box"] +103 [label="103: DeclStmt \n n$10=_fun___cast(&base:class Base *,sizeof(class Derived ( sub )(cast)):void ) [line 47]\n *&derived:class Derived *=n$10 [line 47]\n " shape="box"] 103 -> 102 ; @@ -67,7 +67,7 @@ digraph iCFG { 93 -> 104 ; -92 [label="92: DeclStmt \n n$11=*&certificate:class Base & [line 40]\n n$12=_fun___cast(n$11:class Base *,sizeof(class Derived ( sub )(cast)):void ) [line 40]\n *&cert:class Derived *=n$12 [line 40]\n " shape="box"] +92 [label="92: DeclStmt \n n$10=*&certificate:class Base & [line 40]\n n$11=_fun___cast(n$10:class Base *,sizeof(class Derived ( sub )(cast)):void ) [line 40]\n *&cert:class Derived *=n$11 [line 40]\n " shape="box"] 92 -> 91 ; diff --git a/infer/tests/codetoanalyze/objc/frontend/block/dispatch.dot b/infer/tests/codetoanalyze/objc/frontend/block/dispatch.dot index af7b7c8fd..4308b2ee2 100644 --- a/infer/tests/codetoanalyze/objc/frontend/block/dispatch.dot +++ b/infer/tests/codetoanalyze/objc/frontend/block/dispatch.dot @@ -1,49 +1,90 @@ /* @generated */ digraph iCFG { -34 [label="34: DeclStmt \n n$3=_fun_A_sharedInstance() [line 54]\n *&b:class A *=n$3 [line 54]\n " shape="box"] +45 [label="45: DeclStmt \n n$3=_fun_A_sharedInstance() [line 72]\n *&b:class A *=n$3 [line 72]\n " shape="box"] + + + 45 -> 44 ; +44 [label="44: DeclStmt \n *&p:int *=0 [line 73]\n " shape="box"] + + + 44 -> 39 ; +43 [label="43: Return Stmt \n *&return:int =0 [line 77]\n " shape="box"] + + + 43 -> 36 ; +42 [label="42: Return Stmt \n n$1=*&p:int * [line 75]\n n$2=*n$1:int [line 75]\n *&return:int =n$2 [line 75]\n " shape="box"] + + + 42 -> 36 ; +41 [label="41: Prune (false branch) \n PRUNE(((n$0 == 0) == 0), false); [line 74]\n " shape="invhouse"] + + + 41 -> 43 ; +40 [label="40: Prune (true branch) \n PRUNE(((n$0 == 0) != 0), true); [line 74]\n " shape="invhouse"] + + + 40 -> 42 ; +39 [label="39: BinaryOperatorStmt: EQ \n n$0=*&b:class A * [line 74]\n " shape="box"] + + + 39 -> 40 ; + 39 -> 41 ; +38 [label="38: between_join_and_exit \n " shape="box"] + + + 38 -> 36 ; +37 [label="37: + \n " ] + + + 37 -> 38 ; +36 [label="36: Exit main \n " color=yellow style=filled] + + +35 [label="35: Start main\nFormals: \nLocals: p:int * b:class A * \n DECLARE_LOCALS(&return,&p,&b); [line 71]\n " color=yellow style=filled] + + + 35 -> 45 ; +34 [label="34: DeclStmt \n n$29=_fun_A_dispatch_a_block_variable_from_macro() [line 64]\n *&a:class A *=n$29 [line 64]\n " shape="box"] 34 -> 33 ; -33 [label="33: DeclStmt \n *&p:int *=0 [line 55]\n " shape="box"] +33 [label="33: BinaryOperatorStmt: Assign \n n$28=*&a:class A * [line 65]\n *n$28._x:int =5 [line 65]\n " shape="box"] - 33 -> 28 ; -32 [label="32: Return Stmt \n *&return:int =0 [line 59]\n " shape="box"] + 33 -> 32 ; +32 [label="32: Return Stmt \n n$26=*&a:class A * [line 66]\n n$27=*n$26._x:int [line 66]\n *&return:int =(1 / (n$27 - 5)) [line 66]\n " shape="box"] - 32 -> 25 ; -31 [label="31: Return Stmt \n n$1=*&p:int * [line 57]\n n$2=*n$1:int [line 57]\n *&return:int =n$2 [line 57]\n " shape="box"] + 32 -> 31 ; +31 [label="31: Exit A_dispatch_a_block_variable_from_macro_delivers_initialised_object \n " color=yellow style=filled] - 31 -> 25 ; -30 [label="30: Prune (false branch) \n PRUNE(((n$0 == 0) == 0), false); [line 56]\n " shape="invhouse"] +30 [label="30: Start A_dispatch_a_block_variable_from_macro_delivers_initialised_object\nFormals: \nLocals: a:class A * \n DECLARE_LOCALS(&return,&a); [line 63]\n " color=yellow style=filled] - 30 -> 32 ; -29 [label="29: Prune (true branch) \n PRUNE(((n$0 == 0) != 0), true); [line 56]\n " shape="invhouse"] + 30 -> 34 ; +29 [label="29: Return Stmt \n DECLARE_LOCALS(&__objc_anonymous_block_A_dispatch_a_block_variable_from_macro______4); [line 54]\n n$24=_fun___objc_alloc_no_fail(sizeof(class __objc_anonymous_block_A_dispatch_a_block_variable_from_macro______4 ):unsigned long ) [line 54]\n *&__objc_anonymous_block_A_dispatch_a_block_variable_from_macro______4:class __objc_anonymous_block_A_dispatch_a_block_variable_from_macro______4 =n$24 [line 54]\n n$25=*&#GB$A_dispatch_a_block_variable_from_macro_static_storage__:struct objc_object * [line 54]\n *n$24.A_dispatch_a_block_variable_from_macro_static_storage__:struct objc_object *=n$25 [line 54]\n *&initialization_block__:_fn_ (*)=(_fun___objc_anonymous_block_A_dispatch_a_block_variable_from_macro______4) [line 54]\n n$20=*&initialization_block__:_fn_ (*) [line 58]\n n$21=n$20() [line 58]\n n$19=*&#GB$A_dispatch_a_block_variable_from_macro_static_storage__:struct objc_object * [line 59]\n *&return:struct objc_object *=n$19 [line 52]\n " shape="box"] - 29 -> 31 ; -28 [label="28: BinaryOperatorStmt: EQ \n n$0=*&b:class A * [line 56]\n " shape="box"] + 29 -> 25 ; +28 [label="28: BinaryOperatorStmt: Assign \n n$22=_fun___objc_alloc_no_fail(sizeof(class A ):unsigned long ) [line 55]\n n$23=_fun_A_init(n$22:class A *) virtual [line 55]\n *&#GB$A_dispatch_a_block_variable_from_macro_static_storage__:struct objc_object *=n$23 [line 55]\n " shape="box"] - 28 -> 29 ; - 28 -> 30 ; -27 [label="27: between_join_and_exit \n " shape="box"] + 28 -> 27 ; +27 [label="27: Exit __objc_anonymous_block_A_dispatch_a_block_variable_from_macro______4 \n " color=yellow style=filled] - 27 -> 25 ; -26 [label="26: + \n " ] +26 [label="26: Start __objc_anonymous_block_A_dispatch_a_block_variable_from_macro______4\nFormals: \nLocals: \n DECLARE_LOCALS(&return); [line 54]\n " color=yellow style=filled] - 26 -> 27 ; -25 [label="25: Exit main \n " color=yellow style=filled] + 26 -> 28 ; +25 [label="25: Exit A_dispatch_a_block_variable_from_macro \n " color=yellow style=filled] -24 [label="24: Start main\nFormals: \nLocals: p:int * b:class A * \n DECLARE_LOCALS(&return,&p,&b); [line 53]\n " color=yellow style=filled] +24 [label="24: Start A_dispatch_a_block_variable_from_macro\nFormals: \nLocals: initialization_block__:_fn_ (*) \n DECLARE_LOCALS(&return,&initialization_block__); [line 51]\n " color=yellow style=filled] - 24 -> 34 ; + 24 -> 29 ; 23 [label="23: DeclStmt \n DECLARE_LOCALS(&__objc_anonymous_block_A_dispatch_a_block_variable______3); [line 43]\n n$17=_fun___objc_alloc_no_fail(sizeof(class __objc_anonymous_block_A_dispatch_a_block_variable______3 ):unsigned long ) [line 43]\n *&__objc_anonymous_block_A_dispatch_a_block_variable______3:class __objc_anonymous_block_A_dispatch_a_block_variable______3 =n$17 [line 43]\n n$18=*&#GB$A_dispatch_a_block_variable_static_storage__:struct objc_object * [line 43]\n *n$17.A_dispatch_a_block_variable_static_storage__:struct objc_object *=n$18 [line 43]\n *&initialization_block__:_fn_ (*)=(_fun___objc_anonymous_block_A_dispatch_a_block_variable______3) [line 43]\n " shape="box"] diff --git a/infer/tests/codetoanalyze/objc/frontend/block/dispatch.m b/infer/tests/codetoanalyze/objc/frontend/block/dispatch.m index eed705d9b..a82a2402b 100644 --- a/infer/tests/codetoanalyze/objc/frontend/block/dispatch.m +++ b/infer/tests/codetoanalyze/objc/frontend/block/dispatch.m @@ -11,7 +11,7 @@ @interface A : NSObject -@property int x; +@property(nonatomic) int x; + (instancetype)sharedInstance; @@ -48,6 +48,24 @@ return static_storage__; } ++ (instancetype)dispatch_a_block_variable_from_macro { + return ({ + static __typeof__([self new]) static_storage__; + void (^initialization_block__)() = ^{ + static_storage__ = ([self new]); + }; + static dispatch_once_t once_token__; + _dispatch_once(&once_token__, initialization_block__); + static_storage__; + }); +} + ++ (int)dispatch_a_block_variable_from_macro_delivers_initialised_object { + A* a = [A dispatch_a_block_variable_from_macro]; + a->_x = 5; + return 1 / (a->_x - 5); +} + @end int main() { diff --git a/infer/tests/endtoend/objc/BlockDispatchTest.java b/infer/tests/endtoend/objc/BlockDispatchTest.java index 1d8b4638f..0a2907018 100644 --- a/infer/tests/endtoend/objc/BlockDispatchTest.java +++ b/infer/tests/endtoend/objc/BlockDispatchTest.java @@ -32,7 +32,7 @@ public class BlockDispatchTest { private static ImmutableList inferCmd; - public static final String NULL_DEREFERENCE = "NULL_DEREFERENCE"; + public static final String DIVIDE_BY_ZERO = "DIVIDE_BY_ZERO"; @ClassRule public static DebuggableTemporaryFolder folder = @@ -46,15 +46,16 @@ public class BlockDispatchTest { @Test public void whenInferRunsOnMThenNPEIsFound() throws InterruptedException, IOException, InferException { - InferResults inferResults = InferRunner.runInferObjC(inferCmd); + InferResults inferResults = InferRunner.runInferC(inferCmd); String[] procedures = { + "A_dispatch_a_block_variable_from_macro_delivers_initialised_object" }; assertThat( - "Results should contain the expected null pointer exception", + "Results should contain the expected " + DIVIDE_BY_ZERO, inferResults, containsExactly( - NULL_DEREFERENCE, //No NPEs are found. If the call to dispatch_once didn't work - // we would get an NPE. + DIVIDE_BY_ZERO, //No NPEs are found. If the call to dispatch_once didn't work + // we would get an NPE. We find only one divide by zero. FILE, procedures )