From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from www523.your-server.de (www523.your-server.de [159.69.224.22]) by sourceware.org (Postfix) with ESMTPS id CFABE3857344 for ; Fri, 17 Jun 2022 15:54:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CFABE3857344 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tim-lange.me Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tim-lange.me DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tim-lange.me; s=default2108; h=Content-Transfer-Encoding:Content-Type: MIME-Version:Message-Id:To:Subject:From:Date:Sender:Reply-To:Cc:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References; bh=mJB+GNcbCekO5o6vajaKlMoPOZsY0/j8KjYyGw6SLRU=; b=ToyKqTaroL40QondIFeJrdRMZ+ DBdYKs8U7pVM/t/VI4dF3j0+GVE6eVyanmLARdutCox2skSH3zHNjIBTVyTeJ5BQ3IxKW1oqVjmaX 8IzSrncTAfRQjxKUG9vikW4tBBxYtoeEn9XL0p4QCeXfIuVhIsiZ73tlIroDTd3jahj6MyVAmQZwP UaGze1P1Vx5PbMlewpCe84b5WJ7dV4EKXO0mAD3sGJspuPCy7pKurJT2J7VPe47ISfh+OpZwPy7iG g4G/NNqygzucvu1nRQ3ZgSInsKdM2YgNe6BfvjIee2glj1b+6BCbJ+0dJdpiMzf95UsqMN5Q3YY4+ RTozc+vw==; Received: from sslproxy06.your-server.de ([78.46.172.3]) by www523.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1o2EJA-000FTm-LO; Fri, 17 Jun 2022 17:54:52 +0200 Received: from [2a02:908:1864:dbe0::6768] (helo=fedora) by sslproxy06.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1o2EJA-000FNz-GO; Fri, 17 Jun 2022 17:54:52 +0200 Date: Fri, 17 Jun 2022 17:54:46 +0200 From: Tim Lange Subject: [RFC] analyzer: allocation size warning To: David Malcolm , GCC Mailing List Message-Id: X-Mailer: geary/40.0 MIME-Version: 1.0 Content-Type: text/plain; charset=windows-1251; format=flowed Content-Transfer-Encoding: quoted-printable X-Authenticated-Sender: mail@tim-lange.me X-Virus-Scanned: Clear (ClamAV 0.103.6/26575/Fri Jun 17 10:08:05 2022) X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_INFOUSMEBIZ, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Jun 2022 15:54:57 -0000 Hi everyone, tracked in PR105900 [0], I'd like to add support for a new warning on=20 dubious allocation sizes. The new checker emits a warning when the=20 allocation size is not a multiple of the type's size. With the checker,=20 following mistakes are detected: int *arr =3D malloc(3); // forgot to multiply by sizeof arr[0] =3D ...; arr[1] =3D ...; or int *buf =3D malloc (n + sizeof(int)); // probably should be * instead=20 of + Because it is implemented inside the analyzer, it also emits warnings=20 when the buffer is first of type void* and later on casted to something=20 else. Though, this also inherits a limitation. The checker can not=20 distinguish 2 * sizeof(short) from sizeof(int) because sizeof is=20 resolved and constants are folded at the point when the analyzer runs.=20 As a mitigation, I plan to implement a check in the frontend that emits=20 a warning if sizeof(lhs pointee type) is not part of the malloc=20 argument. I'm looking for a first feedback on the phrasing of the diagnostics as=20 well on the preliminary patch [1]. On constant buffer sizes, the warnings looks like this: warning: Allocated buffer size is not a multiple of the pointee's size=20 [CWE-131] [-Wanalyzer-allocation-size] 22 | int *ptr =3D malloc (10 + sizeof(int)); /* { dg-line malloc2 } */ | ^~~~~~~~~~~~~~~~~~~~~~~~~ =91test_2=92: event 1 | | 22 | int *ptr =3D malloc (10 + sizeof(int)); /* { dg-line malloc2 }=20 */ | | ^~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (1) Casting a 14 byte buffer to =91int *=92 leaves 2 trailing=20 bytes; either the allocated size is bogus or the type on the left-hand=20 side is wrong | On symbolic buffer sizes: warning: Allocated buffer size is not a multiple of the pointee's size=20 [CWE-131] [-Wanalyzer-allocation-size] 33 | int *ptr =3D malloc (n + sizeof(int)); /* { dg-line malloc3 } */ | ^~~~~~~~~~~~~~~~~~~~~~~~ =91test_3=92: event 1 | | 33 | int *ptr =3D malloc (n + sizeof(int)); /* { dg-line malloc3 }=20 */ | | ^~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (1) Allocation is incompatible with =91int *=92; either the=20 allocated size is bogus or the type on the left-hand side is wrong | And this is how a simple flow looks like: warning: Allocated buffer size is not a multiple of the pointee's size=20 [CWE-131] [-Wanalyzer-allocation-size] 39 | int *iptr =3D (int *)ptr; /* { dg-line assign } */ | ^~~~ =91test_4=92: events 1-2 | | 38 | void *ptr =3D malloc (n * sizeof (short)); /* { dg-message } */ | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (1) allocated here | 39 | int *iptr =3D (int *)ptr; /* { dg-line assign } */ | | ~~~~ | | | | | (2) =91ptr=92 is incompatible with =91int *=92; either the=20 allocated size at (1) is bogus or the type on the left-hand side is=20 wrong | There are some things to discuss from my side: * The tests with the "toy re-implementation of CPython's object=20 model"[2] fail due to a extra warning emitted. Because the analyzer=20 can't know the calculation actually results in a correct buffer size=20 when viewed as a string_obj later on, it emits a warning, e.g. at line=20 61 in data-model-5.c. The only mitigation would be to disable the=20 warning for structs entirely. Now, the question is to rather have noise=20 on these cases or disable the warning for structs entirely? * I'm unable to emit a warning whenever the cast happens at an=20 assignment with a call as the rhs, e.g. test_1 in allocation-size-4.c.=20 This is because I'm unable to access a region_svalue for the returned=20 value. Even in the new_program_state, the svalue of the lhs is still a=20 conjured_svalue. Maybe David can lead me to a place where I can access=20 the return value's region_svalue or do I have to adapt the engine? * attr-malloc-6.c and pr96639.c did both contain structs without an=20 implementation. Something in the analyzer must have triggered another=20 warning about the usage of those without them having an implementation.=20 I changed those structs to have an empty implementation, such that the=20 additional warning are gone. I think this shouldn't change the test=20 case, so is this change okay? - Tim [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D105900 [1] While all tests except the cpython ones work, I have yet to test it=20 on large C projects [2] FAIL: gcc.dg/analyzer/data-model-5.c (test for excess errors) FAIL: gcc.dg/analyzer/data-model-5b.c (test for excess errors) FAIL: gcc.dg/analyzer/data-model-5c.c (test for excess errors) FAIL: gcc.dg/analyzer/data-model-5d.c (test for excess errors) FAIL: gcc.dg/analyzer/first-field-2.c (test for excess errors) ------- Subject: [PATCH] analyzer: add allocation size warning This patch adds an allocation size checker to the analyzer. The checker warns when the tracked buffer size is not a multiple of the=20 left-hand side pointee's type. This resolves PR analyzer/105900. The patch is not yet fully tested. gcc/analyzer/ChangeLog: * analyzer.opt: Add Wanalyzer-allocation-size. * sm-malloc.cc (class dubious_allocation_size): New=20 pending_diagnostic subclass. (capacity_compatible_with_type): New. (const_operand_in_sval_p): New. (struct_or_union_with_inheritance_p): New. (check_capacity): New. (malloc_state_machine::on_stmt): Add calls to=20 on_pointer_assignment. (malloc_state_machine::on_allocator_call): Add node to=20 parameters and call to on_pointer_assignment. (malloc_state_machine::on_pointer_assignment): New. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/attr-malloc-6.c: Disabled=20 Wanalyzer-allocation-size and added default implementation for FILE. * gcc.dg/analyzer/capacity-1.c: Added dg directives. * gcc.dg/analyzer/malloc-4.c: Disabled=20 Wanalyzer-allocation-size. * gcc.dg/analyzer/pr96639.c: Disabled Wanalyzer-allocation-size=20 and added default implementation for foo and bar. * gcc.dg/analyzer/allocation-size-1.c: New test. * gcc.dg/analyzer/allocation-size-2.c: New test. * gcc.dg/analyzer/allocation-size-3.c: New test. * gcc.dg/analyzer/allocation-size-4.c: New test. Signed-off-by: Tim Lange --- gcc/analyzer/analyzer.opt | 4 + gcc/analyzer/sm-malloc.cc | 363 +++++++++++++++++- .../gcc.dg/analyzer/allocation-size-1.c | 54 +++ .../gcc.dg/analyzer/allocation-size-2.c | 44 +++ .../gcc.dg/analyzer/allocation-size-3.c | 48 +++ .../gcc.dg/analyzer/allocation-size-4.c | 39 ++ gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c | 2 + gcc/testsuite/gcc.dg/analyzer/capacity-1.c | 5 +- gcc/testsuite/gcc.dg/analyzer/malloc-4.c | 6 +- gcc/testsuite/gcc.dg/analyzer/pr96639.c | 2 + 10 files changed, 559 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index 4aea52d3a87..f213989e0bb 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -78,6 +78,10 @@ Wanalyzer-malloc-leak Common Var(warn_analyzer_malloc_leak) Init(1) Warning Warn about code paths in which a heap-allocated pointer leaks. +Wanalyzer-allocation-size +Common Var(warn_analyzer_allocation_size) Init(1) Warning +Warn about code paths in which a buffer is assigned to a incompatible=20 type. + Wanalyzer-mismatching-deallocation Common Var(warn_analyzer_mismatching_deallocation) Init(1) Warning Warn about code paths in which the wrong deallocation function is=20 called. diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc index 3bd40425919..790c9f0e57d 100644 --- a/gcc/analyzer/sm-malloc.cc +++ b/gcc/analyzer/sm-malloc.cc @@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see #include "attribs.h" #include "analyzer/function-set.h" #include "analyzer/program-state.h" +#include "print-tree.h" +#include "gimple-pretty-print.h" #if ENABLE_ANALYZER @@ -428,6 +430,7 @@ private: get_or_create_deallocator (tree deallocator_fndecl); void on_allocator_call (sm_context *sm_ctxt, + const supernode *node, const gcall *call, const deallocator_set *deallocators, bool returns_nonnull =3D false) const; @@ -444,6 +447,16 @@ private: void on_realloc_call (sm_context *sm_ctxt, const supernode *node, const gcall *call) const; + void on_pointer_assignment(sm_context *sm_ctxt, + const supernode *node, + const gassign *assign_stmt, + tree lhs, + tree rhs) const; + void on_pointer_assignment(sm_context *sm_ctxt, + const supernode *node, + const gcall *call, + tree lhs, + tree rhs) const; void on_zero_assignment (sm_context *sm_ctxt, const gimple *stmt, tree lhs) const; @@ -1432,6 +1445,117 @@ private: const char *m_funcname; }; +/* Concrete subclass for casts of pointers that lead to trailing=20 bytes. */ + +class dubious_allocation_size : public malloc_diagnostic +{ +public: + dubious_allocation_size (const malloc_state_machine &sm, tree lhs,=20 tree rhs, + tree size_tree, unsigned HOST_WIDE_INT size_diff) + : malloc_diagnostic(sm, rhs),=20 m_type(dubious_allocation_type::CONSTANT_SIZE), + m_lhs(lhs), m_size_tree(size_tree), m_size_diff(size_diff) + {} + + dubious_allocation_size (const malloc_state_machine &sm, tree lhs,=20 tree rhs, + tree size_tree) + : malloc_diagnostic(sm, rhs),=20 m_type(dubious_allocation_type::MISSING_OPERAND), + m_lhs(lhs), m_size_tree(size_tree), m_size_diff(0) + {} + + const char *get_kind () const final override + { + return "dubious_allocation_size"; + } + + int get_controlling_option () const final override + { + return OPT_Wanalyzer_allocation_size; + } + + bool subclass_equal_p (const pending_diagnostic &base_other) const + final override + { + const dubious_allocation_size &other =3D (const dubious_allocation_size=20 &)base_other; + return malloc_diagnostic::subclass_equal_p(other) + && m_type =3D=3D other.m_type + && same_tree_p (m_lhs, other.m_lhs) + && same_tree_p (m_size_tree, other.m_size_tree) + && m_size_diff =3D=3D other.m_size_diff; + } + + bool emit (rich_location *rich_loc) final override + { + diagnostic_metadata m; + m.add_cwe (131); + return warning_meta (rich_loc, m, get_controlling_option (), + "Allocated buffer size is not a multiple of the pointee's size"); + } + + label_text describe_state_change (const evdesc::state_change &change) + override + { + if (change.m_old_state =3D=3D m_sm.get_start_state () + && unchecked_p (change.m_new_state)) + { + m_alloc_event =3D change.m_event_id; + if (m_type =3D=3D dubious_allocation_type::CONSTANT_SIZE) + { + // TODO: verify that it's the allocation stmt, not a copy + return change.formatted_print ("%E bytes allocated here", + m_size_tree); + } + } + return malloc_diagnostic::describe_state_change (change); + } + + label_text describe_final_event (const evdesc::final_event &ev) final=20 override + { + if (m_type =3D=3D dubious_allocation_type::CONSTANT_SIZE) + { + if (m_alloc_event.known_p ()) + return ev.formatted_print ( + "Casting %qE to %qT leaves %wu trailing bytes; either the" + " allocated size is bogus or the type on the left-hand side is" + " wrong", + m_arg, TREE_TYPE (m_lhs), m_size_diff); + else + return ev.formatted_print ( + "Casting a %E byte buffer to %qT leaves %wu trailing bytes; either" + " the allocated size is bogus or the type on the left-hand side is" + " wrong", + m_size_tree, TREE_TYPE (m_lhs), m_size_diff); + } + else if (m_type =3D=3D dubious_allocation_type::MISSING_OPERAND) + { + if (m_alloc_event.known_p ()) + return ev.formatted_print ( + "%qE is incompatible with %qT; either the allocated size at %@ is" + " bogus or the type on the left-hand side is wrong", + m_arg, TREE_TYPE (m_lhs), &m_alloc_event); + else + return ev.formatted_print ( + "Allocation is incompatible with %qT; either the allocated size is" + " bogus or the type on the left-hand side is wrong", + TREE_TYPE (m_lhs)); + } + + gcc_unreachable (); + return label_text (); + } + +private: + enum dubious_allocation_type { + CONSTANT_SIZE, + MISSING_OPERAND + }; + + dubious_allocation_type m_type; + diagnostic_event_id_t m_alloc_event; + tree m_lhs; + tree m_size_tree; + unsigned HOST_WIDE_INT m_size_diff; +}; + /* struct allocation_state : public state_machine::state. */ /* Implementation of state_machine::state::dump_to_pp vfunc @@ -1633,6 +1757,160 @@ known_allocator_p (const_tree fndecl, const=20 gcall *call) return false; } +/* Returns the trailing bytes on dubious allocation sizes. */ + +static unsigned HOST_WIDE_INT +capacity_compatible_with_type (tree cst, tree pointee_size_tree) +{ + unsigned HOST_WIDE_INT pointee_size =3D TREE_INT_CST_LOW=20 (pointee_size_tree); + if (pointee_size =3D=3D 0) + return 0; + unsigned HOST_WIDE_INT alloc_size =3D TREE_INT_CST_LOW (cst); + + return alloc_size % pointee_size; +} + +/* Returns true if there is a constant tree with + the same constant value inside the sval. */ + +static bool +const_operand_in_sval_p (const svalue *sval, tree size_cst) +{ + auto_vec non_mult_expr; + auto_vec worklist; + worklist.safe_push(sval); + while (!worklist.is_empty()) + { + const svalue *curr =3D worklist.pop (); + curr =3D curr->unwrap_any_unmergeable (); + + switch (curr->get_kind()) + { + default: + break; + case svalue_kind::SK_CONSTANT: + { + const constant_svalue *cst_sval =3D curr->dyn_cast_constant_svalue (); + unsigned HOST_WIDE_INT sval_int + =3D TREE_INT_CST_LOW (cst_sval->get_constant ()); + unsigned HOST_WIDE_INT size_cst_int =3D TREE_INT_CST_LOW (size_cst); + if (sval_int % size_cst_int =3D=3D 0) + return true; + } + break; + case svalue_kind::SK_BINOP: + { + const binop_svalue *b_sval =3D curr->dyn_cast_binop_svalue (); + if (b_sval->get_op () =3D=3D MULT_EXPR) + { + worklist.safe_push (b_sval->get_arg0 ()); + worklist.safe_push (b_sval->get_arg1 ()); + } + else + { + non_mult_expr.safe_push (b_sval->get_arg0 ()); + non_mult_expr.safe_push (b_sval->get_arg1 ()); + } + } + break; + case svalue_kind::SK_UNARYOP: + { + const unaryop_svalue *un_sval =3D curr->dyn_cast_unaryop_svalue (); + worklist.safe_push (un_sval->get_arg ()); + } + break; + case svalue_kind::SK_UNKNOWN: + return true; + } + } + + /* Each expr should be a multiple of the size. + E.g. used to catch n + sizeof(int) errors. */ + bool reduce =3D !non_mult_expr.is_empty (); + while (!non_mult_expr.is_empty() && reduce) + { + const svalue *expr_sval =3D non_mult_expr.pop (); + reduce &=3D const_operand_in_sval_p (expr_sval, size_cst); + } + return reduce; +} + +/* Returns true iff the type is a struct with another struct inside. */ + +static bool +struct_or_union_with_inheritance_p (tree type) +{ + if (!RECORD_OR_UNION_TYPE_P (type)) + return false; + + for (tree f =3D TYPE_FIELDS (type); f; f =3D TREE_CHAIN (f)) + if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (f))) + return true; + + return false; +} + +static void +check_capacity (sm_context *sm_ctxt, + const malloc_state_machine &sm, + const supernode *node, + const gimple *stmt, + tree lhs, + tree rhs, + const svalue *capacity) +{ + tree pointer_type =3D TREE_TYPE (lhs); + gcc_assert (TREE_CODE (pointer_type) =3D=3D POINTER_TYPE); + + tree pointee_type =3D TREE_TYPE (pointer_type); + /* void * is always compatible. */ + if (TREE_CODE (pointee_type) =3D=3D VOID_TYPE) + return; + + if (struct_or_union_with_inheritance_p (pointee_type)) + return; + + tree pointee_size_tree =3D size_in_bytes(pointee_type); + /* The size might be unknown e.g. being a array with n elements + or casting to char * never has any trailing bytes. */ + if (TREE_CODE (pointee_size_tree) !=3D INTEGER_CST + || TREE_INT_CST_LOW (pointee_size_tree) =3D=3D 1) + return; + + switch (capacity->get_kind ()) + { + default: + break; + case svalue_kind::SK_CONSTANT: + { + const constant_svalue *cst_sval =3D capacity->dyn_cast_constant_svalue=20 (); + tree cst =3D cst_sval->get_constant (); + unsigned HOST_WIDE_INT size_diff + =3D capacity_compatible_with_type (cst, pointee_size_tree); + if (size_diff !=3D 0) + { + tree diag_arg =3D sm_ctxt->get_diagnostic_tree (rhs); + sm_ctxt->warn (node, stmt, diag_arg, + new dubious_allocation_size (sm, lhs, diag_arg, + cst, size_diff)); + } + } + break; + case svalue_kind::SK_BINOP: + case svalue_kind::SK_UNARYOP: + { + if (!const_operand_in_sval_p (capacity, pointee_size_tree)) + { + tree diag_arg =3D sm_ctxt->get_diagnostic_tree (rhs); + sm_ctxt->warn (node, stmt, diag_arg, + new dubious_allocation_size (sm, lhs, diag_arg, + pointee_size_tree)); + } + } + break; + } +} + /* Implementation of state_machine::on_stmt vfunc for=20 malloc_state_machine. */ bool @@ -1645,14 +1923,14 @@ malloc_state_machine::on_stmt (sm_context=20 *sm_ctxt, { if (known_allocator_p (callee_fndecl, call)) { - on_allocator_call (sm_ctxt, call, &m_free); + on_allocator_call (sm_ctxt, node, call, &m_free); return true; } if (is_named_call_p (callee_fndecl, "operator new", call, 1)) - on_allocator_call (sm_ctxt, call, &m_scalar_delete); + on_allocator_call (sm_ctxt, node, call, &m_scalar_delete); else if (is_named_call_p (callee_fndecl, "operator new []", call, 1)) - on_allocator_call (sm_ctxt, call, &m_vector_delete); + on_allocator_call (sm_ctxt, node, call, &m_vector_delete); else if (is_named_call_p (callee_fndecl, "operator delete", call, 1) || is_named_call_p (callee_fndecl, "operator delete", call, 2)) { @@ -1707,7 +1985,7 @@ malloc_state_machine::on_stmt (sm_context=20 *sm_ctxt, tree attrs =3D TYPE_ATTRIBUTES (TREE_TYPE (callee_fndecl)); bool returns_nonnull =3D lookup_attribute ("returns_nonnull", attrs); - on_allocator_call (sm_ctxt, call, deallocators, returns_nonnull); + on_allocator_call (sm_ctxt, node, call, deallocators,=20 returns_nonnull); } /* Handle "__attribute__((nonnull))". */ @@ -1763,12 +2041,31 @@ malloc_state_machine::on_stmt (sm_context=20 *sm_ctxt, =3D mutable_this->get_or_create_deallocator (callee_fndecl); on_deallocator_call (sm_ctxt, node, call, d, dealloc_argno); } + + /* Handle returns from function calls. */ + tree lhs =3D gimple_call_lhs (call); + if (lhs && TREE_CODE (TREE_TYPE (lhs)) =3D=3D POINTER_TYPE + && TREE_CODE (gimple_call_return_type (call)) =3D=3D POINTER_TYPE) + on_pointer_assignment (sm_ctxt, node, call, lhs, + gimple_call_fn (call)); } if (tree lhs =3D sm_ctxt->is_zero_assignment (stmt)) if (any_pointer_p (lhs)) on_zero_assignment (sm_ctxt, stmt,lhs); + /* Handle pointer assignments/casts for dubious allocation size. */ + if (const gassign *assign_stmt =3D dyn_cast (stmt)) + { + if (gimple_num_ops (stmt) =3D=3D 2) + { + tree lhs =3D gimple_assign_lhs (assign_stmt); + tree rhs =3D gimple_assign_rhs1 (assign_stmt); + if (any_pointer_p (lhs) && any_pointer_p (rhs)) + on_pointer_assignment (sm_ctxt, node, assign_stmt, lhs, rhs); + } + } + /* Handle dereferences. */ for (unsigned i =3D 0; i < gimple_num_ops (stmt); i++) { @@ -1818,6 +2115,7 @@ malloc_state_machine::on_stmt (sm_context=20 *sm_ctxt, void malloc_state_machine::on_allocator_call (sm_context *sm_ctxt, + const supernode *node, const gcall *call, const deallocator_set *deallocators, bool returns_nonnull) const @@ -1830,6 +2128,9 @@ malloc_state_machine::on_allocator_call=20 (sm_context *sm_ctxt, (returns_nonnull ? deallocators->m_nonnull : deallocators->m_unchecked)); + + if (TREE_CODE (TREE_TYPE (lhs)) =3D=3D POINTER_TYPE) + on_pointer_assignment (sm_ctxt, node, call, lhs, gimple_call_fn=20 (call)); } else { @@ -1968,6 +2269,60 @@ malloc_state_machine::on_realloc_call=20 (sm_context *sm_ctxt, } } +/* Handle assignments between two pointers. + Check for dubious allocation sizes. +*/ + +void +malloc_state_machine::on_pointer_assignment (sm_context *sm_ctxt, + const supernode *node, + const gassign *assign_stmt, + tree lhs, + tree rhs) const +{ + /* Do not warn if lhs and rhs are of the same type to not emit=20 duplicate + warnings on assignments after the cast. */ + if (pending_diagnostic::same_tree_p (TREE_TYPE (lhs), TREE_TYPE=20 (rhs))) + return; + + const program_state *state =3D sm_ctxt->get_old_program_state (); + const svalue *r_value =3D state->m_region_model->get_rvalue (rhs, NULL); + if (const region_svalue *reg =3D dyn_cast =20 (r_value)) + { + const svalue *capacity =3D state->m_region_model->get_capacity + (reg->get_pointee ()); + check_capacity(sm_ctxt, *this, node, assign_stmt, lhs, rhs, capacity); + } +} + +void +malloc_state_machine::on_pointer_assignment (sm_context *sm_ctxt, + const supernode *node, + const gcall *call, + tree lhs, + tree fn_decl) const +{ + /* Do not warn if lhs and rhs are of the same type to not emit=20 duplicate + warnings on assignments after the cast. */ + if (pending_diagnostic::same_tree_p + (TREE_TYPE (lhs), TREE_TYPE (gimple_call_return_type (call)))) + return; + + const program_state *state =3D sm_ctxt->get_new_program_state (); + const svalue *r_value =3D state->m_region_model->get_rvalue (lhs, NULL); + if (const region_svalue *reg =3D dyn_cast =20 (r_value)) + { + const svalue *capacity =3D state->m_region_model->get_capacity + (reg->get_pointee ()); + check_capacity (sm_ctxt, *this, node, call, lhs, fn_decl, capacity); + } + else if (const conjured_svalue *con + =3D dyn_cast (r_value)) + { + // FIXME: How to get a region_svalue? + } +} + /* Implementation of state_machine::on_phi vfunc for=20 malloc_state_machine. */ void diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c=20 b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c new file mode 100644 index 00000000000..5403c5f41f1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c @@ -0,0 +1,54 @@ +#include + +/* Tests with constant buffer sizes */ + +void test_1 (void) +{ + short *ptr =3D malloc (21 * sizeof(short)); + free (ptr); +} + +void test_2 (void) +{ + int *ptr =3D malloc (21 * sizeof (short)); /* { dg-line malloc } */ + free (ptr); + + /* { dg-warning "Allocated buffer size is not a multiple of the=20 pointee's size" "" { target *-*-* } malloc } */ + /* { dg-message "\\(1\\) Casting a 42 byte buffer to 'int \\*' leaves=20 2 trailing bytes" "" { target *-*-* } malloc } */ +} + +void test_3 (void) +{ + void *ptr =3D malloc (21 * sizeof (short)); + short *sptr =3D (short *)ptr; + free (sptr); +} + +void test_4 (void) +{ + void *ptr =3D malloc (21 * sizeof (short)); /* { dg-message } */ + int *iptr =3D (int *)ptr; /* { dg-line assign } */ + free (iptr); + + /* { dg-warning "Allocated buffer size is not a multiple of the=20 pointee's size" "" { target *-*-* } assign } */ + /* { dg-message "\\(2\\) Casting 'ptr' to 'int \\*' leaves 2 trailing=20 bytes" "" { target *-*-* } assign } */ +} + +struct s { + int i; +}; + +void test_5 (void) +{ + struct s *ptr =3D malloc (5 * sizeof (struct s)); + free (ptr); +} + +void test_6 (void) +{ + long *ptr =3D malloc (5 * sizeof (struct s)); /* { dg-line malloc6 } */ + free (ptr); + + /* { dg-warning "" "" { target *-*-* } malloc6 } */ + /* { dg-message "" "" { target *-*-* } malloc6 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c=20 b/gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c new file mode 100644 index 00000000000..e66d2793f13 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c @@ -0,0 +1,44 @@ +#include +#include + +/* Tests with symbolic buffer sizes */ + +void test_1 (void) +{ + int n; + scanf("%i", &n); + short *ptr =3D malloc (n * sizeof(short)); + free (ptr); +} + +void test_2 (void) +{ + int n; + scanf("%i", &n); + int *ptr =3D malloc (n * sizeof (short)); /* { dg-line malloc } */ + free (ptr); + + /* { dg-warning "Allocated buffer size is not a multiple of the=20 pointee's size" "" { target *-*-* } malloc } */ + /* { dg-message "\\(1\\) Allocation is incompatible with 'int \\*'"=20 "" { target *-*-* } malloc } */ +} + +void test_3 (void) +{ + int n; + scanf("%i", &n); + void *ptr =3D malloc (n * sizeof (short)); + short *sptr =3D (short *)ptr; + free (sptr); +} + +void test_4 (void) +{ + int n; + scanf("%i", &n); + void *ptr =3D malloc (n * sizeof (short)); /* { dg-message } */ + int *iptr =3D (int *)ptr; /* { dg-line assign } */ + free (iptr); + + /* { dg-warning "Allocated buffer size is not a multiple of the=20 pointee's size" "" { target *-*-* } assign } */ + /* { dg-message "\\(2\\) 'ptr' is incompatible with 'int \\*'; either=20 the allocated size at \\(1\\)" "" { target *-*-* } assign } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c=20 b/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c new file mode 100644 index 00000000000..dafc0e73c63 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c @@ -0,0 +1,48 @@ +#include +#include + +/* CWE-131 example 5 */ +void test_1(void) +{ + int *id_sequence =3D (int *) malloc (3); /* { dg-line malloc1 } */ + if (id_sequence =3D=3D NULL) exit (1); + + id_sequence[0] =3D 13579; + id_sequence[1] =3D 24680; + id_sequence[2] =3D 97531; + + free (id_sequence); + + /* { dg-warning "" "" { target *-*-* } malloc1 } */ + /* { dg-message "" "" { target *-*-* } malloc1 } */ +} + +void test_2(void) +{ + int *ptr =3D malloc (10 + sizeof(int)); /* { dg-line malloc2 } */ + free (ptr); + + /* { dg-warning "" "" { target *-*-* } malloc2 } */ + /* { dg-message "" "" { target *-*-* } malloc2 } */ +} + +void test_3(void) +{ + int n; + scanf("%i", &n); + int *ptr =3D malloc (n + sizeof (int)); /* { dg-line malloc3 } */ + free (ptr); + + /* { dg-warning "" "" { target *-*-* } malloc3 } */ + /* { dg-message "" "" { target *-*-* } malloc3 } */ +} + +void test_4(void) +{ + int n; + scanf("%i", &n); + int m; + scanf("%i", &m); + int *ptr =3D malloc ((n + m) * sizeof (int)); + free (ptr); +} diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c=20 b/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c new file mode 100644 index 00000000000..4c2b31d6e0a --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c @@ -0,0 +1,39 @@ +#include +#include + +/* Flow warnings */ + +void *create_buffer(int n) +{ + return malloc(n); +} + +void test_1(void) +{ + // FIXME + int *buf =3D create_buffer(42); /* { dg-warning "" "" { xfail *-*-* } }=20 */ + free (buf); +} + +void test_2(void) +{ + void *buf =3D create_buffer(42); /* { dg-message } */ + int *ibuf =3D buf; /* { dg-line assign2 } */ + free (ibuf); + + /* { dg-warning "" "" { target *-*-* } assign2 } */ + /* { dg-message "" "" { target *-*-* } assign2 } */ +} + +void test_3(void) +{ + void *buf =3D malloc(42); /* { dg-message } */ + if (buf !=3D NULL) /* { dg-message } */ + { + int *ibuf =3D buf; /* { dg-line assign3 } */ + free (ibuf); + } + + /* { dg-warning "" "" { target *-*-* } assign3 } */ + /* { dg-message "" "" { target *-*-* } assign3 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c=20 b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c index bd28107d0d7..809ee88cf07 100644 --- a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c @@ -1,7 +1,9 @@ +/* { dg-additional-options -Wno-analyzer-allocation-size } */ /* Adapted from gcc.dg/Wmismatched-dealloc.c. */ #define A(...) __attribute__ ((malloc (__VA_ARGS__))) +struct FILE {}; typedef struct FILE FILE; typedef __SIZE_TYPE__ size_t; diff --git a/gcc/testsuite/gcc.dg/analyzer/capacity-1.c=20 b/gcc/testsuite/gcc.dg/analyzer/capacity-1.c index 2d124833296..94f569e390b 100644 --- a/gcc/testsuite/gcc.dg/analyzer/capacity-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/capacity-1.c @@ -89,8 +89,11 @@ struct s static struct s * __attribute__((noinline)) alloc_s (size_t num) { - struct s *p =3D malloc (sizeof(struct s) + num); + struct s *p =3D malloc (sizeof(struct s) + num); /* { dg-line malloc }=20 */ return p; + + /* { dg-warning "" "" { target *-*-* } malloc } */ + /* { dg-message "" "" { target *-*-* } malloc } */ } struct s * diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-4.c=20 b/gcc/testsuite/gcc.dg/analyzer/malloc-4.c index 908bb28ee50..0ca94250ba2 100644 --- a/gcc/testsuite/gcc.dg/analyzer/malloc-4.c +++ b/gcc/testsuite/gcc.dg/analyzer/malloc-4.c @@ -1,9 +1,9 @@ -/* { dg-additional-options "-Wno-incompatible-pointer-types" } */ +/* { dg-additional-options "-Wno-incompatible-pointer-types=20 -Wno-analyzer-allocation-size" } */ #include -struct foo; -struct bar; +struct foo {}; +struct bar {}; void *hv (struct foo **tm) { void *p =3D __builtin_malloc (4); diff --git a/gcc/testsuite/gcc.dg/analyzer/pr96639.c=20 b/gcc/testsuite/gcc.dg/analyzer/pr96639.c index 02ca3f084a2..6f365c3cb5d 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr96639.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr96639.c @@ -1,3 +1,5 @@ +/* { dg-additional-options -Wno-analyzer-allocation-size } */ + void *calloc (__SIZE_TYPE__, __SIZE_TYPE__); int --=20 2.36.1