public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PUSHED] sim: fixes for libopcodes styled disassembler
@ 2022-04-04 21:46 Andrew Burgess
  2022-09-03  6:57 ` Jan-Benedict Glaw
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Burgess @ 2022-04-04 21:46 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

I've gone ahead and pushed the patch below without review, despite it
being non-trivial.  I stupidly managed to break several sim targets,
and I wanted to undo the breakage asap.

Despite being quite large, I hope nothing in here is too
controversial.  However, if there is any feedback, do please leave it
and I'll be happy to fix any issues in a follow up patch.

Sorry for the breakage, and thank you for your understanding.

Andrew


---

In commit:

  commit 60a3da00bd5407f07d64dff82a4dae98230dfaac
  Date:   Sat Jan 22 11:38:18 2022 +0000

      objdump/opcodes: add syntax highlighting to disassembler output

I broke several sim/ targets by forgetting to update their uses of the
libopcodes disassembler to take account of the new styled printing.

These should all be fixed by this commit.

I've not tried to add actual styled output to the simulator traces,
instead, the styled print routines just ignore the style and print the
output unstyled.
---
 sim/arm/wrapper.c       | 15 ++++++++++++++-
 sim/common/cgen-trace.c | 19 ++++++++++++++++++-
 sim/common/cgen-trace.h |  1 +
 sim/common/sim-trace.c  | 14 +++++++++++++-
 sim/cris/sim-if.c       |  3 ++-
 sim/erc32/interf.c      |  3 ++-
 sim/erc32/sis.c         | 18 +++++++++++++++++-
 sim/erc32/sis.h         |  3 +++
 sim/m32c/trace.c        | 14 +++++++++++++-
 sim/rl78/trace.c        | 14 +++++++++++++-
 sim/rx/trace.c          | 14 +++++++++++++-
 11 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/sim/arm/wrapper.c b/sim/arm/wrapper.c
index ab56bc4e7b8..c97bae89dad 100644
--- a/sim/arm/wrapper.c
+++ b/sim/arm/wrapper.c
@@ -76,6 +76,19 @@ op_printf (char *buf, const char *fmt, ...)
   return ret;
 }
 
+static int ATTRIBUTE_PRINTF (3, 4)
+op_styled_printf (char *buf, enum disassembler_style style,
+		  const char *fmt, ...)
+{
+  int ret;
+  va_list ap;
+
+  va_start (ap, fmt);
+  ret = vsprintf (opbuf + strlen (opbuf), fmt, ap);
+  va_end (ap);
+  return ret;
+}
+
 static int
 sim_dis_read (bfd_vma                     memaddr ATTRIBUTE_UNUSED,
 	      bfd_byte *                  ptr,
@@ -321,7 +334,7 @@ sim_create_inferior (SIM_DESC sd ATTRIBUTE_UNUSED,
     }
 
   memset (& info, 0, sizeof (info));
-  INIT_DISASSEMBLE_INFO (info, stdout, op_printf);
+  INIT_DISASSEMBLE_INFO (info, stdout, op_printf, op_styled_printf);
   info.read_memory_func = sim_dis_read;
   info.arch = bfd_get_arch (abfd);
   info.mach = bfd_get_mach (abfd);
diff --git a/sim/common/cgen-trace.c b/sim/common/cgen-trace.c
index 149ea8a127c..12f6126d43f 100644
--- a/sim/common/cgen-trace.c
+++ b/sim/common/cgen-trace.c
@@ -320,6 +320,22 @@ sim_disasm_sprintf (SFILE *f, const char *format, ...)
   return n;
 }
 
+/* sprintf to a "stream" with styling.  */
+
+int
+sim_disasm_styled_sprintf (SFILE *f, enum disassembler_style style,
+			   const char *format, ...)
+{
+  int n;
+  va_list args;
+
+  va_start (args, format);
+  vsprintf (f->current, format, args);
+  f->current += n = strlen (f->current);
+  va_end (args);
+  return n;
+}
+
 /* Memory read support for an opcodes disassembler.  */
 
 int
@@ -383,7 +399,8 @@ sim_cgen_disassemble_insn (SIM_CPU *cpu, const CGEN_INSN *insn,
 
   sfile.buffer = sfile.current = buf;
   INIT_DISASSEMBLE_INFO (disasm_info, (FILE *) &sfile,
-			 (fprintf_ftype) sim_disasm_sprintf);
+			 (fprintf_ftype) sim_disasm_sprintf,
+			 (fprintf_styled_ftype) sim_disasm_styled_sprintf);
   disasm_info.endian =
     (bfd_big_endian (STATE_PROG_BFD (sd)) ? BFD_ENDIAN_BIG
      : bfd_little_endian (STATE_PROG_BFD (sd)) ? BFD_ENDIAN_LITTLE
diff --git a/sim/common/cgen-trace.h b/sim/common/cgen-trace.h
index 84b4935f148..3e2b4a9e9d7 100644
--- a/sim/common/cgen-trace.h
+++ b/sim/common/cgen-trace.h
@@ -78,6 +78,7 @@ typedef struct {
 
 /* String printer for the disassembler.  */
 extern int sim_disasm_sprintf (SFILE *, const char *, ...) ATTRIBUTE_PRINTF_2;
+extern int sim_disasm_styled_sprintf (SFILE *, enum disassembler_style, const char *, ...) ATTRIBUTE_PRINTF_3;
 
 /* For opcodes based disassemblers.  */
 #ifdef __BFD_H_SEEN__
diff --git a/sim/common/sim-trace.c b/sim/common/sim-trace.c
index 0b63bdb07b6..952c3514087 100644
--- a/sim/common/sim-trace.c
+++ b/sim/common/sim-trace.c
@@ -906,6 +906,18 @@ dis_printf (SIM_CPU *cpu, const char *fmt, ...)
   return 0;
 }
 
+static int ATTRIBUTE_PRINTF (3, 4)
+dis_styled_printf (SIM_CPU *cpu, enum disassembler_style style,
+		   const char *fmt, ...)
+{
+  SIM_DESC sd = CPU_STATE (cpu);
+  va_list ap;
+  va_start (ap, fmt);
+  trace_vprintf (sd, cpu, fmt, ap);
+  va_end (ap);
+  return 0;
+}
+
 void
 trace_disasm (SIM_DESC sd, sim_cpu *cpu, address_word addr)
 {
@@ -922,7 +934,7 @@ trace_disasm (SIM_DESC sd, sim_cpu *cpu, address_word addr)
 			bfd_big_endian (trace_data->dis_bfd),
 			bfd_get_mach (trace_data->dis_bfd),
 			trace_data->dis_bfd);
-      INIT_DISASSEMBLE_INFO (*info, cpu, dis_printf);
+      INIT_DISASSEMBLE_INFO (*info, cpu, dis_printf, dis_styled_printf);
       info->read_memory_func = dis_read;
       info->arch = bfd_get_arch (bfd);
       info->mach = bfd_get_mach (bfd);
diff --git a/sim/cris/sim-if.c b/sim/cris/sim-if.c
index c72edc752b6..3589f5bfdce 100644
--- a/sim/cris/sim-if.c
+++ b/sim/cris/sim-if.c
@@ -1017,7 +1017,8 @@ cris_disassemble_insn (SIM_CPU *cpu,
 
   sfile.buffer = sfile.current = buf;
   INIT_DISASSEMBLE_INFO (disasm_info, (FILE *) &sfile,
-			 (fprintf_ftype) sim_disasm_sprintf);
+			 (fprintf_ftype) sim_disasm_sprintf,
+			 (fprintf_styled_ftype) sim_disasm_styled_sprintf);
   disasm_info.endian = BFD_ENDIAN_LITTLE;
   disasm_info.read_memory_func = sim_disasm_read_memory;
   disasm_info.memory_error_func = sim_disasm_perror_memory;
diff --git a/sim/erc32/interf.c b/sim/erc32/interf.c
index d04d07c08c8..78dec6f4b9b 100644
--- a/sim/erc32/interf.c
+++ b/sim/erc32/interf.c
@@ -244,7 +244,8 @@ sim_open (SIM_OPEN_KIND kind, struct host_callback_struct *callback,
 #ifdef F_GETFL
     termsave = fcntl(0, F_GETFL, 0);
 #endif
-    INIT_DISASSEMBLE_INFO(dinfo, stdout,(fprintf_ftype)fprintf);
+    INIT_DISASSEMBLE_INFO(dinfo, stdout,(fprintf_ftype)fprintf,
+			  fprintf_styled);
 #ifdef HOST_LITTLE_ENDIAN
     dinfo.endian = BFD_ENDIAN_LITTLE;
 #else
diff --git a/sim/erc32/sis.c b/sim/erc32/sis.c
index 66a27d07506..12eb21f15a7 100644
--- a/sim/erc32/sis.c
+++ b/sim/erc32/sis.c
@@ -138,6 +138,21 @@ run_sim(struct pstate *sregs, uint64_t icount, int dis)
     return TIME_OUT;
 }
 
+int
+fprintf_styled (void *stream, enum disassembler_style style,
+		const char *fmt, ...)
+{
+  int ret;
+  FILE *out = (FILE *) stream;
+  va_list args;
+
+  va_start (args, fmt);
+  ret = vfprintf (out, fmt, args);
+  va_end (args);
+
+  return ret;
+}
+
 int
 main(int argc, char **argv)
 {
@@ -212,7 +227,8 @@ main(int argc, char **argv)
 #endif
     sregs.freq = freq;
 
-    INIT_DISASSEMBLE_INFO(dinfo, stdout, (fprintf_ftype) fprintf);
+    INIT_DISASSEMBLE_INFO(dinfo, stdout, (fprintf_ftype) fprintf,
+			  (fprintf_styled_ftype) fprintf_styled);
 #ifdef HOST_LITTLE_ENDIAN
     dinfo.endian = BFD_ENDIAN_LITTLE;
 #else
diff --git a/sim/erc32/sis.h b/sim/erc32/sis.h
index 33f16c77ca4..3a276670402 100644
--- a/sim/erc32/sis.h
+++ b/sim/erc32/sis.h
@@ -18,6 +18,7 @@
 #include "sim/sim.h"
 #include <sim-config.h>
 #include <stdint.h>
+#include "dis-asm.h"
 
 #if HOST_BYTE_ORDER == BIG_ENDIAN
 #define HOST_BIG_ENDIAN
@@ -203,6 +204,8 @@ extern void	init_regs (struct pstate *sregs);
 /* interf.c */
 extern int	run_sim (struct pstate *sregs,
 			 uint64_t icount, int dis);
+extern int      fprintf_styled (void *stream, enum disassembler_style style,
+				const char *fmt, ...) ATTRIBUTE_PRINTF (3, 4);
 
 /* float.c */
 extern int	get_accex (void);
diff --git a/sim/m32c/trace.c b/sim/m32c/trace.c
index 571df1a1e2a..f5171c298c5 100644
--- a/sim/m32c/trace.c
+++ b/sim/m32c/trace.c
@@ -101,6 +101,18 @@ op_printf (char *buf, char *fmt, ...)
   return ret;
 }
 
+static int
+op_styled_printf (char *buf, enum disassembler_style style, char *fmt, ...)
+{
+  int ret;
+  va_list ap;
+
+  va_start (ap, fmt);
+  ret = vsprintf (opbuf + strlen (opbuf), fmt, ap);
+  va_end (ap);
+  return ret;
+}
+
 static bfd *current_bfd;
 
 void
@@ -210,7 +222,7 @@ sim_disasm_one (void)
     {
       initted = 1;
       memset (&info, 0, sizeof (info));
-      INIT_DISASSEMBLE_INFO (info, stdout, op_printf);
+      INIT_DISASSEMBLE_INFO (info, stdout, op_printf, op_styled_printf);
       info.read_memory_func = sim_dis_read;
       info.arch = bfd_get_arch (current_bfd);
       info.mach = bfd_get_mach (current_bfd);
diff --git a/sim/rl78/trace.c b/sim/rl78/trace.c
index df75d793f69..a485c4c004d 100644
--- a/sim/rl78/trace.c
+++ b/sim/rl78/trace.c
@@ -104,6 +104,18 @@ op_printf (char *buf, char *fmt, ...)
   return ret;
 }
 
+static int
+op_styled_printf (char *buf, enum disassembler_style style, char *fmt, ...)
+{
+  int ret;
+  va_list ap;
+
+  va_start (ap, fmt);
+  ret = vsprintf (opbuf + strlen (opbuf), fmt, ap);
+  va_end (ap);
+  return ret;
+}
+
 static bfd *       current_bfd = NULL;
 static asymbol **  symtab = NULL;
 static int         symcount = 0;
@@ -205,7 +217,7 @@ sim_get_current_source_location (const char **  pfilename,
 
       initted = 1;
       memset (& info, 0, sizeof (info));
-      INIT_DISASSEMBLE_INFO (info, stdout, op_printf);
+      INIT_DISASSEMBLE_INFO (info, stdout, op_printf, op_styled_printf);
       info.read_memory_func = sim_dis_read;
       info.arch = bfd_get_arch (current_bfd);
       info.mach = bfd_get_mach (current_bfd);
diff --git a/sim/rx/trace.c b/sim/rx/trace.c
index 6e9a266ca3a..df83dc14a1b 100644
--- a/sim/rx/trace.c
+++ b/sim/rx/trace.c
@@ -109,6 +109,18 @@ op_printf (char *buf, char *fmt, ...)
   return ret;
 }
 
+static int ATTRIBUTE_PRINTF (3, 4)
+op_styled_printf (char *buf, enum disassembler_style style, char *fmt, ...)
+{
+  int ret;
+  va_list ap;
+
+  va_start (ap, fmt);
+  ret = vsprintf (opbuf + strlen (opbuf), fmt, ap);
+  va_end (ap);
+  return ret;
+}
+
 static bfd *       current_bfd = NULL;
 static asymbol **  symtab = NULL;
 static int         symcount = 0;
@@ -209,7 +221,7 @@ sim_get_current_source_location (const char **  pfilename,
 
       initted = 1;
       memset (& info, 0, sizeof (info));
-      INIT_DISASSEMBLE_INFO (info, stdout, op_printf);
+      INIT_DISASSEMBLE_INFO (info, stdout, op_printf, op_styled_printf);
       info.read_memory_func = sim_dis_read;
       info.arch = bfd_get_arch (current_bfd);
       info.mach = bfd_get_mach (current_bfd);
-- 
2.25.4


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

* Re: [PUSHED] sim: fixes for libopcodes styled disassembler
  2022-04-04 21:46 [PUSHED] sim: fixes for libopcodes styled disassembler Andrew Burgess
@ 2022-09-03  6:57 ` Jan-Benedict Glaw
  2022-09-04 17:04   ` Andrew Burgess
  0 siblings, 1 reply; 4+ messages in thread
From: Jan-Benedict Glaw @ 2022-09-03  6:57 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 7261 bytes --]

On Mon, 2022-04-04 22:46:32 +0100, Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> wrote:
> I've gone ahead and pushed the patch below without review, despite it
> being non-trivial.  I stupidly managed to break several sim targets,
> and I wanted to undo the breakage asap.
> 
> Despite being quite large, I hope nothing in here is too
> controversial.  However, if there is any feedback, do please leave it
> and I'll be happy to fix any issues in a follow up patch.
> 
> Sorry for the breakage, and thank you for your understanding.

.../configure --prefix=... --target=sparc-linux
make V=1 all-gdb

[...]
rm -f gdb
g++  -g -O2       -pthread  \
        -o gdb gdb.o ada-exp.o ada-lang.o ada-tasks.o ada-typeprint.o ada-valprint.o ada-varobj.o addrmap.o agent.o alloc.o annotate.o arch-utils.o async-event.o auto-load.o auxv.o ax-gdb.o ax-general.o bcache.o bfd-target.o block.o blockframe.o break-catch-exec.o break-catch-fork.o break-catch-load.o break-catch-sig.o break-catch-syscall.o break-catch-throw.o breakpoint.o bt-utils.o btrace.o build-id.o buildsym-legacy.o buildsym.o c-exp.o c-lang.o c-typeprint.o c-valprint.o c-varobj.o charset.o cli-out.o cli/cli-cmds.o cli/cli-decode.o cli/cli-dump.o cli/cli-interp.o cli/cli-logging.o cli/cli-option.o cli/cli-script.o cli/cli-setshow.o cli/cli-style.o cli/cli-utils.o coff-pe-read.o coffread.o compile/compile-c-support.o compile/compile-c-symbols.o compile/compile-c-types.o compile/compile-cplus-symbols.o compile/compile-cplus-types.o compile/compile-loc2c.o compile/compile-object-load.o compile/compile-object-run.o compile/compile.o complaints.o completer.o copying.o corefile.o corelow.o cp-abi.o cp-name-parser.o cp-namespace.o cp-support.o cp-valprint.o ctfread.o d-exp.o d-lang.o d-namespace.o d-valprint.o dbxread.o dcache.o debug.o debuginfod-support.o dictionary.o disasm-selftests.o disasm.o displaced-stepping.o dtrace-probe.o dummy-frame.o dwarf2/abbrev-cache.o dwarf2/abbrev.o dwarf2/attribute.o dwarf2/comp-unit-head.o dwarf2/cooked-index.o dwarf2/cu.o dwarf2/dwz.o dwarf2/expr.o dwarf2/frame-tailcall.o dwarf2/frame.o dwarf2/index-cache.o dwarf2/index-common.o dwarf2/index-write.o dwarf2/leb.o dwarf2/line-header.o dwarf2/loc.o dwarf2/macro.o dwarf2/read.o dwarf2/section.o dwarf2/stringify.o elf-none-tdep.o elfread.o eval.o event-top.o exceptions.o exec.o expprint.o extension.o f-exp.o f-lang.o f-typeprint.o f-valprint.o filename-seen-cache.o filesystem.o findcmd.o findvar.o frame-base.o frame-unwind.o frame.o gcore-elf.o gcore.o gdb-demangle.o gdb_bfd.o gdbarch-selftests.o gdbtypes.o gmp-utils.o gnu-v2-abi.o gnu-v3-abi.o go-exp.o go-lang.o go-typeprint.o go-valprint.o guile/guile.o inf-child.o inf-loop.o infcall.o infcmd.o inferior.o inflow.o infrun.o inline-frame.o interps.o jit.o language.o linespec.o linux-tdep.o location.o m2-exp.o m2-lang.o m2-typeprint.o m2-valprint.o macrocmd.o macroexp.o macroscope.o macrotab.o main.o maint-test-options.o maint-test-settings.o maint.o mdebugread.o mem-break.o memattr.o memory-map.o memrange.o memtag.o mi/mi-cmd-break.o mi/mi-cmd-catch.o mi/mi-cmd-disas.o mi/mi-cmd-env.o mi/mi-cmd-file.o mi/mi-cmd-info.o mi/mi-cmd-stack.o mi/mi-cmd-target.o mi/mi-cmd-var.o mi/mi-cmds.o mi/mi-common.o mi/mi-console.o mi/mi-getopt.o mi/mi-interp.o mi/mi-main.o mi/mi-out.o mi/mi-parse.o mi/mi-symbol-cmds.o minidebug.o minsyms.o mipsread.o namespace.o objc-lang.o objfiles.o observable.o opencl-lang.o osabi.o osdata.o p-exp.o p-lang.o p-typeprint.o p-valprint.o parse.o posix-hdep.o printcmd.o probe.o process-stratum-target.o producer.o progspace-and-thread.o progspace.o prologue-value.o psymtab.o python/python.o ravenscar-thread.o record-btrace.o record-full.o record.o regcache-dump.o regcache.o reggroups.o remote-fileio.o remote-notif.o remote-sim.o remote.o reverse.o run-on-main-thread.o rust-lang.o rust-parse.o selftest-arch.o sentinel-frame.o ser-base.o ser-event.o ser-pipe.o ser-tcp.o ser-uds.o ser-unix.o serial.o skip.o solib-svr4.o solib-target.o solib.o source-cache.o source.o sparc-linux-tdep.o sparc-ravenscar-thread.o sparc-tdep.o sparc64-linux-tdep.o sparc64-tdep.o split-name.o stabsread.o stack.o stap-probe.o std-regs.o symfile-debug.o symfile-mem.o symfile.o symmisc.o symtab.o target-connection.o target-dcache.o target-descriptions.o target-float.o target-memory.o target.o target/target.o target/waitstatus.o test-target.o thread-iter.o thread.o tid-parse.o top.o tracectf.o tracefile-tfile.o tracefile.o tracepoint.o trad-frame.o tramp-frame.o tui/tui-command.o tui/tui-data.o tui/tui-disasm.o tui/tui-file.o tui/tui-hooks.o tui/tui-interp.o tui/tui-io.o tui/tui-layout.o tui/tui-location.o tui/tui-out.o tui/tui-regs.o tui/tui-source.o tui/tui-stack.o tui/tui-win.o tui/tui-wingeneral.o tui/tui-winsource.o tui/tui.o type-stack.o typeprint.o ui-file.o ui-out.o ui-style.o unittests/array-view-selftests.o unittests/child-path-selftests.o unittests/cli-utils-selftests.o unittests/command-def-selftests.o unittests/common-utils-selftests.o unittests/copy_bitwise-selftests.o unittests/enum-flags-selftests.o unittests/environ-selftests.o unittests/filtered_iterator-selftests.o unittests/format_pieces-selftests.o unittests/function-view-selftests.o unittests/gdb_tilde_expand-selftests.o unittests/gmp-utils-selftests.o unittests/intrusive_list-selftests.o unittests/lookup_name_info-selftests.o unittests/main-thread-selftests.o unittests/memory-map-selftests.o unittests/memrange-selftests.o unittests/mkdir-recursive-selftests.o unittests/observable-selftests.o unittests/offset-type-selftests.o unittests/optional-selftests.o unittests/packed-selftests.o unittests/parallel-for-selftests.o unittests/parse-connection-spec-selftests.o unittests/path-join-selftests.o unittests/ptid-selftests.o unittests/rsp-low-selftests.o unittests/scoped_fd-selftests.o unittests/scoped_ignore_signal-selftests.o unittests/scoped_mmap-selftests.o unittests/scoped_restore-selftests.o unittests/search-memory-selftests.o unittests/string_view-selftests.o unittests/style-selftests.o unittests/tracepoint-selftests.o unittests/tui-selftests.o unittests/ui-file-selftests.o unittests/unique_xmalloc_ptr_char.o unittests/unpack-selftests.o unittests/utils-selftests.o unittests/vec-utils-selftests.o unittests/xml-utils-selftests.o user-regs.o utils.o valarith.o valops.o valprint.o value.o varobj.o version.o xml-builtin.o xml-support.o xml-syscall.o xml-tdesc.o init.o \
          ../sim/erc32/libsim.a ../readline/readline/libreadline.a ../opcodes/libopcodes.a ../libctf/.libs/libctf.a ../bfd/libbfd.a -L./../zlib -lz ../gdbsupport/libgdbsupport.a  ../libiberty/libiberty.a ../libdecnumber/libdecnumber.a    ../libbacktrace/.libs/libbacktrace.a -lncursesw -lm -ldl      ../gnulib/import/libgnu.a    -lgmp      
/usr/bin/ld: ../sim/erc32/libsim.a(interf.o): in function `sim_open':
/var/cache/git/binutils-gdb/sim/erc32/interf.c:247: undefined reference to `fprintf_styled'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:2142: gdb] Error 1
make[1]: Leaving directory '/tmp/sparc/b/gdb'
make: *** [Makefile:13193: all-gdb] Error 2


erc32 has two INIT_DISASSEMBLE_INFO(), of which one doesn't pick the
provided function. Maybe just ordering?

MfG, JBG

-- 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

* Re: [PUSHED] sim: fixes for libopcodes styled disassembler
  2022-09-03  6:57 ` Jan-Benedict Glaw
