public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/4] RISC-V/Linux/native: Determine FLEN dynamically
  2020-01-23 19:40 [PATCH 0/4] RISC-V/Linux `gdbserver' support and associated fixes Maciej W. Rozycki
@ 2020-01-23 19:40 ` Maciej W. Rozycki
  2020-01-24  1:18   ` Jim Wilson
  2020-01-23 19:41 ` [PATCH 2/4] gdbserver: Remove a stale TAGS recipe for config files Maciej W. Rozycki
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-23 19:40 UTC (permalink / raw)
  To: gdb-patches
  Cc: Jim Wilson, Andrew Burgess, Palmer Dabbelt, Tom Tromey, guoren,
	lifang_xia, yunhai_shang, jiangshuai_li

Fix RISC-V native Linux support to handle a 64-bit FPU (FLEN == 64) with 
both RV32 and RV64 systems, which is a part of the current Linux ABI for 
hard-float systems, rather than assuming that (FLEN == XLEN).

We can do better however and not rely on any particular value of FLEN 
and probe for it dynamically, by observing that the PTRACE_GETREGSET 
ptrace(2) call will only accept an exact regset size, and that will 
reflect FLEN.  Therefore iterate over the call with a geometrically 
increasing regset size until a match is marked by a successful ptrace(2) 
call completion or we run beyond the maximum size we can support.

Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG, 
however NFPREG is nowhere defined.

	gdb/
	* riscv-linux-nat.c [!NFPREG] (NFPREG): New macro.
	(riscv_linux_nat_target::read_description): Determine FLEN 
	dynamically.
---
Hi,

 Smoke-testing indicates no immediate problems with this change; full 
regression-testing is still running and will take approximately 3 days.

  Maciej
---
 gdb/riscv-linux-nat.c |   49 +++++++++++++++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 12 deletions(-)

gdb-riscv-linux-nat-flen.diff
Index: binutils-gdb/gdb/riscv-linux-nat.c
===================================================================
--- binutils-gdb.orig/gdb/riscv-linux-nat.c
+++ binutils-gdb/gdb/riscv-linux-nat.c
@@ -28,6 +28,11 @@
 
 #include <sys/ptrace.h>
 
+/* Work around glibc header breakage causing ELF_NFPREG not to be usable.  */
+#ifndef NFPREG
+# define NFPREG 33
+#endif
+
 /* RISC-V Linux native additions to the default linux support.  */
 
 class riscv_linux_nat_target final : public linux_nat_target
@@ -166,8 +171,8 @@ const struct target_desc *
 riscv_linux_nat_target::read_description ()
 {
   struct riscv_gdbarch_features features;
-  struct iovec iov;
   elf_fpregset_t regs;
+  int flen;
   int tid;
 
   /* Figuring out xlen is easy.  */
@@ -175,19 +180,39 @@ riscv_linux_nat_target::read_description
 
   tid = inferior_ptid.lwp ();
 
-  iov.iov_base = &regs;
-  iov.iov_len = sizeof (regs);
+  /* Start with no f-registers.  */
+  features.flen = 0;
 
-  /* Can we fetch the f-registers?  */
-  if (ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET,
-	      (PTRACE_TYPE_ARG3) &iov) == -1)
-    features.flen = 0;		/* No f-registers.  */
-  else
+  /* How much worth of f-registers can we fetch if any?  */
+  for (flen = sizeof (regs.__f.__f[0]); ; flen *= 2)
     {
-      /* TODO: We need a way to figure out the actual length of the
-	 f-registers.  We could have 64-bit x-registers, with 32-bit
-	 f-registers.  For now, just assumed xlen and flen match.  */
-      features.flen = features.xlen;
+      size_t regset_size;
+      struct iovec iov;
+
+      /* Regsets have a uniform slot size, so we count FSCR like an FGR.  */
+      regset_size = ELF_NFPREG * flen;
+      if (regset_size > sizeof (regs))
+	break;
+
+      iov.iov_base = &regs;
+      iov.iov_len = regset_size;
+      if (ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	{
+	  switch (errno)
+	    {
+	    case EINVAL:
+	      continue;
+	    case EIO:
+	      break;
+	    default:
+	      perror_with_name (_("Couldn't get registers"));
+	      break;
+	    }
+	}
+      else
+	features.flen = flen;
+      break;
     }
 
   return riscv_create_target_description (features);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 0/4] RISC-V/Linux `gdbserver' support and associated fixes
@ 2020-01-23 19:40 Maciej W. Rozycki
  2020-01-23 19:40 ` [PATCH 1/4] RISC-V/Linux/native: Determine FLEN dynamically Maciej W. Rozycki
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-23 19:40 UTC (permalink / raw)
  To: gdb-patches
  Cc: Jim Wilson, Andrew Burgess, Palmer Dabbelt, Tom Tromey, guoren,
	lifang_xia, yunhai_shang, jiangshuai_li

