From patchwork Fri Dec 20 16:47:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 37033 Received: (qmail 30506 invoked by alias); 20 Dec 2019 16:53:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 30493 invoked by uid 89); 20 Dec 2019 16:53:53 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy= X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 20 Dec 2019 16:53:50 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 3A87D2064A; Fri, 20 Dec 2019 11:47:31 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id CAD1D20577; Fri, 20 Dec 2019 11:47:08 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 9176D281DC; Fri, 20 Dec 2019 11:47:08 -0500 (EST) X-Gerrit-PatchSet: 3 Date: Fri, 20 Dec 2019 11:47:08 -0500 From: "Sourceware to Gerrit sync (Code Review)" To: Tankut Baris Aktemur , gdb-patches@sourceware.org Cc: Tom Tromey , Andrew Burgess Auto-Submitted: auto-generated X-Gerrit-MessageType: merged Subject: [pushed] infcall, c++: collect more pass-by-reference information X-Gerrit-Change-Id: Ic05bd98a962d07ec3c1ad041f709687eabda3bb9 X-Gerrit-Change-Number: 137 X-Gerrit-ChangeURL: X-Gerrit-Commit: 62bf63d74d54482d42e9d78890ebc0dd4675e23b In-Reply-To: References: Reply-To: noreply@gnutoolchain-gerrit.osci.io, tankut.baris.aktemur@intel.com, tromey@sourceware.org, andrew.burgess@embecosm.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Message-Id: <20191220164708.9176D281DC@gnutoolchain-gerrit.osci.io> Sourceware to Gerrit sync has submitted this change. Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/137 ...................................................................... infcall, c++: collect more pass-by-reference information Walk through a given type to collect information about whether the type is copy constructible, destructible, trivially copyable, trivially copy constructible, trivially destructible. The previous algorithm returned only a boolean result about whether the type is trivially copyable. This patch computes more info. Additionally, it utilizes DWARF attributes that were previously not taken into account; namely, DW_AT_deleted, DW_AT_defaulted, and DW_AT_calling_convention. gdb/ChangeLog: 2019-12-20 Tankut Baris Aktemur * gnu-v3-abi.c (enum definition_style): New enum type. (get_def_style): New function. (is_user_provided_def): New function. (is_implicit_def): New function. (is_copy_or_move_constructor_type): New function. (is_copy_constructor_type): New function. (is_move_constructor_type): New function. (gnuv3_pass_by_reference): Collect language_pass_by_ref_info for a given type. Change-Id: Ic05bd98a962d07ec3c1ad041f709687eabda3bb9 --- M gdb/ChangeLog M gdb/gnu-v3-abi.c 2 files changed, 248 insertions(+), 51 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3e88754..43b86f2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,17 @@ 2019-12-20 Tankut Baris Aktemur + * gnu-v3-abi.c (enum definition_style): New enum type. + (get_def_style): New function. + (is_user_provided_def): New function. + (is_implicit_def): New function. + (is_copy_or_move_constructor_type): New function. + (is_copy_constructor_type): New function. + (is_move_constructor_type): New function. + (gnuv3_pass_by_reference): Collect language_pass_by_ref_info + for a given type. + +2019-12-20 Tankut Baris Aktemur + * language.h (struct language_pass_by_ref_info): New struct. (struct language_defn): Change the signature to return a language_pass_by_ref_info instead of an int. diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c index 35197e5..33be218 100644 --- a/gdb/gnu-v3-abi.c +++ b/gdb/gnu-v3-abi.c @@ -23,6 +23,7 @@ #include "cp-abi.h" #include "cp-support.h" #include "demangle.h" +#include "dwarf2.h" #include "objfiles.h" #include "valprint.h" #include "c-lang.h" @@ -1230,6 +1231,127 @@ return real_stop_pc; } +/* A member function is in one these states. */ + +enum definition_style +{ + DOES_NOT_EXIST_IN_SOURCE, + DEFAULTED_INSIDE, + DEFAULTED_OUTSIDE, + DELETED, + EXPLICIT, +}; + +/* Return how the given field is defined. */ + +static definition_style +get_def_style (struct fn_field *fn, int fieldelem) +{ + if (TYPE_FN_FIELD_DELETED (fn, fieldelem)) + return DELETED; + + if (TYPE_FN_FIELD_ARTIFICIAL (fn, fieldelem)) + return DOES_NOT_EXIST_IN_SOURCE; + + switch (TYPE_FN_FIELD_DEFAULTED (fn, fieldelem)) + { + case DW_DEFAULTED_no: + return EXPLICIT; + case DW_DEFAULTED_in_class: + return DEFAULTED_INSIDE; + case DW_DEFAULTED_out_of_class: + return DEFAULTED_OUTSIDE; + default: + break; + } + + return EXPLICIT; +} + +/* Helper functions to determine whether the given definition style + denotes that the definition is user-provided or implicit. + Being defaulted outside the class decl counts as an explicit + user-definition, while being defaulted inside is implicit. */ + +static bool +is_user_provided_def (definition_style def) +{ + return def == EXPLICIT || def == DEFAULTED_OUTSIDE; +} + +static bool +is_implicit_def (definition_style def) +{ + return def == DOES_NOT_EXIST_IN_SOURCE || def == DEFAULTED_INSIDE; +} + +/* Helper function to decide if METHOD_TYPE is a copy/move + constructor type for CLASS_TYPE. EXPECTED is the expected + type code for the "right-hand-side" argument. + This function is supposed to be used by the IS_COPY_CONSTRUCTOR_TYPE + and IS_MOVE_CONSTRUCTOR_TYPE functions below. Normally, you should + not need to call this directly. */ + +static bool +is_copy_or_move_constructor_type (struct type *class_type, + struct type *method_type, + type_code expected) +{ + /* The method should take at least two arguments... */ + if (TYPE_NFIELDS (method_type) < 2) + return false; + + /* ...and the second argument should be the same as the class + type, with the expected type code... */ + struct type *arg_type = TYPE_FIELD_TYPE (method_type, 1); + + if (TYPE_CODE (arg_type) != expected) + return false; + + struct type *target = check_typedef (TYPE_TARGET_TYPE (arg_type)); + if (!(class_types_same_p (target, class_type))) + return false; + + /* ...and if any of the remaining arguments don't have a default value + then this is not a copy or move constructor, but just a + constructor. */ + for (int i = 2; i < TYPE_NFIELDS (method_type); i++) + { + arg_type = TYPE_FIELD_TYPE (method_type, i); + /* FIXME aktemur/2019-10-31: As of this date, neither + clang++-7.0.0 nor g++-8.2.0 produce a DW_AT_default_value + attribute. GDB is also not set to read this attribute, yet. + Hence, we immediately return false if there are more than + 2 parameters. + GCC bug link: + https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42959 + */ + return false; + } + + return true; +} + +/* Return true if METHOD_TYPE is a copy ctor type for CLASS_TYPE. */ + +static bool +is_copy_constructor_type (struct type *class_type, + struct type *method_type) +{ + return is_copy_or_move_constructor_type (class_type, method_type, + TYPE_CODE_REF); +} + +/* Return true if METHOD_TYPE is a move ctor type for CLASS_TYPE. */ + +static bool +is_move_constructor_type (struct type *class_type, + struct type *method_type) +{ + return is_copy_or_move_constructor_type (class_type, method_type, + TYPE_CODE_RVALUE_REF); +} + /* Return pass-by-reference information for the given TYPE. The rule in the v3 ABI document comes from section 3.1.1. If the @@ -1238,16 +1360,15 @@ is one or perform the copy itself otherwise), pass the address of the copy, and then destroy the temporary (if necessary). - For return values with non-trivial copy constructors or + For return values with non-trivial copy/move constructors or destructors, space will be allocated in the caller, and a pointer will be passed as the first argument (preceding "this"). We don't have a bulletproof mechanism for determining whether a - constructor or destructor is trivial. For GCC and DWARF2 debug - information, we can check the artificial flag. - - We don't do anything with the constructors or destructors, - but we have to get the argument passing right anyway. */ + constructor or destructor is trivial. For GCC and DWARF5 debug + information, we can check the calling_convention attribute, + the 'artificial' flag, the 'defaulted' attribute, and the + 'deleted' attribute. */ static struct language_pass_by_ref_info gnuv3_pass_by_reference (struct type *type) @@ -1260,21 +1381,39 @@ struct language_pass_by_ref_info info = default_pass_by_reference (type); - /* FIXME: Currently, this implementation only fills in the - 'trivially-copyable' field to preserve GDB's existing behavior. */ + bool has_cc_attr = false; + bool is_pass_by_value = false; + bool is_dynamic = false; + definition_style cctor_def = DOES_NOT_EXIST_IN_SOURCE; + definition_style dtor_def = DOES_NOT_EXIST_IN_SOURCE; + definition_style mctor_def = DOES_NOT_EXIST_IN_SOURCE; /* We're only interested in things that can have methods. */ if (TYPE_CODE (type) != TYPE_CODE_STRUCT && TYPE_CODE (type) != TYPE_CODE_UNION) return info; + /* The compiler may have emitted the calling convention attribute. + Note: GCC does not produce this attribute as of version 9.2.1. + Bug link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92418 */ + if (TYPE_CPLUS_CALLING_CONVENTION (type) == DW_CC_pass_by_value) + { + has_cc_attr = true; + is_pass_by_value = true; + /* Do not return immediately. We have to find out if this type + is copy_constructible and destructible. */ + } + + if (TYPE_CPLUS_CALLING_CONVENTION (type) == DW_CC_pass_by_reference) + { + has_cc_attr = true; + is_pass_by_value = false; + } + /* A dynamic class has a non-trivial copy constructor. See c++98 section 12.8 Copying class objects [class.copy]. */ if (gnuv3_dynamic_class (type)) - { - info.trivially_copyable = false; - return info; - } + is_dynamic = true; for (fieldnum = 0; fieldnum < TYPE_NFN_FIELDS (type); fieldnum++) for (fieldelem = 0; fieldelem < TYPE_FN_FIELDLIST_LENGTH (type, fieldnum); @@ -1284,49 +1423,75 @@ const char *name = TYPE_FN_FIELDLIST_NAME (type, fieldnum); struct type *fieldtype = TYPE_FN_FIELD_TYPE (fn, fieldelem); - /* If this function is marked as artificial, it is compiler-generated, - and we assume it is trivial. */ - if (TYPE_FN_FIELD_ARTIFICIAL (fn, fieldelem)) - continue; - - /* If we've found a destructor, we must pass this by reference. */ if (name[0] == '~') { - info.trivially_copyable = false; - return info; + /* We've found a destructor. + There should be at most one dtor definition. */ + gdb_assert (dtor_def == DOES_NOT_EXIST_IN_SOURCE); + dtor_def = get_def_style (fn, fieldelem); } - - /* If the mangled name of this method doesn't indicate that it - is a constructor, we're not interested. - - FIXME drow/2007-09-23: We could do this using the name of - the method and the name of the class instead of dealing - with the mangled name. We don't have a convenient function - to strip off both leading scope qualifiers and trailing - template arguments yet. */ - if (!is_constructor_name (TYPE_FN_FIELD_PHYSNAME (fn, fieldelem)) - && !TYPE_FN_FIELD_CONSTRUCTOR (fn, fieldelem)) - continue; - - /* If this method takes two arguments, and the second argument is - a reference to this class, then it is a copy constructor. */ - if (TYPE_NFIELDS (fieldtype) == 2) + else if (is_constructor_name (TYPE_FN_FIELD_PHYSNAME (fn, fieldelem)) + || TYPE_FN_FIELD_CONSTRUCTOR (fn, fieldelem)) { - struct type *arg_type = TYPE_FIELD_TYPE (fieldtype, 1); - - if (TYPE_CODE (arg_type) == TYPE_CODE_REF) + /* FIXME drow/2007-09-23: We could do this using the name of + the method and the name of the class instead of dealing + with the mangled name. We don't have a convenient function + to strip off both leading scope qualifiers and trailing + template arguments yet. */ + if (is_copy_constructor_type (type, fieldtype)) { - struct type *arg_target_type - = check_typedef (TYPE_TARGET_TYPE (arg_type)); - if (class_types_same_p (arg_target_type, type)) - { - info.trivially_copyable = false; - return info; - } + /* There may be more than one cctors. E.g.: one that + take a const parameter and another that takes a + non-const parameter. Such as: + + class K { + K (const K &k)... + K (K &k)... + }; + + It is sufficient for the type to be non-trivial + even only one of the cctors is explicit. + Therefore, update the cctor_def value in the + implicit -> explicit direction, not backwards. */ + + if (is_implicit_def (cctor_def)) + cctor_def = get_def_style (fn, fieldelem); + } + else if (is_move_constructor_type (type, fieldtype)) + { + /* Again, there may be multiple move ctors. Update the + mctor_def value if we found an explicit def and the + existing one is not explicit. Otherwise retain the + existing value. */ + if (is_implicit_def (mctor_def)) + mctor_def = get_def_style (fn, fieldelem); } } } + bool cctor_implicitly_deleted + = (mctor_def != DOES_NOT_EXIST_IN_SOURCE + && cctor_def == DOES_NOT_EXIST_IN_SOURCE); + + bool cctor_explicitly_deleted = (cctor_def == DELETED); + + if (cctor_implicitly_deleted || cctor_explicitly_deleted) + info.copy_constructible = false; + + if (dtor_def == DELETED) + info.destructible = false; + + info.trivially_destructible = is_implicit_def (dtor_def); + + info.trivially_copy_constructible + = (is_implicit_def (cctor_def) + && !is_dynamic); + + info.trivially_copyable + = (info.trivially_copy_constructible + && info.trivially_destructible + && !is_user_provided_def (mctor_def)); + /* Even if all the constructors and destructors were artificial, one of them may have invoked a non-artificial constructor or destructor in a base class. If any base class needs to be passed @@ -1337,15 +1502,35 @@ for (fieldnum = 0; fieldnum < TYPE_NFIELDS (type); fieldnum++) if (!field_is_static (&TYPE_FIELD (type, fieldnum))) { + struct type *field_type = TYPE_FIELD_TYPE (type, fieldnum); + + /* For arrays, make the decision based on the element type. */ + if (TYPE_CODE (field_type) == TYPE_CODE_ARRAY) + field_type = check_typedef (TYPE_TARGET_TYPE (field_type)); + struct language_pass_by_ref_info field_info - = gnuv3_pass_by_reference (TYPE_FIELD_TYPE (type, fieldnum)); + = gnuv3_pass_by_reference (field_type); + + if (!field_info.copy_constructible) + info.copy_constructible = false; + if (!field_info.destructible) + info.destructible = false; if (!field_info.trivially_copyable) - { - info.trivially_copyable = false; - return info; - } + info.trivially_copyable = false; + if (!field_info.trivially_copy_constructible) + info.trivially_copy_constructible = false; + if (!field_info.trivially_destructible) + info.trivially_destructible = false; } + /* Consistency check. */ + if (has_cc_attr && info.trivially_copyable != is_pass_by_value) + { + /* DWARF CC attribute is not the same as the inferred value; + use the DWARF attribute. */ + info.trivially_copyable = is_pass_by_value; + } + return info; }