[2/4] gdbserver use aarch64_create_target_description

Message ID 48281E67-64B6-4558-9790-CB4189187124@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Oct. 25, 2017, 8:29 a.m. UTC
  In gdbserver, instead of accessing tdesc_aarch64 directly, obtain it
from the function aarch64_linux_read_description.

aarch64_linux_read_description then calls down to
aarch64_create_target_description which was added in the previous patch.

Had to add a new makefile rule to catch the arch/ file, otherwise the
make system gets confused between arch64/aarch64.c and features/aarch64.c

Like the previous patch, this does not change any functionality.


Tested on a --enable-targets=all build and aarch64 build with board
files unix, native-gdbserver and unittest.exp.

Alan.

2017-10-25  Alan Hayward  <alan.hayward@arm.com>

gdbserver/
	* Makefile.in: IPA rules for arch/ files.
	* configure.srv: Add new files.
	* linux-aarch64-ipa.c (get_ipa_tdesc): Call
	aarch64_linux_read_description.
	* linux-aarch64-low.c (aarch64_linux_read_description):
	Merge with aarch64_arch_setup.
	(aarch64_arch_setup): Call aarch64_linux_read_description.
	* linux-aarch64-tdesc.c: New file.
	* linux-aarch64-tdesc.h: New file.
  

Comments

Yao Qi Oct. 30, 2017, 12:21 p.m. UTC | #1
Alan Hayward <Alan.Hayward@arm.com> writes:

> Had to add a new makefile rule to catch the arch/ file, otherwise the
> make system gets confused between arch64/aarch64.c and features/aarch64.c
>

I don't understand the reason.  We don't compile features/aarch64.c in
GDBserver.

> diff --git a/gdb/gdbserver/linux-aarch64-tdesc.h b/gdb/gdbserver/linux-aarch64-tdesc.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..48314e934e0c1163fce400b368a3717a27239be8
> --- /dev/null
> +++ b/gdb/gdbserver/linux-aarch64-tdesc.h
> @@ -0,0 +1,20 @@
> +/* Low level support for aarch64, shared between gdbserver and IPA.
> +
> +   Copyright (C) 2016-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/>.  */
> +
> +const struct target_desc * aarch64_linux_read_description ();
> diff --git a/gdb/gdbserver/linux-aarch64-tdesc.c b/gdb/gdbserver/linux-aarch64-tdesc.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..4148ed70c4ca973652e860492cd7cc1a249991d8
> --- /dev/null
> +++ b/gdb/gdbserver/linux-aarch64-tdesc.c
> @@ -0,0 +1,41 @@
> +/* GNU/Linux/aarch64 specific target description, for the remote server
> +   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 "server.h"
> +#include "tdesc.h"
> +#include "arch/aarch64.h"
> +#include "linux-aarch32-low.h"
> +
> +/* The Aarch64 target descriptor.  */

s/descriptor/description/

> +static const struct target_desc *aarch64_tdesc = NULL;
> +

Move it into aarch64_linux_read_description.

> +/* Create the aarch64 target description.  */
> +
> +const struct target_desc *

Remove struct.

> +aarch64_linux_read_description ()
> +{
> +  struct target_desc **tdesc = (struct target_desc **) &aarch64_tdesc;
> +
> +  if (*tdesc == NULL)
> +    {
> +      *tdesc = aarch64_create_target_description ();
> +    }

"{" and "}" are not needed.

> +
> +  return *tdesc;
> +}
  
Alan Hayward Oct. 31, 2017, 3:29 p.m. UTC | #2
> On 30 Oct 2017, at 12:21, Yao Qi <qiyaoltc@gmail.com> wrote:

> 

> Alan Hayward <Alan.Hayward@arm.com> writes:

> 

>> Had to add a new makefile rule to catch the arch/ file, otherwise the

>> make system gets confused between arch64/aarch64.c and features/aarch64.c

>> 

> 

> I don't understand the reason.  We don't compile features/aarch64.c in

> GDBserver.


Sorry, I meant “aarch64-generated.c”, not "features/aarch64.c"

In the existing code, configure.srv has the following:
ipa_obj="linux-aarch64-ipa.o aarch64-ipa.o”

This creates aarch64-ipa.o by compiling aarch64-generated.c using the rule
in Makefile.in:
%-ipa.o: %-generated.c

With the new code we no longer need aarch64-generated.c in IPA.

But, we do need to compile arch64/aarch64.c into IPA.

Using the existing logic, adding it to ipa_obj using the existing rules gives the same
ipa_obj="linux-aarch64-ipa.o aarch64-ipa.o”
Which is wrong because it will just try to compile aarch64-generated.c.

I could have just changed the order of the search in Makefile.in, but this might break
something else. Instead, I added the new rule for "-arch-ipa.c"


Ok with other comments.

> 

>> diff --git a/gdb/gdbserver/linux-aarch64-tdesc.h b/gdb/gdbserver/linux-aarch64-tdesc.h

