From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 82096 invoked by alias); 12 Mar 2018 17:20:18 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 82086 invoked by uid 89); 12 Mar 2018 17:20:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KAM_LOTSOFHASH,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Mar 2018 17:20:14 +0000 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w2CHIlXa146253 for ; Mon, 12 Mar 2018 13:20:13 -0400 Received: from e06smtp15.uk.ibm.com (e06smtp15.uk.ibm.com [195.75.94.111]) by mx0a-001b2d01.pphosted.com with ESMTP id 2gnunxpdcg-1 (version=TLSv1.2 cipher=AES256-SHA256 bits=256 verify=NOT) for ; Mon, 12 Mar 2018 13:20:12 -0400 Received: from localhost by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 12 Mar 2018 17:20:11 -0000 Received: from b06cxnps3074.portsmouth.uk.ibm.com (9.149.109.194) by e06smtp15.uk.ibm.com (192.168.101.145) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 12 Mar 2018 17:20:07 -0000 Received: from d06av22.portsmouth.uk.ibm.com (d06av22.portsmouth.uk.ibm.com [9.149.105.58]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w2CHK7w025690186; Mon, 12 Mar 2018 17:20:07 GMT Received: from d06av22.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 68C8D4C046; Mon, 12 Mar 2018 17:13:24 +0000 (GMT) Received: from d06av22.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 35BE74C052; Mon, 12 Mar 2018 17:13:24 +0000 (GMT) Received: from ThinkPad (unknown [9.152.212.63]) by d06av22.portsmouth.uk.ibm.com (Postfix) with ESMTP; Mon, 12 Mar 2018 17:13:24 +0000 (GMT) Date: Mon, 12 Mar 2018 17:20:00 -0000 From: Philipp Rudo To: Alan Hayward Cc: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH v3 3/8] Commonise tdesc_feature In-Reply-To: References: <757A8B89-2EF0-46BD-BAA6-6E668538B17F@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 18031217-0020-0000-0000-000004027738 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18031217-0021-0000-0000-00004296C8D6 Message-Id: <20180312182005.1aebd3e2@ThinkPad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2018-03-12_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1803120197 X-IsSubscribed: yes X-SW-Source: 2018-03/txt/msg00257.txt.bz2 On Thu, 1 Mar 2018 11:40:00 +0000 Alan Hayward wrote: > This patch commonises tdesc_feature and makes use of it in gdbserver tdesc. > > Previously in gdbserver, target_desc was interchangeable with tdesc_feature. > Now gdbserver target_desc contains a vector of tdesc_feature. > > I am now able to commonise tdesc_create_reg (wanted to do this in the > previous patch). > > I also had to commonise tdesc_type. This is ok, because I will need them > in the next patch. > > Alan. > > 2018-03-01 Alan Hayward > > gdb/ > * common/tdesc.c (tdesc_feature::accept): Move to here. > (tdesc_feature::operator==): Likewise. > (tdesc_create_reg): Likewise. > * common/tdesc.h (tdesc_type_kind): Likewise. > (struct tdesc_type): Likewise. > (struct tdesc_feature): Likewise. > * regformats/regdat.sh: Create a feature. > * target-descriptions.c (tdesc_type_kind): Move from here. > (tdesc_type): Likewise. > (tdesc_type_up): Likewise. > (tdesc_feature): Likewise. > (tdesc_create_reg): Likewise. > > > gdbserver/ > * tdesc.c (~target_desc): Remove implictly deleted items. > (init_target_desc): Iterate all features. > (tdesc_get_features_xml): Use vector. > (tdesc_create_feature): Create feature. > * tdesc.h (tdesc_feature) Remove > (target_desc): Add features. > > > diff --git a/gdb/common/tdesc.h b/gdb/common/tdesc.h > index ac6dfdf7bb4ec2c3bbbed1b091f04308dd741f21..07083e3d1d41c4f86f85ae032b45c1d6f6a105ff 100644 > --- a/gdb/common/tdesc.h > +++ b/gdb/common/tdesc.h > @@ -31,6 +31,7 @@ struct tdesc_type_vector; > struct tdesc_type_with_fields; > struct tdesc_reg; > struct target_desc; > +struct type; This forward declaration looks unnecessary to me. Thanks Philipp > > /* The interface to visit different elements of target description. */ > > @@ -137,6 +138,99 @@ struct tdesc_reg : tdesc_element > > typedef std::unique_ptr tdesc_reg_up; > > +enum tdesc_type_kind > +{ > + /* Predefined types. */ > + TDESC_TYPE_BOOL, > + TDESC_TYPE_INT8, > + TDESC_TYPE_INT16, > + TDESC_TYPE_INT32, > + TDESC_TYPE_INT64, > + TDESC_TYPE_INT128, > + TDESC_TYPE_UINT8, > + TDESC_TYPE_UINT16, > + TDESC_TYPE_UINT32, > + TDESC_TYPE_UINT64, > + TDESC_TYPE_UINT128, > + TDESC_TYPE_CODE_PTR, > + TDESC_TYPE_DATA_PTR, > + TDESC_TYPE_IEEE_SINGLE, > + TDESC_TYPE_IEEE_DOUBLE, > + TDESC_TYPE_ARM_FPA_EXT, > + TDESC_TYPE_I387_EXT, > + > + /* Types defined by a target feature. */ > + TDESC_TYPE_VECTOR, > + TDESC_TYPE_STRUCT, > + TDESC_TYPE_UNION, > + TDESC_TYPE_FLAGS, > + TDESC_TYPE_ENUM > +}; > + > +struct tdesc_type : tdesc_element > +{ > + tdesc_type (const std::string &name_, enum tdesc_type_kind kind_) > + : name (name_), kind (kind_) > + {} > + > + virtual ~tdesc_type () = default; > + > + DISABLE_COPY_AND_ASSIGN (tdesc_type); > + > + /* The name of this type. */ > + std::string name; > + > + /* Identify the kind of this type. */ > + enum tdesc_type_kind kind; > + > + bool operator== (const tdesc_type &other) const > + { > + return name == other.name && kind == other.kind; > + } > + > + bool operator!= (const tdesc_type &other) const > + { > + return !(*this == other); > + } > +}; > + > +typedef std::unique_ptr tdesc_type_up; > + > +/* A feature from a target description. Each feature is a collection > + of other elements, e.g. registers and types. */ > + > +struct tdesc_feature : tdesc_element > +{ > + tdesc_feature (const std::string &name_) > + : name (name_) > + {} > + > + virtual ~tdesc_feature () = default; > + > + DISABLE_COPY_AND_ASSIGN (tdesc_feature); > + > + /* The name of this feature. It may be recognized by the architecture > + support code. */ > + std::string name; > + > + /* The registers associated with this feature. */ > + std::vector registers; > + > + /* The types associated with this feature. */ > + std::vector types; > + > + void accept (tdesc_element_visitor &v) const override; > + > + bool operator== (const tdesc_feature &other) const; > + > + bool operator!= (const tdesc_feature &other) const > + { > + return !(*this == other); > + } > +}; > + > +typedef std::unique_ptr tdesc_feature_up; > + > /* Allocate a new target_desc. */ > target_desc *allocate_target_description (void); > > diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c > index a5f2de4b5ba2adb61dd098d8fc569a6339164586..ef6ed944df20d4613dd0dd338710a4c1ad67bbf2 100644 > --- a/gdb/common/tdesc.c > +++ b/gdb/common/tdesc.c > @@ -38,3 +38,61 @@ tdesc_reg::tdesc_reg (struct tdesc_feature *feature, const std::string &name_, > have easy access to the containing feature when we want it later. */ > tdesc_type = tdesc_named_type (feature, type.c_str ()); > } > + > +void tdesc_feature::accept (tdesc_element_visitor &v) const > +{ > + v.visit_pre (this); > + > + for (const tdesc_type_up &type : types) > + type->accept (v); > + > + for (const tdesc_reg_up ® : registers) > + reg->accept (v); > + > + v.visit_post (this); > +} > + > +bool tdesc_feature::operator== (const tdesc_feature &other) const > +{ > + if (name != other.name) > + return false; > + > + if (registers.size () != other.registers.size ()) > + return false; > + > + for (int ix = 0; ix < registers.size (); ix++) > + { > + const tdesc_reg_up ®1 = registers[ix]; > + const tdesc_reg_up ®2 = other.registers[ix]; > + > + if (reg1 != reg2 && *reg1 != *reg2) > + return false; > + } > + > + if (types.size () != other.types.size ()) > + return false; > + > + for (int ix = 0; ix < types.size (); ix++) > + { > + const tdesc_type_up &type1 = types[ix]; > + const tdesc_type_up &type2 = other.types[ix]; > + > + if (type1 != type2 && *type1 != *type2) > + return false; > + } > + > + return true; > +} > + > +/* See common/tdesc.h. */ > + > +void > +tdesc_create_reg (struct tdesc_feature *feature, const char *name, > + int regnum, int save_restore, const char *group, > + int bitsize, const char *type) > +{ > + tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore, > + group, bitsize, type); > + > + feature->registers.emplace_back (reg); > +} > diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h > index b0b8fa3f9e63590398d0de52359ae8ee0271557b..8534069d616b97dc82df04939a4ac935aa4e1cad 100644 > --- a/gdb/gdbserver/tdesc.h > +++ b/gdb/gdbserver/tdesc.h > @@ -24,16 +24,10 @@ > #include "regdef.h" > #include > > -struct tdesc_feature > -{ > - /* The registers associated with this feature. */ > - std::vector registers; > -}; > - > /* A target description. Inherit from tdesc_feature so that target_desc > can be used as tdesc_feature. */ > > -struct target_desc : tdesc_feature > +struct target_desc > { > /* A vector of elements of register definitions that > describe the inferior's register set. */ > @@ -42,6 +36,9 @@ struct target_desc : tdesc_feature > /* The register cache size, in bytes. */ > int registers_size; > > + /* XML features in this target description. */ > + std::vector features; > + > #ifndef IN_PROCESS_AGENT > /* An array of register names. These are the "expedite" registers: > registers whose values are sent along with stop replies. */ > @@ -56,9 +53,6 @@ struct target_desc : tdesc_feature > fields features, arch, and osabi in tdesc_get_features_xml. */ > const char *xmltarget = NULL; > > - /* XML features in this target description. */ > - VEC (char_ptr) *features = NULL; > - > /* The value of element in the XML, replying GDB. */ > const char *arch = NULL; > > diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c > index 806e87da185623da29d39d9fd74c8722f795e086..72769ff00fd073d93807a4aaa59ec19ec6292641 100644 > --- a/gdb/gdbserver/tdesc.c > +++ b/gdb/gdbserver/tdesc.c > @@ -23,19 +23,11 @@ > > target_desc::~target_desc () > { > - int i; > - > for (reg *reg : reg_defs) > xfree (reg); > > xfree ((char *) arch); > xfree ((char *) osabi); > - > - char *f; > - > - for (i = 0; VEC_iterate (char_ptr, features, i, f); i++) > - xfree (f); > - VEC_free (char_ptr, features); > } > > bool target_desc::operator== (const target_desc &other) const > @@ -73,28 +65,29 @@ init_target_desc (struct target_desc *tdesc) > int offset = 0; > > /* Go through all the features and populate reg_defs. */ > - for (const tdesc_reg_up &treg : tdesc->registers) > - { > - /* Fill in any blank spaces. */ > - while (tdesc->reg_defs.size () < treg->target_regnum) > - { > - struct reg *reg = XCNEW (struct reg); > - reg->name = ""; > - reg->size = 0; > - reg->offset = offset; > - tdesc->reg_defs.push_back (reg); > - } > - > - gdb_assert (treg->target_regnum == 0 > - || treg->target_regnum == tdesc->reg_defs.size ()); > - > - struct reg *reg = XCNEW (struct reg); > - reg->name = treg->name.c_str (); > - reg->size = treg->bitsize; > - reg->offset = offset; > - tdesc->reg_defs.push_back (reg); > - offset += reg->size; > - } > + for (const tdesc_feature_up &feature : tdesc->features) > + for (const tdesc_reg_up &treg : feature->registers) > + { > + /* Fill in any blank spaces. */ > + while (tdesc->reg_defs.size () < treg->target_regnum) > + { > + struct reg *reg = XCNEW (struct reg); > + reg->name = ""; > + reg->size = 0; > + reg->offset = offset; > + tdesc->reg_defs.push_back (reg); > + } > + > + gdb_assert (treg->target_regnum == 0 > + || treg->target_regnum == tdesc->reg_defs.size ()); > + > + struct reg *reg = XCNEW (struct reg); > + reg->name = treg->name.c_str (); > + reg->size = treg->bitsize; > + reg->offset = offset; > + tdesc->reg_defs.push_back (reg); > + offset += reg->size; > + } > > tdesc->registers_size = offset / 8; > > @@ -157,7 +150,7 @@ tdesc_get_features_xml (target_desc *tdesc) > { > /* Either .xmltarget or .features is not NULL. */ > gdb_assert (tdesc->xmltarget != NULL > - || (tdesc->features != NULL > + || (!tdesc->features.empty () > && tdesc->arch != NULL)); > > if (tdesc->xmltarget == NULL) > @@ -177,12 +170,10 @@ tdesc_get_features_xml (target_desc *tdesc) > buffer += ""; > } > > - char *xml; > - > - for (int i = 0; VEC_iterate (char_ptr, tdesc->features, i, xml); i++) > + for (const tdesc_feature_up &feature : tdesc->features) > { > buffer += " - buffer += xml; > + buffer += feature->name; > buffer += "\"/>"; > } > > @@ -195,19 +186,16 @@ tdesc_get_features_xml (target_desc *tdesc) > } > #endif > > -struct tdesc_type > -{}; > - > /* See common/tdesc.h. */ > > struct tdesc_feature * > tdesc_create_feature (struct target_desc *tdesc, const char *name, > const char *xml) > { > -#ifndef IN_PROCESS_AGENT > - VEC_safe_push (char_ptr, tdesc->features, xstrdup (xml)); > -#endif > - return tdesc; > + struct tdesc_feature *new_feature = new tdesc_feature > + (xml != nullptr ? xml : name); > + tdesc->features.emplace_back (new_feature); > + return new_feature; > } > > /* See common/tdesc.h. */ > @@ -252,21 +240,6 @@ tdesc_create_struct (struct tdesc_feature *feature, const char *id) > > /* See common/tdesc.h. */ > > -void > -tdesc_create_reg (struct tdesc_feature *feature, const char *name, > - int regnum, int save_restore, const char *group, > - int bitsize, const char *type) > -{ > - struct target_desc *tdesc = (struct target_desc *) feature; > - > - tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore, > - group, bitsize, type); > - > - tdesc->registers.emplace_back (reg); > -} > - > -/* See common/tdesc.h. */ > - > struct tdesc_type * > tdesc_create_vector (struct tdesc_feature *feature, const char *name, > struct tdesc_type *field_type, int count) > diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh > index ce1627120d9439082709c82c5804724f39477eb1..8c6e191596350fb4e983f8736985d9832f41e2d3 100755 > --- a/gdb/regformats/regdat.sh > +++ b/gdb/regformats/regdat.sh > @@ -131,7 +131,7 @@ do > echo "{" > echo " static struct target_desc tdesc_${name}_s;" > echo " struct target_desc *result = &tdesc_${name}_s;" > - > + echo " struct tdesc_feature *feature = tdesc_create_feature (result, \"${name}\");" > continue > elif test "${type}" = "xmltarget"; then > xmltarget="${entry}" > @@ -149,7 +149,7 @@ do > echo "$0: $1 does not specify \`\`name''." 1>&2 > exit 1 > else > - echo " tdesc_create_reg ((struct tdesc_feature *) result, \"${entry}\"," > + echo " tdesc_create_reg (feature, \"${entry}\"," > echo " 0, 0, NULL, ${type}, NULL);" > > offset=`expr ${offset} + ${type}` > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index 3186bf886fc5f1b62e0ff77a4e5a2bc0fe52b250..a453eddec0cd97b692c4c9ace3f093c3fbc67422 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c > @@ -67,64 +67,6 @@ struct tdesc_type_field > int start, end; > }; > > -enum tdesc_type_kind > -{ > - /* Predefined types. */ > - TDESC_TYPE_BOOL, > - TDESC_TYPE_INT8, > - TDESC_TYPE_INT16, > - TDESC_TYPE_INT32, > - TDESC_TYPE_INT64, > - TDESC_TYPE_INT128, > - TDESC_TYPE_UINT8, > - TDESC_TYPE_UINT16, > - TDESC_TYPE_UINT32, > - TDESC_TYPE_UINT64, > - TDESC_TYPE_UINT128, > - TDESC_TYPE_CODE_PTR, > - TDESC_TYPE_DATA_PTR, > - TDESC_TYPE_IEEE_SINGLE, > - TDESC_TYPE_IEEE_DOUBLE, > - TDESC_TYPE_ARM_FPA_EXT, > - TDESC_TYPE_I387_EXT, > - > - /* Types defined by a target feature. */ > - TDESC_TYPE_VECTOR, > - TDESC_TYPE_STRUCT, > - TDESC_TYPE_UNION, > - TDESC_TYPE_FLAGS, > - TDESC_TYPE_ENUM > -}; > - > -struct tdesc_type : tdesc_element > -{ > - tdesc_type (const std::string &name_, enum tdesc_type_kind kind_) > - : name (name_), kind (kind_) > - {} > - > - virtual ~tdesc_type () = default; > - > - DISABLE_COPY_AND_ASSIGN (tdesc_type); > - > - /* The name of this type. */ > - std::string name; > - > - /* Identify the kind of this type. */ > - enum tdesc_type_kind kind; > - > - bool operator== (const tdesc_type &other) const > - { > - return name == other.name && kind == other.kind; > - } > - > - bool operator!= (const tdesc_type &other) const > - { > - return !(*this == other); > - } > -}; > - > -typedef std::unique_ptr tdesc_type_up; > - > struct tdesc_type_builtin : tdesc_type > { > tdesc_type_builtin (const std::string &name, enum tdesc_type_kind kind) > @@ -428,82 +370,6 @@ make_gdb_type (struct gdbarch *gdbarch, struct tdesc_type *ttype) > return gdb_type.get_type (); > } > > -/* A feature from a target description. Each feature is a collection > - of other elements, e.g. registers and types. */ > - > -struct tdesc_feature : tdesc_element > -{ > - tdesc_feature (const std::string &name_) > - : name (name_) > - {} > - > - virtual ~tdesc_feature () = default; > - > - DISABLE_COPY_AND_ASSIGN (tdesc_feature); > - > - /* The name of this feature. It may be recognized by the architecture > - support code. */ > - std::string name; > - > - /* The registers associated with this feature. */ > - std::vector registers; > - > - /* The types associated with this feature. */ > - std::vector types; > - > - void accept (tdesc_element_visitor &v) const override > - { > - v.visit_pre (this); > - > - for (const tdesc_type_up &type : types) > - type->accept (v); > - > - for (const tdesc_reg_up ® : registers) > - reg->accept (v); > - > - v.visit_post (this); > - } > - > - bool operator== (const tdesc_feature &other) const > - { > - if (name != other.name) > - return false; > - > - if (registers.size () != other.registers.size ()) > - return false; > - > - for (int ix = 0; ix < registers.size (); ix++) > - { > - const tdesc_reg_up ®1 = registers[ix]; > - const tdesc_reg_up ®2 = other.registers[ix]; > - > - if (reg1 != reg2 && *reg1 != *reg2) > - return false; > - } > - > - if (types.size () != other.types.size ()) > - return false; > - > - for (int ix = 0; ix < types.size (); ix++) > - { > - const tdesc_type_up &type1 = types[ix]; > - const tdesc_type_up &type2 = other.types[ix]; > - > - if (type1 != type2 && *type1 != *type2) > - return false; > - } > - > - return true; > - } > - > - bool operator!= (const tdesc_feature &other) const > - { > - return !(*this == other); > - } > -}; > - > -typedef std::unique_ptr tdesc_feature_up; > - > /* A target description. */ > > struct target_desc : tdesc_element > @@ -1358,20 +1224,6 @@ tdesc_use_registers (struct gdbarch *gdbarch, > tdesc_remote_register_number); > set_gdbarch_register_reggroup_p (gdbarch, tdesc_register_reggroup_p); > } > - > > - > -/* See common/tdesc.h. */ > - > -void > -tdesc_create_reg (struct tdesc_feature *feature, const char *name, > - int regnum, int save_restore, const char *group, > - int bitsize, const char *type) > -{ > - tdesc_reg *reg = new tdesc_reg (feature, name, regnum, save_restore, > - group, bitsize, type); > - > - feature->registers.emplace_back (reg); > -} > > /* See common/tdesc.h. */ >