Hi,

 [Including Tom on the cc list as some of these changes will likely 
interfere with the effort to move `gdbserver' to the top level, although I 
think mechanically only.]

 This is a small series adding RISC-V `gdbserver' support with some 
preparatory changes as follows:

- a fix to native RISC-V/Linux FLEN determination,

- a fix to remove a `gdbserver' stale TAGS `make' recipe,

- a fix to make `make TAGS' actually work for `gdbserver',

- actual RISC-V/Linux `gdbserver'.

I think changes #1-#3 are ready to go in.  As to change #4 it might not be 
as it triggers a bug in RISC-V GDB with handling XML register descriptions 
which causes any description supplied to be rejected and therefore this 
part cannot be verified at this point to be correct without the GDB bug 
being fixed first.

 See individual changes for details.

  Maciej

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/4] gdbserver: Remove a stale TAGS recipe for config files
  2020-01-23 19:40 [PATCH 0/4] RISC-V/Linux `gdbserver' support and associated fixes Maciej W. Rozycki
  2020-01-23 19:40 ` [PATCH 1/4] RISC-V/Linux/native: Determine FLEN dynamically Maciej W. Rozycki
@ 2020-01-23 19:41 ` Maciej W. Rozycki
  2020-01-23 21:31   ` Tom Tromey
  2020-01-23 19:42 ` [PATCH 3/4] gdbserver: Make `make TAGS' actually work Maciej W. Rozycki
  2020-01-23 19:45 ` [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support Maciej W. Rozycki
  3 siblings, 1 reply; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-23 19:41 UTC (permalink / raw)
  To: gdb-patches
  Cc: Jim Wilson, Andrew Burgess, Palmer Dabbelt, Tom Tromey, guoren,
	lifang_xia, yunhai_shang, jiangshuai_li

Complement commit 7ea814144a31 ("Fully disentangle gdb and gdbserver"), 
<https://sourceware.org/ml/gdb-patches/2002-02/msg00692.html> (from 
2002!), and remove a recipe to include config files in `make TAGS', 
which are no longer used by `gdbserver' as from that commit.

	gdb/gdbserver/
	* Makefile.in (TAGS): Remove config files from the recipe.
---
 gdb/gdbserver/Makefile.in |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

gdb-gdbserver-tags-xx-files.diff
Index: binutils-gdb/gdb/gdbserver/Makefile.in
===================================================================
--- binutils-gdb.orig/gdb/gdbserver/Makefile.in
+++ binutils-gdb/gdb/gdbserver/Makefile.in
@@ -456,9 +456,7 @@ $(IPA_LIB): $(sort $(IPA_OBJS)) ${CDEPS}
 # specific routine gets the one for the correct machine.
 # The xyzzy stuff below deals with empty DEPFILES
 TAGS:	${TAGFILES}
-	etags `find ${srcdir}/../config -name $(DEPRECATED_TM_FILE) -print` \
-	  `find ${srcdir}/../config -name ${XM_FILE} -print` \
-	  `find ${srcdir}/../config -name ${NAT_FILE} -print` \
+	etags \
 	  `for i in yzzy ${DEPFILES}; do \
 	     if [ x$$i != xyzzy ]; then \
 	       echo ${srcdir}/$$i | sed -e 's/\.o$$/\.c/' ; \

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/4] gdbserver: Make `make TAGS' actually work
  2020-01-23 19:40 [PATCH 0/4] RISC-V/Linux `gdbserver' support and associated fixes Maciej W. Rozycki
  2020-01-23 19:40 ` [PATCH 1/4] RISC-V/Linux/native: Determine FLEN dynamically Maciej W. Rozycki
  2020-01-23 19:41 ` [PATCH 2/4] gdbserver: Remove a stale TAGS recipe for config files Maciej W. Rozycki
@ 2020-01-23 19:42 ` Maciej W. Rozycki
  2020-01-23 21:42   ` Tom Tromey
  2020-01-23 19:45 ` [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support Maciej W. Rozycki
  3 siblings, 1 reply; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-23 19:42 UTC (permalink / raw)
  To: gdb-patches
  Cc: Jim Wilson, Andrew Burgess, Palmer Dabbelt, Tom Tromey, guoren,
	lifang_xia, yunhai_shang, jiangshuai_li

Fix a:

make: *** No rule to make target '.../gdb/gdbserver/arch/arm.c', needed by 'TAGS'.  Stop.

error produced by `make TAGS' by making the list of sources processed 
match actual file locations and by moving host-specific object files 
listed in DEPFILES to nat/ or target/ subdirectories as appropriate so 
that the location of the corresponding source file can be mechanically 
determined.

	gdb/gdbserver/
	* Makefile.in (SFILES): Adjust paths to point to real files.
	(OBS): Move waitstatus.o to target/waitstatus.o.
	(TAGS): Transform paths appropriately.
	(%.o): Rename to...
	(nat/%.o): ... this pattern rule.
	(%.o): Rename to...
	(target/%.o): ... this pattern rule.
	* configure.srv: Adjust paths throughout to include nat/ prefix 
	with the revant files.
	* configure.ac: Add `nat' and `target' to CONFIG_SRC_SUBDIR.
	* configure: Regenerate.
---
Hi,

 NB SFILES is only maintained for the purpose of `make TAGS', so I could 
not persuade myself to put deliberate breakage there.  I find it amazing 
and amusing nobody has noticed that for many years and just been blindly 
putting gibberish there.

  Maciej
---
 gdb/gdbserver/Makefile.in   |   36 +++++++++++++++++---------------
 gdb/gdbserver/configure     |    2 -
 gdb/gdbserver/configure.ac  |    2 -
 gdb/gdbserver/configure.srv |   49 ++++++++++++++++++++++++++------------------
 4 files changed, 51 insertions(+), 38 deletions(-)

gdb-gdbserver-sfiles-paths.diff
Index: binutils-gdb/gdb/gdbserver/Makefile.in
===================================================================
--- binutils-gdb.orig/gdb/gdbserver/Makefile.in
+++ binutils-gdb/gdb/gdbserver/Makefile.in
@@ -198,11 +198,11 @@ SFILES = \
 	$(srcdir)/win32-low.c \
 	$(srcdir)/wincecompat.c \
 	$(srcdir)/x86-low.c \
-	$(srcdir)/arch/arm.c \
-	$(srcdir)/arch/arm-get-next-pcs.c \
-	$(srcdir)/arch/arm-linux.c \
-	$(srcdir)/arch/ppc-linux-common.c \
 	$(srcdir)/../alloc.c \
+	$(srcdir)/../arch/arm.c \
+	$(srcdir)/../arch/arm-get-next-pcs.c \
+	$(srcdir)/../arch/arm-linux.c \
+	$(srcdir)/../arch/ppc-linux-common.c \
 	$(srcdir)/../../gdbsupport/btrace-common.c \
 	$(srcdir)/../../gdbsupport/buffer.c \
 	$(srcdir)/../../gdbsupport/cleanups.c \
@@ -229,15 +229,15 @@ SFILES = \
 	$(srcdir)/../../gdbsupport/safe-strerror.c \
 	$(srcdir)/../../gdbsupport/tdesc.c \
 	$(srcdir)/../../gdbsupport/xml-utils.c \
-	$(srcdir)/nat/aarch64-sve-linux-ptrace.c \
-	$(srcdir)/nat/linux-btrace.c \
-	$(srcdir)/nat/linux-namespaces.c \
-	$(srcdir)/nat/linux-osdata.c \
-	$(srcdir)/nat/linux-personality.c \
-	$(srcdir)/nat/mips-linux-watch.c \
-	$(srcdir)/nat/ppc-linux.c \
-	$(srcdir)/nat/fork-inferior.c \
-	$(srcdir)/target/waitstatus.c
+	$(srcdir)/../nat/aarch64-sve-linux-ptrace.c \
+	$(srcdir)/../nat/linux-btrace.c \
+	$(srcdir)/../nat/linux-namespaces.c \
+	$(srcdir)/../nat/linux-osdata.c \
+	$(srcdir)/../nat/linux-personality.c \
+	$(srcdir)/../nat/mips-linux-watch.c \
+	$(srcdir)/../nat/ppc-linux.c \
+	$(srcdir)/../nat/fork-inferior.c \
+	$(srcdir)/../target/waitstatus.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -295,7 +295,7 @@ OBS = \
 	tracepoint.o \
 	utils.o \
 	version.o \
-	waitstatus.o \
+	target/waitstatus.o \
 	$(DEPFILES) \
 	$(LIBOBJS) \
 	$(XML_BUILTIN)
@@ -459,7 +459,9 @@ TAGS:	${TAGFILES}
 	etags \
 	  `for i in yzzy ${DEPFILES}; do \
 	     if [ x$$i != xyzzy ]; then \
-	       echo ${srcdir}/$$i | sed -e 's/\.o$$/\.c/' ; \
+	       echo ${srcdir}/$$i | sed -e 's/\.o$$/\.c/' \
+		 -e 's,/\(arch\|nat\|target\)/,/../\1/,' \
+		 -e 's,/\(gdbsupport\)/,/../../\1/,'; \
 	     fi; \
 	   done` \
 	  ${TAGFILES}
@@ -633,11 +635,11 @@ gdbsupport/%.o: ../../gdbsupport/%.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
 
-%.o: ../nat/%.c
+nat/%.o: ../nat/%.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
 
-%.o: ../target/%.c
+target/%.o: ../target/%.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
 
Index: binutils-gdb/gdb/gdbserver/configure
===================================================================
--- binutils-gdb.orig/gdb/gdbserver/configure
+++ binutils-gdb/gdb/gdbserver/configure
@@ -6025,7 +6025,7 @@ ac_config_commands="$ac_config_commands
 
 
 # Create sub-directories for objects and dependencies.
-CONFIG_SRC_SUBDIR="arch gdbsupport"
+CONFIG_SRC_SUBDIR="arch gdbsupport nat target"
 
 
 ac_config_commands="$ac_config_commands gdbdepdir"
Index: binutils-gdb/gdb/gdbserver/configure.ac
===================================================================
--- binutils-gdb.orig/gdb/gdbserver/configure.ac
+++ binutils-gdb/gdb/gdbserver/configure.ac
@@ -56,7 +56,7 @@ ACX_NONCANONICAL_HOST
 ZW_CREATE_DEPDIR
 
 # Create sub-directories for objects and dependencies.
-CONFIG_SRC_SUBDIR="arch gdbsupport"
+CONFIG_SRC_SUBDIR="arch gdbsupport nat target"
 AC_SUBST(CONFIG_SRC_SUBDIR)
 
 AC_CONFIG_COMMANDS([gdbdepdir],[
Index: binutils-gdb/gdb/gdbserver/configure.srv
===================================================================
--- binutils-gdb.orig/gdb/gdbserver/configure.srv
+++ binutils-gdb/gdb/gdbserver/configure.srv
@@ -28,21 +28,22 @@ ipa_ppc_linux_regobj="powerpc-32l-ipa.o
 
 # Linux object files.  This is so we don't have to repeat
 # these files over and over again.
-srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-waitpid.o linux-personality.o linux-namespaces.o fork-child.o fork-inferior.o"
+srv_linux_obj="linux-low.o nat/linux-osdata.o nat/linux-procfs.o nat/linux-ptrace.o nat/linux-waitpid.o nat/linux-personality.o nat/linux-namespaces.o fork-child.o nat/fork-inferior.o"
 
 # Input is taken from the "${target}" variable.
 
 case "${target}" in
-  aarch64*-*-linux*)	srv_tgtobj="linux-aarch64-low.o aarch64-linux-hw-point.o"
+  aarch64*-*-linux*)	srv_tgtobj="linux-aarch64-low.o"
+			srv_tgtobj="$srv_tgtobj nat/aarch64-linux-hw-point.o"
 			srv_tgtobj="$srv_tgtobj linux-aarch32-low.o"
 			srv_tgtobj="$srv_tgtobj linux-aarch32-tdesc.o"
 			srv_tgtobj="${srv_tgtobj} arch/aarch32.o"
 			srv_tgtobj="${srv_tgtobj} arch/arm.o"
-			srv_tgtobj="$srv_tgtobj aarch64-linux.o"
+			srv_tgtobj="$srv_tgtobj nat/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 aarch64-sve-linux-ptrace.o"
+			srv_tgtobj="$srv_tgtobj nat/aarch64-sve-linux-ptrace.o"
 			srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
@@ -86,14 +87,18 @@ case "${target}" in
 			srv_linux_thread_db=yes
 			;;
   i[34567]86-*-cygwin*)	srv_regobj=""
-			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="x86-low.o nat/x86-dregs.o win32-low.o"
+			srv_tgtobj="${srv_tgtobj} win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			;;
   i[34567]86-*-linux*)	srv_tgtobj="${srv_tgtobj} arch/i386.o"
-			srv_tgtobj="${srv_tgtobj} $srv_linux_obj linux-x86-low.o x86-low.o x86-dregs.o i387-fp.o"
+			srv_tgtobj="${srv_tgtobj} $srv_linux_obj"
+			srv_tgtobj="${srv_tgtobj} linux-x86-low.o x86-low.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} linux-x86-tdesc.o"
-			srv_tgtobj="${srv_tgtobj} linux-btrace.o x86-linux.o"
-			srv_tgtobj="${srv_tgtobj} x86-linux-dregs.o"
+			srv_tgtobj="${srv_tgtobj} nat/linux-btrace.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-linux.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-linux-dregs.o"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
@@ -102,13 +107,15 @@ case "${target}" in
 			ipa_obj="${ipa_obj} arch/i386-ipa.o"
 			;;
   i[34567]86-*-lynxos*)	srv_regobj=""
-			srv_tgtobj="lynx-low.o lynx-i386-low.o fork-child.o fork-inferior.o"
+			srv_tgtobj="lynx-low.o lynx-i386-low.o fork-child.o"
+			srv_tgtobj="${srv_tgtobj} nat/fork-inferior.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_lynxos=yes
 			;;
   i[34567]86-*-mingw32ce*)
 			srv_regobj=""
-			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="x86-low.o nat/x86-dregs.o win32-low.o"
+			srv_tgtobj="${srv_tgtobj} win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
 			# hostio_last_error implementation is in win32-low.c
@@ -117,7 +124,7 @@ case "${target}" in
 			srv_mingwce=yes
 			;;
   i[34567]86-*-mingw*)	srv_regobj=""
-			srv_tgtobj="x86-low.o x86-dregs.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="x86-low.o nat/x86-dregs.o win32-low.o"				srv_tgtobj="${srv_tgtobj} win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_mingw=yes
 			;;
@@ -159,7 +166,7 @@ case "${target}" in
 			srv_regobj="${srv_regobj} mips64-linux.o"
 			srv_regobj="${srv_regobj} mips64-dsp-linux.o"
 			srv_tgtobj="$srv_linux_obj linux-mips-low.o"
-			srv_tgtobj="${srv_tgtobj} mips-linux-watch.o"
+			srv_tgtobj="${srv_tgtobj} nat/mips-linux-watch.o"
 			srv_xmlfiles="mips-linux.xml"
 			srv_xmlfiles="${srv_xmlfiles} mips-dsp-linux.xml"
 			srv_xmlfiles="${srv_xmlfiles} mips-cpu.xml"
@@ -203,7 +210,8 @@ case "${target}" in
 			srv_regobj="${srv_regobj} powerpc-isa205-ppr-dscr-vsx64l.o"
 			srv_regobj="${srv_regobj} powerpc-isa207-vsx64l.o"
 			srv_regobj="${srv_regobj} powerpc-isa207-htm-vsx64l.o"
-			srv_tgtobj="$srv_linux_obj linux-ppc-low.o ppc-linux.o"
+			srv_tgtobj="$srv_linux_obj linux-ppc-low.o"
+			srv_tgtobj="$srv_linux_obj nat/ppc-linux.o"
 			srv_tgtobj="${srv_tgtobj} arch/ppc-linux-common.o"
 			srv_xmlfiles="rs6000/powerpc-32l.xml"
 			srv_xmlfiles="${srv_xmlfiles} rs6000/powerpc-altivec32l.xml"
@@ -350,12 +358,13 @@ case "${target}" in
 			srv_linux_thread_db=yes
 			;;
   x86_64-*-linux*)	srv_tgtobj="$srv_linux_obj linux-x86-low.o x86-low.o"
-			srv_tgtobj="${srv_tgtobj} x86-dregs.o i387-fp.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} arch/i386.o arch/amd64.o"
 			srv_tgtobj="${srv_tgtobj} linux-x86-tdesc.o"
-			srv_tgtobj="${srv_tgtobj} linux-btrace.o x86-linux.o"
-			srv_tgtobj="${srv_tgtobj} x86-linux-dregs.o"
-			srv_tgtobj="${srv_tgtobj} amd64-linux-siginfo.o"
+			srv_tgtobj="${srv_tgtobj} nat/linux-btrace.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-linux.o"
+			srv_tgtobj="${srv_tgtobj} nat/x86-linux-dregs.o"
+			srv_tgtobj="${srv_tgtobj} nat/amd64-linux-siginfo.o"
 			srv_linux_usrregs=yes # This is for i386 progs.
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
@@ -364,12 +373,14 @@ case "${target}" in
 			ipa_obj="${ipa_obj} arch/amd64-ipa.o"
 			;;
   x86_64-*-mingw*)	srv_regobj=""
-			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="x86-low.o nat/x86-dregs.o i387-fp.o"
+			srv_tgtobj="${srv_tgtobj} win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj=""
-			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="x86-low.o nat/x86-dregs.o i387-fp.o"
+			srv_tgtobj="${srv_tgtobj} win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
 			;;
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support
  2020-01-23 19:40 [PATCH 0/4] RISC-V/Linux `gdbserver' support and associated fixes Maciej W. Rozycki
                   ` (2 preceding siblings ...)
  2020-01-23 19:42 ` [PATCH 3/4] gdbserver: Make `make TAGS' actually work Maciej W. Rozycki
@ 2020-01-23 19:45 ` Maciej W. Rozycki
  2020-01-23 21:55   ` Tom Tromey
  2020-01-24  3:02   ` Jim Wilson
  3 siblings, 2 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-23 19:45 UTC (permalink / raw)
  To: gdb-patches
  Cc: Jim Wilson, Andrew Burgess, Palmer Dabbelt, Tom Tromey, guoren,
	lifang_xia, yunhai_shang, jiangshuai_li

Implement RISC-V/Linux support for both RV64 and RV32 systems, including 
XML target description handling based on features determined, GPR and 
FPR regset support including dynamic sizing of the latter, and software 
breakpoint handling.

Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
however NFPREG is nowhere defined.

	gdb/
	* arch/riscv.h (riscv_create_target_description): Remove `const' 
	qualifier from the return type.
	* arch/riscv.c (riscv_create_target_description): Likewise.

	gdb/gdbserver/
	* linux-riscv-low.c: New file.
	* Makefile.in (SFILES): Add linux-riscv-low.c and arch/riscv.c.
	* configure.srv <riscv*-*-linux*> (srv_tgtobj)
	(srv_linux_regsets, srv_linux_usrregs, srv_linux_thread_db): 
	Define.
---
Hi,

 Posted as an RFC as this hits a major issue with GDB rejecting the XML 
description supplied, e.g.:

warning: while parsing target description (at line 4): Target description specified unknown architecture "riscv:rv64id"
warning: Could not load XML target description; ignoring

with HiFive Unleashed; clearly GDB is unhappy with the `riscv:rv64id' 
architecture name produced by its own shared code, and not surprisingly, 
as it only has `riscv', `riscv:rv64' and `riscv:rv32' available to choose 
from as far as RISC-V targets are concerned, as easily verified with:

(gdb) set architecture

As the architecture names come from BFD it will clearly have to be there 
that the problem is fixed, with the ISA suffixes driving the choice of 
instructions handled by the disassembler and possibly other functions.

 Also I think large parts of `riscv_linux_nat_target::read_description' in 
RISC-V/Linux/native support could be factored out to a piece of code 
shared with `riscv_arch_setup'.  This might be less important a problem, 
but nevertheless I think it should be done sooner than later so as not to 
make the two pieces diverge, and a preparatory change inserted ahead of 
this one for this purpose might actually be the best approach.  I have 
planned to do that once the XML target description acceptance problem has 
been fixed.

 I have run initial lp64d multilib remote testing with HiFive Unleashed as 
the target.  Results were as follows:

		=== gdb Summary ===

# of expected passes		58054
# of unexpected failures	621
# of unexpected successes	4
# of expected failures		49
# of unknown successes		5
# of known failures		72
# of unresolved testcases	110
# of untested testcases		100
# of unsupported tests		232

but no further analysis has been made and also they come from Nov 6th 
only, so they may not be up to date with current GDB.  I thought it would 
be more important to get the XML issue fixed first.  Also native gdbserver 
testing should also be done as well as should native reference testing.

 Questions, comments?

  Maciej
---
 gdb/arch/riscv.c                |    2 
 gdb/arch/riscv.h                |    4 
 gdb/gdbserver/Makefile.in       |    2 
 gdb/gdbserver/configure.srv     |    6 
 gdb/gdbserver/linux-riscv-low.c |  300 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 311 insertions(+), 3 deletions(-)

gdb-riscv-gdbserver-linux.diff
Index: binutils-gdb/gdb/arch/riscv.c
===================================================================
--- binutils-gdb.orig/gdb/arch/riscv.c
+++ binutils-gdb/gdb/arch/riscv.c
@@ -43,7 +43,7 @@ static std::unordered_map<riscv_gdbarch_
 
 /* See arch/riscv.h.  */
 
-const target_desc *
+target_desc *
 riscv_create_target_description (struct riscv_gdbarch_features features)
 {
   /* Have we seen this feature set before?  If we have return the same
Index: binutils-gdb/gdb/arch/riscv.h
===================================================================
--- binutils-gdb.orig/gdb/arch/riscv.h
+++ binutils-gdb/gdb/arch/riscv.h
@@ -69,7 +69,7 @@ struct riscv_gdbarch_features
 /* Create and return a target description that is compatible with
    FEATURES.  */
 
-const target_desc *riscv_create_target_description
-	(struct riscv_gdbarch_features features);
+target_desc *riscv_create_target_description
+  (struct riscv_gdbarch_features features);
 
 #endif /* ARCH_RISCV_H */
Index: binutils-gdb/gdb/gdbserver/Makefile.in
===================================================================
--- binutils-gdb.orig/gdb/gdbserver/Makefile.in
+++ binutils-gdb/gdb/gdbserver/Makefile.in
@@ -177,6 +177,7 @@ SFILES = \
 	$(srcdir)/linux-mips-low.c \
 	$(srcdir)/linux-nios2-low.c \
 	$(srcdir)/linux-ppc-low.c \
+	$(srcdir)/linux-riscv-low.c \
 	$(srcdir)/linux-s390-low.c \
 	$(srcdir)/linux-sh-low.c \
 	$(srcdir)/linux-sparc-low.c \
@@ -203,6 +204,7 @@ SFILES = \
 	$(srcdir)/../arch/arm-get-next-pcs.c \
 	$(srcdir)/../arch/arm-linux.c \
 	$(srcdir)/../arch/ppc-linux-common.c \
+	$(srcdir)/../arch/riscv.c \
 	$(srcdir)/../../gdbsupport/btrace-common.c \
 	$(srcdir)/../../gdbsupport/buffer.c \
 	$(srcdir)/../../gdbsupport/cleanups.c \
Index: binutils-gdb/gdb/gdbserver/configure.srv
===================================================================
--- binutils-gdb.orig/gdb/gdbserver/configure.srv
+++ binutils-gdb/gdb/gdbserver/configure.srv
@@ -267,6 +267,12 @@ case "${target}" in
 			srv_xmlfiles="${srv_xmlfiles} rs6000/power-fpu.xml"
 			srv_lynxos=yes
 			;;
+  riscv*-*-linux*)	srv_tgtobj="arch/riscv.o linux-riscv-low.o"
+			srv_tgtobj="${srv_tgtobj} ${srv_linux_obj}"
+			srv_linux_regsets=yes
+			srv_linux_usrregs=yes
+			srv_linux_thread_db=yes
+			;;
   s390*-*-linux*)	srv_regobj="s390-linux32.o"
 			srv_regobj="${srv_regobj} s390-linux32v1.o"
 			srv_regobj="${srv_regobj} s390-linux32v2.o"
Index: binutils-gdb/gdb/gdbserver/linux-riscv-low.c
===================================================================
--- /dev/null
+++ binutils-gdb/gdb/gdbserver/linux-riscv-low.c
@@ -0,0 +1,300 @@
+/* GNU/Linux/RISC-V specific low level interface, for the remote server
+   for GDB.
+   Copyright (C) 2020 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 "gdb_proc_service.h"
+#include "linux-low.h"
+#include "tdesc.h"
+#include "arch/riscv.h"
+#include "elf/common.h"
+#include "nat/gdb_ptrace.h"
+#include "opcode/riscv.h"
+
+#include <sys/uio.h>
+
+/* Work around glibc header breakage causing ELF_NFPREG not to be usable.  */
+#ifndef NFPREG
+# define NFPREG 33
+#endif
+
+/* Implementation of linux_target_ops method "arch_setup".  */
+
+static void
+riscv_arch_setup (void)
+{
+  static const char *expedite_regs[] = { "sp", "pc", NULL };
+  struct riscv_gdbarch_features features;
+  elf_fpregset_t regs;
+  target_desc *tdesc;
+  int flen;
+  int tid;
+
+  /* Figuring out xlen is easy.  */
+  features.xlen = sizeof (elf_greg_t);
+
+  tid = lwpid_of (current_thread);
+
+  /* Start with no f-registers.  */
+  features.flen = 0;
+
+  /* How much worth of f-registers can we fetch if any?  */
+  for (flen = sizeof (regs.__f.__f[0]); ; flen *= 2)
+    {
+      size_t regset_size;
+      struct iovec iov;
+
+      /* Regsets have a uniform slot size, so we count FSCR like an FGR.  */
+      regset_size = ELF_NFPREG * flen;
+      if (regset_size > sizeof (regs))
+	break;
+
+      iov.iov_base = &regs;
+      iov.iov_len = regset_size;
+      if (ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET,
+		  (PTRACE_TYPE_ARG3) &iov) == -1)
+	{
+	  switch (errno)
+	    {
+	    case EINVAL:
+	      continue;
+	    case EIO:
+	      break;
+	    default:
+	      perror_with_name (_("Couldn't get registers"));
+	      break;
+	    }
+	}
+      else
+	features.flen = flen;
+      break;
+    }
+
+  tdesc = riscv_create_target_description (features);
+  if (!tdesc->expedite_regs)
+    init_target_desc (tdesc, expedite_regs);
+  current_process ()->tdesc = tdesc;
+}
+
+static void
+riscv_fill_gregset (struct regcache *regcache, void *buf)
+{
+  const struct target_desc *tdesc = regcache->tdesc;
+  elf_gregset_t *regset = (elf_gregset_t *) buf;
+  int regno = find_regno (tdesc, "zero");
+  int i;
+
+  collect_register_by_name (regcache, "pc", *regset);
+  for (i = 1; i < ARRAY_SIZE (*regset); i++)
+    collect_register (regcache, regno + i, *regset + i);
+}
+
+static void
+riscv_store_gregset (struct regcache *regcache, const void *buf)
+{
+  const elf_gregset_t *regset = (const elf_gregset_t *) buf;
+  const struct target_desc *tdesc = regcache->tdesc;
+  int regno = find_regno (tdesc, "zero");
+  int i;
+
+  supply_register_by_name (regcache, "pc", *regset);
+  supply_register_zeroed (regcache, regno);
+  for (i = 1; i < ARRAY_SIZE (*regset); i++)
+    supply_register (regcache, regno + i, *regset + i);
+}
+
+static void
+riscv_fill_fpregset (struct regcache *regcache, void *buf)
+{
+  const struct target_desc *tdesc = regcache->tdesc;
+  elf_fpregset_t *regset = (elf_fpregset_t *) buf;
+  int regno = find_regno (tdesc, "ft0");
+  int i;
+
+  for (i = 0; i < ARRAY_SIZE (regset->__d.__f); i++)
+    collect_register (regcache, regno + i, regset->__d.__f + i);
+  collect_register_by_name (regcache, "fcsr", &regset->__d.__fcsr);
+}
+
+static void
+riscv_store_fpregset (struct regcache *regcache, const void *buf)
+{
+  const elf_fpregset_t *regset = (const elf_fpregset_t *) buf;
+  const struct target_desc *tdesc = regcache->tdesc;
+  int regno = find_regno (tdesc, "ft0");
+  int i;
+
+  for (i = 0; i < ARRAY_SIZE (regset->__d.__f); i++)
+    supply_register (regcache, regno + i, regset->__d.__f + i);
+  supply_register_by_name (regcache, "fcsr", &regset->__d.__fcsr);
+}
+
+static struct regset_info riscv_regsets[] = {
+  { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_PRSTATUS,
+    sizeof (elf_gregset_t), GENERAL_REGS,
+    riscv_fill_gregset, riscv_store_gregset },
+  { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_FPREGSET,
+    sizeof (elf_fpregset_t), FP_REGS,
+    riscv_fill_fpregset, riscv_store_fpregset },
+  NULL_REGSET
+};
+
+static struct regsets_info riscv_regsets_info =
+  {
+    riscv_regsets, /* regsets */
+    0, /* num_regsets */
+    NULL, /* disabled_regsets */
+  };
+
+static struct regs_info riscv_regs =
+  {
+    NULL, /* regset_bitmap */
+    NULL, /* usrregs */
+    &riscv_regsets_info,
+  };
+
+/* Implementation of linux_target_ops method "regs_info".  */
+
+static const struct regs_info *
+riscv_regs_info (void)
+{
+  return &riscv_regs;
+}
+
+/* Implementation of linux_target_ops method "fetch_register".  */
+
+static int
+riscv_fetch_register (struct regcache *regcache, int regno)
+{
+  const struct target_desc *tdesc = regcache->tdesc;
+
+  if (regno != find_regno (tdesc, "zero"))
+    return 0;
+  supply_register_zeroed (regcache, regno);
+  return 1;
+}
+
+/* Implementation of linux_target_ops method "get_pc".  */
+
+static CORE_ADDR
+riscv_get_pc (struct regcache *regcache)
+{
+  elf_gregset_t regset;
+
+  if (sizeof (regset[0]) == 8)
+    return linux_get_pc_64bit (regcache);
+  else
+    return linux_get_pc_32bit (regcache);
+}
+
+/* Implementation of linux_target_ops method "set_pc".  */
+
+static void
+riscv_set_pc (struct regcache *regcache, CORE_ADDR newpc)
+{
+  elf_gregset_t regset;
+
+  if (sizeof (regset[0]) == 8)
+    linux_set_pc_64bit (regcache, newpc);
+  else
+    linux_set_pc_32bit (regcache, newpc);
+}
+
+/* Correct in either endianness.  */
+static const uint16_t riscv_ibreakpoint[] = { 0x0073, 0x0010 };
+static const uint16_t riscv_cbreakpoint = 0x9002;
+
+/* Implementation of linux_target_ops method "breakpoint_kind_from_pc".  */
+
+static int
+riscv_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
+{
+  union
+    {
+      gdb_byte bytes[2];
+      uint16_t insn;
+    }
+  buf;
+
+  if (target_read_memory (*pcptr, buf.bytes, sizeof (buf.insn)) == 0
+      && riscv_insn_length (buf.insn == sizeof (riscv_ibreakpoint)))
+    return sizeof (riscv_ibreakpoint);
+  else
+    return sizeof (riscv_cbreakpoint);
+}
+
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind".  */
+
+static const gdb_byte *
+riscv_sw_breakpoint_from_kind (int kind, int *size)
+{
+  *size = kind;
+  switch (kind)
+    {
+      case sizeof (riscv_ibreakpoint):
+	return (const gdb_byte *) &riscv_ibreakpoint;
+      default:
+	return (const gdb_byte *) &riscv_cbreakpoint;
+    }
+}
+
+/* Implementation of linux_target_ops method "breakpoint_at".  */
+
+static int
+riscv_breakpoint_at (CORE_ADDR pc)
+{
+  union
+    {
+      gdb_byte bytes[2];
+      uint16_t insn;
+    }
+  buf;
+
+  if (target_read_memory (pc, buf.bytes, sizeof (buf.insn)) == 0
+      && (buf.insn == riscv_cbreakpoint
+	  || (buf.insn == riscv_ibreakpoint[0]
+	      && target_read_memory (pc + sizeof (buf.insn), buf.bytes,
+				     sizeof (buf.insn)) == 0
+	      && buf.insn == riscv_ibreakpoint[1])))
+    return 1;
+  else
+    return 0;
+}
+
+struct linux_target_ops the_low_target =
+{
+  riscv_arch_setup,
+  riscv_regs_info,
+  NULL, /* cannot_fetch_register */
+  NULL, /* cannot_store_register */
+  riscv_fetch_register,
+  riscv_get_pc,
+  riscv_set_pc,
+  riscv_breakpoint_kind_from_pc,
+  riscv_sw_breakpoint_from_kind,
+  NULL, /* get_next_pcs */
+  0,    /* decr_pc_after_break */
+  riscv_breakpoint_at,
+};
+
+void
+initialize_low_arch (void)
+{
+  initialize_regsets_info (&riscv_regsets_info);
+}

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] gdbserver: Remove a stale TAGS recipe for config files
  2020-01-23 19:41 ` [PATCH 2/4] gdbserver: Remove a stale TAGS recipe for config files Maciej W. Rozycki
@ 2020-01-23 21:31   ` Tom Tromey
  2020-01-24 13:01     ` Maciej W. Rozycki
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2020-01-23 21:31 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: gdb-patches, Jim Wilson, Andrew Burgess, Palmer Dabbelt,
	Tom Tromey, guoren, lifang_xia, yunhai_shang, jiangshuai_li

>>>>> "Maciej" == Maciej W Rozycki <macro@wdc.com> writes:

Maciej> Complement commit 7ea814144a31 ("Fully disentangle gdb and gdbserver"), 
Maciej> <https://sourceware.org/ml/gdb-patches/2002-02/msg00692.html> (from 
Maciej> 2002!), and remove a recipe to include config files in `make TAGS', 
Maciej> which are no longer used by `gdbserver' as from that commit.

Maciej> 	gdb/gdbserver/
Maciej> 	* Makefile.in (TAGS): Remove config files from the recipe.

Thank you.  This is ok.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] gdbserver: Make `make TAGS' actually work
  2020-01-23 19:42 ` [PATCH 3/4] gdbserver: Make `make TAGS' actually work Maciej W. Rozycki
