From 605bc5e01a702b5029ea5f17618e12215accddfd Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Wed, 27 Mar 2019 10:22:35 -0700 Subject: [PATCH] [pulse] fix some tests and add interproc tests Summary: Some of these tests were wrong, eg `~lambda()` calls `lambda()` then... takes the bitwise complement or something? The intent was to call the destructor. Add interprocedural tests for later. Reviewed By: jberdine Differential Revision: D14324762 fbshipit-source-id: 40d2c32f5 --- .../codetoanalyze/cpp/pulse/closures.cpp | 9 +- .../cpp/pulse/interprocedural.cpp | 89 +++++++++++++++++++ .../tests/codetoanalyze/cpp/pulse/issues.exp | 23 +++-- .../cpp/pulse/use_after_delete.cpp | 18 ++-- .../cpp/pulse/use_after_destructor.cpp | 5 +- 5 files changed, 119 insertions(+), 25 deletions(-) create mode 100644 infer/tests/codetoanalyze/cpp/pulse/interprocedural.cpp diff --git a/infer/tests/codetoanalyze/cpp/pulse/closures.cpp b/infer/tests/codetoanalyze/cpp/pulse/closures.cpp index 8ebb8c412..429b448cd 100644 --- a/infer/tests/codetoanalyze/cpp/pulse/closures.cpp +++ b/infer/tests/codetoanalyze/cpp/pulse/closures.cpp @@ -10,6 +10,7 @@ struct S { int f; S() { f = 1; } + ~S() {} }; int ref_capture_destroy_invoke_bad() { @@ -96,11 +97,9 @@ std::function ref_capture_read_lambda_ok() { f; // reading (but not invoking) the lambda doesn't use its captured vars } -// we'll miss this because we count invoking a lambda object as a use of its -// captured vars, not the lambda object itself. -void FN_delete_lambda_then_call_bad() { - auto lambda = [] { return 1; }; - ~lambda(); +void delete_lambda_then_call_bad() { + std::function lambda = [] { return 1; }; + lambda.~function(); return lambda(); } diff --git a/infer/tests/codetoanalyze/cpp/pulse/interprocedural.cpp b/infer/tests/codetoanalyze/cpp/pulse/interprocedural.cpp new file mode 100644 index 000000000..17398b07c --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/pulse/interprocedural.cpp @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2019-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +struct X { + int f; +}; + +void skip(struct X& x) {} +void skip_ptr(struct X* x) {} + +int wraps_read_inner(struct X& x) { return x.f; } + +int wraps_read(struct X& x) { return wraps_read_inner(x); } + +void wraps_write_inner(struct X& x, int i) { x.f = i; } + +void wraps_write(struct X& x, int i) { wraps_write_inner(x, i); } + +void wraps_delete_inner(struct X* x) { delete x; } + +void wraps_delete(struct X* x) { wraps_delete_inner(x); } + +void FP_delete_then_skip_ok(struct X& x) { + delete (&x); + skip(x); +} + +void FP_delete_then_skip_ptr_ok(struct X* x) { + delete x; + skip_ptr(x); +} + +void delete_then_read_bad(struct X& x) { + delete (&x); + wraps_read(x); +} + +void FN_delete_then_write_bad(struct X& x) { + wraps_delete(&x); + wraps_read(x); +} + +void FN_delete_inner_then_write_bad(struct X& x) { + wraps_delete_inner(&x); + wraps_read(x); +} + +void read_write_then_delete_good(struct X& x) { + wraps_write(x, 10); + wraps_read(x); + wraps_delete(&x); +} + +int two_cells(struct X* x, struct X* y) { + x->f = 32; + y->f = 52; + return x->f * y->f; +} + +void aliasing_call(struct X* x) { two_cells(x, x); } + +struct Y { + int* p; +}; + +void store(struct Y* y, int* p) { y->p = p; } + +void call_store(struct Y* y) { + int x = 42; + store(y, &x); +} + +extern bool nondet_choice(); + +struct Y* FP_may_return_invalid_ptr_ok() { + struct Y* y = new Y(); + if (nondet_choice()) { + delete y; + } + return y; +} + +void FN_feed_invalid_into_access_bad() { + struct Y* y = FP_may_return_invalid_ptr_ok(); + call_store(y); +} diff --git a/infer/tests/codetoanalyze/cpp/pulse/issues.exp b/infer/tests/codetoanalyze/cpp/pulse/issues.exp index d446a7045..5f14b015f 100644 --- a/infer/tests/codetoanalyze/cpp/pulse/issues.exp +++ b/infer/tests/codetoanalyze/cpp/pulse/issues.exp @@ -1,14 +1,19 @@ codetoanalyze/cpp/pulse/basics.cpp, multiple_invalidations_branch_bad, 6, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete ptr` at line 58 here,accessed `*(ptr)` here] codetoanalyze/cpp/pulse/basics.cpp, multiple_invalidations_loop_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete ptr` at line 68 here,accessed `ptr` here] -codetoanalyze/cpp/pulse/closures.cpp, implicit_ref_capture_destroy_invoke_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `S_S(&(s),&(0$?%__sil_tmpSIL_materialize_temp__n$12))`,`&(s)` captured as `s`,invalidated by destructor call `S_~S(s)` at line 29 here,accessed `&(f)` here] -codetoanalyze/cpp/pulse/closures.cpp, ref_capture_destroy_invoke_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `S_S(&(s))`,`&(s)` captured as `s`,invalidated by destructor call `S_~S(s)` at line 20 here,accessed `&(f)` here] +codetoanalyze/cpp/pulse/closures.cpp, delete_lambda_then_call_bad, 3, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `std::function<_fn_>_function(&(lambda),&(0$?%__sil_tmpSIL_materialize_temp__n$8))`,invalidated by destructor call `std::function<_fn_>_~function(lambda)` at line 102 here,accessed `lambda` here] +codetoanalyze/cpp/pulse/closures.cpp, implicit_ref_capture_destroy_invoke_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `S_S(&(s),&(0$?%__sil_tmpSIL_materialize_temp__n$12))`,`&(s)` captured as `s`,invalidated by destructor call `S_~S(s)` at line 30 here,accessed `&(f)` here] +codetoanalyze/cpp/pulse/closures.cpp, ref_capture_destroy_invoke_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `S_S(&(s))`,`&(s)` captured as `s`,invalidated by destructor call `S_~S(s)` at line 21 here,accessed `&(f)` here] +codetoanalyze/cpp/pulse/interprocedural.cpp, FP_delete_then_skip_ok, 2, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete x` at line 27 here,accessed `x` here] +codetoanalyze/cpp/pulse/interprocedural.cpp, FP_delete_then_skip_ptr_ok, 2, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete x` at line 32 here,accessed `x` here] +codetoanalyze/cpp/pulse/interprocedural.cpp, FP_may_return_invalid_ptr_ok, 5, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Y))`,assigned to `y`,invalidated by call to `delete y` at line 81 here,accessed `y` here] +codetoanalyze/cpp/pulse/interprocedural.cpp, delete_then_read_bad, 2, USE_AFTER_DELETE, no_bucket, ERROR, [invalidated by call to `delete x` at line 37 here,accessed `x` here] codetoanalyze/cpp/pulse/join.cpp, invalidate_node_alias_bad, 12, USE_AFTER_DELETE, no_bucket, ERROR, [assigned to `result`,invalidated by call to `delete result` at line 30 here,accessed `*(result)` here] codetoanalyze/cpp/pulse/returns.cpp, returns::return_deleted_bad, 4, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(int))`,assigned to `x`,invalidated by call to `delete x` at line 112 here,accessed `x` here] codetoanalyze/cpp/pulse/returns.cpp, returns::return_literal_stack_reference_bad, 0, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, [] codetoanalyze/cpp/pulse/returns.cpp, returns::return_stack_pointer_bad, 1, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, [] codetoanalyze/cpp/pulse/returns.cpp, returns::return_variable_stack_reference1_bad, 1, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, [] codetoanalyze/cpp/pulse/returns.cpp, returns::return_variable_stack_reference2_bad, 1, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, [] -codetoanalyze/cpp/pulse/use_after_delete.cpp, delete_in_branch_bad, 5, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Simple))`,assigned to `s`,invalidated by call to `delete s` at line 57 here,accessed `s` here] +codetoanalyze/cpp/pulse/use_after_delete.cpp, delete_in_branch_bad, 5, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Simple))`,assigned to `s`,invalidated by call to `delete s` at line 57 here,accessed `s->f` here] codetoanalyze/cpp/pulse/use_after_delete.cpp, delete_in_loop_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Simple))`,assigned to `s`,invalidated by call to `delete s` at line 82 here,accessed `s` here] codetoanalyze/cpp/pulse/use_after_delete.cpp, deref_deleted_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Simple))`,assigned to `s`,invalidated by call to `delete s` at line 18 here,accessed `s` here] codetoanalyze/cpp/pulse/use_after_delete.cpp, double_delete_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Simple))`,assigned to `s`,invalidated by call to `delete s` at line 50 here,accessed `s` here] @@ -16,12 +21,12 @@ codetoanalyze/cpp/pulse/use_after_delete.cpp, reassign_field_of_deleted_bad, 3, codetoanalyze/cpp/pulse/use_after_delete.cpp, return_deleted_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Simple))`,assigned to `s`,invalidated by call to `delete s` at line 24 here,accessed `s` here] codetoanalyze/cpp/pulse/use_after_delete.cpp, use_in_branch_bad, 4, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Simple))`,assigned to `s`,invalidated by call to `delete s` at line 73 here,accessed `s` here] codetoanalyze/cpp/pulse/use_after_delete.cpp, use_in_loop_bad, 4, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(Simple))`,assigned to `s`,invalidated by call to `delete s` at line 102 here,accessed `s->f` here] -codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::double_destructor_bad, 5, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `use_after_destructor::S_S(&(s),1)`,invalidated by destructor call `use_after_destructor::S_~S(s)` at line 64 here,accessed `s` here] -codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::placement_new_aliasing1_bad, 5, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(use_after_destructor::S))`,assigned to `s`,invalidated by call to `delete alias` at line 148 here,accessed `s` here] -codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::placement_new_aliasing2_bad, 5, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(use_after_destructor::S))`,assigned to `s`,returned from call to `(sizeof(use_after_destructor::S),s)`,assigned to `alias`,invalidated by call to `delete s` at line 156 here,accessed `alias` here] -codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::placement_new_aliasing3_bad, 6, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(use_after_destructor::S))`,assigned to `s`,assigned to `alias`,invalidated by call to `delete s` at line 165 here,accessed `alias` here] -codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::use_after_destructor_bad, 3, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `use_after_destructor::S_S(&(s),1)`,invalidated by destructor call `use_after_destructor::S_~S(s)` at line 71 here,accessed `*(s.f)` here] -codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::use_after_scope4_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `use_after_destructor::C_C(&(c),3)`,invalidated by destructor call `use_after_destructor::C_~C(c)` at line 210 here,accessed `pc->f` here] +codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::double_destructor_bad, 5, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `use_after_destructor::S_S(&(s),1)`,invalidated by destructor call `use_after_destructor::S_~S(s)` at line 65 here,accessed `s` here] +codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::placement_new_aliasing1_bad, 5, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(use_after_destructor::S))`,assigned to `s`,invalidated by call to `delete alias` at line 149 here,accessed `s` here] +codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::placement_new_aliasing2_bad, 5, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(use_after_destructor::S))`,assigned to `s`,returned from call to `(sizeof(use_after_destructor::S),s)`,assigned to `alias`,invalidated by call to `delete s` at line 157 here,accessed `alias` here] +codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::placement_new_aliasing3_bad, 6, USE_AFTER_DELETE, no_bucket, ERROR, [returned from call to `__new(sizeof(use_after_destructor::S))`,assigned to `s`,assigned to `alias`,invalidated by call to `delete s` at line 166 here,accessed `alias` here] +codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::use_after_destructor_bad, 3, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `use_after_destructor::S_S(&(s),1)`,invalidated by destructor call `use_after_destructor::S_~S(s)` at line 72 here,accessed `*(s.f)` here] +codetoanalyze/cpp/pulse/use_after_destructor.cpp, use_after_destructor::use_after_scope4_bad, 6, USE_AFTER_DESTRUCTOR, no_bucket, ERROR, [returned from call to `use_after_destructor::C_C(&(c),3)`,invalidated by destructor call `use_after_destructor::C_~C(c)` at line 211 here,accessed `pc->f` here] codetoanalyze/cpp/pulse/use_after_free.cpp, double_free_simple_bad, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidated by call to `free(x)` at line 15 here,accessed `x` here] codetoanalyze/cpp/pulse/use_after_free.cpp, use_after_free_simple_bad, 2, USE_AFTER_FREE, no_bucket, ERROR, [invalidated by call to `free(x)` at line 10 here,accessed `*(x)` here] codetoanalyze/cpp/pulse/vector.cpp, FP_init_fill_then_push_back_loop_ok, 6, VECTOR_INVALIDATION, no_bucket, ERROR, [returned from call to `std::vector::at(&(vec),(unsigned long) 1)`,assigned to `elt`,potentially invalidated by call to `std::vector::push_back(&(vec), ..)` at line 65 here,accessed `*(elt)` here] diff --git a/infer/tests/codetoanalyze/cpp/pulse/use_after_delete.cpp b/infer/tests/codetoanalyze/cpp/pulse/use_after_delete.cpp index 6fd5a084c..4842d2c31 100644 --- a/infer/tests/codetoanalyze/cpp/pulse/use_after_delete.cpp +++ b/infer/tests/codetoanalyze/cpp/pulse/use_after_delete.cpp @@ -14,7 +14,7 @@ struct Simple { }; void deref_deleted_bad() { - auto s = new Simple{1}; + auto* s = new Simple{1}; delete s; Simple tmp = *s; } @@ -51,12 +51,12 @@ void double_delete_bad() { delete s; } -Simple* delete_in_branch_bad(bool b) { +void delete_in_branch_bad(bool b) { auto s = new Simple{1}; if (b) { delete s; } - return s; + s->f = 7; } void delete_in_branch_ok(bool b) { @@ -105,29 +105,29 @@ void use_in_loop_bad() { } } -Simple* gated_delete_abort_ok(bool b) { +void gated_delete_abort_ok(bool b) { auto s = new Simple{1}; if (b) { delete s; std::abort(); } - return s; + s->f = 7; } -Simple* gated_exit_abort_ok(bool b) { +void gated_exit_abort_ok(bool b) { auto s = new Simple{1}; if (b) { delete s; exit(1); } - return s; + s->f = 7; } -Simple* gated_delete_throw_ok(bool b) { +void gated_delete_throw_ok(bool b) { auto s = new Simple{1}; if (b) { delete s; throw 5; } - return s; + s->f = 7; } diff --git a/infer/tests/codetoanalyze/cpp/pulse/use_after_destructor.cpp b/infer/tests/codetoanalyze/cpp/pulse/use_after_destructor.cpp index 948331f8f..49a14af9f 100644 --- a/infer/tests/codetoanalyze/cpp/pulse/use_after_destructor.cpp +++ b/infer/tests/codetoanalyze/cpp/pulse/use_after_destructor.cpp @@ -38,12 +38,13 @@ int reinit_after_explicit_destructor_ok() { return *s.f; } -int reinit_after_explicit_destructor2_ok() { +int reinit_after_explicit_destructor2_bad() { S s(1); S s2(2); s.~S(); - s = s2; + s = s2; // operator= return *s.f; + // [s2.f] is [s.f] so gets deleted twice } void placement_new_explicit_destructor_ok() {