@ 2022-09-04 17:04   ` Andrew Burgess
  2022-09-05 10:38     ` Jan-Benedict Glaw
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Burgess @ 2022-09-04 17:04 UTC (permalink / raw)
  To: Jan-Benedict Glaw; +Cc: gdb-patches

* Jan-Benedict Glaw <jbglaw@lug-owl.de> [2022-09-03 08:57:32 +0200]:

> On Mon, 2022-04-04 22:46:32 +0100, Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> wrote:
> > I've gone ahead and pushed the patch below without review, despite it
> > being non-trivial.  I stupidly managed to break several sim targets,
> > and I wanted to undo the breakage asap.
> > 
> > Despite being quite large, I hope nothing in here is too
> > controversial.  However, if there is any feedback, do please leave it
> > and I'll be happy to fix any issues in a follow up patch.
> > 
> > Sorry for the breakage, and thank you for your understanding.
> 
> .../configure --prefix=... --target=sparc-linux
> make V=1 all-gdb
> 
> [...]
> rm -f gdb
> g++  -g -O2       -pthread  \
>         -o gdb gdb.o ada-exp.o ada-lang.o ada-tasks.o ada-typeprint.o ada-valprint.o ada-varobj.o addrmap.o agent.o alloc.o annotate.o arch-utils.o async-event.o auto-load.o auxv.o ax-gdb.o ax-general.o bcache.o bfd-target.o block.o blockframe.o break-catch-exec.o break-catch-fork.o break-catch-load.o break-catch-sig.o break-catch-syscall.o break-catch-throw.o breakpoint.o bt-utils.o btrace.o build-id.o buildsym-legacy.o buildsym.o c-exp.o c-lang.o c-typeprint.o c-valprint.o c-varobj.o charset.o cli-out.o cli/cli-cmds.o cli/cli-decode.o cli/cli-dump.o cli/cli-interp.o cli/cli-logging.o cli/cli-option.o cli/cli-script.o cli/cli-setshow.o cli/cli-style.o cli/cli-utils.o coff-pe-read.o coffread.o compile/compile-c-support.o compile/compile-c-symbols.o compile/compile-c-types.o compile/compile-cplus-symbols.o compile/compile-cplus-types.o compile/compile-loc2c.o compile/compile-object-load.o compile/compile-object-run.o compile/compile.o complaints.o completer.o copying.o corefile.o corelow.o cp-abi.o cp-name-parser.o cp-namespace.o cp-support.o cp-valprint.o ctfread.o d-exp.o d-lang.o d-namespace.o d-valprint.o dbxread.o dcache.o debug.o debuginfod-support.o dictionary.o disasm-selftests.o disasm.o displaced-stepping.o dtrace-probe.o dummy-frame.o dwarf2/abbrev-cache.o dwarf2/abbrev.o dwarf2/attribute.o dwarf2/comp-unit-head.o dwarf2/cooked-index.o dwarf2/cu.o dwarf2/dwz.o dwarf2/expr.o dwarf2/frame-tailcall.o dwarf2/frame.o dwarf2/index-cache.o dwarf2/index-common.o dwarf2/index-write.o dwarf2/leb.o dwarf2/line-header.o dwarf2/loc.o dwarf2/macro.o dwarf2/read.o dwarf2/section.o dwarf2/stringify.o elf-none-tdep.o elfread.o eval.o event-top.o exceptions.o exec.o expprint.o extension.o f-exp.o f-lang.o f-typeprint.o f-valprint.o filename-seen-cache.o filesystem.o findcmd.o findvar.o frame-base.o frame-unwind.o frame.o gcore-elf.o gcore.o gdb-demangle.o gdb_bfd.o gdbarch-selftests.o gdbtypes.o gmp-utils.o gnu-v2-abi.o gnu-v3-abi.o go-exp.o go-lang.o go-typeprint.o go-valprint.o guile/guile.o inf-child.o inf-loop.o infcall.o infcmd.o inferior.o inflow.o infrun.o inline-frame.o interps.o jit.o language.o linespec.o linux-tdep.o location.o m2-exp.o m2-lang.o m2-typeprint.o m2-valprint.o macrocmd.o macroexp.o macroscope.o macrotab.o main.o maint-test-options.o maint-test-settings.o maint.o mdebugread.o mem-break.o memattr.o memory-map.o memrange.o memtag.o mi/mi-cmd-break.o mi/mi-cmd-catch.o mi/mi-cmd-disas.o mi/mi-cmd-env.o mi/mi-cmd-file.o mi/mi-cmd-info.o mi/mi-cmd-stack.o mi/mi-cmd-target.o mi/mi-cmd-var.o mi/mi-cmds.o mi/mi-common.o mi/mi-console.o mi/mi-getopt.o mi/mi-interp.o mi/mi-main.o mi/mi-out.o mi/mi-parse.o mi/mi-symbol-cmds.o minidebug.o minsyms.o mipsread.o namespace.o objc-lang.o objfiles.o observable.o opencl-lang.o osabi.o osdata.o p-exp.o p-lang.o p-typeprint.o p-valprint.o parse.o posix-hdep.o printcmd.o probe.o process-stratum-target.o producer.o progspace-and-thread.o progspace.o prologue-value.o psymtab.o python/python.o ravenscar-thread.o record-btrace.o record-full.o record.o regcache-dump.o regcache.o reggroups.o remote-fileio.o remote-notif.o remote-sim.o remote.o reverse.o run-on-main-thread.o rust-lang.o rust-parse.o selftest-arch.o sentinel-frame.o ser-base.o ser-event.o ser-pipe.o ser-tcp.o ser-uds.o ser-unix.o serial.o skip.o solib-svr4.o solib-target.o solib.o source-cache.o source.o sparc-linux-tdep.o sparc-ravenscar-thread.o sparc-tdep.o sparc64-linux-tdep.o sparc64-tdep.o split-name.o stabsread.o stack.o stap-probe.o std-regs.o symfile-debug.o symfile-mem.o symfile.o symmisc.o symtab.o target-connection.o target-dcache.o target-descriptions.o target-float.o target-memory.o target.o target/target.o target/waitstatus.o test-target.o thread-iter.o thread.o tid-parse.o top.o tracectf.o tracefile-tfile.o tracefile.o tracepoint.o trad-frame.o tramp-frame.o tui/tui-command.o tui/tui-data.o tui/tui-disasm.o tui/tui-file.o tui/tui-hooks.o tui/tui-interp.o tui/tui-io.o tui/tui-layout.o tui/tui-location.o tui/tui-out.o tui/tui-regs.o tui/tui-source.o tui/tui-stack.o tui/tui-win.o tui/tui-wingeneral.o tui/tui-winsource.o tui/tui.o type-stack.o typeprint.o ui-file.o ui-out.o ui-style.o unittests/array-view-selftests.o unittests/child-path-selftests.o unittests/cli-utils-selftests.o unittests/command-def-selftests.o unittests/common-utils-selftests.o unittests/copy_bitwise-selftests.o unittests/enum-flags-selftests.o unittests/environ-selftests.o unittests/filtered_iterator-selftests.o unittests/format_pieces-selftests.o unittests/function-view-selftests.o unittests/gdb_tilde_expand-selftests.o unittests/gmp-utils-selftests.o unittests/intrusive_list-selftests.o unittests/lookup_name_info-selftests.o unittests/main-thread-selftests.o unittests/memory-map-selftests.o unittests/memrange-selftests.o unittests/mkdir-recursive-selftests.o unittests/observable-selftests.o unittests/offset-type-selftests.o unittests/optional-selftests.o unittests/packed-selftests.o unittests/parallel-for-selftests.o unittests/parse-connection-spec-selftests.o unittests/path-join-selftests.o unittests/ptid-selftests.o unittests/rsp-low-selftests.o unittests/scoped_fd-selftests.o unittests/scoped_ignore_signal-selftests.o unittests/scoped_mmap-selftests.o unittests/scoped_restore-selftests.o unittests/search-memory-selftests.o unittests/string_view-selftests.o unittests/style-selftests.o unittests/tracepoint-selftests.o unittests/tui-selftests.o unittests/ui-file-selftests.o unittests/unique_xmalloc_ptr_char.o unittests/unpack-selftests.o unittests/utils-selftests.o unittests/vec-utils-selftests.o unittests/xml-utils-selftests.o user-regs.o utils.o valarith.o valops.o valprint.o value.o varobj.o version.o xml-builtin.o xml-support.o xml-syscall.o xml-tdesc.o init.o \
>           ../sim/erc32/libsim.a ../readline/readline/libreadline.a ../opcodes/libopcodes.a ../libctf/.libs/libctf.a ../bfd/libbfd.a -L./../zlib -lz ../gdbsupport/libgdbsupport.a  ../libiberty/libiberty.a ../libdecnumber/libdecnumber.a    ../libbacktrace/.libs/libbacktrace.a -lncursesw -lm -ldl      ../gnulib/import/libgnu.a    -lgmp      
> /usr/bin/ld: ../sim/erc32/libsim.a(interf.o): in function `sim_open':
> /var/cache/git/binutils-gdb/sim/erc32/interf.c:247: undefined reference to `fprintf_styled'
> collect2: error: ld returned 1 exit status
> make[1]: *** [Makefile:2142: gdb] Error 1
> make[1]: Leaving directory '/tmp/sparc/b/gdb'
> make: *** [Makefile:13193: all-gdb] Error 2
> 
> 
> erc32 has two INIT_DISASSEMBLE_INFO(), of which one doesn't pick the
> provided function. Maybe just ordering?