@ 2020-01-23 21:42   ` Tom Tromey
  2020-01-24 14:15     ` Maciej W. Rozycki
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2020-01-23 21:42 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: gdb-patches, Jim Wilson, Andrew Burgess, Palmer Dabbelt,
	Tom Tromey, guoren, lifang_xia, yunhai_shang, jiangshuai_li

>>>>> "Maciej" == Maciej W Rozycki <macro@wdc.com> writes:

Maciej> error produced by `make TAGS' by making the list of sources processed 
Maciej> match actual file locations and by moving host-specific object files 
Maciej> listed in DEPFILES to nat/ or target/ subdirectories as appropriate so 
Maciej> that the location of the corresponding source file can be mechanically 
Maciej> determined.

Maciej> 	gdb/gdbserver/
Maciej> 	* Makefile.in (SFILES): Adjust paths to point to real files.
Maciej> 	(OBS): Move waitstatus.o to target/waitstatus.o.
Maciej> 	(TAGS): Transform paths appropriately.
Maciej> 	(%.o): Rename to...
Maciej> 	(nat/%.o): ... this pattern rule.
Maciej> 	(%.o): Rename to...
Maciej> 	(target/%.o): ... this pattern rule.
Maciej> 	* configure.srv: Adjust paths throughout to include nat/ prefix 
Maciej> 	with the revant files.
Maciej> 	* configure.ac: Add `nat' and `target' to CONFIG_SRC_SUBDIR.
Maciej> 	* configure: Regenerate.

