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 8383B385828B for ; Fri, 17 Jun 2022 19:23:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8383B385828B 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:References:In-Reply-To:Message-Id:Cc:To:Subject:From:Date:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=WrSZr/vmVJZpszOXzba0TxgUYe4RJgrJH0pLPMQLIPY=; b=Gp/mCW8bspYXyENepde06W8+yy ZFkk/DddQcfnGgV7hi7ttDSRTfgNPDamqWAdi728GzH0itD7zB2dDOxJI7H7JVx9OEv2Urbjkqz5P AQla4iGRwSn9zavIYCVRq3bXXE44V2yzABdE0GuOt/PSE4Fn1EuPQj632bnLK78OkOZgx+i9X4VPm gGZazXI9dyPQ3MuFO8B0eWNuOlq14+T/cZjDTV3PhWdR4nXtvKOjSVYtNE8MefM2FMTCehsDTusq6 YByVSs8f/nsN1rCS0kSM54zqj/GZWxnonK1IYlH2nrnIEWmvEeIClbap1n65G9IUHOqTG9TWUVJEJ jYt+FAGw==; Received: from sslproxy02.your-server.de ([78.47.166.47]) by www523.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1o2HZE-000CLd-Ue; Fri, 17 Jun 2022 21:23:41 +0200 Received: from [2a02:908:1864:dbe0::6768] (helo=fedora) by sslproxy02.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1o2HZE-0008Ab-O2; Fri, 17 Jun 2022 21:23:40 +0200 Date: Fri, 17 Jun 2022 21:23:34 +0200 From: Tim Lange Subject: Re: [RFC] analyzer: allocation size warning To: Prathamesh Kulkarni Cc: David Malcolm , GCC Mailing List Message-Id: In-Reply-To: References: 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=-10.1 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 19:23:47 -0000 On Fr, Jun 17 2022 at 22:45:42 +0530, Prathamesh Kulkarni=20 wrote: > On Fri, 17 Jun 2022 at 21:25, Tim Lange wrote: >>=20 >> Hi everyone, > Hi Tim, > Thanks for posting the POC patch! > Just a couple of comments (inline) Hi Prathamesh, thanks for looking at it. >>=20 >> tracked in PR105900 [0], I'd like to add support for a new warning=20 >> on >> dubious allocation sizes. The new checker emits a warning when the >> allocation size is not a multiple of the type's size. With the=20 >> checker, >> 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 *=20 >> instead >> of + >> Because it is implemented inside the analyzer, it also emits=20 >> warnings >> when the buffer is first of type void* and later on casted to=20 >> something >> else. Though, this also inherits a limitation. The checker can not >> distinguish 2 * sizeof(short) from sizeof(int) because sizeof is >> resolved and constants are folded at the point when the analyzer=20 >> runs. >> As a mitigation, I plan to implement a check in the frontend that=20 >> emits >> a warning if sizeof(lhs pointee type) is not part of the malloc >> argument. > IMHO, warning if sizeof(lhs pointee_type) is not present inside > malloc, might not be a good idea because it > would reject valid calls to malloc. > For eg: > (1) > size_t size =3D sizeof(int); > int *p =3D malloc (size); >=20 > (2) > void *p =3D malloc (sizeof(int)); > int *q =3D p; Hm, that's right. Maybe only warn when there is a sizeof(type) in the=20 argument and the lhs pointee_type !=3D type (except for void*, maybe=20 char* and "inherited" structs)? >>=20 >> I'm looking for a first feedback on the phrasing of the diagnostics=20 >> as >> well on the preliminary patch [1]. >>=20 >> On constant buffer sizes, the warnings looks like this: >> warning: Allocated buffer size is not a multiple of the pointee's=20 >> size >> [CWE-131] [-Wanalyzer-allocation-size] >> 22 | int *ptr =3D malloc (10 + sizeof(int)); /* { dg-line malloc2=20 >> } */ >> | ^~~~~~~~~~~~~~~~~~~~~~~~~ >> =91test_2=92: event 1 >> | >> | 22 | int *ptr =3D malloc (10 + sizeof(int)); /* { dg-line=20 >> malloc2 } >> */ >> | | ^~~~~~~~~~~~~~~~~~~~~~~~~ >> | | | >> | | (1) Casting a 14 byte buffer to =91int *=92 leaves 2=20 >> trailing >> bytes; either the allocated size is bogus or the type on the=20 >> left-hand >> side is wrong >> | >>=20 >> On symbolic buffer sizes: >> warning: Allocated buffer size is not a multiple of the pointee's=20 >> size >> [CWE-131] [-Wanalyzer-allocation-size] >> 33 | int *ptr =3D malloc (n + sizeof(int)); /* { dg-line malloc3 }=20 >> */ >> | ^~~~~~~~~~~~~~~~~~~~~~~~ >> =91test_3=92: event 1 >> | >> | 33 | int *ptr =3D malloc (n + sizeof(int)); /* { dg-line=20 >> malloc3 } >> */ >> | | ^~~~~~~~~~~~~~~~~~~~~~~~ >> | | | >> | | (1) Allocation is incompatible with =91int *=92; either the >> allocated size is bogus or the type on the left-hand side is wrong >> | > Won't the warning be incorrect if 'n' is a multiple of sizeof(int) ? > I assume by symbolic buffer size, 'n' is not known at compile time. * VLAs are resolved to n * sizeof(type) when the analyzer runs and work=20 fine. * Flows with if (cond) n =3D ...; else n =3D ...; are tracked by the=20 analyzer with a widening_svalue and can be handled (While thinking=20 about this answer, I noticed my patch is missing this case. Thanks!) * In case of more complicated flows, the analyzer's buffer size=20 tracking resorts to unknown_svalue. If any variable in an expression is=20 unknown, no warning will be emitted. * Generally, when requesting memory for a variable type, accepting an=20 arbitrary number doesn't sound right. I do warn, e.g. if 'n' is a=20 conjured_svalue (e.g. a from scanf call). I think only the last case could in theory be a false-positive. I've=20 noticed that this is the case when 'n' is guarded by an if making sure=20 n is only a multiple of sizeof(type). In theory, I can fix this case=20 too as the analysis is path-sensitive. Do you know of some other case where 'n' might be an unknown value=20 neither guarded an if condition nor resorted to 'unknown' by a=20 complicated flow but still correct? - Tim >=20 > Thanks, > Prathamesh >>=20 >> And this is how a simple flow looks like: >> warning: Allocated buffer size is not a multiple of the pointee's=20 >> size >> [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=20 >> } */ >> | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >> | | | >> | | (1) allocated here >> | 39 | int *iptr =3D (int *)ptr; /* { dg-line assign } */ >> | | ~~~~ >> | | | >> | | (2) =91ptr=92 is incompatible with =91int *=92; either the >> allocated size at (1) is bogus or the type on the left-hand side is >> wrong >> | >>=20 >> There are some things to discuss from my side: >> * The tests with the "toy re-implementation of CPython's object >> model"[2] fail due to a extra warning emitted. Because the analyzer >> can't know the calculation actually results in a correct buffer size >> when viewed as a string_obj later on, it emits a warning, e.g. at=20 >> line >> 61 in data-model-5.c. The only mitigation would be to disable the >> warning for structs entirely. Now, the question is to rather have=20 >> noise >> on these cases or disable the warning for structs entirely? >> * I'm unable to emit a warning whenever the cast happens at an >> assignment with a call as the rhs, e.g. test_1 in=20 >> allocation-size-4.c. >> This is because I'm unable to access a region_svalue for the=20 >> returned >> value. Even in the new_program_state, the svalue of the lhs is=20 >> still a >> conjured_svalue. Maybe David can lead me to a place where I can=20 >> access >> 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 >> implementation. Something in the analyzer must have triggered=20 >> another >> warning about the usage of those without them having an=20 >> implementation. >> I changed those structs to have an empty implementation, such that=20 >> the >> additional warning are gone. I think this shouldn't change the test >> case, so is this change okay? >>=20 >> - Tim >>=20 >> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D105900 >> [1] While all tests except the cpython ones work, I have yet to=20 >> test it >> 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) >>=20 >> ------- >>=20 >> Subject: [PATCH] analyzer: add allocation size warning >>=20 >> This patch adds an allocation size checker to the analyzer. >> The checker warns when the tracked buffer size is not a multiple of=20 >> the >> left-hand side pointee's type. This resolves PR analyzer/105900. >>=20 >> The patch is not yet fully tested. >>=20 >> gcc/analyzer/ChangeLog: >>=20 >> * analyzer.opt: Add Wanalyzer-allocation-size. >> * sm-malloc.cc (class dubious_allocation_size): New >> 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 >> on_pointer_assignment. >> (malloc_state_machine::on_allocator_call): Add node to >> parameters and call to on_pointer_assignment. >> (malloc_state_machine::on_pointer_assignment): New. >>=20 >> gcc/testsuite/ChangeLog: >>=20 >> * gcc.dg/analyzer/attr-malloc-6.c: Disabled >> 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 >> Wanalyzer-allocation-size. >> * gcc.dg/analyzer/pr96639.c: Disabled=20 >> Wanalyzer-allocation-size >> 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. >>=20 >> 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=20 >> gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c >> create mode 100644=20 >> gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c >> create mode 100644=20 >> gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c >> create mode 100644=20 >> gcc/testsuite/gcc.dg/analyzer/allocation-size-4.c >>=20 >> 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. >>=20 >> +Wanalyzer-allocation-size >> +Common Var(warn_analyzer_allocation_size) Init(1) Warning >> +Warn about code paths in which a buffer is assigned to a=20 >> incompatible >> type. >> + >> Wanalyzer-mismatching-deallocation >> Common Var(warn_analyzer_mismatching_deallocation) Init(1) Warning >> Warn about code paths in which the wrong deallocation function is >> 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" >>=20 >> #if ENABLE_ANALYZER >>=20 >> @@ -428,6 +430,7 @@ private: >> get_or_create_deallocator (tree deallocator_fndecl); >>=20 >> 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; >> }; >>=20 >> +/* Concrete subclass for casts of pointers that lead to trailing >> bytes. */ >> + >> +class dubious_allocation_size : public malloc_diagnostic >> +{ >> +public: >> + dubious_allocation_size (const malloc_state_machine &sm, tree lhs, >> tree rhs, >> + tree size_tree, unsigned HOST_WIDE_INT size_diff) >> + : malloc_diagnostic(sm, rhs), >> 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, >> tree rhs, >> + tree size_tree) >> + : malloc_diagnostic(sm, rhs), >> 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=20 >> dubious_allocation_size >> &)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=20 >> &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)=20 >> final >> 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;=20 >> either" >> + " the allocated size is bogus or the type on the left-hand side=20 >> 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=20 >> 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. */ >>=20 >> /* Implementation of state_machine::state::dump_to_pp vfunc >> @@ -1633,6 +1757,160 @@ known_allocator_p (const_tree fndecl, const >> gcall *call) >> return false; >> } >>=20 >> +/* 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 >> (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=20 >> (); >> + 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=20 >> 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=20 >> capacity->dyn_cast_constant_svalue >> (); >> + 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 >> malloc_state_machine. */ >>=20 >> bool >> @@ -1645,14 +1923,14 @@ malloc_state_machine::on_stmt (sm_context >> *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; >> } >>=20 >> 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,=20 >> 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,=20 >> 1) >> || is_named_call_p (callee_fndecl, "operator delete", call, 2)) >> { >> @@ -1707,7 +1985,7 @@ malloc_state_machine::on_stmt (sm_context >> *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, >> returns_nonnull); >> } >>=20 >> /* Handle "__attribute__((nonnull))". */ >> @@ -1763,12 +2041,31 @@ malloc_state_machine::on_stmt (sm_context >> *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)); >> } >>=20 >> if (tree lhs =3D sm_ctxt->is_zero_assignment (stmt)) >> if (any_pointer_p (lhs)) >> on_zero_assignment (sm_ctxt, stmt,lhs); >>=20 >> + /* Handle pointer assignments/casts for dubious allocation size.=20 >> */ >> + if (const gassign *assign_stmt =3D dyn_cast =20 >> (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 >> *sm_ctxt, >>=20 >> 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 >> (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 >> (call)); >> } >> else >> { >> @@ -1968,6 +2269,60 @@ malloc_state_machine::on_realloc_call >> (sm_context *sm_ctxt, >> } >> } >>=20 >> +/* 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 >> duplicate >> + warnings on assignments after the cast. */ >> + if (pending_diagnostic::same_tree_p (TREE_TYPE (lhs), TREE_TYPE >> (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,=20 >> NULL); >> + if (const region_svalue *reg =3D dyn_cast >> (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,=20 >> 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 >> 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,=20 >> NULL); >> + if (const region_svalue *reg =3D dyn_cast >> (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,=20 >> 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 >> malloc_state_machine. */ >>=20 >> void >> diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-1.c >> 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 >> pointee's size" "" { target *-*-* } malloc } */ >> + /* { dg-message "\\(1\\) Casting a 42 byte buffer to 'int \\*'=20 >> leaves >> 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 >> pointee's size" "" { target *-*-* } assign } */ >> + /* { dg-message "\\(2\\) Casting 'ptr' to 'int \\*' leaves 2=20 >> trailing >> 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=20 >> } */ >> + free (ptr); >> + >> + /* { dg-warning "" "" { target *-*-* } malloc6 } */ >> + /* { dg-message "" "" { target *-*-* } malloc6 } */ >> +} >> diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-2.c >> 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 >> 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 >> pointee's size" "" { target *-*-* } assign } */ >> + /* { dg-message "\\(2\\) 'ptr' is incompatible with 'int \\*';=20 >> either >> the allocated size at \\(1\\)" "" { target *-*-* } assign } */ >> +} >> diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c >> 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 >> 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 >> 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. */ >>=20 >> #define A(...) __attribute__ ((malloc (__VA_ARGS__))) >>=20 >> +struct FILE {}; >> typedef struct FILE FILE; >> typedef __SIZE_TYPE__ size_t; >>=20 >> diff --git a/gcc/testsuite/gcc.dg/analyzer/capacity-1.c >> 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=20 >> malloc } >> */ >> return p; >> + >> + /* { dg-warning "" "" { target *-*-* } malloc } */ >> + /* { dg-message "" "" { target *-*-* } malloc } */ >> } >>=20 >> struct s * >> diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-4.c >> 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 >> -Wno-analyzer-allocation-size" } */ >>=20 >> #include >>=20 >> -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 >> 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__); >>=20 >> int >> -- >> 2.36.1 >>=20 >>=20 >>=20