Thanks for pointing this out.

I pushed the patch below which should fix this problem.   Let me know
if you still have any problems with the build.

Thanks,
Andrew

---

commit a411a714f31da81e6c57317f1d392e166053dff1
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sun Sep 4 17:49:11 2022 +0100

    sim/erc32: fix gdb with simulator build
    
    In commit:
    
      commit 7b01c1cc1d111ba0afa51e60fa9842d3b971e2d1
      Date:   Mon Apr 4 22:38:04 2022 +0100
    
          sim: fixes for libopcodes styled disassembler
    
    changes were made to the simulator source to handle the new libopcodes
    disassembler styling API.
    
    Unfortunately, these changes broke building GDB with the erc32 (sparc)
    simulator, like this:
    
      ../src/configure --target=sparc-linux
      make all-gdb
      ....
      /usr/bin/ld: ../sim/erc32/libsim.a(interf.o): in function `sim_open':
      /tmp/build/sim/../../src/sim/erc32/interf.c:247: undefined reference to `fprintf_styled'
      collect2: error: ld returned 1 exit status
    
    The problem is that in commit 7b01c1cc1d11 the fprintf_styled function
    was added into sis.c.  This file is only used when building the 'run'
    binary, that is, the standalone simulator, and is not included in the
    libsim.a library.
    
    Now, the obvious fix would be to move fprintf_styled into libsim.a,
    however, that turns out to be tricky.
    
    The erc32 simulator currently has two copies of the function run_sim,
    one in sis.c, and one in interf.c, both of these copies are global.
    
    Currently, the 'run' binary links fine, though I suspect this might be
    pure luck.  When I tried moving fprintf_styled into interf.c, I ran
    into multiple-definition (of run_sim) errors.  I suspect that by
    requiring the linker to pull in fprintf_styled from libsim.a I was
    changing the order in which symbols were loaded, and the linker was
    now seeing both copies of run_sim, while currently we only see one
    copy.
    
    The ideal solution of course, would be to merge the two similar, but
    slightly different copies of run_sim, and just use the one copy.  Then
    we could safely move fprintf_styled into interf.c too, and all would
    be good.
    
    But I don't have time right now to start debugging the erc32
    simulator, so I wanted a solution that fixes the build without
    introducing multiple definition errors.
    
    The easiest solution I think is to just have two copies of
    fprintf_styled, one in sis.c, and one in interf.c.  Unlike run_sim,
    these two copies are both static, so we will not run into multiple
    definition issues with this function.  The functions themselves are
    not very big, so it's not a huge amount of duplicate code.
    
    I am very aware that this is not an ideal solution, and I would
    welcome anyone who wants to take on fixing the run_sim problem
    properly, and then cleanup the fprintf_styled duplication.