Thanks for the patch.

Maciej>  NB SFILES is only maintained for the purpose of `make TAGS', so I could 
Maciej> not persuade myself to put deliberate breakage there.  I find it amazing 
Maciej> and amusing nobody has noticed that for many years and just been blindly 
Maciej> putting gibberish there.

LOL.  Well, anything not tested will surely break :)

This looks ok to me.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support
  2020-01-23 19:45 ` [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support Maciej W. Rozycki
@ 2020-01-23 21:55   ` Tom Tromey
  2020-01-24 19:14     ` Maciej W. Rozycki
  2020-01-24  3:02   ` Jim Wilson
  1 sibling, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2020-01-23 21:55 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: gdb-patches, Jim Wilson, Andrew Burgess, Palmer Dabbelt,
	Tom Tromey, guoren, lifang_xia, yunhai_shang, jiangshuai_li

>>>>> "Maciej" == Maciej W Rozycki <macro@wdc.com> writes:

Maciej> Implement RISC-V/Linux support for both RV64 and RV32 systems, including 
Maciej> XML target description handling based on features determined, GPR and 
Maciej> FPR regset support including dynamic sizing of the latter, and software 
Maciej> breakpoint handling.

I saw a couple of small nits here.

Maciej> Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
Maciej> however NFPREG is nowhere defined.

In case you haven't already, please report this to glibc.

Maciej> As the architecture names come from BFD it will clearly have to be there 
Maciej> that the problem is fixed, with the ISA suffixes driving the choice of 
Maciej> instructions handled by the disassembler and possibly other functions.

Seems reasonable enough.  I don't really know much about this area
though.

Maciej>  Also I think large parts of `riscv_linux_nat_target::read_description' in 
Maciej> RISC-V/Linux/native support could be factored out to a piece of code 
Maciej> shared with `riscv_arch_setup'.  This might be less important a problem, 
Maciej> but nevertheless I think it should be done sooner than later so as not to 
Maciej> make the two pieces diverge

Agreed.

Maciej> +/* Implementation of linux_target_ops method "regs_info".  */
Maciej> +
Maciej> +static const struct regs_info *
Maciej> +riscv_regs_info (void)

We stopped using "(void)" in new code, in favor of just "()".
This showed up in a few spots.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] RISC-V/Linux/native: Determine FLEN dynamically
  2020-01-23 19:40 ` [PATCH 1/4] RISC-V/Linux/native: Determine FLEN dynamically Maciej W. Rozycki
@ 2020-01-24  1:18   ` Jim Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Jim Wilson @ 2020-01-24  1:18 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: gdb-patches, Andrew Burgess, Palmer Dabbelt, Tom Tromey, Guo Ren,
	Lifang Xia, yunhai_shang, jiangshuai_li

On Thu, Jan 23, 2020 at 11:40 AM Maciej W. Rozycki <macro@wdc.com> wrote:
>         gdb/
>         * riscv-linux-nat.c [!NFPREG] (NFPREG): New macro.
>         (riscv_linux_nat_target::read_description): Determine FLEN
>         dynamically.

This looks good to me, though I'm not a formal riscv linux gdb maintainer.

Jim

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support
  2020-01-23 19:45 ` [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support Maciej W. Rozycki
  2020-01-23 21:55   ` Tom Tromey
@ 2020-01-24  3:02   ` Jim Wilson
  2020-01-24 20:06     ` Maciej W. Rozycki
  1 sibling, 1 reply; 14+ messages in thread
From: Jim Wilson @ 2020-01-24  3:02 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: gdb-patches, Andrew Burgess, Palmer Dabbelt, Tom Tromey, Guo Ren,
	Lifang Xia, yunhai_shang, jiangshuai_li

On Thu, Jan 23, 2020 at 11:42 AM Maciej W. Rozycki <macro@wdc.com> wrote:
> Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
> however NFPREG is nowhere defined.

I remember that this was discussed on the glibc lists, but I don't
know if a formal bug report was filed.  We unfortunately don't have an
active glibc maintainer, and I'm helping to maintain so much stuff
that I'm not fixing a glibc bug unless it is blocking someone's
progress, and this does not appear to be a blocker for this patch.

>         gdb/
>         * arch/riscv.h (riscv_create_target_description): Remove `const'
>         qualifier from the return type.
>         * arch/riscv.c (riscv_create_target_description): Likewise.
>
>         gdb/gdbserver/
>         * linux-riscv-low.c: New file.
>         * Makefile.in (SFILES): Add linux-riscv-low.c and arch/riscv.c.
>         * configure.srv <riscv*-*-linux*> (srv_tgtobj)
>         (srv_linux_regsets, srv_linux_usrregs, srv_linux_thread_db):
>         Define.

I didn't really review the patch since I've never done any gdbserver
work.  But I did build and test it and it worked for me.  My gdb
testsuite results look roughly the same with and without the patch
set.

I did have to add a missing build_gdbserver=yes line to the
riscv*-*-linux* block in configure.tgt.  I will submit that as a patch
if you don't.

>  Posted as an RFC as this hits a major issue with GDB rejecting the XML
> description supplied, e.g.:
>
> warning: while parsing target description (at line 4): Target description specified unknown architecture "riscv:rv64id"
> warning: Could not load XML target description; ignoring

The problem isn't only in bfd.  gdb/arch/riscv.c
riscv_create_target_description() creates the same kind of strings.
Anyways, this seems to be a simple issue with bfd/cpu-riscv.c using
bfd_default_scan.  It needs to be fixed to use a riscv specific scan
function that ignores trailing characters in the string when they
don't affect the bfd arch match.  I was able to reproduce the problem,
and wrote a bfd patch that is working for me.  This should not be a
blocking factor for the patch, as I can get the bfd patch in tomorrow
or so, unless maybe you want to try it first.

Jim

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] gdbserver: Remove a stale TAGS recipe for config files
  2020-01-23 21:31   ` Tom Tromey
@ 2020-01-24 13:01     ` Maciej W. Rozycki
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-24 13:01 UTC (permalink / raw)
  To: Tom Tromey
  Cc: gdb-patches, Jim Wilson, Andrew Burgess, Palmer Dabbelt, guoren,
	lifang_xia, yunhai_shang, jiangshuai_li

On Thu, 23 Jan 2020, Tom Tromey wrote:

> Maciej> Complement commit 7ea814144a31 ("Fully disentangle gdb and gdbserver"), 
> Maciej> <https://sourceware.org/ml/gdb-patches/2002-02/msg00692.html> (from 
> Maciej> 2002!), and remove a recipe to include config files in `make TAGS', 
> Maciej> which are no longer used by `gdbserver' as from that commit.
> 
> Maciej> 	gdb/gdbserver/
> Maciej> 	* Makefile.in (TAGS): Remove config files from the recipe.
> 
> Thank you.  This is ok.

 Now committed, thanks for your review.

  Maciej

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] gdbserver: Make `make TAGS' actually work
  2020-01-23 21:42   ` Tom Tromey
