* [RFC 0/7] Make GDB builtin target descriptions more flexible @ 2017-05-11 15:55 Yao Qi 2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi ` (7 more replies) 0 siblings, 8 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw) To: gdb-patches; +Cc: alan.hayward This patch series is to change GDB builtin target descriptions more flexible, by removing pre-generated ones. Instead, these builtin target descriptions can be got lazily and dynamically. GDB builtin target descriptions are created from initialize_tdesc_* functions in features/*.c files. This patch series demonstrate what does target descriptions look like by only touching i386-linux target descriptions. There are some shortcoming in GDB target description, 1) All builtin target descriptions are pre-defined. Since all GDB target descriptions are pre-defined, it is not flexible to compose features for different target descriptions. Suppose, some architecture has three hardware features (like avx or mpx in x86), A, B, and C. B and C can be optional. During to the current GDB target description limitation, we need to define four target descriptions A, A-B, A-C, A-B-C. If we need to add a new optional feature D, we need to double target descriptions. 2) Target feature is not parameterized. Registers in the same target feature may have different register sizes in different target descriptions. For example, the register size in "org.gnu.gdb.power.core" and "org.gnu.gdb.mips.cpu" varies between 32-bit variant and 64-bit variant. As a result, there are two xml files for the same feature respectively. Only 1) is addressed in this patch series for i386-linux target. If people like what this patch series does, I'll gradually change other target descriptions to the new style. That is why I post this RFC. GDBserver target description needs change as well, to make it more flexible too, but GDBserver changes can be independent with GDB changes, as long as the basic xml format is not changed. Patch 1 is to move mips target descriptions from -nat.c to -tdep.c, so that I can test them on x86_64-linux. I've posted it separately https://sourceware.org/ml/gdb-patches/2017-05/msg00204.html, include it here to give more context. Patch 2 adds a good unit test to verify we can get the same target description from both xml files and c files. It makes sure my following changes don't break anything on target descriptions, but it, as a unit test case, can go in independently. Patch 4 is the major part of this series, and the following patches changes i386-linux target descriptions, which become more flexible, so that we can compose these target features in a free way. Regression tested on x86_64-linux{-m32,-m64} and ppc64-linux. *** BLURB HERE *** Yao Qi (7): Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c Add unit test to builtin tdesc generated by xml Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml Share code in initialize_tdesc_ functions Centralize i386 linux target descriptions Lazily and dynamically create i386-linux target descriptions Remove builtin tdesc_i386_*_linux gdb/features/aarch64.c | 184 ++++--- gdb/features/arc-arcompact.c | 119 +++-- gdb/features/arc-v2.c | 119 +++-- gdb/features/arm/arm-with-iwmmxt.c | 126 +++-- gdb/features/arm/arm-with-m-fpa-layout.c | 76 ++- gdb/features/arm/arm-with-m-vfp-d16.c | 106 ++-- gdb/features/arm/arm-with-m.c | 57 ++- gdb/features/arm/arm-with-neon.c | 153 ++++-- gdb/features/arm/arm-with-vfpv2.c | 106 ++-- gdb/features/arm/arm-with-vfpv3.c | 138 +++-- gdb/features/i386/amd64-avx-avx512-linux.c | 403 +++++++++------ gdb/features/i386/amd64-avx-avx512.c | 366 +++++++------ gdb/features/i386/amd64-avx-linux.c | 244 ++++++--- gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c | 445 ++++++++++------ gdb/features/i386/amd64-avx-mpx-avx512-pku.c | 408 +++++++++------ gdb/features/i386/amd64-avx-mpx-linux.c | 271 ++++++---- gdb/features/i386/amd64-avx-mpx.c | 234 +++++---- gdb/features/i386/amd64-avx.c | 207 +++++--- gdb/features/i386/amd64-linux.c | 199 ++++--- gdb/features/i386/amd64-mpx-linux.c | 226 +++++--- gdb/features/i386/amd64-mpx.c | 189 ++++--- gdb/features/i386/amd64.c | 162 +++--- gdb/features/i386/i386-avx-avx512-linux.c | 213 +++++--- gdb/features/i386/i386-avx-avx512.c | 205 +++++--- gdb/features/i386/i386-avx-linux.c | 168 +++--- gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c | 251 ++++++--- gdb/features/i386/i386-avx-mpx-avx512-pku.c | 247 ++++++--- gdb/features/i386/i386-avx-mpx-linux.c | 192 ++++--- gdb/features/i386/i386-avx-mpx.c | 186 ++++--- gdb/features/i386/i386-avx.c | 159 ++++-- gdb/features/i386/i386-linux.c | 142 +++-- gdb/features/i386/i386-linux.xml | 2 +- gdb/features/i386/i386-mmx-linux.c | 105 ++-- gdb/features/i386/i386-mmx.c | 96 ++-- gdb/features/i386/i386-mpx-linux.c | 164 +++--- gdb/features/i386/i386-mpx.c | 157 ++++-- gdb/features/i386/i386.c | 130 +++-- gdb/features/i386/x32-avx-avx512-linux.c | 403 +++++++++------ gdb/features/i386/x32-avx-avx512.c | 366 +++++++------ gdb/features/i386/x32-avx-linux.c | 244 ++++++--- gdb/features/i386/x32-avx.c | 207 +++++--- gdb/features/i386/x32-linux.c | 199 ++++--- gdb/features/i386/x32.c | 162 +++--- gdb/features/microblaze-with-stack-protect.c | 155 +++--- gdb/features/microblaze.c | 136 ++--- gdb/features/mips-dsp-linux.c | 258 ++++++---- gdb/features/mips-linux.c | 224 +++++--- gdb/features/mips64-dsp-linux.c | 256 +++++---- gdb/features/mips64-linux.c | 222 +++++--- gdb/features/nds32.c | 190 ++++--- gdb/features/nios2-linux.c | 120 +++-- gdb/features/nios2.c | 120 +++-- gdb/features/rs6000/powerpc-32.c | 182 ++++--- gdb/features/rs6000/powerpc-32l.c | 204 +++++--- gdb/features/rs6000/powerpc-403.c | 355 +++++++------ gdb/features/rs6000/powerpc-403gc.c | 367 +++++++------ gdb/features/rs6000/powerpc-405.c | 290 ++++++----- gdb/features/rs6000/powerpc-505.c | 313 ++++++----- gdb/features/rs6000/powerpc-601.c | 324 +++++++----- gdb/features/rs6000/powerpc-602.c | 328 +++++++----- gdb/features/rs6000/powerpc-603.c | 328 +++++++----- gdb/features/rs6000/powerpc-604.c | 327 +++++++----- gdb/features/rs6000/powerpc-64.c | 182 ++++--- gdb/features/rs6000/powerpc-64l.c | 204 +++++--- gdb/features/rs6000/powerpc-7400.c | 379 ++++++++------ gdb/features/rs6000/powerpc-750.c | 354 +++++++------ gdb/features/rs6000/powerpc-860.c | 401 +++++++++------ gdb/features/rs6000/powerpc-altivec32.c | 273 ++++++---- gdb/features/rs6000/powerpc-altivec32l.c | 291 ++++++----- gdb/features/rs6000/powerpc-altivec64.c | 273 ++++++---- gdb/features/rs6000/powerpc-altivec64l.c | 291 ++++++----- gdb/features/rs6000/powerpc-cell32l.c | 293 ++++++----- gdb/features/rs6000/powerpc-cell64l.c | 293 ++++++----- gdb/features/rs6000/powerpc-e500.c | 184 ++++--- gdb/features/rs6000/powerpc-e500l.c | 206 +++++--- gdb/features/rs6000/powerpc-isa205-32l.c | 204 +++++--- gdb/features/rs6000/powerpc-isa205-64l.c | 204 +++++--- gdb/features/rs6000/powerpc-isa205-altivec32l.c | 291 ++++++----- gdb/features/rs6000/powerpc-isa205-altivec64l.c | 291 ++++++----- gdb/features/rs6000/powerpc-isa205-vsx32l.c | 368 +++++++------ gdb/features/rs6000/powerpc-isa205-vsx64l.c | 368 +++++++------ gdb/features/rs6000/powerpc-vsx32.c | 350 +++++++------ gdb/features/rs6000/powerpc-vsx32l.c | 368 +++++++------ gdb/features/rs6000/powerpc-vsx64.c | 350 +++++++------ gdb/features/rs6000/powerpc-vsx64l.c | 368 +++++++------ gdb/features/rs6000/rs6000.c | 184 ++++--- gdb/features/s390-linux32.c | 173 ++++--- gdb/features/s390-linux32v1.c | 175 ++++--- gdb/features/s390-linux32v2.c | 177 ++++--- gdb/features/s390-linux64.c | 205 +++++--- gdb/features/s390-linux64v1.c | 207 +++++--- gdb/features/s390-linux64v2.c | 209 +++++--- gdb/features/s390-te-linux64.c | 262 ++++++---- gdb/features/s390-tevx-linux64.c | 343 ++++++++----- gdb/features/s390-vx-linux64.c | 290 +++++++---- gdb/features/s390x-linux64.c | 173 ++++--- gdb/features/s390x-linux64v1.c | 175 ++++--- gdb/features/s390x-linux64v2.c | 177 ++++--- gdb/features/s390x-te-linux64.c | 230 ++++++--- gdb/features/s390x-tevx-linux64.c | 311 ++++++----- gdb/features/s390x-vx-linux64.c | 258 ++++++---- gdb/features/tic6x-c62x-linux.c | 90 ++-- gdb/features/tic6x-c62x.c | 90 ++-- gdb/features/tic6x-c64x-linux.c | 169 +++--- gdb/features/tic6x-c64x.c | 169 +++--- gdb/features/tic6x-c64xp-linux.c | 192 ++++--- gdb/features/tic6x-c64xp.c | 192 ++++--- gdb/i386-linux-tdep.c | 103 +++- gdb/i386-linux-tdep.h | 10 +- gdb/mips-linux-nat.c | 11 - gdb/mips-linux-tdep.c | 11 + gdb/mips-linux-tdep.h | 6 + gdb/target-descriptions.c | 570 ++++++++++++++++++--- gdb/target-descriptions.h | 20 + gdb/testsuite/gdb.xml/maint_print_struct.exp | 4 +- gdb/x86-linux-nat.c | 24 +- gdb/xml-tdesc.c | 7 +- 117 files changed, 15741 insertions(+), 9497 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC 7/7] Remove builtin tdesc_i386_*_linux 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi @ 2017-05-11 15:55 ` Yao Qi 2017-05-16 12:02 ` Philipp Rudo 2017-05-17 15:46 ` Pedro Alves 2017-05-11 15:55 ` [RFC 1/7] Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c Yao Qi ` (6 subsequent siblings) 7 siblings, 2 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw) To: gdb-patches With the previous patch, i386-linux target descriptions are dynamically generated, so don't need all these i386-linux builtin target descriptions. This patch removes the code in initialize_tdesc_i386_*_linux to create these tdesc_i386_*_linux. These initialize_tdesc_i386_*_linux functions are still kept to call selftests::record_xml to record the builtin xml file target descriptions are transformed to c files and compiled into GDB, so that we can verify that target descriptions from these xml files are tested (see changes in xml_builtin_tdesc_test). Function tdesc_print_intiailize_tdesc_p is added to control the scope of this removal. In order to highlight the change, I don't re-indent the code. gdb: 2017-05-09 Yao Qi <yao.qi@linaro.org> * target-descriptions.c (tdesc_print_intiailize_tdesc_p): New function. (maint_print_c_tdesc_cmd): Call tdesc_print_intiailize_tdesc_p and print code differently. (xml_files): New. (selftests::record_xml): New function. (selftests::xml_builtin_tdesc_test): Add test. * target-descriptions.h (selftests::record_xml): Declare. * features/i386/i386-avx-avx512-linux.c: Regenerated. * features/i386/i386-avx-linux.c: Regenerated. * features/i386/i386-avx-mpx-avx512-pku-linux.c: Regenerated. * features/i386/i386-avx-mpx-linux.c: Regenerated. * features/i386/i386-linux.c: Regenerated. * features/i386/i386-mmx-linux.c: Regenerated. * features/i386/i386-mpx-linux.c: Regenerated. --- gdb/features/i386/i386-avx-avx512-linux.c | 18 +------- gdb/features/i386/i386-avx-linux.c | 17 +------- gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c | 20 +-------- gdb/features/i386/i386-avx-mpx-linux.c | 18 +------- gdb/features/i386/i386-linux.c | 16 +------- gdb/features/i386/i386-mmx-linux.c | 15 +------ gdb/features/i386/i386-mpx-linux.c | 17 +------- gdb/target-descriptions.c | 50 +++++++++++++++++++++-- gdb/target-descriptions.h | 4 ++ 9 files changed, 58 insertions(+), 117 deletions(-) diff --git a/gdb/features/i386/i386-avx-avx512-linux.c b/gdb/features/i386/i386-avx-avx512-linux.c index e1aec3c..2f47a40 100644 --- a/gdb/features/i386/i386-avx-avx512-linux.c +++ b/gdb/features/i386/i386-avx-avx512-linux.c @@ -220,26 +220,10 @@ create_feature_org_gnu_gdb_i386_avx512 (struct target_desc *result, long regnum) } #endif /* _ORG_GNU_GDB_I386_AVX512 */ -struct target_desc *tdesc_i386_avx_avx512_linux; static void initialize_tdesc_i386_avx_avx512_linux (void) { - struct target_desc *result = allocate_target_description (); - - set_tdesc_architecture (result, bfd_scan_arch ("i386")); - - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); - - long regnum = 0; - - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_avx (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_avx512 (result, regnum); - - tdesc_i386_avx_avx512_linux = result; #if GDB_SELF_TEST - selftests::record_xml_tdesc ("i386/i386-avx-avx512-linux.xml", tdesc_i386_avx_avx512_linux); + selftests::record_xml ("i386/i386-avx-avx512-linux.xml"); #endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-linux.c b/gdb/features/i386/i386-avx-linux.c index 5645dbf..2e85e0d 100644 --- a/gdb/features/i386/i386-avx-linux.c +++ b/gdb/features/i386/i386-avx-linux.c @@ -186,25 +186,10 @@ create_feature_org_gnu_gdb_i386_avx (struct target_desc *result, long regnum) } #endif /* _ORG_GNU_GDB_I386_AVX */ -struct target_desc *tdesc_i386_avx_linux; static void initialize_tdesc_i386_avx_linux (void) { - struct target_desc *result = allocate_target_description (); - - set_tdesc_architecture (result, bfd_scan_arch ("i386")); - - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); - - long regnum = 0; - - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_avx (result, regnum); - - tdesc_i386_avx_linux = result; #if GDB_SELF_TEST - selftests::record_xml_tdesc ("i386/i386-avx-linux.xml", tdesc_i386_avx_linux); + selftests::record_xml ("i386/i386-avx-linux.xml"); #endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c b/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c index c65b515..433f596 100644 --- a/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c +++ b/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c @@ -287,28 +287,10 @@ create_feature_org_gnu_gdb_i386_pkeys (struct target_desc *result, long regnum) } #endif /* _ORG_GNU_GDB_I386_PKEYS */ -struct target_desc *tdesc_i386_avx_mpx_avx512_pku_linux; static void initialize_tdesc_i386_avx_mpx_avx512_pku_linux (void) { - struct target_desc *result = allocate_target_description (); - - set_tdesc_architecture (result, bfd_scan_arch ("i386")); - - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); - - long regnum = 0; - - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_avx (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_avx512 (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_pkeys (result, regnum); - - tdesc_i386_avx_mpx_avx512_pku_linux = result; #if GDB_SELF_TEST - selftests::record_xml_tdesc ("i386/i386-avx-mpx-avx512-pku-linux.xml", tdesc_i386_avx_mpx_avx512_pku_linux); + selftests::record_xml ("i386/i386-avx-mpx-avx512-pku-linux.xml"); #endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-mpx-linux.c b/gdb/features/i386/i386-avx-mpx-linux.c index 49d25f2..9ff8f25 100644 --- a/gdb/features/i386/i386-avx-mpx-linux.c +++ b/gdb/features/i386/i386-avx-mpx-linux.c @@ -238,26 +238,10 @@ create_feature_org_gnu_gdb_i386_mpx (struct target_desc *result, long regnum) } #endif /* _ORG_GNU_GDB_I386_MPX */ -struct target_desc *tdesc_i386_avx_mpx_linux; static void initialize_tdesc_i386_avx_mpx_linux (void) { - struct target_desc *result = allocate_target_description (); - - set_tdesc_architecture (result, bfd_scan_arch ("i386")); - - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); - - long regnum = 0; - - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_avx (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum); - - tdesc_i386_avx_mpx_linux = result; #if GDB_SELF_TEST - selftests::record_xml_tdesc ("i386/i386-avx-mpx-linux.xml", tdesc_i386_avx_mpx_linux); + selftests::record_xml ("i386/i386-avx-mpx-linux.xml"); #endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-linux.c b/gdb/features/i386/i386-linux.c index 51719c5..50ffbda 100644 --- a/gdb/features/i386/i386-linux.c +++ b/gdb/features/i386/i386-linux.c @@ -164,24 +164,10 @@ create_feature_org_gnu_gdb_i386_linux (struct target_desc *result, long regnum) } #endif /* _ORG_GNU_GDB_I386_LINUX */ -struct target_desc *tdesc_i386_linux; static void initialize_tdesc_i386_linux (void) { - struct target_desc *result = allocate_target_description (); - - set_tdesc_architecture (result, bfd_scan_arch ("i386")); - - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); - - long regnum = 0; - - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); - - tdesc_i386_linux = result; #if GDB_SELF_TEST - selftests::record_xml_tdesc ("i386/i386-linux.xml", tdesc_i386_linux); + selftests::record_xml ("i386/i386-linux.xml"); #endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-mmx-linux.c b/gdb/features/i386/i386-mmx-linux.c index 5141692..fe308ba 100644 --- a/gdb/features/i386/i386-mmx-linux.c +++ b/gdb/features/i386/i386-mmx-linux.c @@ -88,23 +88,10 @@ create_feature_org_gnu_gdb_i386_linux (struct target_desc *result, long regnum) } #endif /* _ORG_GNU_GDB_I386_LINUX */ -struct target_desc *tdesc_i386_mmx_linux; static void initialize_tdesc_i386_mmx_linux (void) { - struct target_desc *result = allocate_target_description (); - - set_tdesc_architecture (result, bfd_scan_arch ("i386")); - - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); - - long regnum = 0; - - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); - - tdesc_i386_mmx_linux = result; #if GDB_SELF_TEST - selftests::record_xml_tdesc ("i386/i386-mmx-linux.xml", tdesc_i386_mmx_linux); + selftests::record_xml ("i386/i386-mmx-linux.xml"); #endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-mpx-linux.c b/gdb/features/i386/i386-mpx-linux.c index 9c722cf..15a1ebe 100644 --- a/gdb/features/i386/i386-mpx-linux.c +++ b/gdb/features/i386/i386-mpx-linux.c @@ -216,25 +216,10 @@ create_feature_org_gnu_gdb_i386_mpx (struct target_desc *result, long regnum) } #endif /* _ORG_GNU_GDB_I386_MPX */ -struct target_desc *tdesc_i386_mpx_linux; static void initialize_tdesc_i386_mpx_linux (void) { - struct target_desc *result = allocate_target_description (); - - set_tdesc_architecture (result, bfd_scan_arch ("i386")); - - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); - - long regnum = 0; - - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); - regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum); - - tdesc_i386_mpx_linux = result; #if GDB_SELF_TEST - selftests::record_xml_tdesc ("i386/i386-mpx-linux.xml", tdesc_i386_mpx_linux); + selftests::record_xml ("i386/i386-mpx-linux.xml"); #endif /* GDB_SELF_TEST */ } diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c index a81ae0d..da44e37 100644 --- a/gdb/target-descriptions.c +++ b/gdb/target-descriptions.c @@ -2005,6 +2005,15 @@ tdesc_feature_unique_name (const struct tdesc_feature* feature) return name; } +/* Whether print code initializing tdesc_FUNCTION or not. */ + +static bool +tdesc_print_intiailize_tdesc_p (const char *function) +{ + return !(strncmp (function, "i386", 4) == 0 + && strncmp (&function[strlen (function) - 6], "_linux", 6) == 0); +} + static void maint_print_c_tdesc_cmd (char *args, int from_tty) { @@ -2262,10 +2271,15 @@ maint_print_c_tdesc_cmd (char *args, int from_tty) printf_unfiltered ("\n"); } - printf_unfiltered ("struct target_desc *tdesc_%s;\n", function); + if (tdesc_print_intiailize_tdesc_p (function)) + printf_unfiltered ("struct target_desc *tdesc_%s;\n", function); + printf_unfiltered ("static void\n"); printf_unfiltered ("initialize_tdesc_%s (void)\n", function); printf_unfiltered ("{\n"); + + if (tdesc_print_intiailize_tdesc_p (function)) + { printf_unfiltered (" struct target_desc *result = allocate_target_description ();\n"); @@ -2320,10 +2334,20 @@ maint_print_c_tdesc_cmd (char *args, int from_tty) printf_unfiltered ("\n"); printf_unfiltered (" tdesc_%s = result;\n", function); + } printf_unfiltered ("#if GDB_SELF_TEST\n"); - printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n", - filename_after_features.data (), function); + + if (tdesc_print_intiailize_tdesc_p (function)) + { + printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n", + filename_after_features.data (), function); + } + else + { + printf_unfiltered (" selftests::record_xml (\"%s\");\n", + filename_after_features.data ()); + } printf_unfiltered ("#endif /* GDB_SELF_TEST */\n"); printf_unfiltered ("}\n"); } @@ -2341,12 +2365,32 @@ record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc) lists.emplace_back (xml_file, tdesc); } +/* XML files, which generate C files and compiled into GDB. */ + +static std::vector<std::string> xml_files; + +void +record_xml (const char *xml_file) +{ + xml_files.emplace_back (xml_file); +} + /* Test these GDB builtin target descriptions are identical to these which are generated by the corresponding xml files. */ static void xml_builtin_tdesc_test (void) { + /* Make sure we have a test for every xml target description. */ + for (auto const &xml : xml_files) + { + auto r = std::find_if (std::begin (lists), std::end (lists), + [&xml](decltype (lists)::const_reference e) + -> bool {return e.first == xml;}); + + SELF_CHECK (r != std::end (lists)); + } + std::string feature_dir (ldirname (__FILE__)); struct stat st; diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h index 78416ae..2667bcd 100644 --- a/gdb/target-descriptions.h +++ b/gdb/target-descriptions.h @@ -266,6 +266,10 @@ namespace selftests { void record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc); + +/* Record c file generated by XML_FILE is compiled into GDB. */ + +void record_xml (const char *xml_file); } #endif -- 1.9.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 7/7] Remove builtin tdesc_i386_*_linux 2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi @ 2017-05-16 12:02 ` Philipp Rudo 2017-05-17 15:46 ` Pedro Alves 1 sibling, 0 replies; 32+ messages in thread From: Philipp Rudo @ 2017-05-16 12:02 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches Hi Yao, On Thu, 11 May 2017 16:55:05 +0100 Yao Qi <qiyaoltc@gmail.com> wrote: [...] > diff --git a/gdb/features/i386/i386-mpx-linux.c > b/gdb/features/i386/i386-mpx-linux.c index 9c722cf..15a1ebe 100644 > --- a/gdb/features/i386/i386-mpx-linux.c > +++ b/gdb/features/i386/i386-mpx-linux.c > @@ -216,25 +216,10 @@ create_feature_org_gnu_gdb_i386_mpx (struct target_desc > *result, long regnum) } > #endif /* _ORG_GNU_GDB_I386_MPX */ > > -struct target_desc *tdesc_i386_mpx_linux; > static void > initialize_tdesc_i386_mpx_linux (void) > { > - struct target_desc *result = allocate_target_description (); > - > - set_tdesc_architecture (result, bfd_scan_arch ("i386")); > - > - set_tdesc_osabi (result, osabi_from_tdesc_string ("GNU/Linux")); > - > - long regnum = 0; > - > - regnum = create_feature_org_gnu_gdb_i386_core_i386 (result, regnum); > - regnum = create_feature_org_gnu_gdb_i386_sse (result, regnum); > - regnum = create_feature_org_gnu_gdb_i386_linux (result, regnum); > - regnum = create_feature_org_gnu_gdb_i386_mpx (result, regnum); > - > - tdesc_i386_mpx_linux = result; > #if GDB_SELF_TEST > - selftests::record_xml_tdesc ("i386/i386-mpx-linux.xml", > tdesc_i386_mpx_linux); > + selftests::record_xml ("i386/i386-mpx-linux.xml"); > #endif /* GDB_SELF_TEST */ > } Do you really still need the initialize_tdesc_* functions when you initialize the target description dynamically? Can't you move the seftest somewhere else and get rid of it in this case? This would make the code slightly nicer. > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index a81ae0d..da44e37 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c > @@ -2005,6 +2005,15 @@ tdesc_feature_unique_name (const struct tdesc_feature* > feature) return name; > } > > +/* Whether print code initializing tdesc_FUNCTION or not. */ > + > +static bool > +tdesc_print_intiailize_tdesc_p (const char *function) > +{ > + return !(strncmp (function, "i386", 4) == 0 common/common-utils.h:startswith (function, "i386") ? > + && strncmp (&function[strlen (function) - 6], "_linux", 6) == 0); common/common-utils.h:endswith (function, "_linux") ? (From my concat_path patch https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html) Even nicer would be if you don't check the function name at all but check e.g. a flag a target must set or use a feature method. Otherwise you will get an extremely long and unreadable expression once more targets use this method. Philipp > +} > + > static void > maint_print_c_tdesc_cmd (char *args, int from_tty) > { > @@ -2262,10 +2271,15 @@ maint_print_c_tdesc_cmd (char *args, int from_tty) > printf_unfiltered ("\n"); > } > > - printf_unfiltered ("struct target_desc *tdesc_%s;\n", function); > + if (tdesc_print_intiailize_tdesc_p (function)) > + printf_unfiltered ("struct target_desc *tdesc_%s;\n", function); > + > printf_unfiltered ("static void\n"); > printf_unfiltered ("initialize_tdesc_%s (void)\n", function); > printf_unfiltered ("{\n"); > + > + if (tdesc_print_intiailize_tdesc_p (function)) > + { > printf_unfiltered > (" struct target_desc *result = allocate_target_description ();\n"); > > @@ -2320,10 +2334,20 @@ maint_print_c_tdesc_cmd (char *args, int from_tty) > > printf_unfiltered ("\n"); > printf_unfiltered (" tdesc_%s = result;\n", function); > + } > > printf_unfiltered ("#if GDB_SELF_TEST\n"); > - printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n", > - filename_after_features.data (), function); > + > + if (tdesc_print_intiailize_tdesc_p (function)) > + { > + printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", > tdesc_%s);\n", > + filename_after_features.data (), function); > + } > + else > + { > + printf_unfiltered (" selftests::record_xml (\"%s\");\n", > + filename_after_features.data ()); > + } > printf_unfiltered ("#endif /* GDB_SELF_TEST */\n"); > printf_unfiltered ("}\n"); > } > @@ -2341,12 +2365,32 @@ record_xml_tdesc (std::string xml_file, const struct > target_desc *tdesc) lists.emplace_back (xml_file, tdesc); > } > > +/* XML files, which generate C files and compiled into GDB. */ > + > +static std::vector<std::string> xml_files; > + > +void > +record_xml (const char *xml_file) > +{ > + xml_files.emplace_back (xml_file); > +} > + > /* Test these GDB builtin target descriptions are identical to these which > are generated by the corresponding xml files. */ > > static void > xml_builtin_tdesc_test (void) > { > + /* Make sure we have a test for every xml target description. */ > + for (auto const &xml : xml_files) > + { > + auto r = std::find_if (std::begin (lists), std::end (lists), > + [&xml](decltype (lists)::const_reference e) > + -> bool {return e.first == xml;}); > + > + SELF_CHECK (r != std::end (lists)); > + } > + > std::string feature_dir (ldirname (__FILE__)); > struct stat st; > > diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h > index 78416ae..2667bcd 100644 > --- a/gdb/target-descriptions.h > +++ b/gdb/target-descriptions.h > @@ -266,6 +266,10 @@ namespace selftests { > > void record_xml_tdesc (std::string xml_file, > const struct target_desc *tdesc); > + > +/* Record c file generated by XML_FILE is compiled into GDB. */ > + > +void record_xml (const char *xml_file); > } > #endif > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 7/7] Remove builtin tdesc_i386_*_linux 2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi 2017-05-16 12:02 ` Philipp Rudo @ 2017-05-17 15:46 ` Pedro Alves 1 sibling, 0 replies; 32+ messages in thread From: Pedro Alves @ 2017-05-17 15:46 UTC (permalink / raw) To: Yao Qi, gdb-patches On 05/11/2017 04:55 PM, Yao Qi wrote: > > Function tdesc_print_intiailize_tdesc_p is added to control the scope typo: "intiailize" > * target-descriptions.c (tdesc_print_intiailize_tdesc_p): New Ditto. (multiple places). > +static bool > +tdesc_print_intiailize_tdesc_p (const char *function) In the code too. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC 1/7] Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi 2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi @ 2017-05-11 15:55 ` Yao Qi 2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi ` (5 subsequent siblings) 7 siblings, 0 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw) To: gdb-patches Target description initialization are called in -tdep.c, instead of -nat.c I've posted it separately https://sourceware.org/ml/gdb-patches/2017-05/msg00204.html, include it here to give more context. gdb: 2017-05-09 Yao Qi <yao.qi@linaro.org> * mips-linux-nat.c: Move include features/mips*-linux.c to mips-linux-tdep.c. (_initialize_mips_linux_nat): Move initialize_tdesc_mips* calls to mips-linux-tdep.c. * mips-linux-tdep.c: Include features/mips*-linux.c (_initialize_mips_linux_tdep): Call initialize_tdesc_mips* functions. * mips-linux-tdep.h (tdesc_mips_linux): Declare. (tdesc_mips_dsp_linux, tdesc_mips64_linux): Declare. (tdesc_mips64_dsp_linux): Declare. --- gdb/mips-linux-nat.c | 11 ----------- gdb/mips-linux-tdep.c | 11 +++++++++++ gdb/mips-linux-tdep.h | 6 ++++++ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c index 8041d84..1fd3365 100644 --- a/gdb/mips-linux-nat.c +++ b/gdb/mips-linux-nat.c @@ -38,11 +38,6 @@ #include "nat/mips-linux-watch.h" -#include "features/mips-linux.c" -#include "features/mips-dsp-linux.c" -#include "features/mips64-linux.c" -#include "features/mips64-dsp-linux.c" - #ifndef PTRACE_GET_THREAD_AREA #define PTRACE_GET_THREAD_AREA 25 #endif @@ -803,10 +798,4 @@ triggers a breakpoint or watchpoint."), linux_nat_add_target (t); linux_nat_set_new_thread (t, mips_linux_new_thread); - - /* Initialize the standard target descriptions. */ - initialize_tdesc_mips_linux (); - initialize_tdesc_mips_dsp_linux (); - initialize_tdesc_mips64_linux (); - initialize_tdesc_mips64_dsp_linux (); } diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c index 48a582a..0f22a92 100644 --- a/gdb/mips-linux-tdep.c +++ b/gdb/mips-linux-tdep.c @@ -40,6 +40,11 @@ #include "xml-syscall.h" #include "gdb_signals.h" +#include "features/mips-linux.c" +#include "features/mips-dsp-linux.c" +#include "features/mips64-linux.c" +#include "features/mips64-dsp-linux.c" + static struct target_so_ops mips_svr4_so_ops; /* This enum represents the signals' numbers on the MIPS @@ -1764,4 +1769,10 @@ _initialize_mips_linux_tdep (void) GDB_OSABI_LINUX, mips_linux_init_abi); } + + /* Initialize the standard target descriptions. */ + initialize_tdesc_mips_linux (); + initialize_tdesc_mips_dsp_linux (); + initialize_tdesc_mips64_linux (); + initialize_tdesc_mips64_dsp_linux (); } diff --git a/gdb/mips-linux-tdep.h b/gdb/mips-linux-tdep.h index 407b577..cca4798 100644 --- a/gdb/mips-linux-tdep.h +++ b/gdb/mips-linux-tdep.h @@ -105,3 +105,9 @@ enum { /* Return 1 if MIPS_RESTART_REGNUM is usable. */ int mips_linux_restart_reg_p (struct gdbarch *gdbarch); + +/* Target descriptions. */ +extern struct target_desc *tdesc_mips_linux; +extern struct target_desc *tdesc_mips64_linux; +extern struct target_desc *tdesc_mips_dsp_linux; +extern struct target_desc *tdesc_mips64_dsp_linux; -- 1.9.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi 2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi 2017-05-11 15:55 ` [RFC 1/7] Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c Yao Qi @ 2017-05-11 15:55 ` Yao Qi 2017-05-16 12:00 ` Philipp Rudo 2017-05-17 15:41 ` Pedro Alves 2017-05-11 15:55 ` [RFC 3/7] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml Yao Qi ` (4 subsequent siblings) 7 siblings, 2 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw) To: gdb-patches I need to modify how GDB generate C files under features/ directory from xml files. I realize that we don't have a test to verify that target description got from xml files equals to the target description created by C files generated from the same xml files. GDB have two "groups" of target descriptions, one is builtin target descriptions, and the other is got by parsing xml files (xml files can be got from remote target or via "set tdesc filename XXX"). The builtin target descriptions are created from feature/*.c files, which are generated from xml files as well, GDB <---------------- XML files ^ | | | `----- C files <------, the reason that we transform xml files to c files is that GDB is still able to have some target description without xml parsing support. What this test does is to compare two target descriptions, one is from xml files, and the other is from c files (which is generated from the same xml files). They are must be identical, otherwise, there is something wrong in c files generation from xml files. gdb: 2017-05-07 Yao Qi <yao.qi@linaro.org> * target-descriptions.c (tdesc_reg_equals): New function. (tdesc_type_equals): New function. (tdesc_feature_equals): New function. (tdesc_equals): New function. (maint_print_c_tdesc_cmd): Print C code for unit test. [GDB_SELF_TEST] (lists): New variable. [GDB_SELF_TEST] (selftests::record_xml_tdesc): New function. [GDB_SELF_TEST] (xml_builtin_tdesc_test): New function. (_initialize_target_descriptions) [GDB_SELF_TEST]: Call register_self_test. * target-descriptions.h (tdesc_equals): Declare. (selftests::record_xml_tdesc): Declare. * features/aarch64.c: Regenerated. * features/arc-arcompact.c: Regenerated. * features/arc-v2.c: Regenerated. * features/arm/arm-with-iwmmxt.c: Regenerated. * features/arm/arm-with-m-fpa-layout.c: Regenerated. * features/arm/arm-with-m-vfp-d16.c: Regenerated. * features/arm/arm-with-m.c: Regenerated. * features/arm/arm-with-neon.c: Regenerated. * features/arm/arm-with-vfpv2.c: Regenerated. * features/arm/arm-with-vfpv3.c: Regenerated. * features/i386/amd64-avx-avx512-linux.c: Regenerated. * features/i386/amd64-avx-avx512.c: Regenerated. * features/i386/amd64-avx-linux.c: Regenerated. * features/i386/amd64-avx-mpx-avx512-pku-linux.c: Regenerated. * features/i386/amd64-avx-mpx-avx512-pku.c: Regenerated. * features/i386/amd64-avx-mpx-linux.c: Regenerated. * features/i386/amd64-avx-mpx.c: Regenerated. * features/i386/amd64-avx.c: Regenerated. * features/i386/amd64-linux.c: Regenerated. * features/i386/amd64-mpx-linux.c: Regenerated. * features/i386/amd64-mpx.c: Regenerated. * features/i386/amd64.c: Regenerated. * features/i386/i386-avx-avx512-linux.c: Regenerated. * features/i386/i386-avx-avx512.c: Regenerated. * features/i386/i386-avx-linux.c: Regenerated. * features/i386/i386-avx-mpx-avx512-pku-linux.c: Regenerated. * features/i386/i386-avx-mpx-avx512-pku.c: Regenerated. * features/i386/i386-avx-mpx-linux.c: Regenerated. * features/i386/i386-avx-mpx.c: Regenerated. * features/i386/i386-avx.c: Regenerated. * features/i386/i386-linux.c: Regenerated. * features/i386/i386-mmx-linux.c: Regenerated. * features/i386/i386-mmx.c: Regenerated. * features/i386/i386-mpx-linux.c: Regenerated. * features/i386/i386-mpx.c: Regenerated. * features/i386/i386.c: Regenerated. * features/i386/x32-avx-avx512-linux.c: Regenerated. * features/i386/x32-avx-avx512.c: Regenerated. * features/i386/x32-avx-linux.c: Regenerated. * features/i386/x32-avx.c: Regenerated. * features/i386/x32-linux.c: Regenerated. * features/i386/x32.c: Regenerated. * features/microblaze-with-stack-protect.c: Regenerated. * features/microblaze.c: Regenerated. * features/mips-dsp-linux.c: Regenerated. * features/mips-linux.c: Regenerated. * features/mips64-dsp-linux.c: Regenerated. * features/mips64-linux.c: Regenerated. * features/nds32.c: Regenerated. * features/nios2-linux.c: Regenerated. * features/nios2.c: Regenerated. * features/rs6000/powerpc-32.c: Regenerated. * features/rs6000/powerpc-32l.c: Regenerated. * features/rs6000/powerpc-403.c: Regenerated. * features/rs6000/powerpc-403gc.c: Regenerated. * features/rs6000/powerpc-405.c: Regenerated. * features/rs6000/powerpc-505.c: Regenerated. * features/rs6000/powerpc-601.c: Regenerated. * features/rs6000/powerpc-602.c: Regenerated. * features/rs6000/powerpc-603.c: Regenerated. * features/rs6000/powerpc-604.c: Regenerated. * features/rs6000/powerpc-64.c: Regenerated. * features/rs6000/powerpc-64l.c: Regenerated. * features/rs6000/powerpc-7400.c: Regenerated. * features/rs6000/powerpc-750.c: Regenerated. * features/rs6000/powerpc-860.c: Regenerated. * features/rs6000/powerpc-altivec32.c: Regenerated. * features/rs6000/powerpc-altivec32l.c: Regenerated. * features/rs6000/powerpc-altivec64.c: Regenerated. * features/rs6000/powerpc-altivec64l.c: Regenerated. * features/rs6000/powerpc-cell32l.c: Regenerated. * features/rs6000/powerpc-cell64l.c: Regenerated. * features/rs6000/powerpc-e500.c: Regenerated. * features/rs6000/powerpc-e500l.c: Regenerated. * features/rs6000/powerpc-isa205-32l.c: Regenerated. * features/rs6000/powerpc-isa205-64l.c: Regenerated. * features/rs6000/powerpc-isa205-altivec32l.c: Regenerated. * features/rs6000/powerpc-isa205-altivec64l.c: Regenerated. * features/rs6000/powerpc-isa205-vsx32l.c: Regenerated. * features/rs6000/powerpc-isa205-vsx64l.c: Regenerated. * features/rs6000/powerpc-vsx32.c: Regenerated. * features/rs6000/powerpc-vsx32l.c: Regenerated. * features/rs6000/powerpc-vsx64.c: Regenerated. * features/rs6000/powerpc-vsx64l.c: Regenerated. * features/rs6000/rs6000.c: Regenerated. * features/s390-linux32.c: Regenerated. * features/s390-linux32v1.c: Regenerated. * features/s390-linux32v2.c: Regenerated. * features/s390-linux64.c: Regenerated. * features/s390-linux64v1.c: Regenerated. * features/s390-linux64v2.c: Regenerated. * features/s390-te-linux64.c: Regenerated. * features/s390-tevx-linux64.c: Regenerated. * features/s390-vx-linux64.c: Regenerated. * features/s390x-linux64.c: Regenerated. * features/s390x-linux64v1.c: Regenerated. * features/s390x-linux64v2.c: Regenerated. * features/s390x-te-linux64.c: Regenerated. * features/s390x-tevx-linux64.c: Regenerated. * features/s390x-vx-linux64.c: Regenerated. * features/tic6x-c62x-linux.c: Regenerated. * features/tic6x-c62x.c: Regenerated. * features/tic6x-c64x-linux.c: Regenerated. * features/tic6x-c64x.c: Regenerated. * features/tic6x-c64xp-linux.c: Regenerated. * features/tic6x-c64xp.c: Regenerated. --- gdb/features/aarch64.c | 3 + gdb/features/arc-arcompact.c | 3 + gdb/features/arc-v2.c | 3 + gdb/features/arm/arm-with-iwmmxt.c | 3 + gdb/features/arm/arm-with-m-fpa-layout.c | 3 + gdb/features/arm/arm-with-m-vfp-d16.c | 3 + gdb/features/arm/arm-with-m.c | 3 + gdb/features/arm/arm-with-neon.c | 3 + gdb/features/arm/arm-with-vfpv2.c | 3 + gdb/features/arm/arm-with-vfpv3.c | 3 + gdb/features/i386/amd64-avx-avx512-linux.c | 5 +- gdb/features/i386/amd64-avx-avx512.c | 3 + gdb/features/i386/amd64-avx-linux.c | 3 + gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c | 5 +- gdb/features/i386/amd64-avx-mpx-avx512-pku.c | 3 + gdb/features/i386/amd64-avx-mpx-linux.c | 3 + gdb/features/i386/amd64-avx-mpx.c | 3 + gdb/features/i386/amd64-avx.c | 3 + gdb/features/i386/amd64-linux.c | 3 + gdb/features/i386/amd64-mpx-linux.c | 3 + gdb/features/i386/amd64-mpx.c | 3 + gdb/features/i386/amd64.c | 3 + gdb/features/i386/i386-avx-avx512-linux.c | 3 + gdb/features/i386/i386-avx-avx512.c | 3 + gdb/features/i386/i386-avx-linux.c | 3 + gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c | 3 + gdb/features/i386/i386-avx-mpx-avx512-pku.c | 3 + gdb/features/i386/i386-avx-mpx-linux.c | 3 + gdb/features/i386/i386-avx-mpx.c | 3 + gdb/features/i386/i386-avx.c | 3 + gdb/features/i386/i386-linux.c | 3 + gdb/features/i386/i386-mmx-linux.c | 3 + gdb/features/i386/i386-mmx.c | 3 + gdb/features/i386/i386-mpx-linux.c | 3 + gdb/features/i386/i386-mpx.c | 3 + gdb/features/i386/i386.c | 3 + gdb/features/i386/x32-avx-avx512-linux.c | 3 + gdb/features/i386/x32-avx-avx512.c | 3 + gdb/features/i386/x32-avx-linux.c | 3 + gdb/features/i386/x32-avx.c | 3 + gdb/features/i386/x32-linux.c | 3 + gdb/features/i386/x32.c | 3 + gdb/features/microblaze-with-stack-protect.c | 3 + gdb/features/microblaze.c | 3 + gdb/features/mips-dsp-linux.c | 3 + gdb/features/mips-linux.c | 3 + gdb/features/mips64-dsp-linux.c | 3 + gdb/features/mips64-linux.c | 3 + gdb/features/nds32.c | 3 + gdb/features/nios2-linux.c | 3 + gdb/features/nios2.c | 3 + gdb/features/rs6000/powerpc-32.c | 3 + gdb/features/rs6000/powerpc-32l.c | 3 + gdb/features/rs6000/powerpc-403.c | 3 + gdb/features/rs6000/powerpc-403gc.c | 3 + gdb/features/rs6000/powerpc-405.c | 3 + gdb/features/rs6000/powerpc-505.c | 3 + gdb/features/rs6000/powerpc-601.c | 3 + gdb/features/rs6000/powerpc-602.c | 3 + gdb/features/rs6000/powerpc-603.c | 3 + gdb/features/rs6000/powerpc-604.c | 3 + gdb/features/rs6000/powerpc-64.c | 3 + gdb/features/rs6000/powerpc-64l.c | 3 + gdb/features/rs6000/powerpc-7400.c | 3 + gdb/features/rs6000/powerpc-750.c | 3 + gdb/features/rs6000/powerpc-860.c | 3 + gdb/features/rs6000/powerpc-altivec32.c | 3 + gdb/features/rs6000/powerpc-altivec32l.c | 3 + gdb/features/rs6000/powerpc-altivec64.c | 3 + gdb/features/rs6000/powerpc-altivec64l.c | 3 + gdb/features/rs6000/powerpc-cell32l.c | 3 + gdb/features/rs6000/powerpc-cell64l.c | 3 + gdb/features/rs6000/powerpc-e500.c | 3 + gdb/features/rs6000/powerpc-e500l.c | 3 + gdb/features/rs6000/powerpc-isa205-32l.c | 3 + gdb/features/rs6000/powerpc-isa205-64l.c | 3 + gdb/features/rs6000/powerpc-isa205-altivec32l.c | 3 + gdb/features/rs6000/powerpc-isa205-altivec64l.c | 3 + gdb/features/rs6000/powerpc-isa205-vsx32l.c | 3 + gdb/features/rs6000/powerpc-isa205-vsx64l.c | 3 + gdb/features/rs6000/powerpc-vsx32.c | 3 + gdb/features/rs6000/powerpc-vsx32l.c | 3 + gdb/features/rs6000/powerpc-vsx64.c | 3 + gdb/features/rs6000/powerpc-vsx64l.c | 3 + gdb/features/rs6000/rs6000.c | 3 + gdb/features/s390-linux32.c | 3 + gdb/features/s390-linux32v1.c | 3 + gdb/features/s390-linux32v2.c | 3 + gdb/features/s390-linux64.c | 3 + gdb/features/s390-linux64v1.c | 3 + gdb/features/s390-linux64v2.c | 3 + gdb/features/s390-te-linux64.c | 3 + gdb/features/s390-tevx-linux64.c | 3 + gdb/features/s390-vx-linux64.c | 3 + gdb/features/s390x-linux64.c | 3 + gdb/features/s390x-linux64v1.c | 3 + gdb/features/s390x-linux64v2.c | 3 + gdb/features/s390x-te-linux64.c | 3 + gdb/features/s390x-tevx-linux64.c | 3 + gdb/features/s390x-vx-linux64.c | 3 + gdb/features/tic6x-c62x-linux.c | 3 + gdb/features/tic6x-c62x.c | 3 + gdb/features/tic6x-c64x-linux.c | 3 + gdb/features/tic6x-c64x.c | 3 + gdb/features/tic6x-c64xp-linux.c | 3 + gdb/features/tic6x-c64xp.c | 3 + gdb/target-descriptions.c | 160 +++++++++++++++++++++ gdb/target-descriptions.h | 16 +++ 108 files changed, 496 insertions(+), 2 deletions(-) diff --git a/gdb/features/aarch64.c b/gdb/features/aarch64.c index e9eaed8..0aa0c54 100644 --- a/gdb/features/aarch64.c +++ b/gdb/features/aarch64.c @@ -188,4 +188,7 @@ initialize_tdesc_aarch64 (void) tdesc_create_reg (feature, "fpcr", 67, 1, NULL, 32, "int"); tdesc_aarch64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("aarch64.xml", tdesc_aarch64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arc-arcompact.c b/gdb/features/arc-arcompact.c index a527cc2..2624007 100644 --- a/gdb/features/arc-arcompact.c +++ b/gdb/features/arc-arcompact.c @@ -72,4 +72,7 @@ initialize_tdesc_arc_arcompact (void) tdesc_create_reg (feature, "status32", 35, 1, NULL, 32, "status32_type"); tdesc_arc_arcompact = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arc-arcompact.xml", tdesc_arc_arcompact); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arc-v2.c b/gdb/features/arc-v2.c index b2bdfb5..7533c1b 100644 --- a/gdb/features/arc-v2.c +++ b/gdb/features/arc-v2.c @@ -76,4 +76,7 @@ initialize_tdesc_arc_v2 (void) tdesc_create_reg (feature, "status32", 35, 1, NULL, 32, "status32_type"); tdesc_arc_v2 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arc-v2.xml", tdesc_arc_v2); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arm/arm-with-iwmmxt.c b/gdb/features/arm/arm-with-iwmmxt.c index 1770e03..70e0dbb 100644 --- a/gdb/features/arm/arm-with-iwmmxt.c +++ b/gdb/features/arm/arm-with-iwmmxt.c @@ -79,4 +79,7 @@ initialize_tdesc_arm_with_iwmmxt (void) tdesc_create_reg (feature, "wCGR3", 47, 1, "vector", 32, "int"); tdesc_arm_with_iwmmxt = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arm/arm-with-iwmmxt.xml", tdesc_arm_with_iwmmxt); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arm/arm-with-m-fpa-layout.c b/gdb/features/arm/arm-with-m-fpa-layout.c index f720614..0a74ae9 100644 --- a/gdb/features/arm/arm-with-m-fpa-layout.c +++ b/gdb/features/arm/arm-with-m-fpa-layout.c @@ -43,4 +43,7 @@ initialize_tdesc_arm_with_m_fpa_layout (void) tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int"); tdesc_arm_with_m_fpa_layout = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arm/arm-with-m-fpa-layout.xml", tdesc_arm_with_m_fpa_layout); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arm/arm-with-m-vfp-d16.c b/gdb/features/arm/arm-with-m-vfp-d16.c index 069baac..2586fab 100644 --- a/gdb/features/arm/arm-with-m-vfp-d16.c +++ b/gdb/features/arm/arm-with-m-vfp-d16.c @@ -53,4 +53,7 @@ initialize_tdesc_arm_with_m_vfp_d16 (void) tdesc_create_reg (feature, "fpscr", 42, 1, "float", 32, "int"); tdesc_arm_with_m_vfp_d16 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arm/arm-with-m-vfp-d16.xml", tdesc_arm_with_m_vfp_d16); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arm/arm-with-m.c b/gdb/features/arm/arm-with-m.c index 64d31bb..92c47c5 100644 --- a/gdb/features/arm/arm-with-m.c +++ b/gdb/features/arm/arm-with-m.c @@ -34,4 +34,7 @@ initialize_tdesc_arm_with_m (void) tdesc_create_reg (feature, "xpsr", 25, 1, NULL, 32, "int"); tdesc_arm_with_m = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arm/arm-with-m.xml", tdesc_arm_with_m); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arm/arm-with-neon.c b/gdb/features/arm/arm-with-neon.c index d365c0f..b236458 100644 --- a/gdb/features/arm/arm-with-neon.c +++ b/gdb/features/arm/arm-with-neon.c @@ -71,4 +71,7 @@ initialize_tdesc_arm_with_neon (void) feature = tdesc_create_feature (result, "org.gnu.gdb.arm.neon"); tdesc_arm_with_neon = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arm/arm-with-neon.xml", tdesc_arm_with_neon); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arm/arm-with-vfpv2.c b/gdb/features/arm/arm-with-vfpv2.c index 0ebbfef..e045b01 100644 --- a/gdb/features/arm/arm-with-vfpv2.c +++ b/gdb/features/arm/arm-with-vfpv2.c @@ -53,4 +53,7 @@ initialize_tdesc_arm_with_vfpv2 (void) tdesc_create_reg (feature, "fpscr", 42, 1, "float", 32, "int"); tdesc_arm_with_vfpv2 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arm/arm-with-vfpv2.xml", tdesc_arm_with_vfpv2); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/arm/arm-with-vfpv3.c b/gdb/features/arm/arm-with-vfpv3.c index e235dfa..e8bfea6 100644 --- a/gdb/features/arm/arm-with-vfpv3.c +++ b/gdb/features/arm/arm-with-vfpv3.c @@ -69,4 +69,7 @@ initialize_tdesc_arm_with_vfpv3 (void) tdesc_create_reg (feature, "fpscr", 58, 1, "float", 32, "int"); tdesc_arm_with_vfpv3 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("arm/arm-with-vfpv3.xml", tdesc_arm_with_vfpv3); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-avx-avx512-linux.c b/gdb/features/i386/amd64-avx-avx512-linux.c index 6129ab1..78f3277 100644 --- a/gdb/features/i386/amd64-avx-avx512-linux.c +++ b/gdb/features/i386/amd64-avx-avx512-linux.c @@ -282,7 +282,10 @@ initialize_tdesc_amd64_avx_avx512_linux (void) tdesc_create_reg (feature, "zmm28h", 144, 1, NULL, 256, "v2ui128"); tdesc_create_reg (feature, "zmm29h", 145, 1, NULL, 256, "v2ui128"); tdesc_create_reg (feature, "zmm30h", 146, 1, NULL, 256, "v2ui128"); - tdesc_create_reg (feature, "zmm31h", 146, 1, NULL, 256, "v2ui128"); + tdesc_create_reg (feature, "zmm31h", 147, 1, NULL, 256, "v2ui128"); tdesc_amd64_avx_avx512_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-avx-avx512-linux.xml", tdesc_amd64_avx_avx512_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-avx-avx512.c b/gdb/features/i386/amd64-avx-avx512.c index 8a185c1..b0d8f53 100644 --- a/gdb/features/i386/amd64-avx-avx512.c +++ b/gdb/features/i386/amd64-avx-avx512.c @@ -276,4 +276,7 @@ initialize_tdesc_amd64_avx_avx512 (void) tdesc_create_reg (feature, "zmm31h", 144, 1, NULL, 256, "v2ui128"); tdesc_amd64_avx_avx512 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-avx-avx512.xml", tdesc_amd64_avx_avx512); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-avx-linux.c b/gdb/features/i386/amd64-avx-linux.c index 1d56dbf..3da5273 100644 --- a/gdb/features/i386/amd64-avx-linux.c +++ b/gdb/features/i386/amd64-avx-linux.c @@ -174,4 +174,7 @@ initialize_tdesc_amd64_avx_linux (void) tdesc_create_reg (feature, "ymm15h", 75, 1, NULL, 128, "uint128"); tdesc_amd64_avx_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-avx-linux.xml", tdesc_amd64_avx_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c b/gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c index 248eff7..3d63c21 100644 --- a/gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c +++ b/gdb/features/i386/amd64-avx-mpx-avx512-pku-linux.c @@ -323,7 +323,10 @@ initialize_tdesc_amd64_avx_mpx_avx512_pku_linux (void) tdesc_create_reg (feature, "zmm31h", 153, 1, NULL, 256, "v2ui128"); feature = tdesc_create_feature (result, "org.gnu.gdb.i386.pkeys"); - tdesc_create_reg (feature, "pkru", 152, 1, NULL, 32, "uint32"); + tdesc_create_reg (feature, "pkru", 154, 1, NULL, 32, "uint32"); tdesc_amd64_avx_mpx_avx512_pku_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-avx-mpx-avx512-pku-linux.xml", tdesc_amd64_avx_mpx_avx512_pku_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-avx-mpx-avx512-pku.c b/gdb/features/i386/amd64-avx-mpx-avx512-pku.c index dfe7d77..8c7d40d 100644 --- a/gdb/features/i386/amd64-avx-mpx-avx512-pku.c +++ b/gdb/features/i386/amd64-avx-mpx-avx512-pku.c @@ -317,4 +317,7 @@ initialize_tdesc_amd64_avx_mpx_avx512_pku (void) tdesc_create_reg (feature, "pkru", 151, 1, NULL, 32, "uint32"); tdesc_amd64_avx_mpx_avx512_pku = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-avx-mpx-avx512-pku.xml", tdesc_amd64_avx_mpx_avx512_pku); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-avx-mpx-linux.c b/gdb/features/i386/amd64-avx-mpx-linux.c index 26c1339..59d6f98 100644 --- a/gdb/features/i386/amd64-avx-mpx-linux.c +++ b/gdb/features/i386/amd64-avx-mpx-linux.c @@ -212,4 +212,7 @@ initialize_tdesc_amd64_avx_mpx_linux (void) tdesc_create_reg (feature, "bndstatus", 81, 1, NULL, 64, "status"); tdesc_amd64_avx_mpx_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-avx-mpx-linux.xml", tdesc_amd64_avx_mpx_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-avx-mpx.c b/gdb/features/i386/amd64-avx-mpx.c index ab56f42..f744408 100644 --- a/gdb/features/i386/amd64-avx-mpx.c +++ b/gdb/features/i386/amd64-avx-mpx.c @@ -203,4 +203,7 @@ initialize_tdesc_amd64_avx_mpx (void) tdesc_create_reg (feature, "bndstatus", 78, 1, NULL, 64, "status"); tdesc_amd64_avx_mpx = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-avx-mpx.xml", tdesc_amd64_avx_mpx); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-avx.c b/gdb/features/i386/amd64-avx.c index 42bd69a..403b802 100644 --- a/gdb/features/i386/amd64-avx.c +++ b/gdb/features/i386/amd64-avx.c @@ -165,4 +165,7 @@ initialize_tdesc_amd64_avx (void) tdesc_create_reg (feature, "ymm15h", 72, 1, NULL, 128, "uint128"); tdesc_amd64_avx = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-avx.xml", tdesc_amd64_avx); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-linux.c b/gdb/features/i386/amd64-linux.c index 0e921ba9..8585838 100644 --- a/gdb/features/i386/amd64-linux.c +++ b/gdb/features/i386/amd64-linux.c @@ -156,4 +156,7 @@ initialize_tdesc_amd64_linux (void) tdesc_create_reg (feature, "gs_base", 59, 1, NULL, 64, "int"); tdesc_amd64_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-linux.xml", tdesc_amd64_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-mpx-linux.c b/gdb/features/i386/amd64-mpx-linux.c index e26a74a..8fb5150 100644 --- a/gdb/features/i386/amd64-mpx-linux.c +++ b/gdb/features/i386/amd64-mpx-linux.c @@ -194,4 +194,7 @@ initialize_tdesc_amd64_mpx_linux (void) tdesc_create_reg (feature, "bndstatus", 65, 1, NULL, 64, "status"); tdesc_amd64_mpx_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-mpx-linux.xml", tdesc_amd64_mpx_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64-mpx.c b/gdb/features/i386/amd64-mpx.c index 41f0e78..ddce80e 100644 --- a/gdb/features/i386/amd64-mpx.c +++ b/gdb/features/i386/amd64-mpx.c @@ -185,4 +185,7 @@ initialize_tdesc_amd64_mpx (void) tdesc_create_reg (feature, "bndstatus", 62, 1, NULL, 64, "status"); tdesc_amd64_mpx = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64-mpx.xml", tdesc_amd64_mpx); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/amd64.c b/gdb/features/i386/amd64.c index b875a9b..6def0df 100644 --- a/gdb/features/i386/amd64.c +++ b/gdb/features/i386/amd64.c @@ -147,4 +147,7 @@ initialize_tdesc_amd64 (void) tdesc_create_reg (feature, "mxcsr", 56, 1, "vector", 32, "i386_mxcsr"); tdesc_amd64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/amd64.xml", tdesc_amd64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-avx512-linux.c b/gdb/features/i386/i386-avx-avx512-linux.c index 81149d5..3ebe591 100644 --- a/gdb/features/i386/i386-avx-avx512-linux.c +++ b/gdb/features/i386/i386-avx-avx512-linux.c @@ -167,4 +167,7 @@ initialize_tdesc_i386_avx_avx512_linux (void) tdesc_create_reg (feature, "zmm7h", 65, 1, NULL, 256, "v2ui128"); tdesc_i386_avx_avx512_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-avx-avx512-linux.xml", tdesc_i386_avx_avx512_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-avx512.c b/gdb/features/i386/i386-avx-avx512.c index 1075ca0..0932772 100644 --- a/gdb/features/i386/i386-avx-avx512.c +++ b/gdb/features/i386/i386-avx-avx512.c @@ -162,4 +162,7 @@ initialize_tdesc_i386_avx_avx512 (void) tdesc_create_reg (feature, "zmm7h", 64, 1, NULL, 256, "v2ui128"); tdesc_i386_avx_avx512 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-avx-avx512.xml", tdesc_i386_avx_avx512); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-linux.c b/gdb/features/i386/i386-avx-linux.c index 4a8c6b5..4bcd36b 100644 --- a/gdb/features/i386/i386-avx-linux.c +++ b/gdb/features/i386/i386-avx-linux.c @@ -146,4 +146,7 @@ initialize_tdesc_i386_avx_linux (void) tdesc_create_reg (feature, "ymm7h", 49, 1, NULL, 128, "uint128"); tdesc_i386_avx_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-avx-linux.xml", tdesc_i386_avx_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c b/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c index f90c834..b083c2b 100644 --- a/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c +++ b/gdb/features/i386/i386-avx-mpx-avx512-pku-linux.c @@ -208,4 +208,7 @@ initialize_tdesc_i386_avx_mpx_avx512_pku_linux (void) tdesc_create_reg (feature, "pkru", 72, 1, NULL, 32, "uint32"); tdesc_i386_avx_mpx_avx512_pku_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-avx-mpx-avx512-pku-linux.xml", tdesc_i386_avx_mpx_avx512_pku_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-mpx-avx512-pku.c b/gdb/features/i386/i386-avx-mpx-avx512-pku.c index 08d9b4b..790dd6d 100644 --- a/gdb/features/i386/i386-avx-mpx-avx512-pku.c +++ b/gdb/features/i386/i386-avx-mpx-avx512-pku.c @@ -203,4 +203,7 @@ initialize_tdesc_i386_avx_mpx_avx512_pku (void) tdesc_create_reg (feature, "pkru", 71, 1, NULL, 32, "uint32"); tdesc_i386_avx_mpx_avx512_pku = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-avx-mpx-avx512-pku.xml", tdesc_i386_avx_mpx_avx512_pku); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-mpx-linux.c b/gdb/features/i386/i386-avx-mpx-linux.c index 4b27bfc..997e200 100644 --- a/gdb/features/i386/i386-avx-mpx-linux.c +++ b/gdb/features/i386/i386-avx-mpx-linux.c @@ -184,4 +184,7 @@ initialize_tdesc_i386_avx_mpx_linux (void) tdesc_create_reg (feature, "bndstatus", 55, 1, NULL, 64, "status"); tdesc_i386_avx_mpx_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-avx-mpx-linux.xml", tdesc_i386_avx_mpx_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx-mpx.c b/gdb/features/i386/i386-avx-mpx.c index b27b40a..86e7697 100644 --- a/gdb/features/i386/i386-avx-mpx.c +++ b/gdb/features/i386/i386-avx-mpx.c @@ -179,4 +179,7 @@ initialize_tdesc_i386_avx_mpx (void) tdesc_create_reg (feature, "bndstatus", 54, 1, NULL, 64, "status"); tdesc_i386_avx_mpx = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-avx-mpx.xml", tdesc_i386_avx_mpx); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-avx.c b/gdb/features/i386/i386-avx.c index 1cb0f9e..4dc74cf 100644 --- a/gdb/features/i386/i386-avx.c +++ b/gdb/features/i386/i386-avx.c @@ -141,4 +141,7 @@ initialize_tdesc_i386_avx (void) tdesc_create_reg (feature, "ymm7h", 48, 1, NULL, 128, "uint128"); tdesc_i386_avx = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-avx.xml", tdesc_i386_avx); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-linux.c b/gdb/features/i386/i386-linux.c index 42c406b..39328af 100644 --- a/gdb/features/i386/i386-linux.c +++ b/gdb/features/i386/i386-linux.c @@ -136,4 +136,7 @@ initialize_tdesc_i386_linux (void) tdesc_create_reg (feature, "mxcsr", 40, 1, "vector", 32, "i386_mxcsr"); tdesc_i386_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-linux.xml", tdesc_i386_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-mmx-linux.c b/gdb/features/i386/i386-mmx-linux.c index e53b55f..415e7cc 100644 --- a/gdb/features/i386/i386-mmx-linux.c +++ b/gdb/features/i386/i386-mmx-linux.c @@ -75,4 +75,7 @@ initialize_tdesc_i386_mmx_linux (void) tdesc_create_reg (feature, "orig_eax", 41, 1, NULL, 32, "int"); tdesc_i386_mmx_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-mmx-linux.xml", tdesc_i386_mmx_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-mmx.c b/gdb/features/i386/i386-mmx.c index 74f67ed..604e114 100644 --- a/gdb/features/i386/i386-mmx.c +++ b/gdb/features/i386/i386-mmx.c @@ -70,4 +70,7 @@ initialize_tdesc_i386_mmx (void) tdesc_create_reg (feature, "fop", 31, 1, "float", 32, "int"); tdesc_i386_mmx = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-mmx.xml", tdesc_i386_mmx); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-mpx-linux.c b/gdb/features/i386/i386-mpx-linux.c index 43ea192..778568e 100644 --- a/gdb/features/i386/i386-mpx-linux.c +++ b/gdb/features/i386/i386-mpx-linux.c @@ -174,4 +174,7 @@ initialize_tdesc_i386_mpx_linux (void) tdesc_create_reg (feature, "bndstatus", 47, 1, NULL, 64, "status"); tdesc_i386_mpx_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-mpx-linux.xml", tdesc_i386_mpx_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386-mpx.c b/gdb/features/i386/i386-mpx.c index e832d2e..969422e 100644 --- a/gdb/features/i386/i386-mpx.c +++ b/gdb/features/i386/i386-mpx.c @@ -169,4 +169,7 @@ initialize_tdesc_i386_mpx (void) tdesc_create_reg (feature, "bndstatus", 46, 1, NULL, 64, "status"); tdesc_i386_mpx = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386-mpx.xml", tdesc_i386_mpx); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/i386.c b/gdb/features/i386/i386.c index ede73fc..31fc3b6 100644 --- a/gdb/features/i386/i386.c +++ b/gdb/features/i386/i386.c @@ -131,4 +131,7 @@ initialize_tdesc_i386 (void) tdesc_create_reg (feature, "mxcsr", 40, 1, "vector", 32, "i386_mxcsr"); tdesc_i386 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/i386.xml", tdesc_i386); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/x32-avx-avx512-linux.c b/gdb/features/i386/x32-avx-avx512-linux.c index 0467d87..579566f 100644 --- a/gdb/features/i386/x32-avx-avx512-linux.c +++ b/gdb/features/i386/x32-avx-avx512-linux.c @@ -285,4 +285,7 @@ initialize_tdesc_x32_avx_avx512_linux (void) tdesc_create_reg (feature, "zmm31h", 147, 1, NULL, 256, "v2ui128"); tdesc_x32_avx_avx512_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/x32-avx-avx512-linux.xml", tdesc_x32_avx_avx512_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/x32-avx-avx512.c b/gdb/features/i386/x32-avx-avx512.c index a7a2d52..18ede9b 100644 --- a/gdb/features/i386/x32-avx-avx512.c +++ b/gdb/features/i386/x32-avx-avx512.c @@ -276,4 +276,7 @@ initialize_tdesc_x32_avx_avx512 (void) tdesc_create_reg (feature, "zmm31h", 144, 1, NULL, 256, "v2ui128"); tdesc_x32_avx_avx512 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/x32-avx-avx512.xml", tdesc_x32_avx_avx512); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/x32-avx-linux.c b/gdb/features/i386/x32-avx-linux.c index 8406815..d5ca2f1 100644 --- a/gdb/features/i386/x32-avx-linux.c +++ b/gdb/features/i386/x32-avx-linux.c @@ -174,4 +174,7 @@ initialize_tdesc_x32_avx_linux (void) tdesc_create_reg (feature, "ymm15h", 75, 1, NULL, 128, "uint128"); tdesc_x32_avx_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/x32-avx-linux.xml", tdesc_x32_avx_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/x32-avx.c b/gdb/features/i386/x32-avx.c index 7f62e8f..5800f11 100644 --- a/gdb/features/i386/x32-avx.c +++ b/gdb/features/i386/x32-avx.c @@ -165,4 +165,7 @@ initialize_tdesc_x32_avx (void) tdesc_create_reg (feature, "ymm15h", 72, 1, NULL, 128, "uint128"); tdesc_x32_avx = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/x32-avx.xml", tdesc_x32_avx); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/x32-linux.c b/gdb/features/i386/x32-linux.c index ae49549..6485e0c 100644 --- a/gdb/features/i386/x32-linux.c +++ b/gdb/features/i386/x32-linux.c @@ -156,4 +156,7 @@ initialize_tdesc_x32_linux (void) tdesc_create_reg (feature, "gs_base", 59, 1, NULL, 64, "int"); tdesc_x32_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/x32-linux.xml", tdesc_x32_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/i386/x32.c b/gdb/features/i386/x32.c index 6005d99..18d4a1f 100644 --- a/gdb/features/i386/x32.c +++ b/gdb/features/i386/x32.c @@ -147,4 +147,7 @@ initialize_tdesc_x32 (void) tdesc_create_reg (feature, "mxcsr", 56, 1, "vector", 32, "i386_mxcsr"); tdesc_x32 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("i386/x32.xml", tdesc_x32); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/microblaze-with-stack-protect.c b/gdb/features/microblaze-with-stack-protect.c index b39aa19..67588f5 100644 --- a/gdb/features/microblaze-with-stack-protect.c +++ b/gdb/features/microblaze-with-stack-protect.c @@ -76,4 +76,7 @@ initialize_tdesc_microblaze_with_stack_protect (void) tdesc_create_reg (feature, "rshr", 58, 1, NULL, 32, "int"); tdesc_microblaze_with_stack_protect = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("microblaze-with-stack-protect.xml", tdesc_microblaze_with_stack_protect); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/microblaze.c b/gdb/features/microblaze.c index 6c86fc0..53a1e14 100644 --- a/gdb/features/microblaze.c +++ b/gdb/features/microblaze.c @@ -72,4 +72,7 @@ initialize_tdesc_microblaze (void) tdesc_create_reg (feature, "rtlbhi", 56, 1, NULL, 32, "int"); tdesc_microblaze = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("microblaze.xml", tdesc_microblaze); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/mips-dsp-linux.c b/gdb/features/mips-dsp-linux.c index 80ceb22..6d6ab7b 100644 --- a/gdb/features/mips-dsp-linux.c +++ b/gdb/features/mips-dsp-linux.c @@ -107,4 +107,7 @@ initialize_tdesc_mips_dsp_linux (void) tdesc_create_reg (feature, "restart", 79, 1, "system", 32, "int"); tdesc_mips_dsp_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("mips-dsp-linux.xml", tdesc_mips_dsp_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/mips-linux.c b/gdb/features/mips-linux.c index c990119..31e63ec 100644 --- a/gdb/features/mips-linux.c +++ b/gdb/features/mips-linux.c @@ -98,4 +98,7 @@ initialize_tdesc_mips_linux (void) tdesc_create_reg (feature, "restart", 72, 1, "system", 32, "int"); tdesc_mips_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("mips-linux.xml", tdesc_mips_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/mips64-dsp-linux.c b/gdb/features/mips64-dsp-linux.c index bc09078..1c3310d 100644 --- a/gdb/features/mips64-dsp-linux.c +++ b/gdb/features/mips64-dsp-linux.c @@ -105,4 +105,7 @@ initialize_tdesc_mips64_dsp_linux (void) tdesc_create_reg (feature, "restart", 79, 1, "system", 64, "int"); tdesc_mips64_dsp_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("mips64-dsp-linux.xml", tdesc_mips64_dsp_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/mips64-linux.c b/gdb/features/mips64-linux.c index 2ecda9b..bd897ee 100644 --- a/gdb/features/mips64-linux.c +++ b/gdb/features/mips64-linux.c @@ -96,4 +96,7 @@ initialize_tdesc_mips64_linux (void) tdesc_create_reg (feature, "restart", 72, 1, "system", 64, "int"); tdesc_mips64_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("mips64-linux.xml", tdesc_mips64_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/nds32.c b/gdb/features/nds32.c index 21f63f5..9fecfc7 100644 --- a/gdb/features/nds32.c +++ b/gdb/features/nds32.c @@ -89,4 +89,7 @@ initialize_tdesc_nds32 (void) tdesc_create_reg (feature, "ifc_lp", 67, 1, NULL, 32, "int"); tdesc_nds32 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("nds32.xml", tdesc_nds32); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/nios2-linux.c b/gdb/features/nios2-linux.c index 3288f79..b585b4e 100644 --- a/gdb/features/nios2-linux.c +++ b/gdb/features/nios2-linux.c @@ -68,4 +68,7 @@ initialize_tdesc_nios2_linux (void) tdesc_create_reg (feature, "mpuacc", 48, 1, NULL, 32, "uint32"); tdesc_nios2_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("nios2-linux.xml", tdesc_nios2_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/nios2.c b/gdb/features/nios2.c index 0cedc12..9536850 100644 --- a/gdb/features/nios2.c +++ b/gdb/features/nios2.c @@ -66,4 +66,7 @@ initialize_tdesc_nios2 (void) tdesc_create_reg (feature, "mpuacc", 48, 1, NULL, 32, "uint32"); tdesc_nios2 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("nios2.xml", tdesc_nios2); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-32.c b/gdb/features/rs6000/powerpc-32.c index 5ee5d9c..ae6e946 100644 --- a/gdb/features/rs6000/powerpc-32.c +++ b/gdb/features/rs6000/powerpc-32.c @@ -90,4 +90,7 @@ initialize_tdesc_powerpc_32 (void) tdesc_create_reg (feature, "fpscr", 70, 1, "float", 32, "int"); tdesc_powerpc_32 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-32.xml", tdesc_powerpc_32); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-32l.c b/gdb/features/rs6000/powerpc-32l.c index 971fd4b..f7478a4 100644 --- a/gdb/features/rs6000/powerpc-32l.c +++ b/gdb/features/rs6000/powerpc-32l.c @@ -94,4 +94,7 @@ initialize_tdesc_powerpc_32l (void) tdesc_create_reg (feature, "trap", 72, 1, NULL, 32, "int"); tdesc_powerpc_32l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-32l.xml", tdesc_powerpc_32l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-403.c b/gdb/features/rs6000/powerpc-403.c index a9106f5..50df451 100644 --- a/gdb/features/rs6000/powerpc-403.c +++ b/gdb/features/rs6000/powerpc-403.c @@ -164,4 +164,7 @@ initialize_tdesc_powerpc_403 (void) tdesc_create_reg (feature, "pbu2", 142, 1, NULL, 32, "int"); tdesc_powerpc_403 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-403.xml", tdesc_powerpc_403); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-403gc.c b/gdb/features/rs6000/powerpc-403gc.c index 402b747..b63da57 100644 --- a/gdb/features/rs6000/powerpc-403gc.c +++ b/gdb/features/rs6000/powerpc-403gc.c @@ -170,4 +170,7 @@ initialize_tdesc_powerpc_403gc (void) tdesc_create_reg (feature, "tblu", 148, 1, NULL, 32, "int"); tdesc_powerpc_403gc = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-403gc.xml", tdesc_powerpc_403gc); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-405.c b/gdb/features/rs6000/powerpc-405.c index bcfa144..d647b3a 100644 --- a/gdb/features/rs6000/powerpc-405.c +++ b/gdb/features/rs6000/powerpc-405.c @@ -133,4 +133,7 @@ initialize_tdesc_powerpc_405 (void) tdesc_create_reg (feature, "usprg0", 161, 1, NULL, 32, "int"); tdesc_powerpc_405 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-405.xml", tdesc_powerpc_405); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-505.c b/gdb/features/rs6000/powerpc-505.c index 09b0c7a..c85bca2 100644 --- a/gdb/features/rs6000/powerpc-505.c +++ b/gdb/features/rs6000/powerpc-505.c @@ -143,4 +143,7 @@ initialize_tdesc_powerpc_505 (void) tdesc_create_reg (feature, "nri", 121, 1, NULL, 32, "int"); tdesc_powerpc_505 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-505.xml", tdesc_powerpc_505); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-601.c b/gdb/features/rs6000/powerpc-601.c index f30f5e6..77a294b 100644 --- a/gdb/features/rs6000/powerpc-601.c +++ b/gdb/features/rs6000/powerpc-601.c @@ -147,4 +147,7 @@ initialize_tdesc_powerpc_601 (void) tdesc_create_reg (feature, "rtcl", 126, 1, NULL, 32, "int"); tdesc_powerpc_601 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-601.xml", tdesc_powerpc_601); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-602.c b/gdb/features/rs6000/powerpc-602.c index 7696717..73c3529 100644 --- a/gdb/features/rs6000/powerpc-602.c +++ b/gdb/features/rs6000/powerpc-602.c @@ -150,4 +150,7 @@ initialize_tdesc_powerpc_602 (void) tdesc_create_reg (feature, "lt", 130, 1, NULL, 32, "int"); tdesc_powerpc_602 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-602.xml", tdesc_powerpc_602); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-603.c b/gdb/features/rs6000/powerpc-603.c index d5dae39..ce28f9e 100644 --- a/gdb/features/rs6000/powerpc-603.c +++ b/gdb/features/rs6000/powerpc-603.c @@ -150,4 +150,7 @@ initialize_tdesc_powerpc_603 (void) tdesc_create_reg (feature, "rpa", 130, 1, NULL, 32, "int"); tdesc_powerpc_603 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-603.xml", tdesc_powerpc_603); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-604.c b/gdb/features/rs6000/powerpc-604.c index 44dc8ca..36a77bc 100644 --- a/gdb/features/rs6000/powerpc-604.c +++ b/gdb/features/rs6000/powerpc-604.c @@ -150,4 +150,7 @@ initialize_tdesc_powerpc_604 (void) tdesc_create_reg (feature, "sda", 128, 1, NULL, 32, "int"); tdesc_powerpc_604 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-604.xml", tdesc_powerpc_604); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-64.c b/gdb/features/rs6000/powerpc-64.c index 160d122..68016c7 100644 --- a/gdb/features/rs6000/powerpc-64.c +++ b/gdb/features/rs6000/powerpc-64.c @@ -90,4 +90,7 @@ initialize_tdesc_powerpc_64 (void) tdesc_create_reg (feature, "fpscr", 70, 1, "float", 32, "int"); tdesc_powerpc_64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-64.xml", tdesc_powerpc_64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-64l.c b/gdb/features/rs6000/powerpc-64l.c index 16a766e..8f911af 100644 --- a/gdb/features/rs6000/powerpc-64l.c +++ b/gdb/features/rs6000/powerpc-64l.c @@ -94,4 +94,7 @@ initialize_tdesc_powerpc_64l (void) tdesc_create_reg (feature, "trap", 72, 1, NULL, 64, "int"); tdesc_powerpc_64l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-64l.xml", tdesc_powerpc_64l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-7400.c b/gdb/features/rs6000/powerpc-7400.c index 69d20c4..17a670a 100644 --- a/gdb/features/rs6000/powerpc-7400.c +++ b/gdb/features/rs6000/powerpc-7400.c @@ -200,4 +200,7 @@ initialize_tdesc_powerpc_7400 (void) tdesc_create_reg (feature, "vrsave", 152, 1, "vector", 32, "int"); tdesc_powerpc_7400 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-7400.xml", tdesc_powerpc_7400); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-750.c b/gdb/features/rs6000/powerpc-750.c index 099a478..7c04e14 100644 --- a/gdb/features/rs6000/powerpc-750.c +++ b/gdb/features/rs6000/powerpc-750.c @@ -163,4 +163,7 @@ initialize_tdesc_powerpc_750 (void) tdesc_create_reg (feature, "thrm3", 142, 1, NULL, 32, "int"); tdesc_powerpc_750 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-750.xml", tdesc_powerpc_750); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-860.c b/gdb/features/rs6000/powerpc-860.c index 0692feb..68b72d4 100644 --- a/gdb/features/rs6000/powerpc-860.c +++ b/gdb/features/rs6000/powerpc-860.c @@ -187,4 +187,7 @@ initialize_tdesc_powerpc_860 (void) tdesc_create_reg (feature, "md_dbram1", 165, 1, NULL, 32, "int"); tdesc_powerpc_860 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-860.xml", tdesc_powerpc_860); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-altivec32.c b/gdb/features/rs6000/powerpc-altivec32.c index 285e87d..aa302ea 100644 --- a/gdb/features/rs6000/powerpc-altivec32.c +++ b/gdb/features/rs6000/powerpc-altivec32.c @@ -152,4 +152,7 @@ initialize_tdesc_powerpc_altivec32 (void) tdesc_create_reg (feature, "vrsave", 104, 1, "vector", 32, "int"); tdesc_powerpc_altivec32 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-altivec32.xml", tdesc_powerpc_altivec32); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-altivec32l.c b/gdb/features/rs6000/powerpc-altivec32l.c index 447ed47..1e7ed1b 100644 --- a/gdb/features/rs6000/powerpc-altivec32l.c +++ b/gdb/features/rs6000/powerpc-altivec32l.c @@ -156,4 +156,7 @@ initialize_tdesc_powerpc_altivec32l (void) tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int"); tdesc_powerpc_altivec32l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-altivec32l.xml", tdesc_powerpc_altivec32l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-altivec64.c b/gdb/features/rs6000/powerpc-altivec64.c index 1e9a61d..2e61480 100644 --- a/gdb/features/rs6000/powerpc-altivec64.c +++ b/gdb/features/rs6000/powerpc-altivec64.c @@ -152,4 +152,7 @@ initialize_tdesc_powerpc_altivec64 (void) tdesc_create_reg (feature, "vrsave", 104, 1, "vector", 32, "int"); tdesc_powerpc_altivec64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-altivec64.xml", tdesc_powerpc_altivec64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-altivec64l.c b/gdb/features/rs6000/powerpc-altivec64l.c index 10ecd8a..bfd9b57 100644 --- a/gdb/features/rs6000/powerpc-altivec64l.c +++ b/gdb/features/rs6000/powerpc-altivec64l.c @@ -156,4 +156,7 @@ initialize_tdesc_powerpc_altivec64l (void) tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int"); tdesc_powerpc_altivec64l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-altivec64l.xml", tdesc_powerpc_altivec64l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-cell32l.c b/gdb/features/rs6000/powerpc-cell32l.c index 7d33dc2..dd483b0 100644 --- a/gdb/features/rs6000/powerpc-cell32l.c +++ b/gdb/features/rs6000/powerpc-cell32l.c @@ -158,4 +158,7 @@ initialize_tdesc_powerpc_cell32l (void) tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int"); tdesc_powerpc_cell32l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-cell32l.xml", tdesc_powerpc_cell32l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-cell64l.c b/gdb/features/rs6000/powerpc-cell64l.c index 6054c26..6e763f2 100644 --- a/gdb/features/rs6000/powerpc-cell64l.c +++ b/gdb/features/rs6000/powerpc-cell64l.c @@ -158,4 +158,7 @@ initialize_tdesc_powerpc_cell64l (void) tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int"); tdesc_powerpc_cell64l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-cell64l.xml", tdesc_powerpc_cell64l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-e500.c b/gdb/features/rs6000/powerpc-e500.c index aaca3a7..27570bb 100644 --- a/gdb/features/rs6000/powerpc-e500.c +++ b/gdb/features/rs6000/powerpc-e500.c @@ -91,4 +91,7 @@ initialize_tdesc_powerpc_e500 (void) tdesc_create_reg (feature, "spefscr", 74, 1, NULL, 32, "int"); tdesc_powerpc_e500 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-e500.xml", tdesc_powerpc_e500); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-e500l.c b/gdb/features/rs6000/powerpc-e500l.c index de03862..3f753c8 100644 --- a/gdb/features/rs6000/powerpc-e500l.c +++ b/gdb/features/rs6000/powerpc-e500l.c @@ -95,4 +95,7 @@ initialize_tdesc_powerpc_e500l (void) tdesc_create_reg (feature, "trap", 72, 1, NULL, 32, "int"); tdesc_powerpc_e500l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-e500l.xml", tdesc_powerpc_e500l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-isa205-32l.c b/gdb/features/rs6000/powerpc-isa205-32l.c index 1b5bd6d..37e1179 100644 --- a/gdb/features/rs6000/powerpc-isa205-32l.c +++ b/gdb/features/rs6000/powerpc-isa205-32l.c @@ -94,4 +94,7 @@ initialize_tdesc_powerpc_isa205_32l (void) tdesc_create_reg (feature, "trap", 72, 1, NULL, 32, "int"); tdesc_powerpc_isa205_32l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-isa205-32l.xml", tdesc_powerpc_isa205_32l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-isa205-64l.c b/gdb/features/rs6000/powerpc-isa205-64l.c index 31bfc87..b0bbd3d 100644 --- a/gdb/features/rs6000/powerpc-isa205-64l.c +++ b/gdb/features/rs6000/powerpc-isa205-64l.c @@ -94,4 +94,7 @@ initialize_tdesc_powerpc_isa205_64l (void) tdesc_create_reg (feature, "trap", 72, 1, NULL, 64, "int"); tdesc_powerpc_isa205_64l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-isa205-64l.xml", tdesc_powerpc_isa205_64l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-isa205-altivec32l.c b/gdb/features/rs6000/powerpc-isa205-altivec32l.c index 6c216ce..e162982 100644 --- a/gdb/features/rs6000/powerpc-isa205-altivec32l.c +++ b/gdb/features/rs6000/powerpc-isa205-altivec32l.c @@ -156,4 +156,7 @@ initialize_tdesc_powerpc_isa205_altivec32l (void) tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int"); tdesc_powerpc_isa205_altivec32l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-isa205-altivec32l.xml", tdesc_powerpc_isa205_altivec32l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-isa205-altivec64l.c b/gdb/features/rs6000/powerpc-isa205-altivec64l.c index 2c206aa..93850fb 100644 --- a/gdb/features/rs6000/powerpc-isa205-altivec64l.c +++ b/gdb/features/rs6000/powerpc-isa205-altivec64l.c @@ -156,4 +156,7 @@ initialize_tdesc_powerpc_isa205_altivec64l (void) tdesc_create_reg (feature, "vrsave", 106, 1, "vector", 32, "int"); tdesc_powerpc_isa205_altivec64l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-isa205-altivec64l.xml", tdesc_powerpc_isa205_altivec64l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-isa205-vsx32l.c b/gdb/features/rs6000/powerpc-isa205-vsx32l.c index 4659ce1..05ef8c2 100644 --- a/gdb/features/rs6000/powerpc-isa205-vsx32l.c +++ b/gdb/features/rs6000/powerpc-isa205-vsx32l.c @@ -190,4 +190,7 @@ initialize_tdesc_powerpc_isa205_vsx32l (void) tdesc_create_reg (feature, "vs31h", 138, 1, NULL, 64, "uint64"); tdesc_powerpc_isa205_vsx32l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-isa205-vsx32l.xml", tdesc_powerpc_isa205_vsx32l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-isa205-vsx64l.c b/gdb/features/rs6000/powerpc-isa205-vsx64l.c index 64b12b9..f6a94c4 100644 --- a/gdb/features/rs6000/powerpc-isa205-vsx64l.c +++ b/gdb/features/rs6000/powerpc-isa205-vsx64l.c @@ -190,4 +190,7 @@ initialize_tdesc_powerpc_isa205_vsx64l (void) tdesc_create_reg (feature, "vs31h", 138, 1, NULL, 64, "uint64"); tdesc_powerpc_isa205_vsx64l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-isa205-vsx64l.xml", tdesc_powerpc_isa205_vsx64l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-vsx32.c b/gdb/features/rs6000/powerpc-vsx32.c index ba1fcb6..1a17e04 100644 --- a/gdb/features/rs6000/powerpc-vsx32.c +++ b/gdb/features/rs6000/powerpc-vsx32.c @@ -186,4 +186,7 @@ initialize_tdesc_powerpc_vsx32 (void) tdesc_create_reg (feature, "vs31h", 136, 1, NULL, 64, "uint64"); tdesc_powerpc_vsx32 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-vsx32.xml", tdesc_powerpc_vsx32); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-vsx32l.c b/gdb/features/rs6000/powerpc-vsx32l.c index 013e392..27b47ad 100644 --- a/gdb/features/rs6000/powerpc-vsx32l.c +++ b/gdb/features/rs6000/powerpc-vsx32l.c @@ -190,4 +190,7 @@ initialize_tdesc_powerpc_vsx32l (void) tdesc_create_reg (feature, "vs31h", 138, 1, NULL, 64, "uint64"); tdesc_powerpc_vsx32l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-vsx32l.xml", tdesc_powerpc_vsx32l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-vsx64.c b/gdb/features/rs6000/powerpc-vsx64.c index ca02323..9bbe42e 100644 --- a/gdb/features/rs6000/powerpc-vsx64.c +++ b/gdb/features/rs6000/powerpc-vsx64.c @@ -186,4 +186,7 @@ initialize_tdesc_powerpc_vsx64 (void) tdesc_create_reg (feature, "vs31h", 136, 1, NULL, 64, "uint64"); tdesc_powerpc_vsx64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-vsx64.xml", tdesc_powerpc_vsx64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/powerpc-vsx64l.c b/gdb/features/rs6000/powerpc-vsx64l.c index 31bb224..9df1bdb 100644 --- a/gdb/features/rs6000/powerpc-vsx64l.c +++ b/gdb/features/rs6000/powerpc-vsx64l.c @@ -190,4 +190,7 @@ initialize_tdesc_powerpc_vsx64l (void) tdesc_create_reg (feature, "vs31h", 138, 1, NULL, 64, "uint64"); tdesc_powerpc_vsx64l = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/powerpc-vsx64l.xml", tdesc_powerpc_vsx64l); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/rs6000/rs6000.c b/gdb/features/rs6000/rs6000.c index d4e93a5..0bde256 100644 --- a/gdb/features/rs6000/rs6000.c +++ b/gdb/features/rs6000/rs6000.c @@ -91,4 +91,7 @@ initialize_tdesc_rs6000 (void) tdesc_create_reg (feature, "fpscr", 71, 1, "float", 32, "int"); tdesc_rs6000 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("rs6000/rs6000.xml", tdesc_rs6000); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-linux32.c b/gdb/features/s390-linux32.c index 6d13094..c7be0ce 100644 --- a/gdb/features/s390-linux32.c +++ b/gdb/features/s390-linux32.c @@ -75,4 +75,7 @@ initialize_tdesc_s390_linux32 (void) tdesc_create_reg (feature, "orig_r2", 51, 1, "system", 32, "uint32"); tdesc_s390_linux32 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-linux32.xml", tdesc_s390_linux32); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-linux32v1.c b/gdb/features/s390-linux32v1.c index f773fc1..f9bc149 100644 --- a/gdb/features/s390-linux32v1.c +++ b/gdb/features/s390-linux32v1.c @@ -76,4 +76,7 @@ initialize_tdesc_s390_linux32v1 (void) tdesc_create_reg (feature, "last_break", 52, 0, "system", 32, "code_ptr"); tdesc_s390_linux32v1 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-linux32v1.xml", tdesc_s390_linux32v1); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-linux32v2.c b/gdb/features/s390-linux32v2.c index 2317752..cf5a94a 100644 --- a/gdb/features/s390-linux32v2.c +++ b/gdb/features/s390-linux32v2.c @@ -77,4 +77,7 @@ initialize_tdesc_s390_linux32v2 (void) tdesc_create_reg (feature, "system_call", 53, 1, "system", 32, "uint32"); tdesc_s390_linux32v2 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-linux32v2.xml", tdesc_s390_linux32v2); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-linux64.c b/gdb/features/s390-linux64.c index 3c7145b..663e6db 100644 --- a/gdb/features/s390-linux64.c +++ b/gdb/features/s390-linux64.c @@ -91,4 +91,7 @@ initialize_tdesc_s390_linux64 (void) tdesc_create_reg (feature, "orig_r2", 67, 1, "system", 32, "uint32"); tdesc_s390_linux64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-linux64.xml", tdesc_s390_linux64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-linux64v1.c b/gdb/features/s390-linux64v1.c index 72bd894..26bac20 100644 --- a/gdb/features/s390-linux64v1.c +++ b/gdb/features/s390-linux64v1.c @@ -92,4 +92,7 @@ initialize_tdesc_s390_linux64v1 (void) tdesc_create_reg (feature, "last_break", 68, 0, "system", 32, "code_ptr"); tdesc_s390_linux64v1 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-linux64v1.xml", tdesc_s390_linux64v1); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-linux64v2.c b/gdb/features/s390-linux64v2.c index a1757da..c375644 100644 --- a/gdb/features/s390-linux64v2.c +++ b/gdb/features/s390-linux64v2.c @@ -93,4 +93,7 @@ initialize_tdesc_s390_linux64v2 (void) tdesc_create_reg (feature, "system_call", 69, 1, "system", 32, "uint32"); tdesc_s390_linux64v2 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-linux64v2.xml", tdesc_s390_linux64v2); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-te-linux64.c b/gdb/features/s390-te-linux64.c index 0a3aedf..c5e6e53 100644 --- a/gdb/features/s390-te-linux64.c +++ b/gdb/features/s390-te-linux64.c @@ -115,4 +115,7 @@ initialize_tdesc_s390_te_linux64 (void) tdesc_create_reg (feature, "tr15", 89, 1, "tdb", 64, "uint64"); tdesc_s390_te_linux64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-te-linux64.xml", tdesc_s390_te_linux64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-tevx-linux64.c b/gdb/features/s390-tevx-linux64.c index 5bc3eec..7d8fc5f 100644 --- a/gdb/features/s390-tevx-linux64.c +++ b/gdb/features/s390-tevx-linux64.c @@ -185,4 +185,7 @@ initialize_tdesc_s390_tevx_linux64 (void) tdesc_create_reg (feature, "v31", 121, 1, NULL, 128, "vec128"); tdesc_s390_tevx_linux64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-tevx-linux64.xml", tdesc_s390_tevx_linux64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390-vx-linux64.c b/gdb/features/s390-vx-linux64.c index c3ffa16..c1d64aa 100644 --- a/gdb/features/s390-vx-linux64.c +++ b/gdb/features/s390-vx-linux64.c @@ -163,4 +163,7 @@ initialize_tdesc_s390_vx_linux64 (void) tdesc_create_reg (feature, "v31", 101, 1, NULL, 128, "vec128"); tdesc_s390_vx_linux64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390-vx-linux64.xml", tdesc_s390_vx_linux64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390x-linux64.c b/gdb/features/s390x-linux64.c index 04502c6..4bede5e 100644 --- a/gdb/features/s390x-linux64.c +++ b/gdb/features/s390x-linux64.c @@ -75,4 +75,7 @@ initialize_tdesc_s390x_linux64 (void) tdesc_create_reg (feature, "orig_r2", 51, 1, "system", 64, "uint64"); tdesc_s390x_linux64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390x-linux64.xml", tdesc_s390x_linux64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390x-linux64v1.c b/gdb/features/s390x-linux64v1.c index 05bfd53..353655f 100644 --- a/gdb/features/s390x-linux64v1.c +++ b/gdb/features/s390x-linux64v1.c @@ -76,4 +76,7 @@ initialize_tdesc_s390x_linux64v1 (void) tdesc_create_reg (feature, "last_break", 52, 0, "system", 64, "code_ptr"); tdesc_s390x_linux64v1 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390x-linux64v1.xml", tdesc_s390x_linux64v1); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390x-linux64v2.c b/gdb/features/s390x-linux64v2.c index 4108cc0..e1edb51 100644 --- a/gdb/features/s390x-linux64v2.c +++ b/gdb/features/s390x-linux64v2.c @@ -77,4 +77,7 @@ initialize_tdesc_s390x_linux64v2 (void) tdesc_create_reg (feature, "system_call", 53, 1, "system", 32, "uint32"); tdesc_s390x_linux64v2 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390x-linux64v2.xml", tdesc_s390x_linux64v2); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390x-te-linux64.c b/gdb/features/s390x-te-linux64.c index f75d900..3dee7c2 100644 --- a/gdb/features/s390x-te-linux64.c +++ b/gdb/features/s390x-te-linux64.c @@ -99,4 +99,7 @@ initialize_tdesc_s390x_te_linux64 (void) tdesc_create_reg (feature, "tr15", 73, 1, "tdb", 64, "uint64"); tdesc_s390x_te_linux64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390x-te-linux64.xml", tdesc_s390x_te_linux64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390x-tevx-linux64.c b/gdb/features/s390x-tevx-linux64.c index 327cd23..d841528 100644 --- a/gdb/features/s390x-tevx-linux64.c +++ b/gdb/features/s390x-tevx-linux64.c @@ -169,4 +169,7 @@ initialize_tdesc_s390x_tevx_linux64 (void) tdesc_create_reg (feature, "v31", 105, 1, NULL, 128, "vec128"); tdesc_s390x_tevx_linux64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390x-tevx-linux64.xml", tdesc_s390x_tevx_linux64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/s390x-vx-linux64.c b/gdb/features/s390x-vx-linux64.c index e66da70..9bac54b 100644 --- a/gdb/features/s390x-vx-linux64.c +++ b/gdb/features/s390x-vx-linux64.c @@ -147,4 +147,7 @@ initialize_tdesc_s390x_vx_linux64 (void) tdesc_create_reg (feature, "v31", 85, 1, NULL, 128, "vec128"); tdesc_s390x_vx_linux64 = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("s390x-vx-linux64.xml", tdesc_s390x_vx_linux64); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/tic6x-c62x-linux.c b/gdb/features/tic6x-c62x-linux.c index 8dd426d..46cbce6 100644 --- a/gdb/features/tic6x-c62x-linux.c +++ b/gdb/features/tic6x-c62x-linux.c @@ -53,4 +53,7 @@ initialize_tdesc_tic6x_c62x_linux (void) tdesc_create_reg (feature, "PC", 33, 1, NULL, 32, "code_ptr"); tdesc_tic6x_c62x_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("tic6x-c62x-linux.xml", tdesc_tic6x_c62x_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/tic6x-c62x.c b/gdb/features/tic6x-c62x.c index 2089aaf..5c7c898 100644 --- a/gdb/features/tic6x-c62x.c +++ b/gdb/features/tic6x-c62x.c @@ -51,4 +51,7 @@ initialize_tdesc_tic6x_c62x (void) tdesc_create_reg (feature, "PC", 33, 1, NULL, 32, "code_ptr"); tdesc_tic6x_c62x = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("tic6x-c62x.xml", tdesc_tic6x_c62x); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/tic6x-c64x-linux.c b/gdb/features/tic6x-c64x-linux.c index 2752358..310fe5a 100644 --- a/gdb/features/tic6x-c64x-linux.c +++ b/gdb/features/tic6x-c64x-linux.c @@ -87,4 +87,7 @@ initialize_tdesc_tic6x_c64x_linux (void) tdesc_create_reg (feature, "B31", 65, 1, NULL, 32, "uint32"); tdesc_tic6x_c64x_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("tic6x-c64x-linux.xml", tdesc_tic6x_c64x_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/tic6x-c64x.c b/gdb/features/tic6x-c64x.c index 0feda24..c4bbb3a 100644 --- a/gdb/features/tic6x-c64x.c +++ b/gdb/features/tic6x-c64x.c @@ -85,4 +85,7 @@ initialize_tdesc_tic6x_c64x (void) tdesc_create_reg (feature, "B31", 65, 1, NULL, 32, "uint32"); tdesc_tic6x_c64x = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("tic6x-c64x.xml", tdesc_tic6x_c64x); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/tic6x-c64xp-linux.c b/gdb/features/tic6x-c64xp-linux.c index c1bee4c..df89570 100644 --- a/gdb/features/tic6x-c64xp-linux.c +++ b/gdb/features/tic6x-c64xp-linux.c @@ -92,4 +92,7 @@ initialize_tdesc_tic6x_c64xp_linux (void) tdesc_create_reg (feature, "RILC", 68, 1, NULL, 32, "uint32"); tdesc_tic6x_c64xp_linux = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("tic6x-c64xp-linux.xml", tdesc_tic6x_c64xp_linux); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/features/tic6x-c64xp.c b/gdb/features/tic6x-c64xp.c index 160b854..6cc6430 100644 --- a/gdb/features/tic6x-c64xp.c +++ b/gdb/features/tic6x-c64xp.c @@ -90,4 +90,7 @@ initialize_tdesc_tic6x_c64xp (void) tdesc_create_reg (feature, "RILC", 68, 1, NULL, 32, "uint32"); tdesc_tic6x_c64xp = result; +#if GDB_SELF_TEST + selftests::record_xml_tdesc ("tic6x-c64xp.xml", tdesc_tic6x_c64xp); +#endif /* GDB_SELF_TEST */ } diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c index 9a7e2dd..754f15b 100644 --- a/gdb/target-descriptions.c +++ b/gdb/target-descriptions.c @@ -468,6 +468,101 @@ tdesc_osabi (const struct target_desc *target_desc) return target_desc->osabi; } +/* Return true if REG1 and REG2 are identical. */ + +static bool +tdesc_reg_equals (const struct tdesc_reg *reg1, + const struct tdesc_reg *reg2) +{ + return (strcmp (reg1->name, reg2->name) == 0 + && reg1->target_regnum == reg2->target_regnum + && reg1->save_restore == reg2->save_restore + && reg1->bitsize == reg2->bitsize + && (reg1->group == reg2->group + || strcmp (reg1->group, reg2->group) == 0) + && strcmp (reg1->type, reg2->type) == 0); +} + +/* Return true if TYPE1 and TYPE2 are identical. */ + +static bool +tdesc_type_equals (const struct tdesc_type *type1, + const struct tdesc_type *type2) +{ + return (strcmp (type1->name, type2->name) == 0 + && type1->kind == type2->kind); +} + +/* Return true if FEATURE1 and FEATURE2 are identical. */ + +static bool +tdesc_feature_equals (const struct tdesc_feature *feature1, + const struct tdesc_feature *feature2) +{ + if (feature1 == feature2) + return true; + + if (strcmp (feature1->name, feature2->name) != 0) + return false; + + struct tdesc_reg *reg; + + for (int ix = 0; + VEC_iterate (tdesc_reg_p, feature1->registers, ix, reg); + ix++) + { + struct tdesc_reg *reg2 + = VEC_index (tdesc_reg_p, feature2->registers, ix); + + if (!tdesc_reg_equals (reg, reg2)) + return false; + } + + struct tdesc_type *type; + + for (int ix = 0; + VEC_iterate (tdesc_type_p, feature1->types, ix, type); + ix++) + { + struct tdesc_type *type2 + = VEC_index (tdesc_type_p, feature2->types, ix); + + if (!tdesc_type_equals (type, type2)) + return false; + } + + return true; +} + +bool +tdesc_equals (const struct target_desc *tdesc1, + const struct target_desc *tdesc2) +{ + if (tdesc1->arch != tdesc2->arch) + return false; + + if (tdesc1->osabi != tdesc2->osabi) + return false; + + if (VEC_length (tdesc_feature_p, tdesc1->features) + != VEC_length (tdesc_feature_p, tdesc2->features)) + return false; + + struct tdesc_feature *feature; + + for (int ix = 0; + VEC_iterate (tdesc_feature_p, tdesc1->features, ix, feature); + ix++) + { + struct tdesc_feature *feature2 + = VEC_index (tdesc_feature_p, tdesc2->features, ix); + + if (!tdesc_feature_equals (feature, feature2)) + return false; + } + return true; +} + \f /* Return 1 if this target description includes any registers. */ @@ -1734,6 +1829,12 @@ maint_print_c_tdesc_cmd (char *args, int from_tty) error (_("The current target description did not come from an XML file.")); filename = lbasename (target_description_filename); + std::string filename_after_features (target_description_filename); + auto loc = filename_after_features.rfind ("/features/"); + + if (loc != std::string::npos) + filename_after_features = filename_after_features.substr (loc + 10); + function = (char *) alloca (strlen (filename) + 1); for (inp = filename, outp = function; *inp != '\0'; inp++) if (*inp == '.') @@ -1979,9 +2080,64 @@ feature = tdesc_create_feature (result, \"%s\");\n", } printf_unfiltered (" tdesc_%s = result;\n", function); + + printf_unfiltered ("#if GDB_SELF_TEST\n"); + printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n", + filename_after_features.data (), function); + printf_unfiltered ("#endif /* GDB_SELF_TEST */\n"); printf_unfiltered ("}\n"); } +#if GDB_SELF_TEST +#include "selftest.h" + +namespace selftests { + +static std::vector<std::pair<std::string, const struct target_desc *>> lists; + +void +record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc) +{ + lists.emplace_back (xml_file, tdesc); +} + +/* Test these GDB builtin target descriptions are identical to these which + are generated by the corresponding xml files. */ + +static void +xml_builtin_tdesc_test (void) +{ + std::string feature_dir (ldirname (__FILE__)); + struct stat st; + + /* Look for the features directory. If the directory of __FILE__ can't + be found, __FILE__ is a file name with relative path. Guess that + GDB is executed in testsuite directory like ../gdb, because I don't + expect that GDB is invoked somewhere else and run self tests. */ + if (stat (feature_dir.data (), &st) < 0) + { + feature_dir.insert (0, SLASH_STRING); + feature_dir.insert (0, ".."); + + /* If still can't find the path, something is wrong. */ + SELF_CHECK (stat (feature_dir.data (), &st) == 0); + } + + feature_dir = feature_dir + SLASH_STRING + "features"; + + for (auto const &e : lists) + { + std::string tdesc_xml = (feature_dir + SLASH_STRING + e.first); + const struct target_desc *tdesc + = file_read_description_xml (tdesc_xml.data ()); + + SELF_CHECK (tdesc != NULL); + SELF_CHECK (tdesc_equals (tdesc, e.second)); + } +} +} +#endif /* GDB_SELF_TEST */ + /* Provide a prototype to silence -Wmissing-prototypes. */ extern initialize_file_ftype _initialize_target_descriptions; @@ -2022,4 +2178,8 @@ GDB will read the description from the target."), add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\ Print the current target description as a C source file."), &maintenanceprintlist); + +#if GDB_SELF_TEST + register_self_test (selftests::xml_builtin_tdesc_test); +#endif } diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h index 361ac97..78416ae 100644 --- a/gdb/target-descriptions.h +++ b/gdb/target-descriptions.h @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *); int tdesc_compatible_p (const struct target_desc *, const struct bfd_arch_info *); +/* Compare target descriptions TDESC1 and TDESC2, return true if they + are identical. */ + +bool tdesc_equals (const struct target_desc *tdesc1, + const struct target_desc *tdesc2); + /* Return the string value of a property named KEY, or NULL if the property was not specified. */ @@ -253,4 +259,14 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name, int regnum, int save_restore, const char *group, int bitsize, const char *type); +#if GDB_SELF_TEST +namespace selftests { + +/* Record the target description TDESC generated by XML_FILE. */ + +void record_xml_tdesc (std::string xml_file, + const struct target_desc *tdesc); +} +#endif + #endif /* TARGET_DESCRIPTIONS_H */ -- 1.9.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi @ 2017-05-16 12:00 ` Philipp Rudo 2017-05-16 15:46 ` Yao Qi 2017-05-17 16:06 ` Pedro Alves 2017-05-17 15:41 ` Pedro Alves 1 sibling, 2 replies; 32+ messages in thread From: Philipp Rudo @ 2017-05-16 12:00 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches Hi Yao, I really like the direction this series is going to. Generating the target description dynamically during runtime definitely makes the code much nicer and reduces the number of XML files in the feature directory. The only thing you could have done to make it better is to get rid of the conversion to C altogether and create the target description directly by parsing the XML file. I don't see a benefit in the conversion and removing it would simplify the code even more. But that is a long term goal and your patch set definitely helps towards that goal. See more comments below On Thu, 11 May 2017 16:55:00 +0100 Yao Qi <qiyaoltc@gmail.com> wrote: [...] > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index 9a7e2dd..754f15b 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c > @@ -468,6 +468,101 @@ tdesc_osabi (const struct target_desc *target_desc) > return target_desc->osabi; > } > > +/* Return true if REG1 and REG2 are identical. */ > + > +static bool > +tdesc_reg_equals (const struct tdesc_reg *reg1, > + const struct tdesc_reg *reg2) > +{ > + return (strcmp (reg1->name, reg2->name) == 0 > + && reg1->target_regnum == reg2->target_regnum > + && reg1->save_restore == reg2->save_restore > + && reg1->bitsize == reg2->bitsize > + && (reg1->group == reg2->group > + || strcmp (reg1->group, reg2->group) == 0) > + && strcmp (reg1->type, reg2->type) == 0); > +} inline? strcmp (...) == 0 -> utils.h:streq (...)? Makes it at least a little shorter. > +/* Return true if TYPE1 and TYPE2 are identical. */ > + > +static bool > +tdesc_type_equals (const struct tdesc_type *type1, > + const struct tdesc_type *type2) > +{ > + return (strcmp (type1->name, type2->name) == 0 > + && type1->kind == type2->kind); > +} Same as above. > +/* Return true if FEATURE1 and FEATURE2 are identical. */ > + > +static bool > +tdesc_feature_equals (const struct tdesc_feature *feature1, > + const struct tdesc_feature *feature2) > +{ > + if (feature1 == feature2) > + return true; > + > + if (strcmp (feature1->name, feature2->name) != 0) > + return false; > + > + struct tdesc_reg *reg; > + > + for (int ix = 0; > + VEC_iterate (tdesc_reg_p, feature1->registers, ix, reg); > + ix++) > + { > + struct tdesc_reg *reg2 > + = VEC_index (tdesc_reg_p, feature2->registers, ix); > + > + if (!tdesc_reg_equals (reg, reg2)) > + return false; > + } > + > + struct tdesc_type *type; > + > + for (int ix = 0; > + VEC_iterate (tdesc_type_p, feature1->types, ix, type); > + ix++) > + { > + struct tdesc_type *type2 > + = VEC_index (tdesc_type_p, feature2->types, ix); > + > + if (!tdesc_type_equals (type, type2)) > + return false; > + } > + > + return true; > +} > + > +bool > +tdesc_equals (const struct target_desc *tdesc1, > + const struct target_desc *tdesc2) > +{ > + if (tdesc1->arch != tdesc2->arch) > + return false; > + > + if (tdesc1->osabi != tdesc2->osabi) > + return false; > + > + if (VEC_length (tdesc_feature_p, tdesc1->features) > + != VEC_length (tdesc_feature_p, tdesc2->features)) > + return false; > + > + struct tdesc_feature *feature; > + > + for (int ix = 0; > + VEC_iterate (tdesc_feature_p, tdesc1->features, ix, feature); > + ix++) > + { > + struct tdesc_feature *feature2 > + = VEC_index (tdesc_feature_p, tdesc2->features, ix); > + > + if (!tdesc_feature_equals (feature, feature2)) > + return false; > + } > + return true; > +} > + > \f > > /* Return 1 if this target description includes any registers. */ > @@ -1734,6 +1829,12 @@ maint_print_c_tdesc_cmd (char *args, int from_tty) > error (_("The current target description did not come from an XML > file.")); > > filename = lbasename (target_description_filename); > + std::string filename_after_features (target_description_filename); > + auto loc = filename_after_features.rfind ("/features/"); > + > + if (loc != std::string::npos) > + filename_after_features = filename_after_features.substr (loc + 10); This piece is hard to read. This should do the same and (for my taste) is better readable std::string filename_with_subdir (target_description_filename); std::string subdir (lbasename (ldirname (target_description_filename))); if (subdir.campare ("features") != 0) filename_with_subdir = concat_path (subdir, filename); If you prefer your way, please replace '10' by strlen ("/features/") (or similar) in the last line. > function = (char *) alloca (strlen (filename) + 1); > for (inp = filename, outp = function; *inp != '\0'; inp++) > if (*inp == '.') > @@ -1979,9 +2080,64 @@ feature = tdesc_create_feature (result, \"%s\");\n", > } > > printf_unfiltered (" tdesc_%s = result;\n", function); > + > + printf_unfiltered ("#if GDB_SELF_TEST\n"); > + printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n", > + filename_after_features.data (), function); > + printf_unfiltered ("#endif /* GDB_SELF_TEST */\n"); > printf_unfiltered ("}\n"); > } > > +#if GDB_SELF_TEST > +#include "selftest.h" > + > +namespace selftests { > + > +static std::vector<std::pair<std::string, const struct target_desc *>> lists; Can you rename the vector? Just 'lists' is pretty vague. > +void > +record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc) > +{ > + lists.emplace_back (xml_file, tdesc); > +} > + > +/* Test these GDB builtin target descriptions are identical to these which > + are generated by the corresponding xml files. */ > + > +static void > +xml_builtin_tdesc_test (void) > +{ > + std::string feature_dir (ldirname (__FILE__)); > + struct stat st; > + > + /* Look for the features directory. If the directory of __FILE__ can't > + be found, __FILE__ is a file name with relative path. Guess that > + GDB is executed in testsuite directory like ../gdb, because I don't > + expect that GDB is invoked somewhere else and run self tests. */ > + if (stat (feature_dir.data (), &st) < 0) > + { > + feature_dir.insert (0, SLASH_STRING); > + feature_dir.insert (0, ".."); This would be a perfect spot for concat_path (patch 2 from my lk series https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html). Then it would read feature_dir = concat_path ("..", feature_dir); I actually want to bring some of my patches upstream (mostly the s390-linux-tdep split up patch) to reduce maintenance cost of my series. This would be a good time for this patch, too. > + > + /* If still can't find the path, something is wrong. */ > + SELF_CHECK (stat (feature_dir.data (), &st) == 0); > + } > + > + feature_dir = feature_dir + SLASH_STRING + "features"; feature_dir = concat_path (feature_dir, "features"); ? > + > + for (auto const &e : lists) > + { > + std::string tdesc_xml = (feature_dir + SLASH_STRING + e.first); std::string tdesc_xml = concat_path (feature_dir, e.first); ? Thanks Philipp > + const struct target_desc *tdesc > + = file_read_description_xml (tdesc_xml.data ()); > + > + SELF_CHECK (tdesc != NULL); > + SELF_CHECK (tdesc_equals (tdesc, e.second)); > + } > +} > +} > +#endif /* GDB_SELF_TEST */ > + > /* Provide a prototype to silence -Wmissing-prototypes. */ > extern initialize_file_ftype _initialize_target_descriptions; > > @@ -2022,4 +2178,8 @@ GDB will read the description from the target."), > add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\ > Print the current target description as a C source file."), > &maintenanceprintlist); > + > +#if GDB_SELF_TEST > + register_self_test (selftests::xml_builtin_tdesc_test); > +#endif > } > diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h > index 361ac97..78416ae 100644 > --- a/gdb/target-descriptions.h > +++ b/gdb/target-descriptions.h > @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *); > int tdesc_compatible_p (const struct target_desc *, > const struct bfd_arch_info *); > > +/* Compare target descriptions TDESC1 and TDESC2, return true if they > + are identical. */ > + > +bool tdesc_equals (const struct target_desc *tdesc1, > + const struct target_desc *tdesc2); > + > /* Return the string value of a property named KEY, or NULL if the > property was not specified. */ > > @@ -253,4 +259,14 @@ void tdesc_create_reg (struct tdesc_feature *feature, > const char *name, int regnum, int save_restore, const char *group, > int bitsize, const char *type); > > +#if GDB_SELF_TEST > +namespace selftests { > + > +/* Record the target description TDESC generated by XML_FILE. */ > + > +void record_xml_tdesc (std::string xml_file, > + const struct target_desc *tdesc); > +} > +#endif > + > #endif /* TARGET_DESCRIPTIONS_H */ ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-16 12:00 ` Philipp Rudo @ 2017-05-16 15:46 ` Yao Qi 2017-05-17 9:09 ` Philipp Rudo 2017-05-17 16:06 ` Pedro Alves 1 sibling, 1 reply; 32+ messages in thread From: Yao Qi @ 2017-05-16 15:46 UTC (permalink / raw) To: Philipp Rudo; +Cc: gdb-patches Philipp Rudo <prudo@linux.vnet.ibm.com> writes: Hi Philipp, Thanks for your comments. > The only thing you could have done to make it better is to get rid of the > conversion to C altogether and create the target description directly by > parsing the XML file. I don't see a benefit in the conversion and removing it > would simplify the code even more. But that is a long term goal and your patch > set definitely helps towards that goal. As I described in the commit log, "the reason that we transform xml files to c files is that GDB is still able to have some target description without xml parsing support." so that XML parsing is not a hard requirement for GDB. Nowadays, XML parsing is only used for remote debugging. In native debugging, GDB just uses these pre-generated builtin target descriptions, without parsing any XML. Suppose XML parsing is always there, at least on Linux host, GDB can create a big buffer contains various target features in the runtime. We can do this on top of patch 6/7, like this, (take i386-linux as an example), /* Return a string having XML contents reflecting the right target description according to XCR0. */ std::string i386_linux_read_description (uint64_t xcr0) { std::string buffer; buffer.append ("<?xml version="1.0"?>"); buffer.append ("<!DOCTYPE target SYSTEM "gdb-target.dtd">"); buffer.append ("<target>"); buffer.append (" <architecture>i386</architecture>"); buffer.append (" <osabi>GNU/Linux</osabi>"); if (xcr0 & X86_XSTATE_X87) { /* Open 32bit-core.xml and copy its content here. */ } if (xcr0 & X86_XSTATE_SSE) { /* Open 32bit-sse.xml and copy its content here. */ } .... buffer.append ("</target>"); return buffer; } Even further, this code can be shared between GDB and GDBserver, so that GDBserver can compose the XML contents and send it to GDB. Note that GDBserver doesn't need to parse the XML. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-16 15:46 ` Yao Qi @ 2017-05-17 9:09 ` Philipp Rudo 0 siblings, 0 replies; 32+ messages in thread From: Philipp Rudo @ 2017-05-17 9:09 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches Hi Yao, On Tue, 16 May 2017 16:46:11 +0100 Yao Qi <qiyaoltc@gmail.com> wrote: > Philipp Rudo <prudo@linux.vnet.ibm.com> writes: > > Hi Philipp, > Thanks for your comments. > > > The only thing you could have done to make it better is to get rid of the > > conversion to C altogether and create the target description directly by > > parsing the XML file. I don't see a benefit in the conversion and removing > > it would simplify the code even more. But that is a long term goal and > > your patch set definitely helps towards that goal. > > As I described in the commit log, "the reason that we transform xml files > to c files is that GDB is still able to have some target description > without xml parsing support." so that XML parsing is not a hard > requirement for GDB. Nowadays, XML parsing is only used for remote > debugging. In native debugging, GDB just uses these pre-generated > builtin target descriptions, without parsing any XML. Ok, I see my mistake now. I thought XML parsing would always be present. Then the target descriptions could be handled similar to syscalls. But that was clearly focused too much on GDB on Linux. For other systems and GDBserver this needn't necessarily work... Sorry for the mistake. > Suppose XML parsing is always there, at least on Linux host, GDB can > create a big buffer contains various target features in the runtime. We > can do this on top of patch 6/7, like this, (take i386-linux as an > example), > > /* Return a string having XML contents reflecting the right target > description according to XCR0. */ > > std::string > i386_linux_read_description (uint64_t xcr0) > { > std::string buffer; > > buffer.append ("<?xml version="1.0"?>"); > buffer.append ("<!DOCTYPE target SYSTEM "gdb-target.dtd">"); > buffer.append ("<target>"); > buffer.append (" <architecture>i386</architecture>"); > buffer.append (" <osabi>GNU/Linux</osabi>"); > > if (xcr0 & X86_XSTATE_X87) > { > /* Open 32bit-core.xml and copy its content here. */ > } > > if (xcr0 & X86_XSTATE_SSE) > { > /* Open 32bit-sse.xml and copy its content here. */ > } > > .... > > buffer.append ("</target>"); > > return buffer; > } > > Even further, this code can be shared between GDB and GDBserver, so that > GDBserver can compose the XML contents and send it to GDB. Note that > GDBserver doesn't need to parse the XML. That would be neat! Thanks Philipp ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-16 12:00 ` Philipp Rudo 2017-05-16 15:46 ` Yao Qi @ 2017-05-17 16:06 ` Pedro Alves 2017-05-30 8:00 ` Philipp Rudo 1 sibling, 1 reply; 32+ messages in thread From: Pedro Alves @ 2017-05-17 16:06 UTC (permalink / raw) To: Philipp Rudo, Yao Qi; +Cc: gdb-patches On 05/16/2017 01:00 PM, Philipp Rudo wrote: >> + /* Look for the features directory. If the directory of __FILE__ can't >> + be found, __FILE__ is a file name with relative path. Guess that >> + GDB is executed in testsuite directory like ../gdb, because I don't >> + expect that GDB is invoked somewhere else and run self tests. */ >> + if (stat (feature_dir.data (), &st) < 0) >> + { >> + feature_dir.insert (0, SLASH_STRING); >> + feature_dir.insert (0, ".."); > > This would be a perfect spot for concat_path (patch 2 from my lk series > https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html). > Then it would read > > feature_dir = concat_path ("..", feature_dir); > > I actually want to bring some of my patches upstream (mostly the > s390-linux-tdep split up patch) to reduce maintenance cost of my > series. This would be a good time for this patch, too. Could that patch be split out of the series, and justified on its own grounds? Is there some representative place in the codebase that you could cleanup a bit using the new API, just to show it working nicely? Also, a unit test would be nice. Also, and most importantly, seems to me that patch still has a lot inefficiency related to std::string copying, e.g.: +static inline unsigned long +approx_path_length (std::initializer_list<std::string> args, + std::string dir_separator) This is passing in the strings by value / copy. That looks like an aweful lot of malloc/free/strdup behind the scenes. I still think that if we're adding some utility for building paths from dir components, that it'd be preferred to model the API on (a small subset of) std::experimental::filesystem::path, since in a few years that's the API that everyone learning C++ will be learning. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-17 16:06 ` Pedro Alves @ 2017-05-30 8:00 ` Philipp Rudo 2017-06-01 17:53 ` Philipp Rudo 0 siblings, 1 reply; 32+ messages in thread From: Philipp Rudo @ 2017-05-30 8:00 UTC (permalink / raw) To: Pedro Alves; +Cc: Yao Qi, gdb-patches Hi Pedro, sorry for the late answer but I was sick the last 10 days and just returned yesterday... On Wed, 17 May 2017 17:06:13 +0100 Pedro Alves <palves@redhat.com> wrote: > On 05/16/2017 01:00 PM, Philipp Rudo wrote: > > >> + /* Look for the features directory. If the directory of __FILE__ can't > >> + be found, __FILE__ is a file name with relative path. Guess that > >> + GDB is executed in testsuite directory like ../gdb, because I don't > >> + expect that GDB is invoked somewhere else and run self tests. */ > >> + if (stat (feature_dir.data (), &st) < 0) > >> + { > >> + feature_dir.insert (0, SLASH_STRING); > >> + feature_dir.insert (0, ".."); > > > > This would be a perfect spot for concat_path (patch 2 from my lk series > > https://sourceware.org/ml/gdb-patches/2017-03/msg00272.html). > > Then it would read > > > > feature_dir = concat_path ("..", feature_dir); > > > > I actually want to bring some of my patches upstream (mostly the > > s390-linux-tdep split up patch) to reduce maintenance cost of my > > series. This would be a good time for this patch, too. > > Could that patch be split out of the series, and justified on its > own grounds? Is there some representative place in the codebase > that you could cleanup a bit using the new API, just to show it > working nicely? Also, a unit test would be nice. The patch can be split from my series without problem. I already pulled it right to the beginning of the series, as it is independent of all other changes in the set. There are quite some places you can simplify by using the function. Just grep for SLASH_STRING to find most of them. I also had a patch once where I eliminated all use of SLASH_STRING using the concat_path. Unfortunately it had some problems with the mixture of C and C++ strings causing memory leaks and read after free's. I never had the time to clean that up and thus didn't send it to the mailing list ... > Also, and most importantly, seems to me that patch still has > a lot inefficiency related to std::string copying, e.g.: > > +static inline unsigned long > +approx_path_length (std::initializer_list<std::string> args, > + std::string dir_separator) > > This is passing in the strings by value / copy. That looks like > an aweful lot of malloc/free/strdup behind the scenes. True, this can be optimized. > I still think that if we're adding some utility for building > paths from dir components, that it'd be preferred to model > the API on (a small subset of) std::experimental::filesystem::path, > since in a few years that's the API that everyone learning C++ will > be learning. Also true, let me see if I can hack something today. Currently I don't like to do what I should do and there are many meeting today anyway. So this looks like a perfect task for today ;) Philipp > Thanks, > Pedro Alves > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-30 8:00 ` Philipp Rudo @ 2017-06-01 17:53 ` Philipp Rudo 0 siblings, 0 replies; 32+ messages in thread From: Philipp Rudo @ 2017-06-01 17:53 UTC (permalink / raw) To: Pedro Alves; +Cc: Yao Qi, gdb-patches [-- Attachment #1: Type: text/plain, Size: 11936 bytes --] Hi Pedro, On Tue, 30 May 2017 10:00:07 +0200 Philipp Rudo <prudo@linux.vnet.ibm.com> wrote: [...] > > I still think that if we're adding some utility for building > > paths from dir components, that it'd be preferred to model > > the API on (a small subset of) std::experimental::filesystem::path, > > since in a few years that's the API that everyone learning C++ will > > be learning. > > Also true, let me see if I can hack something today. Currently I don't like > to do what I should do and there are many meeting today anyway. So this > looks like a perfect task for today ;) I looked into it a "little" and it got a "little" out of hand... I'm going on vacation, starting tomorrow until next Wednesday, and didn't have the time to include it in GDB yet. So you find the code attached in two files as separate "programm". My plan is once I'm back (and finished the work I should have done this week (sorry Yao and Omair)) to move gdb_path.h to common/gdb_path.h and expand gdb_path-selftest.c with some static_asserts. While working on it I noticed one detail in std::filesystem::path which could be problematic for GDB. The dir separator it uses (preferred_separator) is 'static constexpr char'. So for cross debugging, with different dir separators, it doesn't allow us to distinguish between host and target paths. That's why my implementation uses a simple "char" such that the dir separator can be changed anytime. Otherwise the implementation should be compatible with std::filesystem::path http://en.cppreference.com/w/cpp/filesystem/path Any comment is welcome. Philipp ---------- To be added as binutils-gdb/gdb/common/gdb_path.h gdb_path.h | 542 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 542 insertions(+) ---------- /* Filesystem path handling for GDB. Copyright (C) 2017 Free Software Foundation, Inc. This file is part of GDB. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ #ifndef __COMMON_GDB_PATH__ #define __COMMON_GDB_PATH__ #include <iomanip> #include <iostream> #include <list> #include <string> namespace gdb { namespace filesystem { /* Stub implementation of C++17 std::filesystem::path. */ class path { public: path () noexcept {} template<class T> path (const T &s); /* Replace this path by OTHER. */ template<class T> path &operator= (const T &other); /* Append OTHER to this path. */ template<class T> path &operator/= (const T &other); /* Same as operator/=. */ template<class T> path &append (const T &other) { return *this /= other; } /* Append RHS to LHS and return the resulting path. */ template<class T> friend path operator/ (path lhs, const T &rhs); /* Add OTHER to this path without adding a dir seperator. */ template<class T> path &operator+= (const T &other); /* Same as operator+=. */ template<class T> path &concat (const T &other) { return *this += other; } /* Send this path to an ostream. */ friend std::ostream &operator<< (std::ostream &os, const path &p); /* Directory seperator. */ char preferred_seperator = '/'; /* Erase the content of this path. */ void clear (); /* Concatenate the path and return it as a std::string. */ std::string string () const; /* Same as .string but returns const char *. */ const char * c_str () { return string ().c_str (); } /* Return the root_name of path, e.g. on DOS-like systems the drive name. */ path root_name () const; /* Return the root directory if it exists. */ path root_directory () const; /* Return the root path, i.e. root_name + root_directory. */ path root_path () const; /* Return the relative path from root_path. */ path relative_path () const; /* Return the parent path of this path. */ path parent_path () const; /* Return the filename component of this path. */ path filename () const; /* Return the stem of filename component of this path, i.e. the filename without extension. */ path stem () const; /* Return the extension of filename component of this path. */ path extension () const; /* Check if this path is empty. */ bool empty () const noexcept; protected: /* Root of the filesystem, e.g. "C:" on dos-like filesystems. */ std::string m_root_name = ""; /* Is this path absolute? I.e. do we need to add a dir_sep at the beginning? */ bool m_absolute = false; /* Components of the path relative to root_path. */ std::list <std::string> m_path = {}; /* Helper function. */ /* Does the given string sart with a root_name? Needed for constructor. */ bool has_root_name (const std::string &s) const; /* Is the substring of S starting at position POS a dir_seperator? */ bool is_dirsep (const std::string &s, const size_t pos) const; /* Calculate the size (i.e. number of characters) of the total path including dir_sep's. */ size_t path_size () const; /* Append path component COMP to m_path. */ void append_component (std::string &comp); }; /* See declaration. */ inline bool path::has_root_name (const std::string &s) const { //#if defined (HAVE_DOS_BASED_FILESYSTEM) /* Assume 'C:'-like root_names. */ return s.size () >= 2 && (std::isalpha (s[0]) && s[1] == ':'); //#else return false; //#endif /* HAVE_DOS_BASED_FILESYSTEM */ } /* See declaration. */ inline bool path::is_dirsep (const std::string &s, const size_t pos) const { return s[pos] == preferred_seperator; } /* See declaration. */ size_t path::path_size () const { size_t size = 0; for (auto &p : m_path) size += p.size () + 1; return size; } /* See declaration. */ void path::append_component (std::string &comp) { if (comp == ".") return; if (comp == ".." && !m_path.empty ()) { m_path.pop_back (); return; } m_path.push_back (comp); } /* Constructors. */ template<class T> path::path (const T &s) : path (std::string (s)) {} template<> path::path (const std::string &s) { size_t pos = 0; if (has_root_name (s)) { /* Assume 'C:'-like root_names. */ m_root_name = s.substr (pos, 2); pos += 2; } if (is_dirsep (s, pos)) { m_absolute = true; pos++; } do { /* Remove duplicate dir_seps. */ while (s[pos] == preferred_seperator) pos++; size_t last_pos = pos; pos = s.find (preferred_seperator, pos); std::string comp (s.substr (last_pos, pos - last_pos)); append_component (comp); } while (pos != std::string::npos); } template<> path::path (const path &other) { *this = other; } /* See declaration. */ std::ostream & operator<< (std::ostream &os, const path &p) { os << std::quoted (p.string ()); return os; } /* See declaration. */ template<class T> path & path::operator= (const T &other) { return *this = path (other); } /* See declaration. */ template<> path & path::operator= (const path &other) { preferred_seperator = other.preferred_seperator; m_root_name = other.m_root_name; m_absolute = other.m_absolute; m_path = other.m_path; return *this; } /* See declaration. */ template<> path & path::operator/= (const path &other) { for (auto comp : other.m_path) append_component (comp); return *this; } /* See declaration. */ template<class T> path & path::operator/= (const T &other) { return *this /= path (other); } /* See declaration. */ template<class T> path operator/ (path lhs, const T &rhs) { return lhs /= rhs; } /* See declaration. */ template<> path & path::operator+= (const path &other) { /* Ignore a possible root_name in other. */ m_path.back () += other.m_path.front (); m_path.insert (m_path.end (), ++other.m_path.begin (), other.m_path.end ()); return *this; } /* See declaration. */ template<class T> path & path::operator+= (const T &other) { return *this += path (other); } /* See declaration. */ void path::clear () { m_root_name.clear (); m_path.clear (); m_absolute = false; } /* See declaration. */ std::string path::string () const { std::string ret; if (empty ()) return ""; ret.reserve (path_size ()); ret += m_root_name; if (m_absolute) ret += preferred_seperator; for (auto p = m_path.begin (); p != m_path.end (); p++) ret += *p + preferred_seperator; /* Remove trailing dir_sep. */ ret.pop_back (); return ret; } /* See declaration. */ path path::root_name () const { return empty () ? path () : path (m_root_name); } /* See declaration. */ path path::root_directory () const { return m_absolute ? path (&preferred_seperator) : path (); } /* See declaration. */ path path::root_path () const { return root_name () / root_directory (); } /* See declaration. */ path path::relative_path () const { if (empty ()) return path (); path ret (*this); ret.m_root_name = ""; ret.m_absolute = false; return ret; } /* See declaration. */ path path::parent_path () const { if (empty ()) return path (); path ret (*this); ret.m_path.pop_back (); return ret; } /* See declaration. */ path path::filename () const { if (empty ()) return path (); return path (m_path.back ()); } /* See declaration. */ path path::stem () const { if (empty ()) return path (); auto pos = m_path.back ().rfind ('.'); if (pos == 0) return path (m_path.back ()); return path (m_path.back ().substr (0, pos)); } /* See declaration. */ path path::extension () const { if (empty ()) return path (); auto pos = m_path.back ().rfind ('.'); if (pos == 0 || pos == std::string::npos) return path (); return path (m_path.back ().substr (pos)); } /* See declaration. */ bool path::empty () const noexcept { return m_path.empty () && m_root_name.empty () && !m_absolute; } } /* namespace filesystem */ enum path_type { HOST_PATH, TARGET_PATH, }; class path : public gdb::filesystem::path { public: path () noexcept {}; template<class T> path (const T& s, enum path_type _type = HOST_PATH) : gdb::filesystem::path (s), type (_type) {} /* Overload .string method to prepend target_prefix. */ std::string string () const; /* Substitute all occurrences of FROM to TO in this path. */ path &substitute (const std::string &from, const std::string &to); /* Type of this path (host or target). */ enum path_type type; private: /* Prefix to be prepended to target paths. */ const std::string m_target_prefix = "target:"; }; /* See declaration. */ std::string path::string () const { std::string ret; if (type == TARGET_PATH) { ret.reserve (path_size () + m_target_prefix.size ()); ret = m_target_prefix + gdb::filesystem::path::string (); } else { ret = gdb::filesystem::path::string (); } return ret; } /* See declaration. */ path & path::substitute (const std::string &from, const std::string &to) { if (from.empty ()) return *this; /* Use TO == "" to remove the component completely. */ if (to.empty ()) { m_path.remove (from); return *this; } for (auto it = m_path.begin (); it != m_path.end (); ++it) { if (*it != from) continue; *it = to; } return *this; } } /* namespace gdb */ #endif /*__COMMON_GDB_PATH__ */ [-- Attachment #2: gdb_path.h --] [-- Type: text/x-chdr, Size: 10141 bytes --] /* Filesystem path handling for GDB. Copyright (C) 2017 Free Software Foundation, Inc. This file is part of GDB. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ #ifndef __COMMON_GDB_PATH__ #define __COMMON_GDB_PATH__ #include <iomanip> #include <iostream> #include <list> #include <string> namespace gdb { namespace filesystem { /* Stub implementation of C++17 std::filesystem::path. */ class path { public: path () noexcept {} template<class T> path (const T &s); /* Replace this path by OTHER. */ template<class T> path &operator= (const T &other); /* Append OTHER to this path. */ template<class T> path &operator/= (const T &other); /* Same as operator/=. */ template<class T> path &append (const T &other) { return *this /= other; } /* Append RHS to LHS and return the resulting path. */ template<class T> friend path operator/ (path lhs, const T &rhs); /* Add OTHER to this path without adding a dir seperator. */ template<class T> path &operator+= (const T &other); /* Same as operator+=. */ template<class T> path &concat (const T &other) { return *this += other; } /* Send this path to an ostream. */ friend std::ostream &operator<< (std::ostream &os, const path &p); /* Directory seperator. */ char preferred_seperator = '/'; /* Erase the content of this path. */ void clear (); /* Concatenate the path and return it as a std::string. */ std::string string () const; /* Same as .string but returns const char *. */ const char * c_str () { return string ().c_str (); } /* Return the root_name of path, e.g. on DOS-like systems the drive name. */ path root_name () const; /* Return the root directory if it exists. */ path root_directory () const; /* Return the root path, i.e. root_name + root_directory. */ path root_path () const; /* Return the relative path from root_path. */ path relative_path () const; /* Return the parent path of this path. */ path parent_path () const; /* Return the filename component of this path. */ path filename () const; /* Return the stem of filename component of this path, i.e. the filename without extension. */ path stem () const; /* Return the extension of filename component of this path. */ path extension () const; /* Check if this path is empty. */ bool empty () const noexcept; protected: /* Root of the filesystem, e.g. "C:" on dos-like filesystems. */ std::string m_root_name = ""; /* Is this path absolute? I.e. do we need to add a dir_sep at the beginning? */ bool m_absolute = false; /* Components of the path relative to root_path. */ std::list <std::string> m_path = {}; /* Helper function. */ /* Does the given string sart with a root_name? Needed for constructor. */ bool has_root_name (const std::string &s) const; /* Is the substring of S starting at position POS a dir_seperator? */ bool is_dirsep (const std::string &s, const size_t pos) const; /* Calculate the size (i.e. number of characters) of the total path including dir_sep's. */ size_t path_size () const; /* Append path component COMP to m_path. */ void append_component (std::string &comp); }; /* See declaration. */ inline bool path::has_root_name (const std::string &s) const { //#if defined (HAVE_DOS_BASED_FILESYSTEM) /* Assume 'C:'-like root_names. */ return s.size () >= 2 && (std::isalpha (s[0]) && s[1] == ':'); //#else return false; //#endif /* HAVE_DOS_BASED_FILESYSTEM */ } /* See declaration. */ inline bool path::is_dirsep (const std::string &s, const size_t pos) const { return s[pos] == preferred_seperator; } /* See declaration. */ size_t path::path_size () const { size_t size = 0; for (auto &p : m_path) size += p.size () + 1; return size; } /* See declaration. */ void path::append_component (std::string &comp) { if (comp == ".") return; if (comp == ".." && !m_path.empty ()) { m_path.pop_back (); return; } m_path.push_back (comp); } /* Constructors. */ template<class T> path::path (const T &s) : path (std::string (s)) {} template<> path::path (const std::string &s) { size_t pos = 0; if (has_root_name (s)) { /* Assume 'C:'-like root_names. */ m_root_name = s.substr (pos, 2); pos += 2; } if (is_dirsep (s, pos)) { m_absolute = true; pos++; } do { /* Remove duplicate dir_seps. */ while (s[pos] == preferred_seperator) pos++; size_t last_pos = pos; pos = s.find (preferred_seperator, pos); std::string comp (s.substr (last_pos, pos - last_pos)); append_component (comp); } while (pos != std::string::npos); } template<> path::path (const path &other) { *this = other; } /* See declaration. */ std::ostream & operator<< (std::ostream &os, const path &p) { os << std::quoted (p.string ()); return os; } /* See declaration. */ template<class T> path & path::operator= (const T &other) { return *this = path (other); } /* See declaration. */ template<> path & path::operator= (const path &other) { preferred_seperator = other.preferred_seperator; m_root_name = other.m_root_name; m_absolute = other.m_absolute; m_path = other.m_path; return *this; } /* See declaration. */ template<> path & path::operator/= (const path &other) { for (auto comp : other.m_path) append_component (comp); return *this; } /* See declaration. */ template<class T> path & path::operator/= (const T &other) { return *this /= path (other); } /* See declaration. */ template<class T> path operator/ (path lhs, const T &rhs) { return lhs /= rhs; } /* See declaration. */ template<> path & path::operator+= (const path &other) { /* Ignore a possible root_name in other. */ m_path.back () += other.m_path.front (); m_path.insert (m_path.end (), ++other.m_path.begin (), other.m_path.end ()); return *this; } /* See declaration. */ template<class T> path & path::operator+= (const T &other) { return *this += path (other); } /* See declaration. */ void path::clear () { m_root_name.clear (); m_path.clear (); m_absolute = false; } /* See declaration. */ std::string path::string () const { std::string ret; if (empty ()) return ""; ret.reserve (path_size ()); ret += m_root_name; if (m_absolute) ret += preferred_seperator; for (auto p = m_path.begin (); p != m_path.end (); p++) ret += *p + preferred_seperator; /* Remove trailing dir_sep. */ ret.pop_back (); return ret; } /* See declaration. */ path path::root_name () const { return empty () ? path () : path (m_root_name); } /* See declaration. */ path path::root_directory () const { return m_absolute ? path (&preferred_seperator) : path (); } /* See declaration. */ path path::root_path () const { return root_name () / root_directory (); } /* See declaration. */ path path::relative_path () const { if (empty ()) return path (); path ret (*this); ret.m_root_name = ""; ret.m_absolute = false; return ret; } /* See declaration. */ path path::parent_path () const { if (empty ()) return path (); path ret (*this); ret.m_path.pop_back (); return ret; } /* See declaration. */ path path::filename () const { if (empty ()) return path (); return path (m_path.back ()); } /* See declaration. */ path path::stem () const { if (empty ()) return path (); auto pos = m_path.back ().rfind ('.'); if (pos == 0) return path (m_path.back ()); return path (m_path.back ().substr (0, pos)); } /* See declaration. */ path path::extension () const { if (empty ()) return path (); auto pos = m_path.back ().rfind ('.'); if (pos == 0 || pos == std::string::npos) return path (); return path (m_path.back ().substr (pos)); } /* See declaration. */ bool path::empty () const noexcept { return m_path.empty () && m_root_name.empty () && !m_absolute; } } /* namespace filesystem */ enum path_type { HOST_PATH, TARGET_PATH, }; class path : public gdb::filesystem::path { public: path () noexcept {}; template<class T> path (const T& s, enum path_type _type = HOST_PATH) : gdb::filesystem::path (s), type (_type) {} /* Overload .string method to prepend target_prefix. */ std::string string () const; /* Substitute all occurrences of FROM to TO in this path. */ path &substitute (const std::string &from, const std::string &to); /* Type of this path (host or target). */ enum path_type type; private: /* Prefix to be prepended to target paths. */ const std::string m_target_prefix = "target:"; }; /* See declaration. */ std::string path::string () const { std::string ret; if (type == TARGET_PATH) { ret.reserve (path_size () + m_target_prefix.size ()); ret = m_target_prefix + gdb::filesystem::path::string (); } else { ret = gdb::filesystem::path::string (); } return ret; } /* See declaration. */ path & path::substitute (const std::string &from, const std::string &to) { if (from.empty ()) return *this; /* Use TO == "" to remove the component completely. */ if (to.empty ()) { m_path.remove (from); return *this; } for (auto it = m_path.begin (); it != m_path.end (); ++it) { if (*it != from) continue; *it = to; } return *this; } } /* namespace gdb */ #endif /*__COMMON_GDB_PATH__ */ [-- Attachment #3: gdb_path-selftest.c --] [-- Type: text/x-c++src, Size: 2388 bytes --] /* Self tests for filesystem path handling for GDB. Copyright (C) 2017 Free Software Foundation, Inc. This file is part of GDB. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <iostream> #include "gdb_path.h" //namespace selftest { namespace fs = gdb::filesystem; static int i = 0; template<class T> void print_path (T &p) { std::cout << i++ << ":\n"; std::cout << "string = " << p.string () << "\n"; std::cout << "c_str = " << p.c_str () << "\n"; std::cout << "root_name = " << p.root_name () << "\n"; std::cout << "root_dir = " << p.root_directory () << "\n"; std::cout << "root_path = " << p.root_path () << "\n"; std::cout << "relative_path = " << p.relative_path () << "\n"; std::cout << "parent_path = " << p.parent_path () << "\n"; std::cout << "filename = " << p.filename () << "\n"; std::cout << "stem = " << p.stem () << "\n"; std::cout << "extension = " << p.extension () << "\n"; std::cout << "\n"; } int main () { fs::path p ("/"); print_path (p); p.clear (); p = "C:1/2///"; print_path (p); p.clear (); p = "C:/1/2///"; print_path (p); p.clear (); p = fs::path ("/1/x/y/z/../../..///2/."); fs::path q = "3"; fs::path r = "4"; fs::path s; s = q / r; p /= s; p /= std::string ("5"); p /= "6"; p.append (".7"); p /= "."; p /= "x"; p /= ".."; print_path (p); p += fs::path (".foo"); print_path (p); p += ".bar"; print_path (p); fs::path t; print_path (t); gdb::path gp ("/a/b/c/$x/$y/e/f"); print_path (gp); gp.type = gdb::TARGET_PATH; gp.substitute ("$x", "d"); gp.substitute ("$y", ""); print_path (gp); gp.clear (); gp /= "../foo"; print_path (gp); } //} /* namespace selftest */ ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi 2017-05-16 12:00 ` Philipp Rudo @ 2017-05-17 15:41 ` Pedro Alves 2017-05-18 9:54 ` Yao Qi 1 sibling, 1 reply; 32+ messages in thread From: Pedro Alves @ 2017-05-17 15:41 UTC (permalink / raw) To: Yao Qi, gdb-patches On 05/11/2017 04:55 PM, Yao Qi wrote: > filename = lbasename (target_description_filename); > + std::string filename_after_features (target_description_filename); > + auto loc = filename_after_features.rfind ("/features/"); > + > + if (loc != std::string::npos) > + filename_after_features = filename_after_features.substr (loc + 10); > + > function = (char *) alloca (strlen (filename) + 1); > for (inp = filename, outp = function; *inp != '\0'; inp++) > if (*inp == '.') > @@ -1979,9 +2080,64 @@ feature = tdesc_create_feature (result, \"%s\");\n", > } > > printf_unfiltered (" tdesc_%s = result;\n", function); > + > + printf_unfiltered ("#if GDB_SELF_TEST\n"); > + printf_unfiltered (" selftests::record_xml_tdesc (\"%s\", tdesc_%s);\n", > + filename_after_features.data (), function); Since you're using the string as a C string -- c_str() is clearer than data(). > + printf_unfiltered ("#endif /* GDB_SELF_TEST */\n"); > printf_unfiltered ("}\n"); > } > > +#if GDB_SELF_TEST > +#include "selftest.h" > + > +namespace selftests { > + > +static std::vector<std::pair<std::string, const struct target_desc *>> lists; I think this "lists" either needs a comment or a more descriptive name. > + > +void > +record_xml_tdesc (std::string xml_file, const struct target_desc *tdesc) > +{ > + lists.emplace_back (xml_file, tdesc); Write: lists.emplace_back (std::move (xml_file), tdesc); to avoid another copy. Though, I can't tell why do we need to store a dynamically allocated std::string, when seemingly a "const char *" would do? All callers pass in a string literal, and all you do with the strings in the list is iterate over all list elements and build new strings from those. > +} > + > +/* Test these GDB builtin target descriptions are identical to these which > + are generated by the corresponding xml files. */ > + > +static void > +xml_builtin_tdesc_test (void) (void) -> () > +{ > + std::string feature_dir (ldirname (__FILE__)); > + struct stat st; Ugh. Obviously this can't work if gdb is installed / copied elsewhere, remote host testing, etc. > + > + /* Look for the features directory. If the directory of __FILE__ can't > + be found, __FILE__ is a file name with relative path. Guess that > + GDB is executed in testsuite directory like ../gdb, because I don't > + expect that GDB is invoked somewhere else and run self tests. */ > + if (stat (feature_dir.data (), &st) < 0) > + { > + feature_dir.insert (0, SLASH_STRING); > + feature_dir.insert (0, ".."); > + > + /* If still can't find the path, something is wrong. */ > + SELF_CHECK (stat (feature_dir.data (), &st) == 0); Do we want to flag this as an error / unit test failure? Maybe it should be a warning instead? > + } > + > + feature_dir = feature_dir + SLASH_STRING + "features"; feature_dir += SLASH_STRING + "features"; > + > + for (auto const &e : lists) > + { > + std::string tdesc_xml = (feature_dir + SLASH_STRING + e.first); > + const struct target_desc *tdesc > + = file_read_description_xml (tdesc_xml.data ()); > + > + SELF_CHECK (tdesc != NULL); > + SELF_CHECK (tdesc_equals (tdesc, e.second)); > + } > +} > +} > +#endif /* GDB_SELF_TEST */ > + > /* Provide a prototype to silence -Wmissing-prototypes. */ > extern initialize_file_ftype _initialize_target_descriptions; > > @@ -2022,4 +2178,8 @@ GDB will read the description from the target."), > add_cmd ("c-tdesc", class_maintenance, maint_print_c_tdesc_cmd, _("\ > Print the current target description as a C source file."), > &maintenanceprintlist); > + > +#if GDB_SELF_TEST > + register_self_test (selftests::xml_builtin_tdesc_test); > +#endif > } > diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h > index 361ac97..78416ae 100644 > --- a/gdb/target-descriptions.h > +++ b/gdb/target-descriptions.h > @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *); > int tdesc_compatible_p (const struct target_desc *, > const struct bfd_arch_info *); > > +/* Compare target descriptions TDESC1 and TDESC2, return true if they > + are identical. */ > + > +bool tdesc_equals (const struct target_desc *tdesc1, > + const struct target_desc *tdesc2); Any reason this and the other equals functions aren't operator== implementations? It's not obvious since the comments say "identical", which would maybe suggest that there may be some property that is not being compared and thus this is not strict value equality, but then function name says "equals". > + > /* Return the string value of a property named KEY, or NULL if the > property was not specified. */ > > @@ -253,4 +259,14 @@ void tdesc_create_reg (struct tdesc_feature *feature, const char *name, > int regnum, int save_restore, const char *group, > int bitsize, const char *type); > > +#if GDB_SELF_TEST > +namespace selftests { > + > +/* Record the target description TDESC generated by XML_FILE. */ > + > +void record_xml_tdesc (std::string xml_file, > + const struct target_desc *tdesc); Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-17 15:41 ` Pedro Alves @ 2017-05-18 9:54 ` Yao Qi 2017-05-18 11:34 ` Pedro Alves 0 siblings, 1 reply; 32+ messages in thread From: Yao Qi @ 2017-05-18 9:54 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Pedro Alves <palves@redhat.com> writes: >> +{ >> + std::string feature_dir (ldirname (__FILE__)); >> + struct stat st; > > Ugh. Obviously this can't work if gdb is installed / copied elsewhere, > remote host testing, etc. > I thought about this, but I can't figure out one better than __FILE__. What I want to do is to find srcdir, and open these xml files during unit tests. Since it is a unit test, I expect gdb is executed in either builddir/gdb or builddir/gdb/testsuite. I don't see a case that people build gdb in one place, and run unit/self tests somewhere else. >> + >> + /* Look for the features directory. If the directory of __FILE__ can't >> + be found, __FILE__ is a file name with relative path. Guess that >> + GDB is executed in testsuite directory like ../gdb, because I don't >> + expect that GDB is invoked somewhere else and run self tests. */ >> + if (stat (feature_dir.data (), &st) < 0) >> + { >> + feature_dir.insert (0, SLASH_STRING); >> + feature_dir.insert (0, ".."); >> + >> + /* If still can't find the path, something is wrong. */ >> + SELF_CHECK (stat (feature_dir.data (), &st) == 0); > > Do we want to flag this as an error / unit test failure? > Maybe it should be a warning instead? > We can skip this test if it can't find "features" directory in source, but something wrong can be easily ignored if we do so. >> --- a/gdb/target-descriptions.h >> +++ b/gdb/target-descriptions.h >> @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *); >> int tdesc_compatible_p (const struct target_desc *, >> const struct bfd_arch_info *); >> >> +/* Compare target descriptions TDESC1 and TDESC2, return true if they >> + are identical. */ >> + >> +bool tdesc_equals (const struct target_desc *tdesc1, >> + const struct target_desc *tdesc2); > > Any reason this and the other equals functions aren't operator== > implementations? tdesc_reg, tdesc_type, tdesc_feature and target_desc should be class-fied first, including adding proper ctor, dtor, etc. I thought it must be a lot of work, so I don't do that. I can do that if it doesn't take me much time. > It's not obvious since the comments say "identical", which would maybe > suggest that > there may be some property that is not being compared and thus this is not > strict value equality, but then function name says "equals". I think "identical" implies "strict value equality". The information I want to deliver is that this function compares all properties, return true if they are exactly the same. Is it this better, /* Return true if target description TDESC1 and TDESC2 are equal. */ -- Yao (齐尧) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-18 9:54 ` Yao Qi @ 2017-05-18 11:34 ` Pedro Alves 2017-05-19 15:47 ` Yao Qi 0 siblings, 1 reply; 32+ messages in thread From: Pedro Alves @ 2017-05-18 11:34 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 05/18/2017 10:54 AM, Yao Qi wrote: > Pedro Alves <palves@redhat.com> writes: > >>> +{ >>> + std::string feature_dir (ldirname (__FILE__)); >>> + struct stat st; >> >> Ugh. Obviously this can't work if gdb is installed / copied elsewhere, >> remote host testing, etc. >> > > I thought about this, but I can't figure out one better than __FILE__. > What I want to do is to find srcdir, and open these xml files during > unit tests. Since it is a unit test, I expect gdb is executed in either > builddir/gdb or builddir/gdb/testsuite. I don't see a case that people > build gdb in one place, and run unit/self tests somewhere else. Not sure -- does it work in remote host testing scenarios? Recall that unit testing is invoked by the normal testsuite, from gdb.gdb/unittest.exp. An alternative would be to add a specific command to run these tests, which would take the path as argument, like "maint check xml-descriptions /path/to/features/", and then run that from gdb.gdb/unittest.exp. > >>> + >>> + /* Look for the features directory. If the directory of __FILE__ can't >>> + be found, __FILE__ is a file name with relative path. Guess that >>> + GDB is executed in testsuite directory like ../gdb, because I don't >>> + expect that GDB is invoked somewhere else and run self tests. */ >>> + if (stat (feature_dir.data (), &st) < 0) >>> + { >>> + feature_dir.insert (0, SLASH_STRING); >>> + feature_dir.insert (0, ".."); >>> + >>> + /* If still can't find the path, something is wrong. */ >>> + SELF_CHECK (stat (feature_dir.data (), &st) == 0); >> >> Do we want to flag this as an error / unit test failure? >> Maybe it should be a warning instead? >> > > We can skip this test if it can't find "features" directory in source, but > something wrong can be easily ignored if we do so. > >>> --- a/gdb/target-descriptions.h >>> +++ b/gdb/target-descriptions.h >>> @@ -162,6 +162,12 @@ enum gdb_osabi tdesc_osabi (const struct target_desc *); >>> int tdesc_compatible_p (const struct target_desc *, >>> const struct bfd_arch_info *); >>> >>> +/* Compare target descriptions TDESC1 and TDESC2, return true if they >>> + are identical. */ >>> + >>> +bool tdesc_equals (const struct target_desc *tdesc1, >>> + const struct target_desc *tdesc2); >> >> Any reason this and the other equals functions aren't operator== >> implementations? > > tdesc_reg, tdesc_type, tdesc_feature and target_desc should be > class-fied first, including adding proper ctor, dtor, etc. There's no such requirement in the language. > I thought it > must be a lot of work, so I don't do that. I can do that if it doesn't > take me much time. But you don't need to do that to implement operators. > >> It's not obvious since the comments say "identical", which would maybe >> suggest that >> there may be some property that is not being compared and thus this is not >> strict value equality, but then function name says "equals". > > I think "identical" implies "strict value equality". Yeah, I think I got it backwards (been a long time since I used a programming language that has such a distinction), but the point still stands -- "identical" and "equals" can mean slightly different things so I'd rather not mix them up to avoid confusion. > The information I > want to deliver is that this function compares all properties, return > true if they are exactly the same. Is it this better, > > /* Return true if target description TDESC1 and TDESC2 are equal. */ Sure, that makes the comment matches the function name, so it's fine. Say "target descriptions", plural, though. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-18 11:34 ` Pedro Alves @ 2017-05-19 15:47 ` Yao Qi 2017-05-22 8:51 ` Yao Qi 0 siblings, 1 reply; 32+ messages in thread From: Yao Qi @ 2017-05-19 15:47 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Pedro Alves <palves@redhat.com> writes: > Not sure -- does it work in remote host testing scenarios? It doesn't work in remote host. > Recall that unit testing is invoked by the normal testsuite, from > gdb.gdb/unittest.exp. > > An alternative would be to add a specific command to run these tests, > which would take the path as argument, like > "maint check xml-descriptions /path/to/features/", and then run that > from gdb.gdb/unittest.exp. It is difficult to get it working for remote host either, because we need to copy all features/*.xml to the remote host. Can we just skip this unit test if it can't find "features" directory? std::string feature_dir (ldirname (__FILE__)); struct stat st; /* Look for the features directory. If the directory of __FILE__ can't be found, __FILE__ is a file name with relative path. Guess that GDB is executed in testsuite directory like ../gdb, because I don't expect that GDB is invoked somewhere else and run self tests. */ if (stat (feature_dir.data (), &st) < 0) { feature_dir.insert (0, SLASH_STRING); feature_dir.insert (0, ".."); /* If still can't find the path, skip it. */ return; } -- Yao (齐尧) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 2/7] Add unit test to builtin tdesc generated by xml 2017-05-19 15:47 ` Yao Qi @ 2017-05-22 8:51 ` Yao Qi 0 siblings, 0 replies; 32+ messages in thread From: Yao Qi @ 2017-05-22 8:51 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Yao Qi <qiyaoltc@gmail.com> writes: > It is difficult to get it working for remote host either, because we > need to copy all features/*.xml to the remote host. > > Can we just skip this unit test if it can't find "features" directory? We need to copy these .xml files to host sooner or later. Let me sort it out. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC 3/7] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi ` (2 preceding siblings ...) 2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi @ 2017-05-11 15:55 ` Yao Qi 2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux target descriptions Yao Qi ` (3 subsequent siblings) 7 siblings, 0 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw) To: gdb-patches Exchange the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml, to align with other i386 linux .xml files. gdb: 2017-04-27 Yao Qi <yao.qi@linaro.org> * features/i386/i386-linux.xml: Exchange the order of including 32bit-linux.xml and 32bit-sse.xml. * features/i386/i386-linux.c: Regenerated. --- gdb/features/i386/i386-linux.c | 6 +++--- gdb/features/i386/i386-linux.xml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gdb/features/i386/i386-linux.c b/gdb/features/i386/i386-linux.c index 39328af..649dcd6 100644 --- a/gdb/features/i386/i386-linux.c +++ b/gdb/features/i386/i386-linux.c @@ -71,9 +71,6 @@ initialize_tdesc_i386_linux (void) tdesc_create_reg (feature, "fooff", 30, 1, "float", 32, "int"); tdesc_create_reg (feature, "fop", 31, 1, "float", 32, "int"); - feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux"); - tdesc_create_reg (feature, "orig_eax", 41, 1, NULL, 32, "int"); - feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse"); field_type = tdesc_named_type (feature, "ieee_single"); tdesc_create_vector (feature, "v4f", field_type, 4); @@ -135,6 +132,9 @@ initialize_tdesc_i386_linux (void) tdesc_create_reg (feature, "xmm7", 39, 1, NULL, 128, "vec128"); tdesc_create_reg (feature, "mxcsr", 40, 1, "vector", 32, "i386_mxcsr"); + feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux"); + tdesc_create_reg (feature, "orig_eax", 41, 1, NULL, 32, "int"); + tdesc_i386_linux = result; #if GDB_SELF_TEST selftests::record_xml_tdesc ("i386/i386-linux.xml", tdesc_i386_linux); diff --git a/gdb/features/i386/i386-linux.xml b/gdb/features/i386/i386-linux.xml index f9aa311..17f9a1a 100644 --- a/gdb/features/i386/i386-linux.xml +++ b/gdb/features/i386/i386-linux.xml @@ -12,6 +12,6 @@ <architecture>i386</architecture> <osabi>GNU/Linux</osabi> <xi:include href="32bit-core.xml"/> - <xi:include href="32bit-linux.xml"/> <xi:include href="32bit-sse.xml"/> + <xi:include href="32bit-linux.xml"/> </target> -- 1.9.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC 6/7] Lazily and dynamically create i386-linux target descriptions 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi ` (3 preceding siblings ...) 2017-05-11 15:55 ` [RFC 3/7] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml Yao Qi @ 2017-05-11 15:55 ` Yao Qi 2017-05-11 18:14 ` John Baldwin 2017-05-17 15:43 ` Pedro Alves 2017-05-11 15:55 ` [RFC 5/7] Centralize i386 linux " Yao Qi ` (2 subsequent siblings) 7 siblings, 2 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw) To: gdb-patches Instead of using pre-generated target descriptions, this patch changes GDB to lazily and dynamically create target descriptions according to the target hardware capability (xcr0 in i386). This support any combination of target features. This patch also adds a unit test to make sure dynamically generated tdesc are identical to these generated from xml files. gdb: 2017-04-27 Yao Qi <yao.qi@linaro.org> * i386-linux-tdep.c (i386_linux_read_description): Generate target description if it doesn't exist. [GDB_SELF_TEST] (i386_linux_read_description_test): New unit test. (_initialize_i386_linux_tdep) [GDB_SELF_TEST]: Register unit test. --- gdb/i386-linux-tdep.c | 85 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 18 deletions(-) diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index 1bc1a6f..615cca5 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -681,27 +681,50 @@ i386_linux_core_read_xcr0 (bfd *abfd) const struct target_desc * i386_linux_read_description (uint64_t xcr0) { - switch ((xcr0 & X86_XSTATE_ALL_MASK)) + if (xcr0 == 0) + return NULL; + + static struct target_desc *i386_linux_tdescs \ + [2/*X87*/][2/*SSE*/][2/*AVX*/][2/*MPX*/][2/*AVX512*/][2/*PKRU*/] = {}; + struct target_desc **tdesc; + + tdesc = &i386_linux_tdescs[(xcr0 & X86_XSTATE_X87) ? 1 : 0] + [(xcr0 & X86_XSTATE_SSE) ? 1 : 0] + [(xcr0 & X86_XSTATE_AVX) ? 1 : 0] + [(xcr0 & X86_XSTATE_MPX) ? 1 : 0] + [(xcr0 & X86_XSTATE_AVX512) ? 1 : 0] + [(xcr0 & X86_XSTATE_PKRU) ? 1 : 0]; + + if (*tdesc == NULL) { - case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK: - return tdesc_i386_avx_mpx_avx512_pku_linux; - case X86_XSTATE_AVX_AVX512_MASK: - return tdesc_i386_avx_avx512_linux; - case X86_XSTATE_MPX_MASK: - return tdesc_i386_mpx_linux; - case X86_XSTATE_AVX_MPX_MASK: - return tdesc_i386_avx_mpx_linux; - case X86_XSTATE_AVX_MASK: - return tdesc_i386_avx_linux; - case X86_XSTATE_SSE_MASK: - return tdesc_i386_linux; - case X86_XSTATE_X87_MASK: - return tdesc_i386_mmx_linux; - default: - break; + *tdesc = allocate_target_description (); + set_tdesc_architecture (*tdesc, bfd_scan_arch ("i386")); + set_tdesc_osabi (*tdesc, osabi_from_tdesc_string ("GNU/Linux")); + + long regnum = 0; + + if (xcr0 & X86_XSTATE_X87) + regnum = create_feature_org_gnu_gdb_i386_core_i386 (*tdesc, regnum); + + if (xcr0 & X86_XSTATE_SSE) + regnum = create_feature_org_gnu_gdb_i386_sse (*tdesc, regnum); + + regnum = create_feature_org_gnu_gdb_i386_linux (*tdesc, regnum); + + if (xcr0 & X86_XSTATE_AVX) + regnum = create_feature_org_gnu_gdb_i386_avx (*tdesc, regnum); + + if (xcr0 & X86_XSTATE_MPX) + regnum = create_feature_org_gnu_gdb_i386_mpx (*tdesc, regnum); + + if (xcr0 & X86_XSTATE_AVX512) + regnum = create_feature_org_gnu_gdb_i386_avx512 (*tdesc, regnum); + + if (xcr0 & X86_XSTATE_PKRU) + regnum = create_feature_org_gnu_gdb_i386_pkeys (*tdesc, regnum); } - return NULL; + return *tdesc; } /* Get Linux/x86 target description from core dump. */ @@ -1101,4 +1124,30 @@ _initialize_i386_linux_tdep (void) initialize_tdesc_i386_avx_mpx_linux (); initialize_tdesc_i386_avx_avx512_linux (); initialize_tdesc_i386_avx_mpx_avx512_pku_linux (); + +#if GDB_SELF_TEST + struct xml_and_mask + { + const char *xml_file_name; + uint64_t mask; + }; + + struct xml_and_mask array[] = { + { "i386/i386-linux.xml", X86_XSTATE_SSE_MASK }, + { "i386/i386-mmx-linux.xml", X86_XSTATE_X87_MASK }, + { "i386/i386-avx-linux.xml", X86_XSTATE_AVX_MASK }, + { "i386/i386-mpx-linux.xml", X86_XSTATE_MPX_MASK }, + { "i386/i386-avx-mpx-linux.xml", X86_XSTATE_AVX_MPX_MASK }, + { "i386/i386-avx-avx512-linux.xml", X86_XSTATE_AVX_AVX512_MASK }, + { "i386/i386-avx-mpx-avx512-pku-linux.xml", + X86_XSTATE_AVX_MPX_AVX512_PKU_MASK }, + }; + + for (auto &a : array) + { + auto tdesc = i386_linux_read_description (a.mask); + + selftests::record_xml_tdesc (a.xml_file_name, tdesc); + } +#endif /* GDB_SELF_TEST */ } -- 1.9.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions 2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux target descriptions Yao Qi @ 2017-05-11 18:14 ` John Baldwin 2017-05-11 21:03 ` Yao Qi 2017-05-17 15:43 ` Pedro Alves 1 sibling, 1 reply; 32+ messages in thread From: John Baldwin @ 2017-05-11 18:14 UTC (permalink / raw) To: gdb-patches; +Cc: Yao Qi On Thursday, May 11, 2017 04:55:04 PM Yao Qi wrote: > Instead of using pre-generated target descriptions, this patch > changes GDB to lazily and dynamically create target descriptions > according to the target hardware capability (xcr0 in i386). > This support any combination of target features. > > This patch also adds a unit test to make sure dynamically generated > tdesc are identical to these generated from xml files. I definitely like this approach of composing the tdesc. For the non-Linux case there are already amd64_target_description() and i386_target_description() functions in the respective -tdep.c files that could use the same treatment. Not sure if you wanted to fix all of x86 in the same patch series or do it as a separate followup? -- John Baldwin ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions 2017-05-11 18:14 ` John Baldwin @ 2017-05-11 21:03 ` Yao Qi 0 siblings, 0 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 21:03 UTC (permalink / raw) To: John Baldwin; +Cc: gdb-patches On 17-05-11 10:04:39, John Baldwin wrote: > On Thursday, May 11, 2017 04:55:04 PM Yao Qi wrote: > > Instead of using pre-generated target descriptions, this patch > > changes GDB to lazily and dynamically create target descriptions > > according to the target hardware capability (xcr0 in i386). > > This support any combination of target features. > > > > This patch also adds a unit test to make sure dynamically generated > > tdesc are identical to these generated from xml files. > > I definitely like this approach of composing the tdesc. For the > non-Linux case there are already amd64_target_description() and > i386_target_description() functions in the respective -tdep.c files > that could use the same treatment. Not sure if you wanted to fix all > of x86 in the same patch series or do it as a separate followup? > The later, but I can fix all x86 in one patch if people think it is more reasonable to do so. I plan to change one arch each time, and gradually change all archs to use the new style of target description. -- Yao ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions 2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux target descriptions Yao Qi 2017-05-11 18:14 ` John Baldwin @ 2017-05-17 15:43 ` Pedro Alves 2017-05-18 15:12 ` Yao Qi 1 sibling, 1 reply; 32+ messages in thread From: Pedro Alves @ 2017-05-17 15:43 UTC (permalink / raw) To: Yao Qi, gdb-patches On 05/11/2017 04:55 PM, Yao Qi wrote: > +#if GDB_SELF_TEST > + struct xml_and_mask > + { > + const char *xml_file_name; > + uint64_t mask; > + }; Minor nit: I'd instinctively find it more natural to read the list below as { input, output } (left to right), while you have have { output, input }. > + > + struct xml_and_mask array[] = { > + { "i386/i386-linux.xml", X86_XSTATE_SSE_MASK }, > + { "i386/i386-mmx-linux.xml", X86_XSTATE_X87_MASK }, > + { "i386/i386-avx-linux.xml", X86_XSTATE_AVX_MASK }, > + { "i386/i386-mpx-linux.xml", X86_XSTATE_MPX_MASK }, > + { "i386/i386-avx-mpx-linux.xml", X86_XSTATE_AVX_MPX_MASK }, > + { "i386/i386-avx-avx512-linux.xml", X86_XSTATE_AVX_AVX512_MASK }, > + { "i386/i386-avx-mpx-avx512-pku-linux.xml", > + X86_XSTATE_AVX_MPX_AVX512_PKU_MASK }, About these xml files. Is your idea that: #1 - we remove these xml description files at some point, keeping only the description fragments which are currently xi:included by the xml files above? #2 - or, we'll still continue adding new xml files and grow this list here? #3 - or, we'll keep the existing xml files as representative / legacy, and use them in the unit tests going forward, just to make sure the machinery builds correct descriptions? (I don't expect to answer to be #2, I just put it there for completeness.) > + }; > + > + for (auto &a : array) > + { > + auto tdesc = i386_linux_read_description (a.mask); > + > + selftests::record_xml_tdesc (a.xml_file_name, tdesc); > + } > +#endif /* GDB_SELF_TEST */ > } > Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions 2017-05-17 15:43 ` Pedro Alves @ 2017-05-18 15:12 ` Yao Qi 2017-05-19 10:15 ` Pedro Alves 0 siblings, 1 reply; 32+ messages in thread From: Yao Qi @ 2017-05-18 15:12 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Pedro Alves <palves@redhat.com> writes: >> + >> + struct xml_and_mask array[] = { >> + { "i386/i386-linux.xml", X86_XSTATE_SSE_MASK }, >> + { "i386/i386-mmx-linux.xml", X86_XSTATE_X87_MASK }, >> + { "i386/i386-avx-linux.xml", X86_XSTATE_AVX_MASK }, >> + { "i386/i386-mpx-linux.xml", X86_XSTATE_MPX_MASK }, >> + { "i386/i386-avx-mpx-linux.xml", X86_XSTATE_AVX_MPX_MASK }, >> + { "i386/i386-avx-avx512-linux.xml", X86_XSTATE_AVX_AVX512_MASK }, >> + { "i386/i386-avx-mpx-avx512-pku-linux.xml", >> + X86_XSTATE_AVX_MPX_AVX512_PKU_MASK }, > > About these xml files. Is your idea that: > > #1 - we remove these xml description files at some point, keeping only > the description fragments which are currently xi:included by the xml > files above? > #2 - or, we'll still continue adding new xml files and grow this > list here? > #3 - or, we'll keep the existing xml files as representative / legacy, > and use them in the unit tests going forward, just to make sure > the machinery builds correct descriptions? > > (I don't expect to answer to be #2, I just put it there for completeness.) In short, #2 and #3. IIUC, you are not against "continue adding new xml files" in #2, you are against that we have to maintain the list here as we add new xml files. Even we finished the transition for all ports to dynamic tdesc creation, we still need to add new xml files to features/ directory for new hardware features, because GDBserver needs them (regformats/*.dat need them). We may add i386/i386-avx-mpx-avx512-linux.xml, GDB doesn't need it, but GDBserver still needs it. Given we need to add such .xml file, it is better to grow the list so that we can do the test. So the answer is #2. Suppose one day, we don't need these XML files to generate regformats/*.dat for GDBserver, we can remove all i386-*linux.xml files except i386-avx-mpx-avx512-pku-linux.xml, but I still want to move them to somewhere in testsuite directory to keep them as tests. Then, it becomes #3. The purpose of this test is to make sure these lazily/dynamically created tdesc (using create_feature_org_gnu_gdb_XXX functions) equal to the tdesc created from XML files. The dynamic tdesc creation is still new, needs more tests. During the process that we change other targets (amd64, arm, mips, etc) to the dynamic tdesc creation, the lists like this are expected to be added for each target. When we finish the transition to dynamic tdesc creation for all ports, and dynamic tdesc creation is quite stable, we can remove all these tests and lists, but I prefer to keeping the tests. Before we remove these tests and lists, we still need to update the lists when, 1) add a new port with target description, a new list should be added in its -tdep.c file. 2) add a new xml target description for the existing port, Let me think about how to generate this list instead of manually maintain this list. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions 2017-05-18 15:12 ` Yao Qi @ 2017-05-19 10:15 ` Pedro Alves 2017-05-19 14:27 ` Yao Qi 0 siblings, 1 reply; 32+ messages in thread From: Pedro Alves @ 2017-05-19 10:15 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 05/18/2017 04:12 PM, Yao Qi wrote: > Pedro Alves <palves@redhat.com> writes: > >>> + >>> + struct xml_and_mask array[] = { >>> + { "i386/i386-linux.xml", X86_XSTATE_SSE_MASK }, >>> + { "i386/i386-mmx-linux.xml", X86_XSTATE_X87_MASK }, >>> + { "i386/i386-avx-linux.xml", X86_XSTATE_AVX_MASK }, >>> + { "i386/i386-mpx-linux.xml", X86_XSTATE_MPX_MASK }, >>> + { "i386/i386-avx-mpx-linux.xml", X86_XSTATE_AVX_MPX_MASK }, >>> + { "i386/i386-avx-avx512-linux.xml", X86_XSTATE_AVX_AVX512_MASK }, >>> + { "i386/i386-avx-mpx-avx512-pku-linux.xml", >>> + X86_XSTATE_AVX_MPX_AVX512_PKU_MASK }, >> >> About these xml files. Is your idea that: >> >> #1 - we remove these xml description files at some point, keeping only >> the description fragments which are currently xi:included by the xml >> files above? >> #2 - or, we'll still continue adding new xml files and grow this >> list here? >> #3 - or, we'll keep the existing xml files as representative / legacy, >> and use them in the unit tests going forward, just to make sure >> the machinery builds correct descriptions? >> >> (I don't expect to answer to be #2, I just put it there for completeness.) > > In short, #2 and #3. > > IIUC, you are not against "continue adding new xml files" in #2, you are > against that we have to maintain the list here as we add new xml files. Well, I wasn't ever expecting we'd end up with #2. I guess I don't see the point of going through all of this if we end up with having to add manually-written xml descriptions covering all feature combinations, anyway? I could see keeping the xml files pushed in the tree, but make them generated instead? It could be gdb that generates them from that xml_and_mask array. We'd share the code with gdbserver, so that gdbserver would generate xml files on the fly to send to gdb. Or we could do the generation completely outside gdb, with some file describing the potential combinations (in spirit similar to that xml_and_mask array), much like we generate the syscall xml files. > Even we finished the transition for all ports to dynamic tdesc creation, > we still need to add new xml files to features/ directory for new > hardware features, because GDBserver needs them (regformats/*.dat need > them). We may add i386/i386-avx-mpx-avx512-linux.xml, GDB > doesn't need it, but GDBserver still needs it. Given we need to add > such .xml file, it is better to grow the list so that we can do the > test. So the answer is #2. Suppose one day, we don't need these XML > files to generate regformats/*.dat for GDBserver, we can remove all > i386-*linux.xml files except i386-avx-mpx-avx512-pku-linux.xml, but I > still want to move them to somewhere in testsuite directory to keep them > as tests. Then, it becomes #3. OK. But if there's no plan to do the gdbserver side of the work, I kind of call into question the more-complicated state we're getting into, for an indeterminately long time. > The purpose of this test is to make sure these lazily/dynamically > created tdesc (using create_feature_org_gnu_gdb_XXX functions) equal to > the tdesc created from XML files. The dynamic tdesc creation is still > new, needs more tests. > > During the process that we change other targets (amd64, arm, mips, etc) > to the dynamic tdesc creation, the lists like this are expected to be > added for each target. > > When we finish the transition to dynamic tdesc creation for all ports, > and dynamic tdesc creation is quite stable, we can remove all these > tests and lists, but I prefer to keeping the tests. I think most ports are not troublesome, WRT to explosion of target description feature set combinations. Maybe a better approach would be to fully transition an architecture all the way to dynamic generation, gdb and gdbserver? > > Before we remove these tests and lists, we still need to update the > lists when, > > 1) add a new port with target description, a new list should be added > in its -tdep.c file. > 2) add a new xml target description for the existing port, > > Let me think about how to generate this list instead of manually > maintain this list. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 6/7] Lazily and dynamically create i386-linux target descriptions 2017-05-19 10:15 ` Pedro Alves @ 2017-05-19 14:27 ` Yao Qi 0 siblings, 0 replies; 32+ messages in thread From: Yao Qi @ 2017-05-19 14:27 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Pedro Alves <palves@redhat.com> writes: > Well, I wasn't ever expecting we'd end up with #2. I guess I don't see > the point of going through all of this if we end up with having to add > manually-written xml descriptions covering all feature combinations, anyway? > These changes are about GDB, but we still need manually-written xml files because GDBserver still needs it. There are still some benefits, - GDB binary becomes smaller, see https://sourceware.org/ml/gdb-patches/2017-05/msg00299.html - We don't have to generate a lot of gdb/features/*.c files, so don't need to include them. In i386-linux, we only need features/i386/i386-avx-mpx-avx512-pku-linux.c and all other i386*linux.c can be removed. > I could see keeping the xml files pushed in the tree, but make them > generated instead? It could be gdb that generates them from that > xml_and_mask array. We'd share the code with gdbserver, so that gdbserver > would generate xml files on the fly to send to gdb. > > Or we could do the generation completely outside gdb, with some file > describing the potential combinations (in spirit similar to that xml_and_mask > array), much like we generate the syscall xml files. > I try to stop using the approach we pre-generate them (both xml files and c files). >> Even we finished the transition for all ports to dynamic tdesc creation, >> we still need to add new xml files to features/ directory for new >> hardware features, because GDBserver needs them (regformats/*.dat need >> them). We may add i386/i386-avx-mpx-avx512-linux.xml, GDB >> doesn't need it, but GDBserver still needs it. Given we need to add >> such .xml file, it is better to grow the list so that we can do the >> test. So the answer is #2. Suppose one day, we don't need these XML >> files to generate regformats/*.dat for GDBserver, we can remove all >> i386-*linux.xml files except i386-avx-mpx-avx512-pku-linux.xml, but I >> still want to move them to somewhere in testsuite directory to keep them >> as tests. Then, it becomes #3. > > OK. But if there's no plan to do the gdbserver side of the work, > I kind of call into question the more-complicated state we're getting > into, for an indeterminately long time. I do plan to do GDBserver side, but I don't have a clear picture on how to do it yet. I post this RFC, because this is only about GDB. This series can make GDB better and keep GDBserver unchanged, it is still a progress to me. > >> The purpose of this test is to make sure these lazily/dynamically >> created tdesc (using create_feature_org_gnu_gdb_XXX functions) equal to >> the tdesc created from XML files. The dynamic tdesc creation is still >> new, needs more tests. >> >> During the process that we change other targets (amd64, arm, mips, etc) >> to the dynamic tdesc creation, the lists like this are expected to be >> added for each target. >> >> When we finish the transition to dynamic tdesc creation for all ports, >> and dynamic tdesc creation is quite stable, we can remove all these >> tests and lists, but I prefer to keeping the tests. > > I think most ports are not troublesome, WRT to explosion of > target description feature set combinations. Maybe a better approach > would be to fully transition an architecture all the way to > dynamic generation, gdb and gdbserver? ARM, MIPS, I386 and S390 have this problem to different extents. I can take GDBserver into account at this stage. I did it last month, but gave up because it was too brain-damaging. Let me try again. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC 5/7] Centralize i386 linux target descriptions 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi ` (4 preceding siblings ...) 2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux target descriptions Yao Qi @ 2017-05-11 15:55 ` Yao Qi 2017-05-11 16:06 ` [RFC 0/7] Make GDB builtin target descriptions more flexible Eli Zaretskii 2017-05-11 20:55 ` [RFC 4/7] Share code in initialize_tdesc_ functions Yao Qi 7 siblings, 0 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 15:55 UTC (permalink / raw) To: gdb-patches This patch moves all the tdesc_i386*_linux target descriptions to a function i386_linux_read_description, which returns the right target description according to xcr0. gdb: 2017-04-27 Yao Qi <yao.qi@linaro.org> * i386-linux-tdep.c (i386_linux_read_description): New function. (i386_linux_core_read_description): Call i386_linux_read_description. * i386-linux-tdep.h (i386_linux_read_description): Declare. (tdesc_i386_linux, tdesc_i386_mmx_linux): Remove declarations. (tdesc_i386_avx_linux, tdesc_i386_mpx_linux): Likewise (tdesc_i386_avx_mpx_linux, tdesc_i386_avx_avx512_linux): Likewise. (tdesc_i386_avx_mpx_avx512_pku_linux): Likewise. * x86-linux-nat.c (x86_linux_read_description): Call i386_linux_read_description. --- gdb/i386-linux-tdep.c | 34 ++++++++++++++++++++++------------ gdb/i386-linux-tdep.h | 10 ++-------- gdb/x86-linux-nat.c | 24 ++++++++---------------- 3 files changed, 32 insertions(+), 36 deletions(-) diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index 1909d61..1bc1a6f 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -678,16 +678,9 @@ i386_linux_core_read_xcr0 (bfd *abfd) return xcr0; } -/* Get Linux/x86 target description from core dump. */ - -static const struct target_desc * -i386_linux_core_read_description (struct gdbarch *gdbarch, - struct target_ops *target, - bfd *abfd) +const struct target_desc * +i386_linux_read_description (uint64_t xcr0) { - /* Linux/i386. */ - uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd); - switch ((xcr0 & X86_XSTATE_ALL_MASK)) { case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK: @@ -708,10 +701,27 @@ i386_linux_core_read_description (struct gdbarch *gdbarch, break; } + return NULL; +} + +/* Get Linux/x86 target description from core dump. */ + +static const struct target_desc * +i386_linux_core_read_description (struct gdbarch *gdbarch, + struct target_ops *target, + bfd *abfd) +{ + /* Linux/i386. */ + uint64_t xcr0 = i386_linux_core_read_xcr0 (abfd); + const struct target_desc * tdesc = i386_linux_read_description (xcr0); + + if (tdesc != NULL) + return tdesc; + if (bfd_get_section_by_name (abfd, ".reg-xfp") != NULL) - return tdesc_i386_linux; + return i386_linux_read_description (X86_XSTATE_SSE_MASK); else - return tdesc_i386_mmx_linux; + return i386_linux_read_description (X86_XSTATE_X87_MASK); } /* Similar to i386_supply_fpregset, but use XSAVE extended state. */ @@ -835,7 +845,7 @@ i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) set_gdbarch_num_regs (gdbarch, I386_LINUX_NUM_REGS); if (! tdesc_has_registers (tdesc)) - tdesc = tdesc_i386_linux; + tdesc = i386_linux_read_description (X86_XSTATE_SSE_MASK); tdep->tdesc = tdesc; feature = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.linux"); diff --git a/gdb/i386-linux-tdep.h b/gdb/i386-linux-tdep.h index 8d4c7ca..1ca8d6b 100644 --- a/gdb/i386-linux-tdep.h +++ b/gdb/i386-linux-tdep.h @@ -42,14 +42,8 @@ extern uint64_t i386_linux_core_read_xcr0 (bfd *abfd); extern void i386_linux_handle_segmentation_fault (struct gdbarch *gdbarch, struct ui_out *uiout); -/* Linux target description. */ -extern struct target_desc *tdesc_i386_linux; -extern struct target_desc *tdesc_i386_mmx_linux; -extern struct target_desc *tdesc_i386_avx_linux; -extern struct target_desc *tdesc_i386_mpx_linux; -extern struct target_desc *tdesc_i386_avx_mpx_linux; -extern struct target_desc *tdesc_i386_avx_avx512_linux; -extern struct target_desc *tdesc_i386_avx_mpx_avx512_pku_linux; +/* Return the target description according to XCR0. */ +extern const struct target_desc *i386_linux_read_description (uint64_t xcr0); /* Format of XSAVE extended state is: struct diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c index 40a1b62..4eafce6 100644 --- a/gdb/x86-linux-nat.c +++ b/gdb/x86-linux-nat.c @@ -163,7 +163,7 @@ x86_linux_read_description (struct target_ops *ops) { have_ptrace_getfpxregs = 0; have_ptrace_getregset = TRIBOOL_FALSE; - return tdesc_i386_mmx_linux; + return i386_linux_read_description (X86_XSTATE_X87_MASK); } } #endif @@ -240,21 +240,13 @@ x86_linux_read_description (struct target_ops *ops) } else { - switch (xcr0_features_bits) - { - case X86_XSTATE_AVX_MPX_AVX512_PKU_MASK: - return tdesc_i386_avx_mpx_avx512_pku_linux; - case X86_XSTATE_AVX_AVX512_MASK: - return tdesc_i386_avx_avx512_linux; - case X86_XSTATE_MPX_MASK: - return tdesc_i386_mpx_linux; - case X86_XSTATE_AVX_MPX_MASK: - return tdesc_i386_avx_mpx_linux; - case X86_XSTATE_AVX_MASK: - return tdesc_i386_avx_linux; - default: - return tdesc_i386_linux; - } + const struct target_desc * tdesc + = i386_linux_read_description (xcr0_features_bits); + + if (tdesc == NULL) + tdesc = i386_linux_read_description (X86_XSTATE_SSE_MASK); + + return tdesc; } gdb_assert_not_reached ("failed to return tdesc"); -- 1.9.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 0/7] Make GDB builtin target descriptions more flexible 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi ` (5 preceding siblings ...) 2017-05-11 15:55 ` [RFC 5/7] Centralize i386 linux " Yao Qi @ 2017-05-11 16:06 ` Eli Zaretskii 2017-05-11 20:56 ` Yao Qi 2017-05-11 20:55 ` [RFC 4/7] Share code in initialize_tdesc_ functions Yao Qi 7 siblings, 1 reply; 32+ messages in thread From: Eli Zaretskii @ 2017-05-11 16:06 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches, alan.hayward > From: Yao Qi <qiyaoltc@gmail.com> > Cc: alan.hayward@arm.com > Date: Thu, 11 May 2017 16:54:58 +0100 > > Patch 4 is the major part of this series But the series you've sent doesn't seem to include patch 4... ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 0/7] Make GDB builtin target descriptions more flexible 2017-05-11 16:06 ` [RFC 0/7] Make GDB builtin target descriptions more flexible Eli Zaretskii @ 2017-05-11 20:56 ` Yao Qi 0 siblings, 0 replies; 32+ messages in thread From: Yao Qi @ 2017-05-11 20:56 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches, alan.hayward On 17-05-11 19:06:21, Eli Zaretskii wrote: > > From: Yao Qi <qiyaoltc@gmail.com> > > Cc: alan.hayward@arm.com > > Date: Thu, 11 May 2017 16:54:58 +0100 > > > > Patch 4 is the major part of this series > > But the series you've sent doesn't seem to include patch 4... The mail was rejected because it is too large. Compress the patch and send it again. -- Yao ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC 4/7] Share code in initialize_tdesc_ functions 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi ` (6 preceding siblings ...) 2017-05-11 16:06 ` [RFC 0/7] Make GDB builtin target descriptions more flexible Eli Zaretskii @ 2017-05-11 20:55 ` Yao Qi 2017-05-16 12:02 ` Philipp Rudo 7 siblings, 1 reply; 32+ messages in thread From: Yao Qi @ 2017-05-11 20:55 UTC (permalink / raw) To: gdb-patches [-- Attachment #1: Type: text/plain, Size: 8436 bytes --] [Resend it as the message is too large, and mail list doesn't accept it. Compress the patch and put it in attachment.] We define target description features in .xml files, and compose target descriptions of different features in .xml too. They are used to generated .c files and these .c files are included into GDB. Each feature may exist in multiple target descriptions, for example, feature avx exists in 4 different i386-linux target descriptions, so that each .c file generated from these 4 target descriptions has a copy of code for this feature, like this, feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx"); tdesc_create_reg (feature, "ymm0h", 42, 1, NULL, 128, "uint128"); tdesc_create_reg (feature, "ymm1h", 43, 1, NULL, 128, "uint128"); that is the code duplication. This patch moves the code for each feature into a function, and each target description just call the function for that feature. it becomes, #ifndef _ORG_GNU_GDB_I386_AVX #define _ORG_GNU_GDB_I386_AVX static void create_feature_org_gnu_gdb_i386_avx (struct target_desc *result) { struct tdesc_feature *feature; feature = tdesc_create_feature (result, "org.gnu.gdb.i386.avx"); tdesc_create_reg (feature, "ymm0h", 42, 1, NULL, 128, "uint128"); static void initialize_tdesc_i386_avx_mpx_avx512_pku_linux (void) { .... create_feature_org_gnu_gdb_i386_avx (result); } So far, everything looks simple, however, two things make it more complicated, 1) different target features have the same feature name, like "org.gnu.gdb.power.core" and "org.gnu.gdb.mips.cpu", so we can't define only one function for "org.gnu.gdb.power.core", 2) the register number of target feature (tdesc_reg::target_regnum) varies in different target descriptions, because the register number is allocated sequentially feature by feature within a target description, so we can't use the c function for a feature generated in one target description for some other target description. In order to address 1), function tdesc_feature_unique_name is added in this patch, which returns the unique name for each feature, even these feature name are the same. To address 2), this patch removes the constant number in generated c functions, instead, use variable "regnum", and get it passed from the c functions. The benefits of doing this are two-folded, - remove the duplication, there is only one copy of code for each feature. With all targets enabled, gdb size is smaller, $ size ./gdb text data bss dec hex filename 23823773 2395640 364776 26584189 195a47d ./gdb with the patch applied, text data bss dec hex filename 23552417 2395648 364808 26312873 19180a9 ./gdb - the composition of features can be more flexible, which will be demonstrated in the following patch, gdb: 2017-05-09 Yao Qi <yao.qi@linaro.org> * target-descriptions.c: Include string and algorithm. (struct tdesc_reg) <target_regnum>: Add comments. (tdesc_reg_equals): Update. (tdesc_remote_register_number): Call abs. (tdesc_feature_unique_name): New function. (maint_print_c_tdesc_cmd): Print function for each feature. * xml-tdesc.c (tdesc_start_reg): Set regnum negative if the value is from "regnum" attribute. (tdesc_start_reg): Call abs. * features/aarch64.c: Regenerated. * features/arc-arcompact.c: Regenerated. * features/arc-v2.c: Regenerated. * features/arm/arm-with-iwmmxt.c: Regenerated. * features/arm/arm-with-m-fpa-layout.c: Regenerated. * features/arm/arm-with-m-vfp-d16.c: Regenerated. * features/arm/arm-with-m.c: Regenerated. * features/arm/arm-with-neon.c: Regenerated. * features/arm/arm-with-vfpv2.c: Regenerated. * features/arm/arm-with-vfpv3.c: Regenerated. * features/i386/amd64-avx-avx512-linux.c: Regenerated. * features/i386/amd64-avx-avx512.c: Regenerated. * features/i386/amd64-avx-linux.c: Regenerated. * features/i386/amd64-avx-mpx-avx512-pku-linux.c: Regenerated. * features/i386/amd64-avx-mpx-avx512-pku.c: Regenerated. * features/i386/amd64-avx-mpx-linux.c: Regenerated. * features/i386/amd64-avx-mpx.c: Regenerated. * features/i386/amd64-avx.c: Regenerated. * features/i386/amd64-linux.c: Regenerated. * features/i386/amd64-mpx-linux.c: Regenerated. * features/i386/amd64-mpx.c: Regenerated. * features/i386/amd64.c: Regenerated. * features/i386/i386-avx-avx512-linux.c: Regenerated. * features/i386/i386-avx-avx512.c: Regenerated. * features/i386/i386-avx-linux.c: Regenerated. * features/i386/i386-avx-mpx-avx512-pku-linux.c: Regenerated. * features/i386/i386-avx-mpx-avx512-pku.c: Regenerated. * features/i386/i386-avx-mpx-linux.c: Regenerated. * features/i386/i386-avx-mpx.c: Regenerated. * features/i386/i386-avx.c: Regenerated. * features/i386/i386-linux.c: Regenerated. * features/i386/i386-mmx-linux.c: Regenerated. * features/i386/i386-mmx.c: Regenerated. * features/i386/i386-mpx-linux.c: Regenerated. * features/i386/i386-mpx.c: Regenerated. * features/i386/i386.c: Regenerated. * features/i386/x32-avx-avx512-linux.c: Regenerated. * features/i386/x32-avx-avx512.c: Regenerated. * features/i386/x32-avx-linux.c: Regenerated. * features/i386/x32-avx.c: Regenerated. * features/i386/x32-linux.c: Regenerated. * features/i386/x32.c: Regenerated. * features/microblaze-with-stack-protect.c: Regenerated. * features/microblaze.c: Regenerated. * features/mips-dsp-linux.c: Regenerated. * features/mips-linux.c: Regenerated. * features/mips64-dsp-linux.c: Regenerated. * features/mips64-linux.c: Regenerated. * features/nds32.c: Regenerated. * features/nios2-linux.c: Regenerated. * features/nios2.c: Regenerated. * features/rs6000/powerpc-32.c: Regenerated. * features/rs6000/powerpc-32l.c: Regenerated. * features/rs6000/powerpc-403.c: Regenerated. * features/rs6000/powerpc-403gc.c: Regenerated. * features/rs6000/powerpc-405.c: Regenerated. * features/rs6000/powerpc-505.c: Regenerated. * features/rs6000/powerpc-601.c: Regenerated. * features/rs6000/powerpc-602.c: Regenerated. * features/rs6000/powerpc-603.c: Regenerated. * features/rs6000/powerpc-604.c: Regenerated. * features/rs6000/powerpc-64.c: Regenerated. * features/rs6000/powerpc-64l.c: Regenerated. * features/rs6000/powerpc-7400.c: Regenerated. * features/rs6000/powerpc-750.c: Regenerated. * features/rs6000/powerpc-860.c: Regenerated. * features/rs6000/powerpc-altivec32.c: Regenerated. * features/rs6000/powerpc-altivec32l.c: Regenerated. * features/rs6000/powerpc-altivec64.c: Regenerated. * features/rs6000/powerpc-altivec64l.c: Regenerated. * features/rs6000/powerpc-cell32l.c: Regenerated. * features/rs6000/powerpc-cell64l.c: Regenerated. * features/rs6000/powerpc-e500.c: Regenerated. * features/rs6000/powerpc-e500l.c: Regenerated. * features/rs6000/powerpc-isa205-32l.c: Regenerated. * features/rs6000/powerpc-isa205-64l.c: Regenerated. * features/rs6000/powerpc-isa205-altivec32l.c: Regenerated. * features/rs6000/powerpc-isa205-altivec64l.c: Regenerated. * features/rs6000/powerpc-isa205-vsx32l.c: Regenerated. * features/rs6000/powerpc-isa205-vsx64l.c: Regenerated. * features/rs6000/powerpc-vsx32.c: Regenerated. * features/rs6000/powerpc-vsx32l.c: Regenerated. * features/rs6000/powerpc-vsx64.c: Regenerated. * features/rs6000/powerpc-vsx64l.c: Regenerated. * features/rs6000/rs6000.c: Regenerated. * features/s390-linux32.c: Regenerated. * features/s390-linux32v1.c: Regenerated. * features/s390-linux32v2.c: Regenerated. * features/s390-linux64.c: Regenerated. * features/s390-linux64v1.c: Regenerated. * features/s390-linux64v2.c: Regenerated. * features/s390-te-linux64.c: Regenerated. * features/s390-tevx-linux64.c: Regenerated. * features/s390-vx-linux64.c: Regenerated. * features/s390x-linux64.c: Regenerated. * features/s390x-linux64v1.c: Regenerated. * features/s390x-linux64v2.c: Regenerated. * features/s390x-te-linux64.c: Regenerated. * features/s390x-tevx-linux64.c: Regenerated. * features/s390x-vx-linux64.c: Regenerated. * features/tic6x-c62x-linux.c: Regenerated. * features/tic6x-c62x.c: Regenerated. * features/tic6x-c64x-linux.c: Regenerated. * features/tic6x-c64x.c: Regenerated. * features/tic6x-c64xp-linux.c: Regenerated. * features/tic6x-c64xp.c: Regenerated. gdb/testsuite: 2017-05-10 Yao Qi <yao.qi@linaro.org> * gdb.xml/maint_print_struct.exp: Update. -- Yao [-- Attachment #2: 4.patch.tar.gz --] [-- Type: application/gzip, Size: 64621 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 4/7] Share code in initialize_tdesc_ functions 2017-05-11 20:55 ` [RFC 4/7] Share code in initialize_tdesc_ functions Yao Qi @ 2017-05-16 12:02 ` Philipp Rudo 2017-05-17 15:43 ` Pedro Alves 0 siblings, 1 reply; 32+ messages in thread From: Philipp Rudo @ 2017-05-16 12:02 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches Hi Yao, On Thu, 11 May 2017 21:55:04 +0100 Yao Qi <qiyaoltc@gmail.com> wrote: > diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c > index 754f15b..a81ae0d 100644 > --- a/gdb/target-descriptions.c > +++ b/gdb/target-descriptions.c [...] > @@ -54,7 +57,10 @@ typedef struct tdesc_reg > char *name; > > /* The register number used by this target to refer to this > - register. This is used for remote p/P packets and to determine > + register. Positive number means it is got by increasing > + the previous register number by one. Negative number means > + it is got from "regnum" attribute in the xml file. > + This is used for remote p/P packets and to determine > the ordering of registers in the remote g/G packets. */ > long target_regnum; I'm not really sure if I like your idea with positive/negative regnums. It could easily lead to hard to find bugs because someone forgot a '-' somewhere. I would prefer an extra field like bool explicit_regnum; or bool automatic_regnum; [...] > +/* Return the unique name of FEATURE. The uniqueness means different > + target description features have different values. This is used > + to differentiate different features with the same name. */ > + > +static std::string > +tdesc_feature_unique_name (const struct tdesc_feature* feature) > +{ > + std::string name = std::string (feature->name); > + > + if (name.compare ("org.gnu.gdb.arm.m-profile") == 0) > + { > + struct tdesc_reg *reg; > + > + for (int ix = 0; > + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); > + ix++) > + if (reg->type != NULL && strcmp (reg->type, "arm_fpa_ext") == 0) > + { > + name.append ("_fpa"); > + break; > + } > + } > + else if (name.compare ("org.gnu.gdb.arm.vfp") == 0) > + { > + name.append ("_"); > + name.append (std::to_string (VEC_length (tdesc_reg_p, > + feature->registers) - 1)); > + } > + else if (name.compare ("org.gnu.gdb.i386.core") == 0) > + { > + struct tdesc_reg *reg = NULL; > + > + for (int ix = 0; > + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); > + ix++) > + { > + if (strcmp (reg->name, "ebp") == 0 > + || strcmp (reg->name, "rbp") == 0) > + break; > + } > + > + name.append ("_"); > + if (strcmp (reg->name, "ebp") == 0) > + name.append ("i386"); > + else > + { > + if (strcmp (reg->type, "data_ptr") == 0) > + name.append ("amd64"); > + else > + name.append ("x32"); > + } > + } > + else if (name.compare ("org.gnu.gdb.power.core") == 0) > + { > + struct tdesc_reg *reg = NULL; > + struct tdesc_reg *reg_mq = NULL; > + struct tdesc_reg *reg_r0 = NULL; > + > + /* Four variants. */ > + for (int ix = 0; > + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); > + ix++) > + { > + if (strcmp (reg->name, "r0") == 0) > + reg_r0 = reg; > + if (strcmp (reg->name, "mq") == 0) > + reg_mq = reg; > + > + if (reg_r0 != NULL && reg_mq != NULL) > + break; > + } > + > + name.append ("_"); > + if (reg_mq == NULL) > + name.append (std::to_string (reg_r0->bitsize)); > + else > + { > + if (reg_mq->target_regnum == -124) > + name.append ("601"); > + else > + name.append ("rs6000"); > + } > + } > + else if (name.compare ("org.gnu.gdb.power.fpu") == 0) > + { > + struct tdesc_reg *reg = NULL; > + > + /* Three variants. */ > + for (int ix = 0; > + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); > + ix++) > + { > + if (strcmp (reg->name, "fpscr") == 0) > + break; > + } > + > + name.append ("_"); > + > + if (reg->target_regnum == -71) > + name.append ("rs6000"); > + else > + name.append (std::to_string (reg->bitsize)); > + } > + else if (name.compare ("org.gnu.gdb.power.linux") == 0) > + { > + struct tdesc_reg *reg = NULL; > + > + for (int ix = 0; > + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); > + ix++) > + { > + if (strcmp (reg->name, "orig_r3") == 0) > + break; > + } > + > + name.append (std::to_string (reg->bitsize)); > + } > + else if (name.compare ("org.gnu.gdb.s390.linux") == 0) > + { > + struct tdesc_reg *reg = NULL; > + struct tdesc_reg *reg_orig_r2 = NULL; > + struct tdesc_reg *reg_sc = NULL; > + struct tdesc_reg *reg_lb = NULL; > + > + /* Six variants. */ > + for (int ix = 0; > + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); > + ix++) > + { > + if (strcmp (reg->name, "orig_r2") == 0) > + reg_orig_r2 = reg; > + if (strcmp (reg->name, "last_break") == 0) > + reg_lb = reg; > + if (strcmp (reg->name, "system_call") == 0) > + reg_sc = reg; > + > + if (reg_orig_r2 != NULL && reg_sc != NULL && reg_lb != NULL) > + break; > + } > + > + name.append ("_"); > + name.append (std::to_string (reg_orig_r2->bitsize)); > + > + if (reg_lb != NULL) > + { > + name.append ("_"); > + > + if (reg_sc == NULL) > + name.append ("v1"); > + else > + name.append ("v2"); > + } > + } > + else if (name.compare ("org.gnu.gdb.s390.core") == 0) > + { > + struct tdesc_reg *reg = NULL; > + struct tdesc_reg *reg_r0 = NULL; > + struct tdesc_reg *reg_r0l = NULL; > + > + /* Six variants. */ > + for (int ix = 0; > + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); > + ix++) > + { > + if (strcmp (reg->name, "r0") == 0) > + reg_r0 = reg; > + if (strcmp (reg->name, "r0l") == 0) > + reg_r0l = reg; > + > + if (reg_r0 != NULL && reg_r0l != NULL) > + break; > + } > + > + name.append ("_"); > + if (reg_r0l != NULL) > + name.append ("s64"); > + else > + name.append (std::to_string (reg_r0->bitsize)); > + } > + else if (name.compare ("org.gnu.gdb.mips.cpu") == 0 > + || name.compare ("org.gnu.gdb.mips.cp0") == 0 > + || name.compare ("org.gnu.gdb.mips.fpu") == 0 > + || name.compare ("org.gnu.gdb.mips.dsp") == 0 > + || name.compare ("org.gnu.gdb.mips.linux") == 0) > + { > + struct tdesc_reg *reg = VEC_index (tdesc_reg_p, feature->registers, 0); > + > + name.append (std::to_string (reg->bitsize)); > + } > + > + std::replace (name.begin (), name.end (), '.', '_'); > + std::replace (name.begin (), name.end (), '-', '_'); > + > + return name; > +} > + This function is actually the part I like least of your implementation. Its way to long and barely readable. The way I understand, it is needed to create unique macro and function names. So why don't you simply use the filename of the XML file where the feature is defined? It already is unique. Or use an gdbarch hook so every architecture can decide for itself how to name them? Philipp ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 4/7] Share code in initialize_tdesc_ functions 2017-05-16 12:02 ` Philipp Rudo @ 2017-05-17 15:43 ` Pedro Alves 2017-05-18 11:21 ` Yao Qi 0 siblings, 1 reply; 32+ messages in thread From: Pedro Alves @ 2017-05-17 15:43 UTC (permalink / raw) To: Philipp Rudo, Yao Qi; +Cc: gdb-patches On 05/16/2017 01:02 PM, Philipp Rudo wrote: > Hi Yao, > > On Thu, 11 May 2017 21:55:04 +0100 > Yao Qi <qiyaoltc@gmail.com> wrote: > >> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c >> index 754f15b..a81ae0d 100644 >> --- a/gdb/target-descriptions.c >> +++ b/gdb/target-descriptions.c > > [...] > >> @@ -54,7 +57,10 @@ typedef struct tdesc_reg >> char *name; >> >> /* The register number used by this target to refer to this >> - register. This is used for remote p/P packets and to determine >> + register. Positive number means it is got by increasing >> + the previous register number by one. Negative number means >> + it is got from "regnum" attribute in the xml file. >> + This is used for remote p/P packets and to determine >> the ordering of registers in the remote g/G packets. */ >> long target_regnum; > > I'm not really sure if I like your idea with positive/negative regnums. It > could easily lead to hard to find bugs because someone forgot a '-' somewhere. > I would prefer an extra field like > > bool explicit_regnum; > or > bool automatic_regnum; > > [...] > >> +/* Return the unique name of FEATURE. The uniqueness means different >> + target description features have different values. This is used >> + to differentiate different features with the same name. */ >> + >> +static std::string >> +tdesc_feature_unique_name (const struct tdesc_feature* feature) >> +{ >> + std::string name = std::string (feature->name); >> + >> + if (name.compare ("org.gnu.gdb.arm.m-profile") == 0) >> + { >> + struct tdesc_reg *reg; >> + >> + for (int ix = 0; >> + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); >> + ix++) >> + if (reg->type != NULL && strcmp (reg->type, "arm_fpa_ext") == 0) >> + { >> + name.append ("_fpa"); >> + break; >> + } >> + } >> + else if (name.compare ("org.gnu.gdb.arm.vfp") == 0) >> + { >> + name.append ("_"); >> + name.append (std::to_string (VEC_length (tdesc_reg_p, >> + feature->registers) - 1)); >> + } >> + else if (name.compare ("org.gnu.gdb.i386.core") == 0) >> + { >> + struct tdesc_reg *reg = NULL; >> + >> + for (int ix = 0; >> + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); >> + ix++) >> + { >> + if (strcmp (reg->name, "ebp") == 0 >> + || strcmp (reg->name, "rbp") == 0) >> + break; >> + } >> + >> + name.append ("_"); >> + if (strcmp (reg->name, "ebp") == 0) >> + name.append ("i386"); >> + else >> + { >> + if (strcmp (reg->type, "data_ptr") == 0) >> + name.append ("amd64"); >> + else >> + name.append ("x32"); >> + } >> + } >> + else if (name.compare ("org.gnu.gdb.power.core") == 0) >> + { >> + struct tdesc_reg *reg = NULL; >> + struct tdesc_reg *reg_mq = NULL; >> + struct tdesc_reg *reg_r0 = NULL; >> + >> + /* Four variants. */ >> + for (int ix = 0; >> + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); >> + ix++) >> + { >> + if (strcmp (reg->name, "r0") == 0) >> + reg_r0 = reg; >> + if (strcmp (reg->name, "mq") == 0) >> + reg_mq = reg; >> + >> + if (reg_r0 != NULL && reg_mq != NULL) >> + break; >> + } >> + >> + name.append ("_"); >> + if (reg_mq == NULL) >> + name.append (std::to_string (reg_r0->bitsize)); >> + else >> + { >> + if (reg_mq->target_regnum == -124) >> + name.append ("601"); >> + else >> + name.append ("rs6000"); >> + } >> + } >> + else if (name.compare ("org.gnu.gdb.power.fpu") == 0) >> + { >> + struct tdesc_reg *reg = NULL; >> + >> + /* Three variants. */ >> + for (int ix = 0; >> + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); >> + ix++) >> + { >> + if (strcmp (reg->name, "fpscr") == 0) >> + break; >> + } >> + >> + name.append ("_"); >> + >> + if (reg->target_regnum == -71) >> + name.append ("rs6000"); >> + else >> + name.append (std::to_string (reg->bitsize)); >> + } >> + else if (name.compare ("org.gnu.gdb.power.linux") == 0) >> + { >> + struct tdesc_reg *reg = NULL; >> + >> + for (int ix = 0; >> + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); >> + ix++) >> + { >> + if (strcmp (reg->name, "orig_r3") == 0) >> + break; >> + } >> + >> + name.append (std::to_string (reg->bitsize)); >> + } >> + else if (name.compare ("org.gnu.gdb.s390.linux") == 0) >> + { >> + struct tdesc_reg *reg = NULL; >> + struct tdesc_reg *reg_orig_r2 = NULL; >> + struct tdesc_reg *reg_sc = NULL; >> + struct tdesc_reg *reg_lb = NULL; >> + >> + /* Six variants. */ >> + for (int ix = 0; >> + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); >> + ix++) >> + { >> + if (strcmp (reg->name, "orig_r2") == 0) >> + reg_orig_r2 = reg; >> + if (strcmp (reg->name, "last_break") == 0) >> + reg_lb = reg; >> + if (strcmp (reg->name, "system_call") == 0) >> + reg_sc = reg; >> + >> + if (reg_orig_r2 != NULL && reg_sc != NULL && reg_lb != NULL) >> + break; >> + } >> + >> + name.append ("_"); >> + name.append (std::to_string (reg_orig_r2->bitsize)); >> + >> + if (reg_lb != NULL) >> + { >> + name.append ("_"); >> + >> + if (reg_sc == NULL) >> + name.append ("v1"); >> + else >> + name.append ("v2"); >> + } >> + } >> + else if (name.compare ("org.gnu.gdb.s390.core") == 0) >> + { >> + struct tdesc_reg *reg = NULL; >> + struct tdesc_reg *reg_r0 = NULL; >> + struct tdesc_reg *reg_r0l = NULL; >> + >> + /* Six variants. */ >> + for (int ix = 0; >> + VEC_iterate (tdesc_reg_p, feature->registers, ix, reg); >> + ix++) >> + { >> + if (strcmp (reg->name, "r0") == 0) >> + reg_r0 = reg; >> + if (strcmp (reg->name, "r0l") == 0) >> + reg_r0l = reg; >> + >> + if (reg_r0 != NULL && reg_r0l != NULL) >> + break; >> + } >> + >> + name.append ("_"); >> + if (reg_r0l != NULL) >> + name.append ("s64"); >> + else >> + name.append (std::to_string (reg_r0->bitsize)); >> + } >> + else if (name.compare ("org.gnu.gdb.mips.cpu") == 0 >> + || name.compare ("org.gnu.gdb.mips.cp0") == 0 >> + || name.compare ("org.gnu.gdb.mips.fpu") == 0 >> + || name.compare ("org.gnu.gdb.mips.dsp") == 0 >> + || name.compare ("org.gnu.gdb.mips.linux") == 0) >> + { >> + struct tdesc_reg *reg = VEC_index (tdesc_reg_p, feature->registers, 0); >> + >> + name.append (std::to_string (reg->bitsize)); >> + } >> + >> + std::replace (name.begin (), name.end (), '.', '_'); >> + std::replace (name.begin (), name.end (), '-', '_'); >> + >> + return name; >> +} >> + > > This function is actually the part I like least of your implementation. Its > way to long and barely readable. The way I understand, it is needed to create > unique macro and function names. So why don't you simply use the filename of > the XML file where the feature is defined? It already is unique. Or use an > gdbarch hook so every architecture can decide for itself how to name them? Agreed. I was reading the patch and thinking how there must be a better way to handle this. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC 4/7] Share code in initialize_tdesc_ functions 2017-05-17 15:43 ` Pedro Alves @ 2017-05-18 11:21 ` Yao Qi 0 siblings, 0 replies; 32+ messages in thread From: Yao Qi @ 2017-05-18 11:21 UTC (permalink / raw) To: Pedro Alves; +Cc: Philipp Rudo, gdb-patches Pedro Alves <palves@redhat.com> writes: Hi Philipp and Pedro, Thanks for your comments. >> This function is actually the part I like least of your implementation. Its >> way to long and barely readable. The way I understand, it is needed to create >> unique macro and function names. So why don't you simply use the filename of >> the XML file where the feature is defined? It already is unique. Or use an >> gdbarch hook so every architecture can decide for itself how to name them? > > Agreed. I was reading the patch and thinking how there must be a better > way to handle this. I dislike my implementation either. I thought about using xml feature file name to distinguish features with the same name. However it doesn't work because the file name is lost after processing xi:include. GDB processes XML target descriptions in two steps, 1) process xi:include, copy feature xml files into a buffer, 2) parse the buffer with gdb-target.dtd then, All tdesc_features are created in step 2, and GDB doesn't know what file is this feature from. For example, there is an XML target description, <target> <xi:include href="32bit-core.xml"/> </target> after step 1, the buffer contains XML contents from 32bit-core.xml, <target> <feature name="org.gnu.gdb.i386.core"> ... </feature> </target> GDB starts parse it in buffer, but doesn't know where is this feature from. I'll think about gdbarch hook you suggested. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2017-06-01 17:53 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-11 15:55 [RFC 0/7] Make GDB builtin target descriptions more flexible Yao Qi 2017-05-11 15:55 ` [RFC 7/7] Remove builtin tdesc_i386_*_linux Yao Qi 2017-05-16 12:02 ` Philipp Rudo 2017-05-17 15:46 ` Pedro Alves 2017-05-11 15:55 ` [RFC 1/7] Move initialize_tdesc_mips* calls from mips-linux-nat.c to mips-linux-tdep.c Yao Qi 2017-05-11 15:55 ` [RFC 2/7] Add unit test to builtin tdesc generated by xml Yao Qi 2017-05-16 12:00 ` Philipp Rudo 2017-05-16 15:46 ` Yao Qi 2017-05-17 9:09 ` Philipp Rudo 2017-05-17 16:06 ` Pedro Alves 2017-05-30 8:00 ` Philipp Rudo 2017-06-01 17:53 ` Philipp Rudo 2017-05-17 15:41 ` Pedro Alves 2017-05-18 9:54 ` Yao Qi 2017-05-18 11:34 ` Pedro Alves 2017-05-19 15:47 ` Yao Qi 2017-05-22 8:51 ` Yao Qi 2017-05-11 15:55 ` [RFC 3/7] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml Yao Qi 2017-05-11 15:55 ` [RFC 6/7] Lazily and dynamically create i386-linux target descriptions Yao Qi 2017-05-11 18:14 ` John Baldwin 2017-05-11 21:03 ` Yao Qi 2017-05-17 15:43 ` Pedro Alves 2017-05-18 15:12 ` Yao Qi 2017-05-19 10:15 ` Pedro Alves 2017-05-19 14:27 ` Yao Qi 2017-05-11 15:55 ` [RFC 5/7] Centralize i386 linux " Yao Qi 2017-05-11 16:06 ` [RFC 0/7] Make GDB builtin target descriptions more flexible Eli Zaretskii 2017-05-11 20:56 ` Yao Qi 2017-05-11 20:55 ` [RFC 4/7] Share code in initialize_tdesc_ functions Yao Qi 2017-05-16 12:02 ` Philipp Rudo 2017-05-17 15:43 ` Pedro Alves 2017-05-18 11:21 ` Yao Qi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).