>> new file mode 100644

>> index 0000000000000000000000000000000000000000..48314e934e0c1163fce400b368a3717a27239be8

>> --- /dev/null

>> +++ b/gdb/gdbserver/linux-aarch64-tdesc.h

>> @@ -0,0 +1,20 @@

>> +/* Low level support for aarch64, shared between gdbserver and IPA.

>> +

>> +   Copyright (C) 2016-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/>.  */

>> +

>> +const struct target_desc * aarch64_linux_read_description ();

>> diff --git a/gdb/gdbserver/linux-aarch64-tdesc.c b/gdb/gdbserver/linux-aarch64-tdesc.c

>> new file mode 100644

>> index 0000000000000000000000000000000000000000..4148ed70c4ca973652e860492cd7cc1a249991d8

>> --- /dev/null

>> +++ b/gdb/gdbserver/linux-aarch64-tdesc.c

>> @@ -0,0 +1,41 @@

>> +/* GNU/Linux/aarch64 specific target description, for the remote server

>> +   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 "server.h"

>> +#include "tdesc.h"

>> +#include "arch/aarch64.h"

>> +#include "linux-aarch32-low.h"

>> +

>> +/* The Aarch64 target descriptor.  */

> 

> s/descriptor/description/

> 

>> +static const struct target_desc *aarch64_tdesc = NULL;

>> +

> 

> Move it into aarch64_linux_read_description.

> 

>> +/* Create the aarch64 target description.  */

>> +

>> +const struct target_desc *

> 

> Remove struct.

> 

>> +aarch64_linux_read_description ()

>> +{

>> +  struct target_desc **tdesc = (struct target_desc **) &aarch64_tdesc;

>> +

>> +  if (*tdesc == NULL)

>> +    {

>> +      *tdesc = aarch64_create_target_description ();

>> +    }

> 

> "{" and "}" are not needed.

> 

>> +

>> +  return *tdesc;

>> +}

> 

> -- 

> Yao (齐尧)
  

Patch

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 8e73563b103f720ddd5e77607c3190a2959903f5..7c155c6cd3824432915c8841b8814362f059870c 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -544,6 +544,10 @@  arch/%.o: ../arch/%.c

 # Rules for objects that go in the in-process agent.

+%-arch-ipa.o: ../arch/%.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
+
 %-ipa.o: %-generated.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 515c6dc8b3e57574286149ebdca37da149218a35..08d8e9875a148f931253bed2340498b6d94b9946 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -27,9 +27,11 @@  srv_hostio_err_objs="hostio-errno.o"
 if $development; then
    srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
    srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
+   srv_aarch64_linux_regobj="aarch64.o"
 else
    srv_i386_linux_regobj=""
    srv_amd64_linux_regobj=""
+   srv_aarch64_linux_regobj=""
 fi

 ipa_ppc_linux_regobj="powerpc-32l-ipa.o powerpc-altivec32l-ipa.o powerpc-cell32l-ipa.o powerpc-vsx32l-ipa.o powerpc-isa205-32l-ipa.o powerpc-isa205-altivec32l-ipa.o powerpc-isa205-vsx32l-ipa.o powerpc-e500l-ipa.o powerpc-64l-ipa.o powerpc-altivec64l-ipa.o powerpc-cell64l-ipa.o powerpc-vsx64l-ipa.o powerpc-isa205-64l-ipa.o powerpc-isa205-altivec64l-ipa.o powerpc-isa205-vsx64l-ipa.o"
@@ -50,13 +52,15 @@  srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-wa

 case "${target}" in
   aarch64*-*-linux*)
-			srv_regobj="aarch64.o"
+			srv_regobj="$srv_aarch64_linux_regobj"
 			srv_regobj="${srv_regobj} arm-with-neon.o"
 			srv_tgtobj="linux-aarch64-low.o aarch64-linux-hw-point.o"
 			srv_tgtobj="$srv_tgtobj linux-aarch32-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/arm.o"
 			srv_tgtobj="$srv_tgtobj aarch64-linux.o"
 			srv_tgtobj="$srv_tgtobj arch/aarch64-insn.o"
+			srv_tgtobj="$srv_tgtobj arch/aarch64.o"
+			srv_tgtobj="$srv_tgtobj linux-aarch64-tdesc.o"
 			srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
 			srv_xmlfiles="aarch64.xml"
 			srv_xmlfiles="${srv_xmlfiles} aarch64-core.xml"
@@ -65,7 +69,9 @@  case "${target}" in
 			srv_xmlfiles="${srv_xmlfiles} arm/arm-with-neon.xml"
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
-			ipa_obj="linux-aarch64-ipa.o aarch64-ipa.o"
+			ipa_obj="linux-aarch64-ipa.o"
+			ipa_obj="${ipa_obj} linux-aarch64-tdesc-ipa.o"
+			ipa_obj="${ipa_obj} aarch64-arch-ipa.o"
 			;;
   arm*-*-linux*)	srv_regobj="reg-arm.o arm-with-iwmmxt.o"
 			srv_regobj="${srv_regobj} arm-with-vfpv2.o"