@ 2020-01-24 14:15     ` Maciej W. Rozycki
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-24 14:15 UTC (permalink / raw)
  To: Tom Tromey
  Cc: gdb-patches, Jim Wilson, Andrew Burgess, Palmer Dabbelt, guoren,
	lifang_xia, yunhai_shang, jiangshuai_li

On Thu, 23 Jan 2020, Tom Tromey wrote:

> Maciej>  NB SFILES is only maintained for the purpose of `make TAGS', so I could 
> Maciej> not persuade myself to put deliberate breakage there.  I find it amazing 
> Maciej> and amusing nobody has noticed that for many years and just been blindly 
> Maciej> putting gibberish there.
> 
> LOL.  Well, anything not tested will surely break :)
> 
> This looks ok to me.

 Applied now, thanks for your review.

  Maciej

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support
  2020-01-23 21:55   ` Tom Tromey
@ 2020-01-24 19:14     ` Maciej W. Rozycki
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-24 19:14 UTC (permalink / raw)
  To: Tom Tromey
  Cc: gdb-patches, Jim Wilson, Andrew Burgess, Palmer Dabbelt, guoren,
	lifang_xia, yunhai_shang, jiangshuai_li

On Thu, 23 Jan 2020, Tom Tromey wrote:

> Maciej> Implement RISC-V/Linux support for both RV64 and RV32 systems, including 
> Maciej> XML target description handling based on features determined, GPR and 
> Maciej> FPR regset support including dynamic sizing of the latter, and software 
> Maciej> breakpoint handling.
> 
> I saw a couple of small nits here.

 Thanks for looking through my proposal!

