From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104243 invoked by alias); 12 Mar 2018 17:20:42 -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 102987 invoked by uid 89); 12 Mar 2018 17:20:41 -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:39 +0000 Received: from pps.filterd (m0098419.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w2CHJUlZ010756 for ; Mon, 12 Mar 2018 13:20:38 -0400 Received: from e06smtp10.uk.ibm.com (e06smtp10.uk.ibm.com [195.75.94.106]) by mx0b-001b2d01.pphosted.com with ESMTP id 2gnwkj0mmu-1 (version=TLSv1.2 cipher=AES256-SHA256 bits=256 verify=NOT) for ; Mon, 12 Mar 2018 13:20:38 -0400 Received: from localhost by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 12 Mar 2018 17:20:36 -0000 Received: from b06cxnps3074.portsmouth.uk.ibm.com (9.149.109.194) by e06smtp10.uk.ibm.com (192.168.101.140) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 12 Mar 2018 17:20:33 -0000 Received: from d06av23.portsmouth.uk.ibm.com (d06av23.portsmouth.uk.ibm.com [9.149.105.59]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w2CHKWTv64290980; Mon, 12 Mar 2018 17:20:32 GMT Received: from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2DABAA4051; Mon, 12 Mar 2018 17:13:24 +0000 (GMT) Received: from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id F10B7A4040; Mon, 12 Mar 2018 17:13:23 +0000 (GMT) Received: from ThinkPad (unknown [9.152.212.63]) by d06av23.portsmouth.uk.ibm.com (Postfix) with ESMTP; Mon, 12 Mar 2018 17:13:23 +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 6/8] Create xml from target descriptions 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-0040-0000-0000-0000041E9312 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18031217-0041-0000-0000-00002621BADF Message-Id: <20180312182031.6093eccd@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=0 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/msg00258.txt.bz2 On Thu, 1 Mar 2018 11:41:04 +0000 Alan Hayward wrote: [...] > diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c > index 115e7cd77942b86dedced946773d1d71950e2bb3..ce5b89045c48c65ac4c6c48f6646f8d5c6ce9d8b 100644 > --- a/gdb/common/tdesc.c > +++ b/gdb/common/tdesc.c > @@ -288,4 +288,158 @@ tdesc_add_enum_value (tdesc_type_with_fields *type, int value, > type->fields.emplace_back (name, > tdesc_predefined_type (TDESC_TYPE_INT32), > value, -1); > -} > \ No newline at end of file This looks like an artefact to me. It also appears in the commit on your branch but not in the code. Very strange ... > +} > + > +void print_xml_feature::visit_post (const target_desc *e) > +{ > + *m_buffer += "\n"; > +} > + > +void print_xml_feature::visit_pre (const tdesc_feature *e) > +{ > + *m_buffer += " + *m_buffer += e->name; > + *m_buffer += "\">\n"; > +} Have you considered using string_printf from common-utils? I think this could make the code a lot better readable. Maybe its even worth adding a string_sprintf function. Thanks Philipp > + > +void print_xml_feature::visit_post (const tdesc_feature *e) > +{ > + *m_buffer += "\n"; > +} > + > +void print_xml_feature::visit (const tdesc_type_builtin *type) > +{ > + error (_("xml output is not supported type \"%s\"."), type->name.c_str ()); > +} > + > +void print_xml_feature::visit (const tdesc_type_vector *type) > +{ > + *m_buffer += " + *m_buffer += type->name; > + *m_buffer += "\" type=\""; > + *m_buffer += type->element_type->name; > + *m_buffer += "\" count=\""; > + *m_buffer += std::to_string (type->count); > + *m_buffer += "\"/>\n"; > +} > + > +void print_xml_feature::visit (const tdesc_type_with_fields *type) > +{ > + struct tdesc_type_field *f; > + const static char *types[] = { "struct", "union", "flags", "enum" }; > + > + gdb_assert (type->kind >= TDESC_TYPE_STRUCT && type->kind <= TDESC_TYPE_ENUM); > + *m_buffer += "<"; > + *m_buffer += types[type->kind - TDESC_TYPE_STRUCT]; > + > + switch (type->kind) > + { > + case TDESC_TYPE_STRUCT: > + case TDESC_TYPE_FLAGS: > + *m_buffer += " id=\""; > + *m_buffer += type->name; > + if (type->size > 0) > + { > + *m_buffer += "\" size=\""; > + *m_buffer += std::to_string (type->size); > + } > + *m_buffer += "\">\n"; > + > + for (const tdesc_type_field &f : type->fields) > + { > + *m_buffer += " + *m_buffer += f.name; > + if (f.start == -1) > + { > + *m_buffer += "\" type=\""; > + *m_buffer += f.type->name; > + } > + else > + { > + *m_buffer += "\" start=\""; > + *m_buffer += std::to_string (f.start); > + *m_buffer += "\" end=\""; > + *m_buffer += std::to_string (f.end); > + } > + > + *m_buffer += "\"/>\n"; > + } > + break; > + > + case TDESC_TYPE_ENUM: > + *m_buffer += " id=\""; > + *m_buffer += type->name; > + *m_buffer += "\">\n"; > + > + for (const tdesc_type_field &f : type->fields) > + { > + *m_buffer += " + *m_buffer += f.name; > + *m_buffer += "\" start=\""; > + *m_buffer += std::to_string (f.start); > + *m_buffer += "\"/>\n"; > + } > + break; > + > + case TDESC_TYPE_UNION: > + *m_buffer += " id=\""; > + *m_buffer += type->name; > + *m_buffer += "\">\n"; > + > + for (const tdesc_type_field &f : type->fields) > + { > + *m_buffer += " + *m_buffer += f.name; > + *m_buffer += "\" type=\""; > + *m_buffer += f.type->name; > + *m_buffer += "\"/>\n"; > + } > + break; > + > + default: > + error (_("xml output is not supported type \"%s\"."), > + type->name.c_str ()); > + } > + > + *m_buffer += " + *m_buffer += types[type->kind - TDESC_TYPE_STRUCT]; > + *m_buffer += ">\n"; > +} > + > +void print_xml_feature::visit (const tdesc_reg *reg) > +{ > + *m_buffer += " + *m_buffer += reg->name; > + *m_buffer += "\" bitsize=\""; > + *m_buffer += std::to_string (reg->bitsize); > + *m_buffer += "\" type=\""; > + *m_buffer += reg->type; > + *m_buffer += "\" regnum=\""; > + *m_buffer += std::to_string (reg->target_regnum); > + if (reg->group.length () > 0) > + { > + *m_buffer += "\" group=\""; > + *m_buffer += reg->group; > + } > + *m_buffer += "\"/>\n"; > +} > + > +void print_xml_feature::visit_pre (const target_desc *e) > +{ > +#ifndef IN_PROCESS_AGENT > + *m_buffer += "\n"; > + *m_buffer += "\n"; > + *m_buffer += "\n"; > + *m_buffer += ""; > + *m_buffer += tdesc_architecture_name (e); > + *m_buffer += "\n"; > + > + const char *osabi = tdesc_osabi_name (e); > + if (osabi != nullptr) > + { > + *m_buffer += ""; > + *m_buffer += osabi; > + *m_buffer += "\n"; > + } > +#endif > +} > diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h > index 8534069d616b97dc82df04939a4ac935aa4e1cad..1c0c4b15f3cf2a7c4afd190963c1ca1ac77e1b59 100644 > --- a/gdb/gdbserver/tdesc.h > +++ b/gdb/gdbserver/tdesc.h > @@ -27,7 +27,7 @@ > /* A target description. Inherit from tdesc_feature so that target_desc > can be used as tdesc_feature. */ > > -struct target_desc > +struct target_desc : tdesc_element > { > /* A vector of elements of register definitions that > describe the inferior's register set. */ > @@ -73,6 +73,8 @@ public: > return !(*this == other); > } > #endif > + > + void accept (tdesc_element_visitor &v) const; > }; > > /* Copy target description SRC to DEST. */ > diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c > index e11344762a3a4114ed9aa459d7d739bd96a90ae5..0d9609bd4c3446b7a15df569923d22fc10105b74 100644 > --- a/gdb/gdbserver/tdesc.c > +++ b/gdb/gdbserver/tdesc.c > @@ -59,6 +59,18 @@ bool target_desc::operator== (const target_desc &other) const > > #endif > > +void target_desc::accept (tdesc_element_visitor &v) const > +{ > +#ifndef IN_PROCESS_AGENT > + v.visit_pre (this); > + > + for (const tdesc_feature_up &feature : features) > + feature->accept (v); > + > + v.visit_post (this); > +#endif > +} > + > void > init_target_desc (struct target_desc *tdesc) > { > @@ -158,8 +170,7 @@ set_tdesc_osabi (struct target_desc *target_desc, const char *name) > target_desc->osabi = xstrdup (name); > } > > -/* Return a string which is of XML format, including XML target > - description to be sent to GDB. */ > +/* See common/tdesc.h. */ > > const char * > tdesc_get_features_xml (target_desc *tdesc) > @@ -171,31 +182,9 @@ tdesc_get_features_xml (target_desc *tdesc) > > if (tdesc->xmltarget == NULL) > { > - std::string buffer ("@"); > - > - buffer += ""; > - buffer += ""; > - buffer += ""; > - buffer += tdesc_architecture_name (tdesc); > - buffer += ""; > - > - const char *osabi = tdesc_osabi_name (tdesc); > - if (osabi != nullptr) > - { > - buffer += ""; > - buffer += osabi; > - buffer += ""; > - } > - > - for (const tdesc_feature_up &feature : tdesc->features) > - { > - buffer += " - buffer += feature->name; > - buffer += "\"/>"; > - } > - > - buffer += ""; > - > + std::string buffer ("@"); > + print_xml_feature v (&buffer); > + tdesc->accept (v); > tdesc->xmltarget = xstrdup (buffer.c_str ()); > } > > diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh > index 8c6e191596350fb4e983f8736985d9832f41e2d3..e6e06bdab0bdecc579686f3525e9f93555e0dd83 100755 > --- a/gdb/regformats/regdat.sh > +++ b/gdb/regformats/regdat.sh > @@ -180,7 +180,6 @@ echo > cat < #ifndef IN_PROCESS_AGENT > result->expedite_regs = expedite_regs_${name}; > - result->xmltarget = xmltarget_${name}; > #endif > > init_target_desc (result); > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index da2c1ce34531c1b23281c42f2dacbc85444ef544..93e73571177c980b4cd1975256c89e8ffb9fcdd6 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c > @@ -333,6 +333,8 @@ struct target_desc : tdesc_element > /* The features associated with this target. */ > std::vector features; > > + char *xmltarget = nullptr; > + > void accept (tdesc_element_visitor &v) const override > { > v.visit_pre (this); > @@ -1667,6 +1669,21 @@ private: > int m_next_regnum = 0; > }; > > +/* See common/tdesc.h. */ > + > +const char * > +tdesc_get_features_xml (target_desc *tdesc) > +{ > + if (tdesc->xmltarget == nullptr) > + { > + std::string buffer ("@"); > + print_xml_feature v (&buffer); > + tdesc->accept (v); > + tdesc->xmltarget = xstrdup (buffer.c_str ()); > + } > + return tdesc->xmltarget; > +} > + > static void > maint_print_c_tdesc_cmd (const char *args, int from_tty) > { > @@ -1760,7 +1777,36 @@ maintenance_check_xml_descriptions (const char *dir, int from_tty) > = file_read_description_xml (tdesc_xml.data ()); > > if (tdesc == NULL || *tdesc != *e.second) > - failed++; > + { > + printf_filtered ( _("Descriptions for %s do not match\n"), e.first); > + failed++; > + continue; > + } > + > + /* Convert both descriptions to xml, and then back again. Confirm all > + descriptions are identical. */ > + > + const char *xml = tdesc_get_features_xml ((target_desc *) tdesc); > + const char *xml2 = tdesc_get_features_xml ((target_desc *) e.second); > + gdb_assert (*xml == '@'); > + gdb_assert (*xml2 == '@'); > + const target_desc *t_trans = target_read_description_xml_string (xml+1); > + const target_desc *t_trans2 = target_read_description_xml_string (xml2+1); > + > + if (t_trans == NULL || t_trans2 == NULL) > + { > + printf_filtered ( > + _("Could not convert descriptions for %s back to xml (%p %p)\n"), > + e.first, t_trans, t_trans2); > + failed++; > + } > + else if (*tdesc != *t_trans || *tdesc != *t_trans2) > + { > + printf_filtered > + (_("Translated descriptions for %s do not match (%d %d)\n"), > + e.first, *tdesc == *t_trans, *tdesc == *t_trans2); > + failed++; > + } > } > printf_filtered (_("Tested %lu XML files, %d failed\n"), > (long) selftests::xml_tdesc.size (), failed); > diff --git a/gdb/xml-tdesc.h b/gdb/xml-tdesc.h > index 8f0679707ad0f1e04d803f955f7fb98b4cc0c8c8..fee60e86dd10e1543b935c3cebce505d4dc828e2 100644 > --- a/gdb/xml-tdesc.h > +++ b/gdb/xml-tdesc.h > @@ -44,5 +44,10 @@ const struct target_desc *target_read_description_xml (struct target_ops *); > otherwise. */ > gdb::optional target_fetch_description_xml (target_ops *ops); > > +/* Take an xml string, parse it, and return the parsed description. Does not > + handle a string containing includes. */ > + > +const struct target_desc *target_read_description_xml_string (const char *); > + > #endif /* XML_TDESC_H */ > > diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c > index 9190d5f3c64ffdc6d7987d651527f597d695c5a6..f793f07c96847a3d61188fff1c5d63952cc37565 100644 > --- a/gdb/xml-tdesc.c > +++ b/gdb/xml-tdesc.c > @@ -752,3 +752,12 @@ target_fetch_description_xml (struct target_ops *ops) > return output; > #endif > } > + > +/* Take an xml string, parse it, and return the parsed description. Does not > + handle a string containing includes. */ > + > +const struct target_desc * > +target_read_description_xml_string (const char *xml_str) > +{ > + return tdesc_parse_xml (xml_str, nullptr, nullptr); > +} >