diff --git a/sim/erc32/interf.c b/sim/erc32/interf.c
index 78dec6f4b9b..f433b9d55ac 100644
--- a/sim/erc32/interf.c
+++ b/sim/erc32/interf.c
@@ -156,6 +156,21 @@ run_sim(struct pstate *sregs, uint64_t icount, int dis)
     return TIME_OUT;
 }
 
+static int
+fprintf_styled (void *stream, enum disassembler_style style,
+		const char *fmt, ...)
+{
+  int ret;
+  FILE *out = (FILE *) stream;
+  va_list args;
+
+  va_start (args, fmt);
+  ret = vfprintf (out, fmt, args);
+  va_end (args);
+
+  return ret;
+}
+
 SIM_DESC
 sim_open (SIM_OPEN_KIND kind, struct host_callback_struct *callback,
 	  struct bfd *abfd, char * const *argv)
diff --git a/sim/erc32/sis.c b/sim/erc32/sis.c
index 12eb21f15a7..1d3ea139c23 100644
--- a/sim/erc32/sis.c
+++ b/sim/erc32/sis.c
@@ -138,7 +138,7 @@ run_sim(struct pstate *sregs, uint64_t icount, int dis)
     return TIME_OUT;
 }
 
-int
+static int
 fprintf_styled (void *stream, enum disassembler_style style,
 		const char *fmt, ...)
 {
diff --git a/sim/erc32/sis.h b/sim/erc32/sis.h
index 3a276670402..71033137f2c 100644
--- a/sim/erc32/sis.h
+++ b/sim/erc32/sis.h
@@ -204,8 +204,6 @@ extern void	init_regs (struct pstate *sregs);
 /* interf.c */
 extern int	run_sim (struct pstate *sregs,
 			 uint64_t icount, int dis);
-extern int      fprintf_styled (void *stream, enum disassembler_style style,
-				const char *fmt, ...) ATTRIBUTE_PRINTF (3, 4);
 
 /* float.c */
 extern int	get_accex (void);


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

* Re: [PUSHED] sim: fixes for libopcodes styled disassembler
  2022-09-04 17:04   ` Andrew Burgess
@ 2022-09-05 10:38     ` Jan-Benedict Glaw
  0 siblings, 0 replies; 4+ messages in thread
From: Jan-Benedict Glaw @ 2022-09-05 10:38 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

On Sun, 2022-09-04 18:04:17 +0100, Andrew Burgess <aburgess@redhat.com> wrote:
> * Jan-Benedict Glaw <jbglaw@lug-owl.de> [2022-09-03 08:57:32 +0200]:
> > On Mon, 2022-04-04 22:46:32 +0100, Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> wrote:
> > > Sorry for the breakage, and thank you for your understanding.
> > 
> > .../configure --prefix=... --target=sparc-linux
> > make V=1 all-gdb
> > /var/cache/git/binutils-gdb/sim/erc32/interf.c:247: undefined reference to `fprintf_styled'
> > collect2: error: ld returned 1 exit status
> > make[1]: *** [Makefile:2142: gdb] Error 1
> > make[1]: Leaving directory '/tmp/sparc/b/gdb'
> > make: *** [Makefile:13193: all-gdb] Error 2
> 
> Thanks for pointing this out.
> 
> I pushed the patch below which should fix this problem.   Let me know
> if you still have any problems with the build.

Fixes all affected / tested targets.

Thanks!
  Jan-Benedict

-- 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

end of thread, other threads:[~2022-09-05 10:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04 21:46 [PUSHED] sim: fixes for libopcodes styled disassembler Andrew Burgess
2022-09-03  6:57 ` Jan-Benedict Glaw
2022-09-04 17:04   ` Andrew Burgess
2022-09-05 10:38     ` Jan-Benedict Glaw

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