> Maciej> Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
> Maciej> however NFPREG is nowhere defined.
> 
> In case you haven't already, please report this to glibc.

 Yep, see: <https://sourceware.org/ml/libc-alpha/2019-10/msg00233.html>, 
and the discussion downthread.

> Maciej> +/* Implementation of linux_target_ops method "regs_info".  */
> Maciej> +
> Maciej> +static const struct regs_info *
> Maciej> +riscv_regs_info (void)
> 
> We stopped using "(void)" in new code, in favor of just "()".
> This showed up in a few spots.

 Right, I'm pretty much in the C world still; will fix that in the next 
iteration.

  Maciej

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support
  2020-01-24  3:02   ` Jim Wilson
@ 2020-01-24 20:06     ` Maciej W. Rozycki
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2020-01-24 20:06 UTC (permalink / raw)
  To: Jim Wilson
  Cc: gdb-patches, Andrew Burgess, Palmer Dabbelt, Tom Tromey, Guo Ren,
	Lifang Xia, yunhai_shang, jiangshuai_li

On Thu, 23 Jan 2020, Jim Wilson wrote:

> > Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
> > however NFPREG is nowhere defined.
> 
> I remember that this was discussed on the glibc lists, but I don't
> know if a formal bug report was filed.  We unfortunately don't have an
> active glibc maintainer, and I'm helping to maintain so much stuff
> that I'm not fixing a glibc bug unless it is blocking someone's
> progress, and this does not appear to be a blocker for this patch.

 I actually volunteered to become a glibc RISC-V machine maintainer, but 