diff --git a/gdb/gdbserver/linux-aarch64-ipa.c b/gdb/gdbserver/linux-aarch64-ipa.c
index ad1059029bb669f4fd6240d8ce8aef126879b6ff..28dc9776a1c8d273e8bc749296a57bc62b8619d5 100644
--- a/gdb/gdbserver/linux-aarch64-ipa.c
+++ b/gdb/gdbserver/linux-aarch64-ipa.c
@@ -25,10 +25,10 @@ 
 #ifdef HAVE_GETAUXVAL
 #include <sys/auxv.h>
 #endif
+#include "linux-aarch64-tdesc.h"

 /* Defined in auto-generated file aarch64.c.  */
 void init_registers_aarch64 (void);
-extern const struct target_desc *tdesc_aarch64;

 /* Each register saved by the jump pad is in a 16 byte cell.  */
 #define FT_CR_SIZE 16
@@ -155,7 +155,7 @@  get_raw_reg (const unsigned char *raw_regs, int regnum)
 const struct target_desc *
 get_ipa_tdesc (int idx)
 {
-  return tdesc_aarch64;
+  return aarch64_linux_read_description ();
 }

 /* Allocate buffer for the jump pads.  The branch instruction has a reach
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index b00d5c502d2be67daf0e7f48025647f191efef3b..dcce287974cb6afd2f530ce1257dbbf25ed25ddc 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -39,10 +39,10 @@ 

 #include "gdb_proc_service.h"
 #include "arch/aarch64.h"
+#include "linux-aarch64-tdesc.h"

 /* Defined in auto-generated files.  */
 void init_registers_aarch64 (void);
-extern const struct target_desc *tdesc_aarch64;

 #ifdef HAVE_SYS_REG_H
 #include <sys/reg.h>
@@ -467,11 +467,10 @@  aarch64_linux_new_fork (struct process_info *parent,
   *child->priv->arch_private = *parent->priv->arch_private;
 }

-/* Return the right target description according to the ELF file of
-   current thread.  */
+/* Implementation of linux_target_ops method "arch_setup".  */

-static const struct target_desc *
-aarch64_linux_read_description (void)
+static void
+aarch64_arch_setup (void)
 {
   unsigned int machine;
   int is_elf64;
@@ -482,17 +481,9 @@  aarch64_linux_read_description (void)
   is_elf64 = linux_pid_exe_is_elf_64_file (tid, &machine);

   if (is_elf64)
-    return tdesc_aarch64;
+    current_process ()->tdesc = aarch64_linux_read_description ();
   else
-    return tdesc_arm_with_neon;
-}
-
-/* Implementation of linux_target_ops method "arch_setup".  */
-
-static void
-aarch64_arch_setup (void)
-{
-  current_process ()->tdesc = aarch64_linux_read_description ();
+    current_process ()->tdesc = tdesc_arm_with_neon;

   aarch64_linux_get_debug_reg_capacity (lwpid_of (current_thread));
 }
diff --git a/gdb/gdbserver/linux-aarch64-tdesc.h b/gdb/gdbserver/linux-aarch64-tdesc.h
new file mode 100644
index 0000000000000000000000000000000000000000..48314e934e0c1163fce400b368a3717a27239be8
--- /dev/null
+++ b/gdb/gdbserver/linux-aarch64-tdesc.h
@@ -0,0 +1,20 @@ 
+/* Low level support for aarch64, shared between gdbserver and IPA.
+
+   Copyright (C) 2016-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/>.  */
+
+const struct target_desc * aarch64_linux_read_description ();
diff --git a/gdb/gdbserver/linux-aarch64-tdesc.c b/gdb/gdbserver/linux-aarch64-tdesc.c
new file mode 100644
index 0000000000000000000000000000000000000000..4148ed70c4ca973652e860492cd7cc1a249991d8
--- /dev/null
+++ b/gdb/gdbserver/linux-aarch64-tdesc.c
@@ -0,0 +1,41 @@ 
+/* GNU/Linux/aarch64 specific target description, for the remote server
+   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 "server.h"
+#include "tdesc.h"
+#include "arch/aarch64.h"
+#include "linux-aarch32-low.h"
+
+/* The Aarch64 target descriptor.  */
+static const struct target_desc *aarch64_tdesc = NULL;
+
+/* Create the aarch64 target description.  */
+
+const struct target_desc *
+aarch64_linux_read_description ()
+{
+  struct target_desc **tdesc = (struct target_desc **) &aarch64_tdesc;
+
+  if (*tdesc == NULL)
+    {
+      *tdesc = aarch64_create_target_description ();
+    }
+
+  return *tdesc;
+}