then I have to admit I haven't been pursuing it particularly actively. :(

> >         gdb/
> >         * arch/riscv.h (riscv_create_target_description): Remove `const'
> >         qualifier from the return type.
> >         * arch/riscv.c (riscv_create_target_description): Likewise.
> >
> >         gdb/gdbserver/
> >         * linux-riscv-low.c: New file.
> >         * Makefile.in (SFILES): Add linux-riscv-low.c and arch/riscv.c.
> >         * configure.srv <riscv*-*-linux*> (srv_tgtobj)
> >         (srv_linux_regsets, srv_linux_usrregs, srv_linux_thread_db):
> >         Define.
> 
> I didn't really review the patch since I've never done any gdbserver
> work.  But I did build and test it and it worked for me.  My gdb
> testsuite results look roughly the same with and without the patch
> set.
> 
> I did have to add a missing build_gdbserver=yes line to the
> riscv*-*-linux* block in configure.tgt.  I will submit that as a patch
> if you don't.

 Indeed, that's useful for a native configuration to build the gdbserver/ 
subdirectory automagically and I missed that, having not looked into it 
(for cross-compilation you need to build `gdbserver' explictly of course).  
Now fixed and included with an updated version, thanks for catching it!

> >  Posted as an RFC as this hits a major issue with GDB rejecting the XML
> > description supplied, e.g.:
> >
> > warning: while parsing target description (at line 4): Target description specified unknown architecture "riscv:rv64id"
> > warning: Could not load XML target description; ignoring
> 
> The problem isn't only in bfd.  gdb/arch/riscv.c
> riscv_create_target_description() creates the same kind of strings.

 Well, that's exactly where `gdbserver' gets its string from.

> Anyways, this seems to be a simple issue with bfd/cpu-riscv.c using
> bfd_default_scan.  It needs to be fixed to use a riscv specific scan
> function that ignores trailing characters in the string when they
> don't affect the bfd arch match.  I was able to reproduce the problem,
> and wrote a bfd patch that is working for me.  This should not be a
> blocking factor for the patch, as I can get the bfd patch in tomorrow
> or so, unless maybe you want to try it first.

 Native regression-testing of 1/4 has now completed successfully, so I'll 
run remote testing now with your patch preapplied and adjustments made to 
4/4.  We'll see how it goes.

  Maciej

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-01-24 19:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-23 19:40 [PATCH 0/4] RISC-V/Linux `gdbserver' support and associated fixes Maciej W. Rozycki
2020-01-23 19:40 ` [PATCH 1/4] RISC-V/Linux/native: Determine FLEN dynamically Maciej W. Rozycki
2020-01-24  1:18   ` Jim Wilson
2020-01-23 19:41 ` [PATCH 2/4] gdbserver: Remove a stale TAGS recipe for config files Maciej W. Rozycki
2020-01-23 21:31   ` Tom Tromey
2020-01-24 13:01     ` Maciej W. Rozycki
2020-01-23 19:42 ` [PATCH 3/4] gdbserver: Make `make TAGS' actually work Maciej W. Rozycki
2020-01-23 21:42   ` Tom Tromey
2020-01-24 14:15     ` Maciej W. Rozycki
2020-01-23 19:45 ` [RFC][PATCH 4/4] gdbserver: Add RISC-V/Linux support Maciej W. Rozycki
2020-01-23 21:55   ` Tom Tromey
2020-01-24 19:14     ` Maciej W. Rozycki
2020-01-24  3:02   ` Jim Wilson
2020-01-24 20:06     ` Maciej W. Rozycki

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).