public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/5] gdb: change jit_debug to a bool
@ 2021-01-09  4:28 Simon Marchi
  2021-01-09  4:28 ` [PATCH 2/5] gdb: convert jit to new-style debug macros Simon Marchi
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Simon Marchi @ 2021-01-09  4:28 UTC (permalink / raw)
  To: gdb-patches

gdb/ChangeLog:

	* jit.c (jit_debug): Change type to bool.
	(_initialize_jit): Adjust.

Change-Id: Ic2b1eec28eafe8ccb2899f38ddc91ba9703cb38e
---
 gdb/jit.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index ad951cdfbb52..474ba838b86b 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -58,9 +58,9 @@ static void jit_inferior_exit_hook (struct inferior *inf);
 
 static struct gdbarch_data *jit_gdbarch_data;
 
-/* Non-zero if we want to see trace of jit level stuff.  */
+/* True if we want to see trace of jit level stuff.  */
 
-static unsigned int jit_debug = 0;
+static bool jit_debug = false;
 
 static void
 show_jit_debug (struct ui_file *file, int from_tty,
@@ -1251,13 +1251,13 @@ _initialize_jit ()
 {
   jit_reader_dir = relocate_gdb_directory (JIT_READER_DIR,
 					   JIT_READER_DIR_RELOCATABLE);
-  add_setshow_zuinteger_cmd ("jit", class_maintenance, &jit_debug,
-			     _("Set JIT debugging."),
-			     _("Show JIT debugging."),
-			     _("When non-zero, JIT debugging is enabled."),
-			     NULL,
-			     show_jit_debug,
-			     &setdebuglist, &showdebuglist);
+  add_setshow_boolean_cmd ("jit", class_maintenance, &jit_debug,
+			   _("Set JIT debugging."),
+			   _("Show JIT debugging."),
+			   _("When non-zero, JIT debugging is enabled."),
+			   NULL,
+			   show_jit_debug,
+			   &setdebuglist, &showdebuglist);
 
   gdb::observers::inferior_created.attach (jit_inferior_created_hook);
   gdb::observers::inferior_execd.attach (jit_inferior_created_hook);
-- 
2.29.2


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

* [PATCH 2/5] gdb: convert jit to new-style debug macros
  2021-01-09  4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
@ 2021-01-09  4:28 ` Simon Marchi
  2021-01-11 21:18   ` Tom Tromey
  2021-01-09  4:28 ` [PATCH 3/5] gdb: convert aarch64 " Simon Marchi
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-01-09  4:28 UTC (permalink / raw)
  To: gdb-patches

Here's a sample output, with infrun debug enabled as well to show
nesting:

    [infrun] fetch_inferior_event: enter
      [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
      [infrun] print_target_wait_results:   4116727.4116727.0 [process 4116727],
      [infrun] print_target_wait_results:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
      [infrun] handle_inferior_event: status->kind = stopped, signal = GDB_SIGNAL_TRAP
      [infrun] start_step_over: enter
        [infrun] start_step_over: stealing global queue of threads to step, length = 0
        [infrun] operator(): step-over queue now empty
      [infrun] start_step_over: exit
      [infrun] handle_signal_stop: stop_pc=0x555555555229
      [infrun] handle_jit_event: handling bp_jit_event
      [jit] jit_read_descriptor: descriptor_addr = 0x5555555580b0
      [jit] jit_register_code: symfile_addr = 0x7000000, symfile_size = 15560
      [jit] jit_bfd_try_read_symtab: symfile_addr = 0x7000000, symfile_size = 15560
      [jit] jit_breakpoint_re_set_internal: breakpoint_addr = 0x555555555229
      [infrun] process_event_stop_test: BPSTAT_WHAT_SINGLE
      [infrun] process_event_stop_test: no stepping, continue
      [infrun] resume_1: step=1, signal=GDB_SIGNAL_0, trap_expected=1, current thread [process 4116727] at 0x555555555229
      [infrun] prepare_to_wait: prepare_to_wait
    [infrun] fetch_inferior_event: exit

gdb/ChangeLog:

	* jit.c (jit_debug_printf): New, use throughout file.

Change-Id: Ic0f5eb3ffc926fb555de4914e7dc1076ada63a97
---
 gdb/jit.c | 62 +++++++++++++++++++++----------------------------------
 1 file changed, 23 insertions(+), 39 deletions(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 474ba838b86b..421054d9359b 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -62,6 +62,11 @@ static struct gdbarch_data *jit_gdbarch_data;
 
 static bool jit_debug = false;
 
+/* Print a "jit" debug statement.  */
+
+#define jit_debug_printf(fmt, ...) \
+  debug_prefixed_printf_cond (jit_debug, "jit", fmt, ##__VA_ARGS__)
+
 static void
 show_jit_debug (struct ui_file *file, int from_tty,
 		struct cmd_list_element *c, const char *value)
@@ -103,9 +108,8 @@ jit_reader_load (const char *file_name)
   reader_init_fn_type *init_fn;
   struct gdb_reader_funcs *funcs = NULL;
 
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog, _("Opening shared object %s.\n"),
-			file_name);
+  jit_debug_printf ("Opening shared object %s", file_name);
+
   gdb_dlhandle_up so = gdb_dlopen (file_name);
 
   init_fn = (reader_init_fn_type *) gdb_dlsym (so, reader_init_fn_sym);
@@ -211,10 +215,7 @@ jit_read_descriptor (gdbarch *gdbarch,
 
   CORE_ADDR addr = MSYMBOL_VALUE_ADDRESS (jiter, objf_data->descriptor);
 
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"jit_read_descriptor, descriptor_addr = %s\n",
-			paddress (gdbarch, addr));
+  jit_debug_printf ("descriptor_addr = %s", paddress (gdbarch, addr));
 
   /* Figure out how big the descriptor is on the remote and how to read it.  */
   ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
@@ -668,9 +669,9 @@ jit_reader_try_read_symtab (struct jit_code_entry *code_entry,
 	status = 0;
     }
 
-  if (jit_debug && status == 0)
-    fprintf_unfiltered (gdb_stdlog,
-			"Could not read symtab using the loaded JIT reader.\n");
+  if (status == 0)
+    jit_debug_printf ("Could not read symtab using the loaded JIT reader.");
+
   return status;
 }
 
@@ -686,12 +687,9 @@ jit_bfd_try_read_symtab (struct jit_code_entry *code_entry,
   struct objfile *objfile;
   const struct bfd_arch_info *b;
 
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"jit_bfd_try_read_symtab, symfile_addr = %s, "
-			"symfile_size = %s\n",
-			paddress (gdbarch, code_entry->symfile_addr),
-			pulongest (code_entry->symfile_size));
+  jit_debug_printf ("symfile_addr = %s, symfile_size = %s",
+		    paddress (gdbarch, code_entry->symfile_addr),
+		    pulongest (code_entry->symfile_size));
 
   gdb_bfd_ref_ptr nbfd (gdb_bfd_open_from_target_memory
       (code_entry->symfile_addr, code_entry->symfile_size, gnutarget));
@@ -752,12 +750,9 @@ jit_register_code (struct gdbarch *gdbarch,
 {
   int success;
 
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"jit_register_code, symfile_addr = %s, "
-			"symfile_size = %s\n",
-			paddress (gdbarch, code_entry->symfile_addr),
-			pulongest (code_entry->symfile_size));
+  jit_debug_printf ("symfile_addr = %s, symfile_size = %s",
+		    paddress (gdbarch, code_entry->symfile_addr),
+		    pulongest (code_entry->symfile_size));
 
   success = jit_reader_try_read_symtab (code_entry, entry_addr);
 
@@ -845,11 +840,7 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace)
       CORE_ADDR addr = MSYMBOL_VALUE_ADDRESS (the_objfile,
 					      objf_data->register_code);
 
-      if (jit_debug)
-	fprintf_unfiltered (gdb_stdlog,
-			    "jit_breakpoint_re_set_internal, "
-			    "breakpoint_addr = %s\n",
-			    paddress (gdbarch, addr));
+      jit_debug_printf ("breakpoint_addr = %s", paddress (gdbarch, addr));
 
       /* Check if we need to re-create the breakpoint.  */
       if (objf_data->cached_code_address == addr)
@@ -893,10 +884,7 @@ jit_unwind_reg_set_impl (struct gdb_unwind_callbacks *cb, int dwarf_regnum,
 					  dwarf_regnum);
   if (gdb_reg == -1)
     {
-      if (jit_debug)
-	fprintf_unfiltered (gdb_stdlog,
-			    _("Could not recognize DWARF regnum %d"),
-			    dwarf_regnum);
+      jit_debug_printf ("Could not recognize DWARF regnum %d", dwarf_regnum);
       value->free (value);
       return;
     }
@@ -987,14 +975,11 @@ jit_frame_sniffer (const struct frame_unwind *self,
   /* Try to coax the provided unwinder to unwind the stack */
   if (funcs->unwind (funcs, &callbacks) == GDB_SUCCESS)
     {
-      if (jit_debug)
-	fprintf_unfiltered (gdb_stdlog, _("Successfully unwound frame using "
-					  "JIT reader.\n"));
+      jit_debug_printf ("Successfully unwound frame using JIT reader.");
       return 1;
     }
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog, _("Could not unwind frame using "
-				      "JIT reader.\n"));
+
+  jit_debug_printf ("Could not unwind frame using JIT reader.");
 
   jit_dealloc_cache (this_frame, *cache);
   *cache = NULL;
@@ -1105,8 +1090,7 @@ jit_inferior_init (inferior *inf)
   struct gdbarch *gdbarch = inf->gdbarch;
   program_space *pspace = inf->pspace;
 
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");
+  jit_debug_printf ("called");
 
   jit_prepend_unwinder (gdbarch);
 
-- 
2.29.2


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

* [PATCH 3/5] gdb: convert aarch64 to new-style debug macros
  2021-01-09  4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
  2021-01-09  4:28 ` [PATCH 2/5] gdb: convert jit to new-style debug macros Simon Marchi
@ 2021-01-09  4:28 ` Simon Marchi
  2021-01-09  4:31   ` Simon Marchi
  2021-01-09  4:28 ` [PATCH 4/5] gdb: convert solib-aix " Simon Marchi
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-01-09  4:28 UTC (permalink / raw)
  To: gdb-patches

I haven't tried this on an actual aarch64 machine, but I am able to
exercise it like this:

    (gdb) set debug aarch64
    (gdb) maintenance selftest aa
    Running selftest aarch64-analyze-prologue.
    [aarch64] aarch64_analyze_prologue: prologue analysis gave up addr=0x14 opcode=0xf94013e0
    Running selftest aarch64-process-record.
    Ran 2 unit tests, 0 failed

gdb/ChangeLog:

	* arch/aarch64-insn.h (aarch64_debug_printf): New.
	* arch/aarch64-insn.c: Use aarch64_debug_printf.
	* aarch64-tdep.c: Use aarch64_debug_printf.

Change-Id: Ifdb40e2816ab8e55a9aabb066d1833d9b5a46094
---
 gdb/aarch64-tdep.c      | 89 ++++++++++++++++-------------------------
 gdb/arch/aarch64-insn.c |  9 ++---
 gdb/arch/aarch64-insn.h |  5 +++
 3 files changed, 42 insertions(+), 61 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 77e6ad700fcd..a51b3341e789 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -388,12 +388,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
 	    regs[rd] = regs[rm];
 	  else
 	    {
-	      if (aarch64_debug)
-		{
-		  debug_printf ("aarch64: prologue analysis gave up "
-				"addr=%s opcode=0x%x (orr x register)\n",
-				core_addr_to_string_nz (start), insn);
-		}
+	      aarch64_debug_printf ("prologue analysis gave up "
+				    "addr=%s opcode=0x%x (orr x register)",
+				    core_addr_to_string_nz (start), insn);
+
 	      break;
 	    }
 	}
@@ -513,10 +511,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
 	    }
 	  else
 	    {
-	      if (aarch64_debug)
-		debug_printf ("aarch64: prologue analysis gave up addr=%s"
-			      " opcode=0x%x (iclass)\n",
-			      core_addr_to_string_nz (start), insn);
+	      aarch64_debug_printf ("prologue analysis gave up addr=%s"
+				    " opcode=0x%x (iclass)",
+				    core_addr_to_string_nz (start), insn);
 	      break;
 	    }
 
@@ -527,12 +524,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
 	}
       else
 	{
-	  if (aarch64_debug)
-	    {
-	      debug_printf ("aarch64: prologue analysis gave up addr=%s"
-			    " opcode=0x%x\n",
-			    core_addr_to_string_nz (start), insn);
-	    }
+	  aarch64_debug_printf ("prologue analysis gave up addr=%s"
+				" opcode=0x%x",
+				core_addr_to_string_nz (start), insn);
+
 	  break;
 	}
     }
@@ -1606,12 +1601,10 @@ pass_in_x (struct gdbarch *gdbarch, struct regcache *regcache,
 	  && (typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION))
 	regval <<= ((X_REGISTER_SIZE - partial_len) * TARGET_CHAR_BIT);
 
-      if (aarch64_debug)
-	{
-	  debug_printf ("arg %d in %s = 0x%s\n", info->argnum,
-			gdbarch_register_name (gdbarch, regnum),
-			phex (regval, X_REGISTER_SIZE));
-	}
+      aarch64_debug_printf ("arg %d in %s = 0x%s", info->argnum,
+			    gdbarch_register_name (gdbarch, regnum),
+			    phex (regval, X_REGISTER_SIZE));
+
       regcache_cooked_write_unsigned (regcache, regnum, regval);
       len -= partial_len;
       buf += partial_len;
@@ -1646,11 +1639,9 @@ pass_in_v (struct gdbarch *gdbarch,
       memcpy (reg, buf, len);
       regcache->cooked_write (regnum, reg);
 
-      if (aarch64_debug)
-	{
-	  debug_printf ("arg %d in %s\n", info->argnum,
-			gdbarch_register_name (gdbarch, regnum));
-	}
+      aarch64_debug_printf ("arg %d in %s", info->argnum,
+			    gdbarch_register_name (gdbarch, regnum));
+
       return 1;
     }
   info->nsrn = 8;
@@ -1680,11 +1671,8 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
   if (align > 16)
     align = 16;
 
-  if (aarch64_debug)
-    {
-      debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
-		    info->nsaa);
-    }
+  aarch64_debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
+			info->nsaa);
 
   item.len = len;
   item.data = buf;
@@ -1833,13 +1821,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   /* The struct_return pointer occupies X8.  */
   if (return_method != return_method_normal)
     {
-      if (aarch64_debug)
-	{
-	  debug_printf ("struct return in %s = 0x%s\n",
-			gdbarch_register_name (gdbarch,
-					       AARCH64_STRUCT_RETURN_REGNUM),
-			paddress (gdbarch, struct_addr));
-	}
+      aarch64_debug_printf
+	("struct return in %s = 0x%s",
+	 gdbarch_register_name (gdbarch, AARCH64_STRUCT_RETURN_REGNUM),
+	 paddress (gdbarch, struct_addr));
+
       regcache_cooked_write_unsigned (regcache, AARCH64_STRUCT_RETURN_REGNUM,
 				      struct_addr);
     }
@@ -2246,12 +2232,10 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
 	  gdb_byte buf[register_size (gdbarch, regno)];
 	  gdb_assert (len <= sizeof (buf));
 
-	  if (aarch64_debug)
-	    {
-	      debug_printf ("read HFA or HVA return value element %d from %s\n",
-			    i + 1,
-			    gdbarch_register_name (gdbarch, regno));
-	    }
+	  aarch64_debug_printf
+	    ("read HFA or HVA return value element %d from %s",
+	     i + 1, gdbarch_register_name (gdbarch, regno));
+
 	  regs->cooked_read (regno, buf);
 
 	  memcpy (valbuf, buf, len);
@@ -2358,12 +2342,9 @@ aarch64_store_return_value (struct type *type, struct regcache *regs,
 	  gdb_byte tmpbuf[register_size (gdbarch, regno)];
 	  gdb_assert (len <= sizeof (tmpbuf));
 
-	  if (aarch64_debug)
-	    {
-	      debug_printf ("write HFA or HVA return value element %d to %s\n",
-			    i + 1,
-			    gdbarch_register_name (gdbarch, regno));
-	    }
+	  aarch64_debug_printf
+	    ("write HFA or HVA return value element %d to %s",
+	     i + 1, gdbarch_register_name (gdbarch, regno));
 
 	  memcpy (tmpbuf, valbuf,
 		  len > V_REGISTER_SIZE ? V_REGISTER_SIZE : len);
@@ -2438,8 +2419,7 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
     {
       if (aarch64_return_in_memory (gdbarch, valtype))
 	{
-	  if (aarch64_debug)
-	    debug_printf ("return value in memory\n");
+	  aarch64_debug_printf ("return value in memory");
 	  return RETURN_VALUE_STRUCT_CONVENTION;
 	}
     }
@@ -2450,8 +2430,7 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
   if (readbuf)
     aarch64_extract_return_value (valtype, regcache, readbuf);
 
-  if (aarch64_debug)
-    debug_printf ("return value in registers\n");
+  aarch64_debug_printf ("return value in registers");
 
   return RETURN_VALUE_REGISTER_CONVENTION;
 }
diff --git a/gdb/arch/aarch64-insn.c b/gdb/arch/aarch64-insn.c
index 125288909137..6a5abf729bb2 100644
--- a/gdb/arch/aarch64-insn.c
+++ b/gdb/arch/aarch64-insn.c
@@ -69,12 +69,9 @@ aarch64_decode_adr (CORE_ADDR addr, uint32_t insn, int *is_adrp,
       else
 	*offset = (immhi | immlo);
 
-      if (aarch64_debug)
-	{
-	  debug_printf ("decode: 0x%s 0x%x %s x%u, #?\n",
-			core_addr_to_string_nz (addr), insn,
-			*is_adrp ?  "adrp" : "adr", *rd);
-	}
+      aarch64_debug_printf ("decode: 0x%s 0x%x %s x%u, #?",
+			    core_addr_to_string_nz (addr), insn,
+			    *is_adrp ?  "adrp" : "adr", *rd);
       return 1;
     }
   return 0;
diff --git a/gdb/arch/aarch64-insn.h b/gdb/arch/aarch64-insn.h
index 57aeb23feab1..1e8c5eac940e 100644
--- a/gdb/arch/aarch64-insn.h
+++ b/gdb/arch/aarch64-insn.h
@@ -21,6 +21,11 @@
 
 extern bool aarch64_debug;
 
+/* Print an "aarch64" debug statement.  */
+
+#define aarch64_debug_printf(fmt, ...) \
+  debug_prefixed_printf_cond (aarch64_debug, "aarch64", fmt, ##__VA_ARGS__)
+
 /* Support routines for instruction parsing.  */
 
 /* Create a mask of X bits.  */
-- 
2.29.2


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

* [PATCH 4/5] gdb: convert solib-aix to new-style debug macros
  2021-01-09  4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
  2021-01-09  4:28 ` [PATCH 2/5] gdb: convert jit to new-style debug macros Simon Marchi
  2021-01-09  4:28 ` [PATCH 3/5] gdb: convert aarch64 " Simon Marchi
@ 2021-01-09  4:28 ` Simon Marchi
  2021-01-11 21:19   ` Tom Tromey
  2021-01-09  4:28 ` [PATCH 5/5] gdb: convert arc " Simon Marchi
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-01-09  4:28 UTC (permalink / raw)
  To: gdb-patches

This is only compile-tested.

gdb/ChangeLog:

	* solib-aix.c (solib_aix_debug_printf): New, use throughout
	file.

Change-Id: I7ec4baa15ab5b8ad786212b8b9de61c2c447bac1
---
 gdb/solib-aix.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 92a3345b06e2..a0dbdde4d49b 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -32,6 +32,11 @@
    this module.  */
 static bool solib_aix_debug;
 
+/* Print an "aix-solib" debug statement.  */
+
+#define solib_aix_debug_printf(fmt, ...) \
+  debug_prefixed_printf_cond (solib_aix_debug, "aix-solib",fmt, ##__VA_ARGS__)
+
 /* Our private data in struct so_list.  */
 
 struct lm_info_aix : public lm_info_base
@@ -250,10 +255,8 @@ solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
       return data->library_list;
     }
 
-  if (solib_aix_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"DEBUG: TARGET_OBJECT_LIBRARIES_AIX = \n%s\n",
-			library_document->data ());
+  solib_aix_debug_printf ("TARGET_OBJECT_LIBRARIES_AIX = %s",
+			  library_document->data ());
 
   data->library_list = solib_aix_parse_libraries (library_document->data ());
   if (!data->library_list.has_value () && warning_msg != NULL)
@@ -374,9 +377,7 @@ solib_aix_free_so (struct so_list *so)
 {
   lm_info_aix *li = (lm_info_aix *) so->lm_info;
 
-  if (solib_aix_debug)
-    fprintf_unfiltered (gdb_stdlog, "DEBUG: solib_aix_free_so (%s)\n",
-			so->so_name);
+  solib_aix_debug_printf ("%s", so->so_name);
 
   delete li;
 }
@@ -685,11 +686,9 @@ solib_aix_get_toc_value (CORE_ADDR pc)
 
   result = (obj_section_addr (data_osect)
 	    + xcoff_get_toc_offset (pc_osect->objfile));
-  if (solib_aix_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"DEBUG: solib_aix_get_toc_value (pc=%s) -> %s\n",
-			core_addr_to_string (pc),
-			core_addr_to_string (result));
+
+  solib_aix_debug_printf ("pc=%s -> %s", core_addr_to_string (pc),
+			  core_addr_to_string (result));
 
   return result;
 }
-- 
2.29.2


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

* [PATCH 5/5] gdb: convert arc to new-style debug macros
  2021-01-09  4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
                   ` (2 preceding siblings ...)
  2021-01-09  4:28 ` [PATCH 4/5] gdb: convert solib-aix " Simon Marchi
@ 2021-01-09  4:28 ` Simon Marchi
  2021-01-09  4:37   ` Simon Marchi
  2021-01-09  4:29 ` [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-01-09  4:28 UTC (permalink / raw)
  To: gdb-patches

Add the standard arc_debug_printf, but also arc_debug_printf_v
(verbose), for when arc_debug is >= 2.  This is similar to the existing
dwarf_read_debug_print_v.

gdb/ChangeLog:

	* arc-tdep.h (arc_debug_printf, arc_debug_printf_v): New.
	* arc-tdep.c: Use arc_debug_printf instead of arc_debug +
	debug_printf.
	* arc-linux-nat.c: Likewise.
	* arc-linux-tdep.c: Likewise.
	* arc-newlib-tdep.c: Likewise.

Change-Id: I5d937566ed7a1925f7982e8809802c8f0560d8c6
---
 gdb/arc-linux-nat.c   |  11 +---
 gdb/arc-linux-tdep.c  |  30 +++-------
 gdb/arc-newlib-tdep.c |   6 +-
 gdb/arc-tdep.c        | 135 ++++++++++++++++--------------------------
 gdb/arc-tdep.h        |  10 ++++
 5 files changed, 76 insertions(+), 116 deletions(-)

diff --git a/gdb/arc-linux-nat.c b/gdb/arc-linux-nat.c
index db5d44312a68..d55623e0e9cf 100644
--- a/gdb/arc-linux-nat.c
+++ b/gdb/arc-linux-nat.c
@@ -219,9 +219,7 @@ void
 fill_fpregset (const struct regcache *regcache,
 	       gdb_fpregset_t *fpregsetp, int regnum)
 {
-  if (arc_debug)
-    debug_printf ("arc-linux-nat: fill_fpregset called.");
-  return;
+  arc_debug_printf ("called");
 }
 
 /* ARC doesn't have separate FP registers.  This function is exported
@@ -230,9 +228,7 @@ fill_fpregset (const struct regcache *regcache,
 void
 supply_fpregset (struct regcache *regcache, const gdb_fpregset_t *fpregsetp)
 {
-  if (arc_debug)
-    debug_printf ("arc-linux-nat: supply_fpregset called.");
-  return;
+  arc_debug_printf ("called");
 }
 
 /* Implement the "read_description" method of linux_nat_target.  */
@@ -295,8 +291,7 @@ ps_err_e
 ps_get_thread_area (struct ps_prochandle *ph, lwpid_t lwpid, int idx,
 		    void **base)
 {
-  if (arc_debug >= 2)
-    debug_printf ("arc-linux-nat: ps_get_thread_area called");
+  arc_debug_printf_v ("called");
 
   if (ptrace (PTRACE_GET_THREAD_AREA, lwpid, NULL, base) != 0)
     return PS_ERR;
diff --git a/gdb/arc-linux-tdep.c b/gdb/arc-linux-tdep.c
index f29279d8fba8..28fa7a791009 100644
--- a/gdb/arc-linux-tdep.c
+++ b/gdb/arc-linux-tdep.c
@@ -158,11 +158,7 @@ arc_linux_is_sigtramp (struct frame_info *this_frame)
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   CORE_ADDR pc = get_frame_pc (this_frame);
 
-  if (arc_debug)
-    {
-      debug_printf ("arc-linux: arc_linux_is_sigtramp, pc=%s\n",
-		    paddress(gdbarch, pc));
-    }
+  arc_debug_printf ("pc=%s", paddress(gdbarch, pc));
 
   static const gdb_byte insns_be_hs[] = {
     0x20, 0x8a, 0x12, 0xc2,	/* mov  r8,nr_rt_sigreturn */
@@ -383,15 +379,12 @@ arc_linux_software_single_step (struct regcache *regcache)
 	  regcache_cooked_read_unsigned (regcache, ARC_LP_COUNT_REGNUM,
 					 &lp_count);
 
-	  if (arc_debug)
-	    {
-	      debug_printf ("arc-linux: lp_start = %s, lp_end = %s, "
-			    "lp_count = %s, next_pc = %s\n",
+	  arc_debug_printf ("lp_start = %s, lp_end = %s, "
+			    "lp_count = %s, next_pc = %s",
 			    paddress (gdbarch, lp_start),
 			    paddress (gdbarch, lp_end),
 			    pulongest (lp_count),
 			    paddress (gdbarch, next_pc));
-	    }
 
 	  if (next_pc == lp_end && lp_count > 1)
 	    {
@@ -436,17 +429,13 @@ arc_linux_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
       if (resolver.minsym != nullptr)
 	{
 	  CORE_ADDR res_addr = BMSYMBOL_VALUE_ADDRESS (resolver);
-	  debug_printf ("arc-linux: skip_solib_resolver (): "
-			"pc = %s, resolver at %s\n",
-			print_core_address (gdbarch, pc),
-			print_core_address (gdbarch, res_addr));
+	  arc_debug_printf ("pc = %s, resolver at %s",
+			    print_core_address (gdbarch, pc),
+			    print_core_address (gdbarch, res_addr));
 	}
       else
-	{
-	  debug_printf ("arc-linux: skip_solib_resolver (): "
-			"pc = %s, no resolver found\n",
-			print_core_address (gdbarch, pc));
-	}
+	arc_debug_printf ("pc = %s, no resolver found",
+			  print_core_address (gdbarch, pc));
     }
 
   if (resolver.minsym != nullptr && BMSYMBOL_VALUE_ADDRESS (resolver) == pc)
@@ -625,8 +614,7 @@ arc_linux_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  if (arc_debug)
-    debug_printf ("arc-linux: GNU/Linux OS/ABI initialization.\n");
+  arc_debug_printf ("GNU/Linux OS/ABI initialization.");
 
   /* Fill in target-dependent info in ARC-private structure.  */
   tdep->is_sigtramp = arc_linux_is_sigtramp;
diff --git a/gdb/arc-newlib-tdep.c b/gdb/arc-newlib-tdep.c
index 8a83aaf6313a..d3ef496ff3a7 100644
--- a/gdb/arc-newlib-tdep.c
+++ b/gdb/arc-newlib-tdep.c
@@ -29,8 +29,7 @@
 static void
 arc_newlib_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
-  if (arc_debug)
-    debug_printf ("arc-newlib: Initialization.\n");
+  arc_debug_printf ("Initialization.");
 
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
@@ -44,8 +43,7 @@ arc_newlib_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
 static enum gdb_osabi
 arc_newlib_osabi_sniffer (bfd *abfd)
 {
-  if (arc_debug)
-    debug_printf ("arc-newlib: OS/ABI sniffer.\n");
+  arc_debug_printf ("OS/ABI sniffer.");
 
   /* crt0.S in libgloss for ARC defines .ivt section for interrupt handlers.
      If this section is not present then this is likely not a newlib - could be
diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index 9b8457112111..3e09d5f89021 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -601,9 +601,8 @@ arc_write_pc (struct regcache *regcache, CORE_ADDR new_pc)
 {
   struct gdbarch *gdbarch = regcache->arch ();
 
-  if (arc_debug)
-    debug_printf ("arc: Writing PC, new value=%s\n",
-		  paddress (gdbarch, new_pc));
+  arc_debug_printf ("Writing PC, new value=%s",
+		    paddress (gdbarch, new_pc));
 
   regcache_cooked_write_unsigned (regcache, gdbarch_pc_regnum (gdbarch),
 				  new_pc);
@@ -614,13 +613,10 @@ arc_write_pc (struct regcache *regcache, CORE_ADDR new_pc)
 
   if ((status32 & ARC_STATUS32_DE_MASK) != 0)
     {
-      if (arc_debug)
-	{
-	  debug_printf ("arc: Changing PC while in delay slot.  Will "
+      arc_debug_printf ("Changing PC while in delay slot.  Will "
 			"reset STATUS32.DE bit to zero.  Value of STATUS32 "
-			"register is 0x%s\n",
+			"register is 0x%s",
 			phex (status32, ARC_REGISTER_SIZE));
-	}
 
       /* Reset bit and write to the cache.  */
       status32 &= ~0x40;
@@ -734,8 +730,7 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		     function_call_return_method return_method,
 		     CORE_ADDR struct_addr)
 {
-  if (arc_debug)
-    debug_printf ("arc: push_dummy_call (nargs = %d)\n", nargs);
+  arc_debug_printf ("nargs = %d", nargs);
 
   int arg_reg = ARC_FIRST_ARG_REGNUM;
 
@@ -751,9 +746,8 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       /* Pass the return address in the first argument register.  */
       regcache_cooked_write_unsigned (regcache, arg_reg, struct_addr);
 
-      if (arc_debug)
-	debug_printf ("arc: struct return address %s passed in R%d",
-		      print_core_address (gdbarch, struct_addr), arg_reg);
+      arc_debug_printf ("struct return address %s passed in R%d",
+			print_core_address (gdbarch, struct_addr), arg_reg);
 
       arg_reg++;
     }
@@ -771,8 +765,7 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
 	  total_space += space;
 
-	  if (arc_debug)
-	    debug_printf ("arc: arg %d: %u bytes -> %u\n", i, len, space);
+	  arc_debug_printf ("arg %d: %u bytes -> %u", i, len, space);
 	}
 
       /* Allocate a buffer to hold a memory image of the arguments.  */
@@ -786,9 +779,8 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	  unsigned int space = align_up (len, 4);
 
 	  memcpy (data, value_contents (args[i]), (size_t) len);
-	  if (arc_debug)
-	    debug_printf ("arc: copying arg %d, val 0x%08x, len %d to mem\n",
-			  i, *((int *) value_contents (args[i])), len);
+	  arc_debug_printf ("copying arg %d, val 0x%08x, len %d to mem",
+			    i, *((int *) value_contents (args[i])), len);
 
 	  data += space;
 	}
@@ -797,9 +789,8 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       data = memory_image;
       while (arg_reg <= ARC_LAST_ARG_REGNUM)
 	{
-	  if (arc_debug)
-	    debug_printf ("arc: passing 0x%02x%02x%02x%02x in register R%d\n",
-			  data[0], data[1], data[2], data[3], arg_reg);
+	  arc_debug_printf ("passing 0x%02x%02x%02x%02x in register R%d",
+			    data[0], data[1], data[2], data[3], arg_reg);
 
 	  /* Note we don't use write_unsigned here, since that would convert
 	     the byte order, but we are already in the correct byte order.  */
@@ -819,8 +810,7 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	 operation).  */
       if (total_space > 0)
 	{
-	  if (arc_debug)
-	    debug_printf ("arc: passing %d bytes on stack\n", total_space);
+	  arc_debug_printf ("passing %d bytes on stack\n", total_space);
 
 	  sp -= total_space;
 	  write_memory (sp, data, (int) total_space);
@@ -916,8 +906,7 @@ arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
 {
   unsigned int len = TYPE_LENGTH (type);
 
-  if (arc_debug)
-    debug_printf ("arc: extract_return_value\n");
+  arc_debug_printf ("called");
 
   if (len <= ARC_REGISTER_SIZE)
     {
@@ -928,8 +917,7 @@ arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
       store_unsigned_integer (valbuf, (int) len,
 			      gdbarch_byte_order (gdbarch), val);
 
-      if (arc_debug)
-	debug_printf ("arc: returning 0x%s\n", phex (val, ARC_REGISTER_SIZE));
+      arc_debug_printf ("returning 0x%s", phex (val, ARC_REGISTER_SIZE));
     }
   else if (len <= ARC_REGISTER_SIZE * 2)
     {
@@ -945,10 +933,9 @@ arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
 			      (int) len - ARC_REGISTER_SIZE,
 			      gdbarch_byte_order (gdbarch), high);
 
-      if (arc_debug)
-	debug_printf ("arc: returning 0x%s%s\n",
-		      phex (high, ARC_REGISTER_SIZE),
-		      phex (low, ARC_REGISTER_SIZE));
+      arc_debug_printf ("returning 0x%s%s",
+			phex (high, ARC_REGISTER_SIZE),
+			phex (low, ARC_REGISTER_SIZE));
     }
   else
     error (_("arc: extract_return_value: type length %u too large"), len);
@@ -970,8 +957,7 @@ arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
 {
   unsigned int len = TYPE_LENGTH (type);
 
-  if (arc_debug)
-    debug_printf ("arc: store_return_value\n");
+  arc_debug_printf ("called");
 
   if (len <= ARC_REGISTER_SIZE)
     {
@@ -982,8 +968,7 @@ arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
 				      gdbarch_byte_order (gdbarch));
       regcache_cooked_write_unsigned (regcache, ARC_R0_REGNUM, val);
 
-      if (arc_debug)
-	debug_printf ("arc: storing 0x%s\n", phex (val, ARC_REGISTER_SIZE));
+      arc_debug_printf ("storing 0x%s", phex (val, ARC_REGISTER_SIZE));
     }
   else if (len <= ARC_REGISTER_SIZE * 2)
     {
@@ -999,10 +984,9 @@ arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
       regcache_cooked_write_unsigned (regcache, ARC_R0_REGNUM, low);
       regcache_cooked_write_unsigned (regcache, ARC_R1_REGNUM, high);
 
-      if (arc_debug)
-	debug_printf ("arc: storing 0x%s%s\n",
-		      phex (high, ARC_REGISTER_SIZE),
-		      phex (low, ARC_REGISTER_SIZE));
+      arc_debug_printf ("storing 0x%s%s",
+			phex (high, ARC_REGISTER_SIZE),
+			phex (low, ARC_REGISTER_SIZE));
     }
   else
     error (_("arc_store_return_value: type length too large."));
@@ -1013,8 +997,7 @@ arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
 static int
 arc_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
 {
-  if (arc_debug)
-    debug_printf ("arc: get_longjmp_target\n");
+  arc_debug_printf ("called");
 
   struct gdbarch *gdbarch = get_frame_arch (frame);
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
@@ -1046,10 +1029,9 @@ arc_return_value (struct gdbarch *gdbarch, struct value *function,
 			  || valtype->code () == TYPE_CODE_UNION
 			  || TYPE_LENGTH (valtype) > 2 * ARC_REGISTER_SIZE);
 
-  if (arc_debug)
-    debug_printf ("arc: return_value (readbuf = %s, writebuf = %s)\n",
-		  host_address_to_string (readbuf),
-		  host_address_to_string (writebuf));
+  arc_debug_printf ("readbuf = %s, writebuf = %s",
+		    host_address_to_string (readbuf),
+		    host_address_to_string (writebuf));
 
   if (writebuf != NULL)
     {
@@ -1400,10 +1382,9 @@ static CORE_ADDR
 arc_analyze_prologue (struct gdbarch *gdbarch, const CORE_ADDR entrypoint,
 		      const CORE_ADDR limit_pc, struct arc_frame_cache *cache)
 {
-  if (arc_debug)
-    debug_printf ("arc: analyze_prologue (entrypoint=%s, limit_pc=%s)\n",
-		  paddress (gdbarch, entrypoint),
-		  paddress (gdbarch, limit_pc));
+  arc_debug_printf ("entrypoint=%s, limit_pc=%s",
+		    paddress (gdbarch, entrypoint),
+		    paddress (gdbarch, limit_pc));
 
   /* Prologue values.  Only core registers can be stored.  */
   pv_t regs[ARC_LAST_CORE_REGNUM + 1];
@@ -1429,9 +1410,8 @@ arc_analyze_prologue (struct gdbarch *gdbarch, const CORE_ADDR entrypoint,
       if (!arc_is_in_prologue (gdbarch, insn, regs, &stack))
 	{
 	  /* Found an instruction that is not in the prologue.  */
-	  if (arc_debug)
-	    debug_printf ("arc: End of prologue reached at address %s\n",
-			  paddress (gdbarch, insn.address));
+	  arc_debug_printf ("End of prologue reached at address %s",
+			    paddress (gdbarch, insn.address));
 	  break;
 	}
 
@@ -1492,8 +1472,7 @@ const static int MAX_PROLOGUE_LENGTH
 static CORE_ADDR
 arc_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
-  if (arc_debug)
-    debug_printf ("arc: skip_prologue\n");
+  arc_debug_printf ("called");
 
   CORE_ADDR func_addr;
   const char *func_name;
@@ -1665,19 +1644,19 @@ static void
 arc_print_frame_cache (struct gdbarch *gdbarch, const char *message,
 		       struct arc_frame_cache *cache, int addresses_known)
 {
-  debug_printf ("arc: frame_info %s\n", message);
-  debug_printf ("arc: prev_sp = %s\n", paddress (gdbarch, cache->prev_sp));
-  debug_printf ("arc: frame_base_reg = %i\n", cache->frame_base_reg);
-  debug_printf ("arc: frame_base_offset = %s\n",
-		plongest (cache->frame_base_offset));
+  arc_debug_printf ("frame_info %s", message);
+  arc_debug_printf ("prev_sp = %s", paddress (gdbarch, cache->prev_sp));
+  arc_debug_printf ("frame_base_reg = %i", cache->frame_base_reg);
+  arc_debug_printf ("frame_base_offset = %s",
+		    plongest (cache->frame_base_offset));
 
   for (int i = 0; i <= ARC_BLINK_REGNUM; i++)
     {
       if (trad_frame_addr_p (cache->saved_regs, i))
-	debug_printf ("arc: saved register %s at %s %s\n",
-		      gdbarch_register_name (gdbarch, i),
-		      (addresses_known) ? "address" : "offset",
-		      paddress (gdbarch, cache->saved_regs[i].addr ()));
+	arc_debug_printf ("saved register %s at %s %s",
+			  gdbarch_register_name (gdbarch, i),
+			  (addresses_known) ? "address" : "offset",
+			      paddress (gdbarch, cache->saved_regs[i].addr ()));
     }
 }
 
@@ -1686,8 +1665,7 @@ arc_print_frame_cache (struct gdbarch *gdbarch, const char *message,
 static struct arc_frame_cache *
 arc_make_frame_cache (struct frame_info *this_frame)
 {
-  if (arc_debug)
-    debug_printf ("arc: frame_cache\n");
+  arc_debug_printf ("called");
 
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
 
@@ -1755,8 +1733,7 @@ static void
 arc_frame_this_id (struct frame_info *this_frame, void **this_cache,
 		   struct frame_id *this_id)
 {
-  if (arc_debug)
-    debug_printf ("arc: frame_this_id\n");
+  arc_debug_printf ("called");
 
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
 
@@ -1851,8 +1828,7 @@ arc_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
 static struct arc_frame_cache *
 arc_make_sigtramp_frame_cache (struct frame_info *this_frame)
 {
-  if (arc_debug)
-    debug_printf ("arc: sigtramp_frame_cache\n");
+  arc_debug_printf ("called");
 
   struct gdbarch_tdep *tdep = gdbarch_tdep (get_frame_arch (this_frame));
 
@@ -1891,8 +1867,7 @@ static void
 arc_sigtramp_frame_this_id (struct frame_info *this_frame,
 			    void **this_cache, struct frame_id *this_id)
 {
-  if (arc_debug)
-    debug_printf ("arc: sigtramp_frame_this_id\n");
+  arc_debug_printf ("called");
 
   if (*this_cache == NULL)
     *this_cache = arc_make_sigtramp_frame_cache (this_frame);
@@ -1911,8 +1886,7 @@ static struct value *
 arc_sigtramp_frame_prev_register (struct frame_info *this_frame,
 				  void **this_cache, int regnum)
 {
-  if (arc_debug)
-    debug_printf ("arc: sigtramp_frame_prev_register (regnum = %d)\n", regnum);
+  arc_debug_printf ("regnum = %d", regnum);
 
   /* Make sure we've initialized the cache.  */
   if (*this_cache == NULL)
@@ -1932,8 +1906,7 @@ arc_sigtramp_frame_sniffer (const struct frame_unwind *self,
 {
   struct gdbarch_tdep *tdep;
 
-  if (arc_debug)
-    debug_printf ("arc: sigtramp_frame_sniffer\n");
+  arc_debug_printf ("called");
 
   tdep = gdbarch_tdep (get_frame_arch (this_frame));
 
@@ -2193,8 +2166,7 @@ arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
 		tdesc_arch_data_up *tdesc_data)
 {
   const struct target_desc *tdesc_loc = info.target_desc;
-  if (arc_debug)
-    debug_printf ("arc: Target description initialization.\n");
+  arc_debug_printf ("Target description initialization.");
 
   /* If target doesn't provide a description, use the default ones.  */
   if (!tdesc_has_registers (tdesc_loc))
@@ -2206,8 +2178,7 @@ arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
     }
   gdb_assert (tdesc_loc != nullptr);
 
-  if (arc_debug)
-    debug_printf ("arc: Have got a target description\n");
+  arc_debug_printf ("Have got a target description");
 
   const struct tdesc_feature *feature_core
     = tdesc_find_feature (tdesc_loc, ARC_CORE_FEATURE_NAME);
@@ -2253,8 +2224,7 @@ arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
 
   if (!valid_p)
     {
-      if (arc_debug)
-	debug_printf ("arc: Target description is not valid\n");
+      arc_debug_printf ("Target description is not valid");
       return false;
     }
 
@@ -2300,8 +2270,7 @@ arc_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   const struct target_desc *tdesc;
   tdesc_arch_data_up tdesc_data;
 
-  if (arc_debug)
-    debug_printf ("arc: Architecture initialization.\n");
+  arc_debug_printf ("Architecture initialization.");
 
   if (!arc_tdesc_init (info, &tdesc, &tdesc_data))
     return nullptr;
diff --git a/gdb/arc-tdep.h b/gdb/arc-tdep.h
index 70fc3d95c483..43a801471772 100644
--- a/gdb/arc-tdep.h
+++ b/gdb/arc-tdep.h
@@ -114,6 +114,16 @@ enum arc_regnum
 
 extern int arc_debug;
 
+/* Print an "arc" debug statement if arc_debug is >= 1.  */
+
+#define arc_debug_printf(fmt, ...) \
+  debug_prefixed_printf_cond (arc_debug >= 1, "arc", fmt, ##__VA_ARGS__)
+
+/* Print an "arc" debug statement if arc_debug is >= 2.  */
+
+#define arc_debug_printf_v(fmt, ...) \
+  debug_prefixed_printf_cond (arc_debug >= 2, "arc", fmt, ##__VA_ARGS__)
+
 /* Target-dependent information.  */
 
 struct gdbarch_tdep
-- 
2.29.2


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

* Re: [PATCH 1/5] gdb: change jit_debug to a bool
  2021-01-09  4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
                   ` (3 preceding siblings ...)
  2021-01-09  4:28 ` [PATCH 5/5] gdb: convert arc " Simon Marchi
@ 2021-01-09  4:29 ` Simon Marchi
  2021-01-11 21:17 ` Tom Tromey
  2021-01-13 14:32 ` Shahab Vahedi
  6 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-01-09  4:29 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Gdb-patches, Mihails Strasuns

Hi Mihails,

Since you deal with JIT a lot, would you mind giving this patch a look?

Thanks!

Simon

On 2021-01-08 11:28 p.m., Simon Marchi wrote:
> gdb/ChangeLog:
> 
> 	* jit.c (jit_debug): Change type to bool.
> 	(_initialize_jit): Adjust.
> 
> Change-Id: Ic2b1eec28eafe8ccb2899f38ddc91ba9703cb38e
> ---
>  gdb/jit.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/jit.c b/gdb/jit.c
> index ad951cdfbb52..474ba838b86b 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -58,9 +58,9 @@ static void jit_inferior_exit_hook (struct inferior *inf);
>  
>  static struct gdbarch_data *jit_gdbarch_data;
>  
> -/* Non-zero if we want to see trace of jit level stuff.  */
> +/* True if we want to see trace of jit level stuff.  */
>  
> -static unsigned int jit_debug = 0;
> +static bool jit_debug = false;
>  
>  static void
>  show_jit_debug (struct ui_file *file, int from_tty,
> @@ -1251,13 +1251,13 @@ _initialize_jit ()
>  {
>    jit_reader_dir = relocate_gdb_directory (JIT_READER_DIR,
>  					   JIT_READER_DIR_RELOCATABLE);
> -  add_setshow_zuinteger_cmd ("jit", class_maintenance, &jit_debug,
> -			     _("Set JIT debugging."),
> -			     _("Show JIT debugging."),
> -			     _("When non-zero, JIT debugging is enabled."),
> -			     NULL,
> -			     show_jit_debug,
> -			     &setdebuglist, &showdebuglist);
> +  add_setshow_boolean_cmd ("jit", class_maintenance, &jit_debug,
> +			   _("Set JIT debugging."),
> +			   _("Show JIT debugging."),
> +			   _("When non-zero, JIT debugging is enabled."),
> +			   NULL,
> +			   show_jit_debug,
> +			   &setdebuglist, &showdebuglist);
>  
>    gdb::observers::inferior_created.attach (jit_inferior_created_hook);
>    gdb::observers::inferior_execd.attach (jit_inferior_created_hook);
> 

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

* Re: [PATCH 3/5] gdb: convert aarch64 to new-style debug macros
  2021-01-09  4:28 ` [PATCH 3/5] gdb: convert aarch64 " Simon Marchi
@ 2021-01-09  4:31   ` Simon Marchi
  2021-01-11 13:18     ` Luis Machado
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-01-09  4:31 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Gdb-patches, Luis Machado

Hi Luis,

Would you mind giving this patch a quick look?

Thanks!

Simon

On 2021-01-08 11:28 p.m., Simon Marchi wrote:
> I haven't tried this on an actual aarch64 machine, but I am able to
> exercise it like this:
> 
>     (gdb) set debug aarch64
>     (gdb) maintenance selftest aa
>     Running selftest aarch64-analyze-prologue.
>     [aarch64] aarch64_analyze_prologue: prologue analysis gave up addr=0x14 opcode=0xf94013e0
>     Running selftest aarch64-process-record.
>     Ran 2 unit tests, 0 failed
> 
> gdb/ChangeLog:
> 
> 	* arch/aarch64-insn.h (aarch64_debug_printf): New.
> 	* arch/aarch64-insn.c: Use aarch64_debug_printf.
> 	* aarch64-tdep.c: Use aarch64_debug_printf.
> 
> Change-Id: Ifdb40e2816ab8e55a9aabb066d1833d9b5a46094
> ---
>  gdb/aarch64-tdep.c      | 89 ++++++++++++++++-------------------------
>  gdb/arch/aarch64-insn.c |  9 ++---
>  gdb/arch/aarch64-insn.h |  5 +++
>  3 files changed, 42 insertions(+), 61 deletions(-)
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 77e6ad700fcd..a51b3341e789 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -388,12 +388,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>  	    regs[rd] = regs[rm];
>  	  else
>  	    {
> -	      if (aarch64_debug)
> -		{
> -		  debug_printf ("aarch64: prologue analysis gave up "
> -				"addr=%s opcode=0x%x (orr x register)\n",
> -				core_addr_to_string_nz (start), insn);
> -		}
> +	      aarch64_debug_printf ("prologue analysis gave up "
> +				    "addr=%s opcode=0x%x (orr x register)",
> +				    core_addr_to_string_nz (start), insn);
> +
>  	      break;
>  	    }
>  	}
> @@ -513,10 +511,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>  	    }
>  	  else
>  	    {
> -	      if (aarch64_debug)
> -		debug_printf ("aarch64: prologue analysis gave up addr=%s"
> -			      " opcode=0x%x (iclass)\n",
> -			      core_addr_to_string_nz (start), insn);
> +	      aarch64_debug_printf ("prologue analysis gave up addr=%s"
> +				    " opcode=0x%x (iclass)",
> +				    core_addr_to_string_nz (start), insn);
>  	      break;
>  	    }
>  
> @@ -527,12 +524,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>  	}
>        else
>  	{
> -	  if (aarch64_debug)
> -	    {
> -	      debug_printf ("aarch64: prologue analysis gave up addr=%s"
> -			    " opcode=0x%x\n",
> -			    core_addr_to_string_nz (start), insn);
> -	    }
> +	  aarch64_debug_printf ("prologue analysis gave up addr=%s"
> +				" opcode=0x%x",
> +				core_addr_to_string_nz (start), insn);
> +
>  	  break;
>  	}
>      }
> @@ -1606,12 +1601,10 @@ pass_in_x (struct gdbarch *gdbarch, struct regcache *regcache,
>  	  && (typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION))
>  	regval <<= ((X_REGISTER_SIZE - partial_len) * TARGET_CHAR_BIT);
>  
> -      if (aarch64_debug)
> -	{
> -	  debug_printf ("arg %d in %s = 0x%s\n", info->argnum,
> -			gdbarch_register_name (gdbarch, regnum),
> -			phex (regval, X_REGISTER_SIZE));
> -	}
> +      aarch64_debug_printf ("arg %d in %s = 0x%s", info->argnum,
> +			    gdbarch_register_name (gdbarch, regnum),
> +			    phex (regval, X_REGISTER_SIZE));
> +
>        regcache_cooked_write_unsigned (regcache, regnum, regval);
>        len -= partial_len;
>        buf += partial_len;
> @@ -1646,11 +1639,9 @@ pass_in_v (struct gdbarch *gdbarch,
>        memcpy (reg, buf, len);
>        regcache->cooked_write (regnum, reg);
>  
> -      if (aarch64_debug)
> -	{
> -	  debug_printf ("arg %d in %s\n", info->argnum,
> -			gdbarch_register_name (gdbarch, regnum));
> -	}
> +      aarch64_debug_printf ("arg %d in %s", info->argnum,
> +			    gdbarch_register_name (gdbarch, regnum));
> +
>        return 1;
>      }
>    info->nsrn = 8;
> @@ -1680,11 +1671,8 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
>    if (align > 16)
>      align = 16;
>  
> -  if (aarch64_debug)
> -    {
> -      debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
> -		    info->nsaa);
> -    }
> +  aarch64_debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
> +			info->nsaa);
>  
>    item.len = len;
>    item.data = buf;
> @@ -1833,13 +1821,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>    /* The struct_return pointer occupies X8.  */
>    if (return_method != return_method_normal)
>      {
> -      if (aarch64_debug)
> -	{
> -	  debug_printf ("struct return in %s = 0x%s\n",
> -			gdbarch_register_name (gdbarch,
> -					       AARCH64_STRUCT_RETURN_REGNUM),
> -			paddress (gdbarch, struct_addr));
> -	}
> +      aarch64_debug_printf
> +	("struct return in %s = 0x%s",
> +	 gdbarch_register_name (gdbarch, AARCH64_STRUCT_RETURN_REGNUM),
> +	 paddress (gdbarch, struct_addr));
> +
>        regcache_cooked_write_unsigned (regcache, AARCH64_STRUCT_RETURN_REGNUM,
>  				      struct_addr);
>      }
> @@ -2246,12 +2232,10 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
>  	  gdb_byte buf[register_size (gdbarch, regno)];
>  	  gdb_assert (len <= sizeof (buf));
>  
> -	  if (aarch64_debug)
> -	    {
> -	      debug_printf ("read HFA or HVA return value element %d from %s\n",
> -			    i + 1,
> -			    gdbarch_register_name (gdbarch, regno));
> -	    }
> +	  aarch64_debug_printf
> +	    ("read HFA or HVA return value element %d from %s",
> +	     i + 1, gdbarch_register_name (gdbarch, regno));
> +
>  	  regs->cooked_read (regno, buf);
>  
>  	  memcpy (valbuf, buf, len);
> @@ -2358,12 +2342,9 @@ aarch64_store_return_value (struct type *type, struct regcache *regs,
>  	  gdb_byte tmpbuf[register_size (gdbarch, regno)];
>  	  gdb_assert (len <= sizeof (tmpbuf));
>  
> -	  if (aarch64_debug)
> -	    {
> -	      debug_printf ("write HFA or HVA return value element %d to %s\n",
> -			    i + 1,
> -			    gdbarch_register_name (gdbarch, regno));
> -	    }
> +	  aarch64_debug_printf
> +	    ("write HFA or HVA return value element %d to %s",
> +	     i + 1, gdbarch_register_name (gdbarch, regno));
>  
>  	  memcpy (tmpbuf, valbuf,
>  		  len > V_REGISTER_SIZE ? V_REGISTER_SIZE : len);
> @@ -2438,8 +2419,7 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
>      {
>        if (aarch64_return_in_memory (gdbarch, valtype))
>  	{
> -	  if (aarch64_debug)
> -	    debug_printf ("return value in memory\n");
> +	  aarch64_debug_printf ("return value in memory");
>  	  return RETURN_VALUE_STRUCT_CONVENTION;
>  	}
>      }
> @@ -2450,8 +2430,7 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
>    if (readbuf)
>      aarch64_extract_return_value (valtype, regcache, readbuf);
>  
> -  if (aarch64_debug)
> -    debug_printf ("return value in registers\n");
> +  aarch64_debug_printf ("return value in registers");
>  
>    return RETURN_VALUE_REGISTER_CONVENTION;
>  }
> diff --git a/gdb/arch/aarch64-insn.c b/gdb/arch/aarch64-insn.c
> index 125288909137..6a5abf729bb2 100644
> --- a/gdb/arch/aarch64-insn.c
> +++ b/gdb/arch/aarch64-insn.c
> @@ -69,12 +69,9 @@ aarch64_decode_adr (CORE_ADDR addr, uint32_t insn, int *is_adrp,
>        else
>  	*offset = (immhi | immlo);
>  
> -      if (aarch64_debug)
> -	{
> -	  debug_printf ("decode: 0x%s 0x%x %s x%u, #?\n",
> -			core_addr_to_string_nz (addr), insn,
> -			*is_adrp ?  "adrp" : "adr", *rd);
> -	}
> +      aarch64_debug_printf ("decode: 0x%s 0x%x %s x%u, #?",
> +			    core_addr_to_string_nz (addr), insn,
> +			    *is_adrp ?  "adrp" : "adr", *rd);
>        return 1;
>      }
>    return 0;
> diff --git a/gdb/arch/aarch64-insn.h b/gdb/arch/aarch64-insn.h
> index 57aeb23feab1..1e8c5eac940e 100644
> --- a/gdb/arch/aarch64-insn.h
> +++ b/gdb/arch/aarch64-insn.h
> @@ -21,6 +21,11 @@
>  
>  extern bool aarch64_debug;
>  
> +/* Print an "aarch64" debug statement.  */
> +
> +#define aarch64_debug_printf(fmt, ...) \
> +  debug_prefixed_printf_cond (aarch64_debug, "aarch64", fmt, ##__VA_ARGS__)
> +
>  /* Support routines for instruction parsing.  */
>  
>  /* Create a mask of X bits.  */
> 

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

* Re: [PATCH 5/5] gdb: convert arc to new-style debug macros
  2021-01-09  4:28 ` [PATCH 5/5] gdb: convert arc " Simon Marchi
@ 2021-01-09  4:37   ` Simon Marchi
  2021-01-13 15:35     ` Shahab Vahedi
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-01-09  4:37 UTC (permalink / raw)
  To: gdb-patches; +Cc: Shahab Vahedi

Hi Shahab.

Would you mind giving a look at this patch?

Thanks!

Simon

On 2021-01-08 11:28 p.m., Simon Marchi via Gdb-patches wrote:
> Add the standard arc_debug_printf, but also arc_debug_printf_v
> (verbose), for when arc_debug is >= 2.  This is similar to the existing
> dwarf_read_debug_print_v.
> 
> gdb/ChangeLog:
> 
> 	* arc-tdep.h (arc_debug_printf, arc_debug_printf_v): New.
> 	* arc-tdep.c: Use arc_debug_printf instead of arc_debug +
> 	debug_printf.
> 	* arc-linux-nat.c: Likewise.
> 	* arc-linux-tdep.c: Likewise.
> 	* arc-newlib-tdep.c: Likewise.
> 
> Change-Id: I5d937566ed7a1925f7982e8809802c8f0560d8c6
> ---
>  gdb/arc-linux-nat.c   |  11 +---
>  gdb/arc-linux-tdep.c  |  30 +++-------
>  gdb/arc-newlib-tdep.c |   6 +-
>  gdb/arc-tdep.c        | 135 ++++++++++++++++--------------------------
>  gdb/arc-tdep.h        |  10 ++++
>  5 files changed, 76 insertions(+), 116 deletions(-)
> 
> diff --git a/gdb/arc-linux-nat.c b/gdb/arc-linux-nat.c
> index db5d44312a68..d55623e0e9cf 100644
> --- a/gdb/arc-linux-nat.c
> +++ b/gdb/arc-linux-nat.c
> @@ -219,9 +219,7 @@ void
>  fill_fpregset (const struct regcache *regcache,
>  	       gdb_fpregset_t *fpregsetp, int regnum)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc-linux-nat: fill_fpregset called.");
> -  return;
> +  arc_debug_printf ("called");
>  }
>  
>  /* ARC doesn't have separate FP registers.  This function is exported
> @@ -230,9 +228,7 @@ fill_fpregset (const struct regcache *regcache,
>  void
>  supply_fpregset (struct regcache *regcache, const gdb_fpregset_t *fpregsetp)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc-linux-nat: supply_fpregset called.");
> -  return;
> +  arc_debug_printf ("called");
>  }
>  
>  /* Implement the "read_description" method of linux_nat_target.  */
> @@ -295,8 +291,7 @@ ps_err_e
>  ps_get_thread_area (struct ps_prochandle *ph, lwpid_t lwpid, int idx,
>  		    void **base)
>  {
> -  if (arc_debug >= 2)
> -    debug_printf ("arc-linux-nat: ps_get_thread_area called");
> +  arc_debug_printf_v ("called");
>  
>    if (ptrace (PTRACE_GET_THREAD_AREA, lwpid, NULL, base) != 0)
>      return PS_ERR;
> diff --git a/gdb/arc-linux-tdep.c b/gdb/arc-linux-tdep.c
> index f29279d8fba8..28fa7a791009 100644
> --- a/gdb/arc-linux-tdep.c
> +++ b/gdb/arc-linux-tdep.c
> @@ -158,11 +158,7 @@ arc_linux_is_sigtramp (struct frame_info *this_frame)
>    struct gdbarch *gdbarch = get_frame_arch (this_frame);
>    CORE_ADDR pc = get_frame_pc (this_frame);
>  
> -  if (arc_debug)
> -    {
> -      debug_printf ("arc-linux: arc_linux_is_sigtramp, pc=%s\n",
> -		    paddress(gdbarch, pc));
> -    }
> +  arc_debug_printf ("pc=%s", paddress(gdbarch, pc));
>  
>    static const gdb_byte insns_be_hs[] = {
>      0x20, 0x8a, 0x12, 0xc2,	/* mov  r8,nr_rt_sigreturn */
> @@ -383,15 +379,12 @@ arc_linux_software_single_step (struct regcache *regcache)
>  	  regcache_cooked_read_unsigned (regcache, ARC_LP_COUNT_REGNUM,
>  					 &lp_count);
>  
> -	  if (arc_debug)
> -	    {
> -	      debug_printf ("arc-linux: lp_start = %s, lp_end = %s, "
> -			    "lp_count = %s, next_pc = %s\n",
> +	  arc_debug_printf ("lp_start = %s, lp_end = %s, "
> +			    "lp_count = %s, next_pc = %s",
>  			    paddress (gdbarch, lp_start),
>  			    paddress (gdbarch, lp_end),
>  			    pulongest (lp_count),
>  			    paddress (gdbarch, next_pc));
> -	    }
>  
>  	  if (next_pc == lp_end && lp_count > 1)
>  	    {
> @@ -436,17 +429,13 @@ arc_linux_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
>        if (resolver.minsym != nullptr)
>  	{
>  	  CORE_ADDR res_addr = BMSYMBOL_VALUE_ADDRESS (resolver);
> -	  debug_printf ("arc-linux: skip_solib_resolver (): "
> -			"pc = %s, resolver at %s\n",
> -			print_core_address (gdbarch, pc),
> -			print_core_address (gdbarch, res_addr));
> +	  arc_debug_printf ("pc = %s, resolver at %s",
> +			    print_core_address (gdbarch, pc),
> +			    print_core_address (gdbarch, res_addr));
>  	}
>        else
> -	{
> -	  debug_printf ("arc-linux: skip_solib_resolver (): "
> -			"pc = %s, no resolver found\n",
> -			print_core_address (gdbarch, pc));
> -	}
> +	arc_debug_printf ("pc = %s, no resolver found",
> +			  print_core_address (gdbarch, pc));
>      }
>  
>    if (resolver.minsym != nullptr && BMSYMBOL_VALUE_ADDRESS (resolver) == pc)
> @@ -625,8 +614,7 @@ arc_linux_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>  
> -  if (arc_debug)
> -    debug_printf ("arc-linux: GNU/Linux OS/ABI initialization.\n");
> +  arc_debug_printf ("GNU/Linux OS/ABI initialization.");
>  
>    /* Fill in target-dependent info in ARC-private structure.  */
>    tdep->is_sigtramp = arc_linux_is_sigtramp;
> diff --git a/gdb/arc-newlib-tdep.c b/gdb/arc-newlib-tdep.c
> index 8a83aaf6313a..d3ef496ff3a7 100644
> --- a/gdb/arc-newlib-tdep.c
> +++ b/gdb/arc-newlib-tdep.c
> @@ -29,8 +29,7 @@
>  static void
>  arc_newlib_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc-newlib: Initialization.\n");
> +  arc_debug_printf ("Initialization.");
>  
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>  
> @@ -44,8 +43,7 @@ arc_newlib_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  static enum gdb_osabi
>  arc_newlib_osabi_sniffer (bfd *abfd)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc-newlib: OS/ABI sniffer.\n");
> +  arc_debug_printf ("OS/ABI sniffer.");
>  
>    /* crt0.S in libgloss for ARC defines .ivt section for interrupt handlers.
>       If this section is not present then this is likely not a newlib - could be
> diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
> index 9b8457112111..3e09d5f89021 100644
> --- a/gdb/arc-tdep.c
> +++ b/gdb/arc-tdep.c
> @@ -601,9 +601,8 @@ arc_write_pc (struct regcache *regcache, CORE_ADDR new_pc)
>  {
>    struct gdbarch *gdbarch = regcache->arch ();
>  
> -  if (arc_debug)
> -    debug_printf ("arc: Writing PC, new value=%s\n",
> -		  paddress (gdbarch, new_pc));
> +  arc_debug_printf ("Writing PC, new value=%s",
> +		    paddress (gdbarch, new_pc));
>  
>    regcache_cooked_write_unsigned (regcache, gdbarch_pc_regnum (gdbarch),
>  				  new_pc);
> @@ -614,13 +613,10 @@ arc_write_pc (struct regcache *regcache, CORE_ADDR new_pc)
>  
>    if ((status32 & ARC_STATUS32_DE_MASK) != 0)
>      {
> -      if (arc_debug)
> -	{
> -	  debug_printf ("arc: Changing PC while in delay slot.  Will "
> +      arc_debug_printf ("Changing PC while in delay slot.  Will "
>  			"reset STATUS32.DE bit to zero.  Value of STATUS32 "
> -			"register is 0x%s\n",
> +			"register is 0x%s",
>  			phex (status32, ARC_REGISTER_SIZE));
> -	}
>  
>        /* Reset bit and write to the cache.  */
>        status32 &= ~0x40;
> @@ -734,8 +730,7 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  		     function_call_return_method return_method,
>  		     CORE_ADDR struct_addr)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: push_dummy_call (nargs = %d)\n", nargs);
> +  arc_debug_printf ("nargs = %d", nargs);
>  
>    int arg_reg = ARC_FIRST_ARG_REGNUM;
>  
> @@ -751,9 +746,8 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>        /* Pass the return address in the first argument register.  */
>        regcache_cooked_write_unsigned (regcache, arg_reg, struct_addr);
>  
> -      if (arc_debug)
> -	debug_printf ("arc: struct return address %s passed in R%d",
> -		      print_core_address (gdbarch, struct_addr), arg_reg);
> +      arc_debug_printf ("struct return address %s passed in R%d",
> +			print_core_address (gdbarch, struct_addr), arg_reg);
>  
>        arg_reg++;
>      }
> @@ -771,8 +765,7 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  
>  	  total_space += space;
>  
> -	  if (arc_debug)
> -	    debug_printf ("arc: arg %d: %u bytes -> %u\n", i, len, space);
> +	  arc_debug_printf ("arg %d: %u bytes -> %u", i, len, space);
>  	}
>  
>        /* Allocate a buffer to hold a memory image of the arguments.  */
> @@ -786,9 +779,8 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  	  unsigned int space = align_up (len, 4);
>  
>  	  memcpy (data, value_contents (args[i]), (size_t) len);
> -	  if (arc_debug)
> -	    debug_printf ("arc: copying arg %d, val 0x%08x, len %d to mem\n",
> -			  i, *((int *) value_contents (args[i])), len);
> +	  arc_debug_printf ("copying arg %d, val 0x%08x, len %d to mem",
> +			    i, *((int *) value_contents (args[i])), len);
>  
>  	  data += space;
>  	}
> @@ -797,9 +789,8 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>        data = memory_image;
>        while (arg_reg <= ARC_LAST_ARG_REGNUM)
>  	{
> -	  if (arc_debug)
> -	    debug_printf ("arc: passing 0x%02x%02x%02x%02x in register R%d\n",
> -			  data[0], data[1], data[2], data[3], arg_reg);
> +	  arc_debug_printf ("passing 0x%02x%02x%02x%02x in register R%d",
> +			    data[0], data[1], data[2], data[3], arg_reg);
>  
>  	  /* Note we don't use write_unsigned here, since that would convert
>  	     the byte order, but we are already in the correct byte order.  */
> @@ -819,8 +810,7 @@ arc_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  	 operation).  */
>        if (total_space > 0)
>  	{
> -	  if (arc_debug)
> -	    debug_printf ("arc: passing %d bytes on stack\n", total_space);
> +	  arc_debug_printf ("passing %d bytes on stack\n", total_space);
>  
>  	  sp -= total_space;
>  	  write_memory (sp, data, (int) total_space);
> @@ -916,8 +906,7 @@ arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
>  {
>    unsigned int len = TYPE_LENGTH (type);
>  
> -  if (arc_debug)
> -    debug_printf ("arc: extract_return_value\n");
> +  arc_debug_printf ("called");
>  
>    if (len <= ARC_REGISTER_SIZE)
>      {
> @@ -928,8 +917,7 @@ arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
>        store_unsigned_integer (valbuf, (int) len,
>  			      gdbarch_byte_order (gdbarch), val);
>  
> -      if (arc_debug)
> -	debug_printf ("arc: returning 0x%s\n", phex (val, ARC_REGISTER_SIZE));
> +      arc_debug_printf ("returning 0x%s", phex (val, ARC_REGISTER_SIZE));
>      }
>    else if (len <= ARC_REGISTER_SIZE * 2)
>      {
> @@ -945,10 +933,9 @@ arc_extract_return_value (struct gdbarch *gdbarch, struct type *type,
>  			      (int) len - ARC_REGISTER_SIZE,
>  			      gdbarch_byte_order (gdbarch), high);
>  
> -      if (arc_debug)
> -	debug_printf ("arc: returning 0x%s%s\n",
> -		      phex (high, ARC_REGISTER_SIZE),
> -		      phex (low, ARC_REGISTER_SIZE));
> +      arc_debug_printf ("returning 0x%s%s",
> +			phex (high, ARC_REGISTER_SIZE),
> +			phex (low, ARC_REGISTER_SIZE));
>      }
>    else
>      error (_("arc: extract_return_value: type length %u too large"), len);
> @@ -970,8 +957,7 @@ arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
>  {
>    unsigned int len = TYPE_LENGTH (type);
>  
> -  if (arc_debug)
> -    debug_printf ("arc: store_return_value\n");
> +  arc_debug_printf ("called");
>  
>    if (len <= ARC_REGISTER_SIZE)
>      {
> @@ -982,8 +968,7 @@ arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
>  				      gdbarch_byte_order (gdbarch));
>        regcache_cooked_write_unsigned (regcache, ARC_R0_REGNUM, val);
>  
> -      if (arc_debug)
> -	debug_printf ("arc: storing 0x%s\n", phex (val, ARC_REGISTER_SIZE));
> +      arc_debug_printf ("storing 0x%s", phex (val, ARC_REGISTER_SIZE));
>      }
>    else if (len <= ARC_REGISTER_SIZE * 2)
>      {
> @@ -999,10 +984,9 @@ arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
>        regcache_cooked_write_unsigned (regcache, ARC_R0_REGNUM, low);
>        regcache_cooked_write_unsigned (regcache, ARC_R1_REGNUM, high);
>  
> -      if (arc_debug)
> -	debug_printf ("arc: storing 0x%s%s\n",
> -		      phex (high, ARC_REGISTER_SIZE),
> -		      phex (low, ARC_REGISTER_SIZE));
> +      arc_debug_printf ("storing 0x%s%s",
> +			phex (high, ARC_REGISTER_SIZE),
> +			phex (low, ARC_REGISTER_SIZE));
>      }
>    else
>      error (_("arc_store_return_value: type length too large."));
> @@ -1013,8 +997,7 @@ arc_store_return_value (struct gdbarch *gdbarch, struct type *type,
>  static int
>  arc_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: get_longjmp_target\n");
> +  arc_debug_printf ("called");
>  
>    struct gdbarch *gdbarch = get_frame_arch (frame);
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> @@ -1046,10 +1029,9 @@ arc_return_value (struct gdbarch *gdbarch, struct value *function,
>  			  || valtype->code () == TYPE_CODE_UNION
>  			  || TYPE_LENGTH (valtype) > 2 * ARC_REGISTER_SIZE);
>  
> -  if (arc_debug)
> -    debug_printf ("arc: return_value (readbuf = %s, writebuf = %s)\n",
> -		  host_address_to_string (readbuf),
> -		  host_address_to_string (writebuf));
> +  arc_debug_printf ("readbuf = %s, writebuf = %s",
> +		    host_address_to_string (readbuf),
> +		    host_address_to_string (writebuf));
>  
>    if (writebuf != NULL)
>      {
> @@ -1400,10 +1382,9 @@ static CORE_ADDR
>  arc_analyze_prologue (struct gdbarch *gdbarch, const CORE_ADDR entrypoint,
>  		      const CORE_ADDR limit_pc, struct arc_frame_cache *cache)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: analyze_prologue (entrypoint=%s, limit_pc=%s)\n",
> -		  paddress (gdbarch, entrypoint),
> -		  paddress (gdbarch, limit_pc));
> +  arc_debug_printf ("entrypoint=%s, limit_pc=%s",
> +		    paddress (gdbarch, entrypoint),
> +		    paddress (gdbarch, limit_pc));
>  
>    /* Prologue values.  Only core registers can be stored.  */
>    pv_t regs[ARC_LAST_CORE_REGNUM + 1];
> @@ -1429,9 +1410,8 @@ arc_analyze_prologue (struct gdbarch *gdbarch, const CORE_ADDR entrypoint,
>        if (!arc_is_in_prologue (gdbarch, insn, regs, &stack))
>  	{
>  	  /* Found an instruction that is not in the prologue.  */
> -	  if (arc_debug)
> -	    debug_printf ("arc: End of prologue reached at address %s\n",
> -			  paddress (gdbarch, insn.address));
> +	  arc_debug_printf ("End of prologue reached at address %s",
> +			    paddress (gdbarch, insn.address));
>  	  break;
>  	}
>  
> @@ -1492,8 +1472,7 @@ const static int MAX_PROLOGUE_LENGTH
>  static CORE_ADDR
>  arc_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: skip_prologue\n");
> +  arc_debug_printf ("called");
>  
>    CORE_ADDR func_addr;
>    const char *func_name;
> @@ -1665,19 +1644,19 @@ static void
>  arc_print_frame_cache (struct gdbarch *gdbarch, const char *message,
>  		       struct arc_frame_cache *cache, int addresses_known)
>  {
> -  debug_printf ("arc: frame_info %s\n", message);
> -  debug_printf ("arc: prev_sp = %s\n", paddress (gdbarch, cache->prev_sp));
> -  debug_printf ("arc: frame_base_reg = %i\n", cache->frame_base_reg);
> -  debug_printf ("arc: frame_base_offset = %s\n",
> -		plongest (cache->frame_base_offset));
> +  arc_debug_printf ("frame_info %s", message);
> +  arc_debug_printf ("prev_sp = %s", paddress (gdbarch, cache->prev_sp));
> +  arc_debug_printf ("frame_base_reg = %i", cache->frame_base_reg);
> +  arc_debug_printf ("frame_base_offset = %s",
> +		    plongest (cache->frame_base_offset));
>  
>    for (int i = 0; i <= ARC_BLINK_REGNUM; i++)
>      {
>        if (trad_frame_addr_p (cache->saved_regs, i))
> -	debug_printf ("arc: saved register %s at %s %s\n",
> -		      gdbarch_register_name (gdbarch, i),
> -		      (addresses_known) ? "address" : "offset",
> -		      paddress (gdbarch, cache->saved_regs[i].addr ()));
> +	arc_debug_printf ("saved register %s at %s %s",
> +			  gdbarch_register_name (gdbarch, i),
> +			  (addresses_known) ? "address" : "offset",
> +			      paddress (gdbarch, cache->saved_regs[i].addr ()));
>      }
>  }
>  
> @@ -1686,8 +1665,7 @@ arc_print_frame_cache (struct gdbarch *gdbarch, const char *message,
>  static struct arc_frame_cache *
>  arc_make_frame_cache (struct frame_info *this_frame)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: frame_cache\n");
> +  arc_debug_printf ("called");
>  
>    struct gdbarch *gdbarch = get_frame_arch (this_frame);
>  
> @@ -1755,8 +1733,7 @@ static void
>  arc_frame_this_id (struct frame_info *this_frame, void **this_cache,
>  		   struct frame_id *this_id)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: frame_this_id\n");
> +  arc_debug_printf ("called");
>  
>    struct gdbarch *gdbarch = get_frame_arch (this_frame);
>  
> @@ -1851,8 +1828,7 @@ arc_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>  static struct arc_frame_cache *
>  arc_make_sigtramp_frame_cache (struct frame_info *this_frame)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: sigtramp_frame_cache\n");
> +  arc_debug_printf ("called");
>  
>    struct gdbarch_tdep *tdep = gdbarch_tdep (get_frame_arch (this_frame));
>  
> @@ -1891,8 +1867,7 @@ static void
>  arc_sigtramp_frame_this_id (struct frame_info *this_frame,
>  			    void **this_cache, struct frame_id *this_id)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: sigtramp_frame_this_id\n");
> +  arc_debug_printf ("called");
>  
>    if (*this_cache == NULL)
>      *this_cache = arc_make_sigtramp_frame_cache (this_frame);
> @@ -1911,8 +1886,7 @@ static struct value *
>  arc_sigtramp_frame_prev_register (struct frame_info *this_frame,
>  				  void **this_cache, int regnum)
>  {
> -  if (arc_debug)
> -    debug_printf ("arc: sigtramp_frame_prev_register (regnum = %d)\n", regnum);
> +  arc_debug_printf ("regnum = %d", regnum);
>  
>    /* Make sure we've initialized the cache.  */
>    if (*this_cache == NULL)
> @@ -1932,8 +1906,7 @@ arc_sigtramp_frame_sniffer (const struct frame_unwind *self,
>  {
>    struct gdbarch_tdep *tdep;
>  
> -  if (arc_debug)
> -    debug_printf ("arc: sigtramp_frame_sniffer\n");
> +  arc_debug_printf ("called");
>  
>    tdep = gdbarch_tdep (get_frame_arch (this_frame));
>  
> @@ -2193,8 +2166,7 @@ arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
>  		tdesc_arch_data_up *tdesc_data)
>  {
>    const struct target_desc *tdesc_loc = info.target_desc;
> -  if (arc_debug)
> -    debug_printf ("arc: Target description initialization.\n");
> +  arc_debug_printf ("Target description initialization.");
>  
>    /* If target doesn't provide a description, use the default ones.  */
>    if (!tdesc_has_registers (tdesc_loc))
> @@ -2206,8 +2178,7 @@ arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
>      }
>    gdb_assert (tdesc_loc != nullptr);
>  
> -  if (arc_debug)
> -    debug_printf ("arc: Have got a target description\n");
> +  arc_debug_printf ("Have got a target description");
>  
>    const struct tdesc_feature *feature_core
>      = tdesc_find_feature (tdesc_loc, ARC_CORE_FEATURE_NAME);
> @@ -2253,8 +2224,7 @@ arc_tdesc_init (struct gdbarch_info info, const struct target_desc **tdesc,
>  
>    if (!valid_p)
>      {
> -      if (arc_debug)
> -	debug_printf ("arc: Target description is not valid\n");
> +      arc_debug_printf ("Target description is not valid");
>        return false;
>      }
>  
> @@ -2300,8 +2270,7 @@ arc_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    const struct target_desc *tdesc;
>    tdesc_arch_data_up tdesc_data;
>  
> -  if (arc_debug)
> -    debug_printf ("arc: Architecture initialization.\n");
> +  arc_debug_printf ("Architecture initialization.");
>  
>    if (!arc_tdesc_init (info, &tdesc, &tdesc_data))
>      return nullptr;
> diff --git a/gdb/arc-tdep.h b/gdb/arc-tdep.h
> index 70fc3d95c483..43a801471772 100644
> --- a/gdb/arc-tdep.h
> +++ b/gdb/arc-tdep.h
> @@ -114,6 +114,16 @@ enum arc_regnum
>  
>  extern int arc_debug;
>  
> +/* Print an "arc" debug statement if arc_debug is >= 1.  */
> +
> +#define arc_debug_printf(fmt, ...) \
> +  debug_prefixed_printf_cond (arc_debug >= 1, "arc", fmt, ##__VA_ARGS__)
> +
> +/* Print an "arc" debug statement if arc_debug is >= 2.  */
> +
> +#define arc_debug_printf_v(fmt, ...) \
> +  debug_prefixed_printf_cond (arc_debug >= 2, "arc", fmt, ##__VA_ARGS__)
> +
>  /* Target-dependent information.  */
>  
>  struct gdbarch_tdep
> 

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

* Re: [PATCH 3/5] gdb: convert aarch64 to new-style debug macros
  2021-01-09  4:31   ` Simon Marchi
@ 2021-01-11 13:18     ` Luis Machado
  2021-01-11 21:30       ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Luis Machado @ 2021-01-11 13:18 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Gdb-patches

Hi,

On 1/9/21 1:31 AM, Simon Marchi wrote:
> Hi Luis,
> 
> Would you mind giving this patch a quick look?
> 
> Thanks!
> 
> Simon
> 
> On 2021-01-08 11:28 p.m., Simon Marchi wrote:
>> I haven't tried this on an actual aarch64 machine, but I am able to
>> exercise it like this:
>>
>>      (gdb) set debug aarch64
>>      (gdb) maintenance selftest aa
>>      Running selftest aarch64-analyze-prologue.
>>      [aarch64] aarch64_analyze_prologue: prologue analysis gave up addr=0x14 opcode=0xf94013e0
>>      Running selftest aarch64-process-record.
>>      Ran 2 unit tests, 0 failed
>>
>> gdb/ChangeLog:
>>
>> 	* arch/aarch64-insn.h (aarch64_debug_printf): New.
>> 	* arch/aarch64-insn.c: Use aarch64_debug_printf.
>> 	* aarch64-tdep.c: Use aarch64_debug_printf.
>>
>> Change-Id: Ifdb40e2816ab8e55a9aabb066d1833d9b5a46094
>> ---
>>   gdb/aarch64-tdep.c      | 89 ++++++++++++++++-------------------------
>>   gdb/arch/aarch64-insn.c |  9 ++---
>>   gdb/arch/aarch64-insn.h |  5 +++
>>   3 files changed, 42 insertions(+), 61 deletions(-)
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 77e6ad700fcd..a51b3341e789 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -388,12 +388,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>   	    regs[rd] = regs[rm];
>>   	  else
>>   	    {
>> -	      if (aarch64_debug)
>> -		{
>> -		  debug_printf ("aarch64: prologue analysis gave up "
>> -				"addr=%s opcode=0x%x (orr x register)\n",
>> -				core_addr_to_string_nz (start), insn);
>> -		}
>> +	      aarch64_debug_printf ("prologue analysis gave up "
>> +				    "addr=%s opcode=0x%x (orr x register)",
>> +				    core_addr_to_string_nz (start), insn);
>> +
>>   	      break;
>>   	    }
>>   	}
>> @@ -513,10 +511,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>   	    }
>>   	  else
>>   	    {
>> -	      if (aarch64_debug)
>> -		debug_printf ("aarch64: prologue analysis gave up addr=%s"
>> -			      " opcode=0x%x (iclass)\n",
>> -			      core_addr_to_string_nz (start), insn);
>> +	      aarch64_debug_printf ("prologue analysis gave up addr=%s"
>> +				    " opcode=0x%x (iclass)",
>> +				    core_addr_to_string_nz (start), insn);
>>   	      break;
>>   	    }
>>   
>> @@ -527,12 +524,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>   	}
>>         else
>>   	{
>> -	  if (aarch64_debug)
>> -	    {
>> -	      debug_printf ("aarch64: prologue analysis gave up addr=%s"
>> -			    " opcode=0x%x\n",
>> -			    core_addr_to_string_nz (start), insn);
>> -	    }
>> +	  aarch64_debug_printf ("prologue analysis gave up addr=%s"
>> +				" opcode=0x%x",
>> +				core_addr_to_string_nz (start), insn);
>> +
>>   	  break;
>>   	}
>>       }
>> @@ -1606,12 +1601,10 @@ pass_in_x (struct gdbarch *gdbarch, struct regcache *regcache,
>>   	  && (typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION))
>>   	regval <<= ((X_REGISTER_SIZE - partial_len) * TARGET_CHAR_BIT);
>>   
>> -      if (aarch64_debug)
>> -	{
>> -	  debug_printf ("arg %d in %s = 0x%s\n", info->argnum,
>> -			gdbarch_register_name (gdbarch, regnum),
>> -			phex (regval, X_REGISTER_SIZE));
>> -	}
>> +      aarch64_debug_printf ("arg %d in %s = 0x%s", info->argnum,
>> +			    gdbarch_register_name (gdbarch, regnum),
>> +			    phex (regval, X_REGISTER_SIZE));
>> +
>>         regcache_cooked_write_unsigned (regcache, regnum, regval);
>>         len -= partial_len;
>>         buf += partial_len;
>> @@ -1646,11 +1639,9 @@ pass_in_v (struct gdbarch *gdbarch,
>>         memcpy (reg, buf, len);
>>         regcache->cooked_write (regnum, reg);
>>   
>> -      if (aarch64_debug)
>> -	{
>> -	  debug_printf ("arg %d in %s\n", info->argnum,
>> -			gdbarch_register_name (gdbarch, regnum));
>> -	}
>> +      aarch64_debug_printf ("arg %d in %s", info->argnum,
>> +			    gdbarch_register_name (gdbarch, regnum));
>> +
>>         return 1;
>>       }
>>     info->nsrn = 8;
>> @@ -1680,11 +1671,8 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
>>     if (align > 16)
>>       align = 16;
>>   
>> -  if (aarch64_debug)
>> -    {
>> -      debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
>> -		    info->nsaa);
>> -    }
>> +  aarch64_debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
>> +			info->nsaa);
>>   
>>     item.len = len;
>>     item.data = buf;
>> @@ -1833,13 +1821,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>>     /* The struct_return pointer occupies X8.  */
>>     if (return_method != return_method_normal)
>>       {
>> -      if (aarch64_debug)
>> -	{
>> -	  debug_printf ("struct return in %s = 0x%s\n",
>> -			gdbarch_register_name (gdbarch,
>> -					       AARCH64_STRUCT_RETURN_REGNUM),
>> -			paddress (gdbarch, struct_addr));
>> -	}
>> +      aarch64_debug_printf
>> +	("struct return in %s = 0x%s",
>> +	 gdbarch_register_name (gdbarch, AARCH64_STRUCT_RETURN_REGNUM),
>> +	 paddress (gdbarch, struct_addr));
>> +

Would it be possible to split this up differently, while still keeping 
the opening parenthesis on the same line as the call?

>>         regcache_cooked_write_unsigned (regcache, AARCH64_STRUCT_RETURN_REGNUM,
>>   				      struct_addr);
>>       }
>> @@ -2246,12 +2232,10 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
>>   	  gdb_byte buf[register_size (gdbarch, regno)];
>>   	  gdb_assert (len <= sizeof (buf));
>>   
>> -	  if (aarch64_debug)
>> -	    {
>> -	      debug_printf ("read HFA or HVA return value element %d from %s\n",
>> -			    i + 1,
>> -			    gdbarch_register_name (gdbarch, regno));
>> -	    }
>> +	  aarch64_debug_printf
>> +	    ("read HFA or HVA return value element %d from %s",
>> +	     i + 1, gdbarch_register_name (gdbarch, regno));
>> +

Same here.

Otherwise, this looks OK. Thanks for converting it.

>>   	  regs->cooked_read (regno, buf);
>>   
>>   	  memcpy (valbuf, buf, len);
>> @@ -2358,12 +2342,9 @@ aarch64_store_return_value (struct type *type, struct regcache *regs,
>>   	  gdb_byte tmpbuf[register_size (gdbarch, regno)];
>>   	  gdb_assert (len <= sizeof (tmpbuf));
>>   
>> -	  if (aarch64_debug)
>> -	    {
>> -	      debug_printf ("write HFA or HVA return value element %d to %s\n",
>> -			    i + 1,
>> -			    gdbarch_register_name (gdbarch, regno));
>> -	    }
>> +	  aarch64_debug_printf
>> +	    ("write HFA or HVA return value element %d to %s",
>> +	     i + 1, gdbarch_register_name (gdbarch, regno));
>>   
>>   	  memcpy (tmpbuf, valbuf,
>>   		  len > V_REGISTER_SIZE ? V_REGISTER_SIZE : len);
>> @@ -2438,8 +2419,7 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
>>       {
>>         if (aarch64_return_in_memory (gdbarch, valtype))
>>   	{
>> -	  if (aarch64_debug)
>> -	    debug_printf ("return value in memory\n");
>> +	  aarch64_debug_printf ("return value in memory");
>>   	  return RETURN_VALUE_STRUCT_CONVENTION;
>>   	}
>>       }
>> @@ -2450,8 +2430,7 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
>>     if (readbuf)
>>       aarch64_extract_return_value (valtype, regcache, readbuf);
>>   
>> -  if (aarch64_debug)
>> -    debug_printf ("return value in registers\n");
>> +  aarch64_debug_printf ("return value in registers");
>>   
>>     return RETURN_VALUE_REGISTER_CONVENTION;
>>   }
>> diff --git a/gdb/arch/aarch64-insn.c b/gdb/arch/aarch64-insn.c
>> index 125288909137..6a5abf729bb2 100644
>> --- a/gdb/arch/aarch64-insn.c
>> +++ b/gdb/arch/aarch64-insn.c
>> @@ -69,12 +69,9 @@ aarch64_decode_adr (CORE_ADDR addr, uint32_t insn, int *is_adrp,
>>         else
>>   	*offset = (immhi | immlo);
>>   
>> -      if (aarch64_debug)
>> -	{
>> -	  debug_printf ("decode: 0x%s 0x%x %s x%u, #?\n",
>> -			core_addr_to_string_nz (addr), insn,
>> -			*is_adrp ?  "adrp" : "adr", *rd);
>> -	}
>> +      aarch64_debug_printf ("decode: 0x%s 0x%x %s x%u, #?",
>> +			    core_addr_to_string_nz (addr), insn,
>> +			    *is_adrp ?  "adrp" : "adr", *rd);
>>         return 1;
>>       }
>>     return 0;
>> diff --git a/gdb/arch/aarch64-insn.h b/gdb/arch/aarch64-insn.h
>> index 57aeb23feab1..1e8c5eac940e 100644
>> --- a/gdb/arch/aarch64-insn.h
>> +++ b/gdb/arch/aarch64-insn.h
>> @@ -21,6 +21,11 @@
>>   
>>   extern bool aarch64_debug;
>>   
>> +/* Print an "aarch64" debug statement.  */
>> +
>> +#define aarch64_debug_printf(fmt, ...) \
>> +  debug_prefixed_printf_cond (aarch64_debug, "aarch64", fmt, ##__VA_ARGS__)
>> +
>>   /* Support routines for instruction parsing.  */
>>   
>>   /* Create a mask of X bits.  */
>>

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

* Re: [PATCH 1/5] gdb: change jit_debug to a bool
  2021-01-09  4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
                   ` (4 preceding siblings ...)
  2021-01-09  4:29 ` [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
@ 2021-01-11 21:17 ` Tom Tromey
  2021-01-11 21:19   ` Simon Marchi
  2021-01-13 14:32 ` Shahab Vahedi
  6 siblings, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-01-11 21:17 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> gdb/ChangeLog:
Simon> 	* jit.c (jit_debug): Change type to bool.
Simon> 	(_initialize_jit): Adjust.

Looks good.  Thanks.

Tom

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

* Re: [PATCH 2/5] gdb: convert jit to new-style debug macros
  2021-01-09  4:28 ` [PATCH 2/5] gdb: convert jit to new-style debug macros Simon Marchi
@ 2021-01-11 21:18   ` Tom Tromey
  2021-01-11 21:20     ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-01-11 21:18 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> gdb/ChangeLog:

Simon> 	* jit.c (jit_debug_printf): New, use throughout file.

Nice, thank you.

Tom

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

* Re: [PATCH 4/5] gdb: convert solib-aix to new-style debug macros
  2021-01-09  4:28 ` [PATCH 4/5] gdb: convert solib-aix " Simon Marchi
@ 2021-01-11 21:19   ` Tom Tromey
  2021-01-11 21:32     ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-01-11 21:19 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> This is only compile-tested.
Simon> gdb/ChangeLog:

Simon> 	* solib-aix.c (solib_aix_debug_printf): New, use throughout
Simon> 	file.

Looks good to me.
Thank you.

Tom

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

* Re: [PATCH 1/5] gdb: change jit_debug to a bool
  2021-01-11 21:17 ` Tom Tromey
@ 2021-01-11 21:19   ` Simon Marchi
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-01-11 21:19 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches



On 2021-01-11 4:17 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> gdb/ChangeLog:
> Simon> 	* jit.c (jit_debug): Change type to bool.
> Simon> 	(_initialize_jit): Adjust.
> 
> Looks good.  Thanks.
> 
> Tom
> 

Thanks, I pushed this one.  It's kind of obvious anyway.

Simon

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

* Re: [PATCH 2/5] gdb: convert jit to new-style debug macros
  2021-01-11 21:18   ` Tom Tromey
@ 2021-01-11 21:20     ` Simon Marchi
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-01-11 21:20 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches



On 2021-01-11 4:18 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> gdb/ChangeLog:
> 
> Simon> 	* jit.c (jit_debug_printf): New, use throughout file.
> 
> Nice, thank you.
> 
> Tom
> 

Thanks.  I'll give Mihails a bit of time to give some feedback.

Simon

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

* Re: [PATCH 3/5] gdb: convert aarch64 to new-style debug macros
  2021-01-11 13:18     ` Luis Machado
@ 2021-01-11 21:30       ` Simon Marchi
  2021-01-11 21:50         ` Luis Machado
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-01-11 21:30 UTC (permalink / raw)
  To: Luis Machado; +Cc: Gdb-patches



On 2021-01-11 8:18 a.m., Luis Machado wrote:
> Hi,
> 
> On 1/9/21 1:31 AM, Simon Marchi wrote:
>> Hi Luis,
>>
>> Would you mind giving this patch a quick look?
>>
>> Thanks!
>>
>> Simon
>>
>> On 2021-01-08 11:28 p.m., Simon Marchi wrote:
>>> I haven't tried this on an actual aarch64 machine, but I am able to
>>> exercise it like this:
>>>
>>>      (gdb) set debug aarch64
>>>      (gdb) maintenance selftest aa
>>>      Running selftest aarch64-analyze-prologue.
>>>      [aarch64] aarch64_analyze_prologue: prologue analysis gave up addr=0x14 opcode=0xf94013e0
>>>      Running selftest aarch64-process-record.
>>>      Ran 2 unit tests, 0 failed
>>>
>>> gdb/ChangeLog:
>>>
>>>     * arch/aarch64-insn.h (aarch64_debug_printf): New.
>>>     * arch/aarch64-insn.c: Use aarch64_debug_printf.
>>>     * aarch64-tdep.c: Use aarch64_debug_printf.
>>>
>>> Change-Id: Ifdb40e2816ab8e55a9aabb066d1833d9b5a46094
>>> ---
>>>   gdb/aarch64-tdep.c      | 89 ++++++++++++++++-------------------------
>>>   gdb/arch/aarch64-insn.c |  9 ++---
>>>   gdb/arch/aarch64-insn.h |  5 +++
>>>   3 files changed, 42 insertions(+), 61 deletions(-)
>>>
>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>> index 77e6ad700fcd..a51b3341e789 100644
>>> --- a/gdb/aarch64-tdep.c
>>> +++ b/gdb/aarch64-tdep.c
>>> @@ -388,12 +388,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>>           regs[rd] = regs[rm];
>>>         else
>>>           {
>>> -          if (aarch64_debug)
>>> -        {
>>> -          debug_printf ("aarch64: prologue analysis gave up "
>>> -                "addr=%s opcode=0x%x (orr x register)\n",
>>> -                core_addr_to_string_nz (start), insn);
>>> -        }
>>> +          aarch64_debug_printf ("prologue analysis gave up "
>>> +                    "addr=%s opcode=0x%x (orr x register)",
>>> +                    core_addr_to_string_nz (start), insn);
>>> +
>>>             break;
>>>           }
>>>       }
>>> @@ -513,10 +511,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>>           }
>>>         else
>>>           {
>>> -          if (aarch64_debug)
>>> -        debug_printf ("aarch64: prologue analysis gave up addr=%s"
>>> -                  " opcode=0x%x (iclass)\n",
>>> -                  core_addr_to_string_nz (start), insn);
>>> +          aarch64_debug_printf ("prologue analysis gave up addr=%s"
>>> +                    " opcode=0x%x (iclass)",
>>> +                    core_addr_to_string_nz (start), insn);
>>>             break;
>>>           }
>>>   @@ -527,12 +524,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>>       }
>>>         else
>>>       {
>>> -      if (aarch64_debug)
>>> -        {
>>> -          debug_printf ("aarch64: prologue analysis gave up addr=%s"
>>> -                " opcode=0x%x\n",
>>> -                core_addr_to_string_nz (start), insn);
>>> -        }
>>> +      aarch64_debug_printf ("prologue analysis gave up addr=%s"
>>> +                " opcode=0x%x",
>>> +                core_addr_to_string_nz (start), insn);
>>> +
>>>         break;
>>>       }
>>>       }
>>> @@ -1606,12 +1601,10 @@ pass_in_x (struct gdbarch *gdbarch, struct regcache *regcache,
>>>         && (typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION))
>>>       regval <<= ((X_REGISTER_SIZE - partial_len) * TARGET_CHAR_BIT);
>>>   -      if (aarch64_debug)
>>> -    {
>>> -      debug_printf ("arg %d in %s = 0x%s\n", info->argnum,
>>> -            gdbarch_register_name (gdbarch, regnum),
>>> -            phex (regval, X_REGISTER_SIZE));
>>> -    }
>>> +      aarch64_debug_printf ("arg %d in %s = 0x%s", info->argnum,
>>> +                gdbarch_register_name (gdbarch, regnum),
>>> +                phex (regval, X_REGISTER_SIZE));
>>> +
>>>         regcache_cooked_write_unsigned (regcache, regnum, regval);
>>>         len -= partial_len;
>>>         buf += partial_len;
>>> @@ -1646,11 +1639,9 @@ pass_in_v (struct gdbarch *gdbarch,
>>>         memcpy (reg, buf, len);
>>>         regcache->cooked_write (regnum, reg);
>>>   -      if (aarch64_debug)
>>> -    {
>>> -      debug_printf ("arg %d in %s\n", info->argnum,
>>> -            gdbarch_register_name (gdbarch, regnum));
>>> -    }
>>> +      aarch64_debug_printf ("arg %d in %s", info->argnum,
>>> +                gdbarch_register_name (gdbarch, regnum));
>>> +
>>>         return 1;
>>>       }
>>>     info->nsrn = 8;
>>> @@ -1680,11 +1671,8 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
>>>     if (align > 16)
>>>       align = 16;
>>>   -  if (aarch64_debug)
>>> -    {
>>> -      debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
>>> -            info->nsaa);
>>> -    }
>>> +  aarch64_debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
>>> +            info->nsaa);
>>>       item.len = len;
>>>     item.data = buf;
>>> @@ -1833,13 +1821,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>>>     /* The struct_return pointer occupies X8.  */
>>>     if (return_method != return_method_normal)
>>>       {
>>> -      if (aarch64_debug)
>>> -    {
>>> -      debug_printf ("struct return in %s = 0x%s\n",
>>> -            gdbarch_register_name (gdbarch,
>>> -                           AARCH64_STRUCT_RETURN_REGNUM),
>>> -            paddress (gdbarch, struct_addr));
>>> -    }
>>> +      aarch64_debug_printf
>>> +    ("struct return in %s = 0x%s",
>>> +     gdbarch_register_name (gdbarch, AARCH64_STRUCT_RETURN_REGNUM),
>>> +     paddress (gdbarch, struct_addr));
>>> +
> 
> Would it be possible to split this up differently, while still keeping the opening parenthesis on the same line as the call?

This doesn't work, the "AARCH64_STRUCT_RETURN_REGNUM" line is too long:

      aarch64_debug_printf ("struct return in %s = 0x%s",
			    gdbarch_register_name (gdbarch,
						   AARCH64_STRUCT_RETURN_REGNUM),
			    paddress (gdbarch, struct_addr));

but I could do:

      aarch64_debug_printf ("struct return in %s = 0x%s",
			    gdbarch_register_name
			      (gdbarch, AARCH64_STRUCT_RETURN_REGNUM),
			    paddress (gdbarch, struct_addr));

aarch64_debug_printf's parenthesis are on the same line, but gdbarch_register_name's
are not, it's a trade-off.

> 
>>>         regcache_cooked_write_unsigned (regcache, AARCH64_STRUCT_RETURN_REGNUM,
>>>                         struct_addr);
>>>       }
>>> @@ -2246,12 +2232,10 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
>>>         gdb_byte buf[register_size (gdbarch, regno)];
>>>         gdb_assert (len <= sizeof (buf));
>>>   -      if (aarch64_debug)
>>> -        {
>>> -          debug_printf ("read HFA or HVA return value element %d from %s\n",
>>> -                i + 1,
>>> -                gdbarch_register_name (gdbarch, regno));
>>> -        }
>>> +      aarch64_debug_printf
>>> +        ("read HFA or HVA return value element %d from %s",
>>> +         i + 1, gdbarch_register_name (gdbarch, regno));
>>> +
> 
> Same here.
> 
> Otherwise, this looks OK. Thanks for converting it.

That would require splitting the string in two though, are you ok with that?

	  aarch64_debug_printf ("read HFA or HVA return value element %d "
				"from %s",
				i + 1, gdbarch_register_name (gdbarch, regno));

Splitting literal strings is higher on my list of things to try to avoid.  As
you saw, when space gets tight, I like to move the opening parenthesis to the
next line.  That makes it so the space we have for the arguments does not depend
on the length of the called function's name.  And since I like long, descriptive
function names, I use it often :).

Simon

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

* Re: [PATCH 4/5] gdb: convert solib-aix to new-style debug macros
  2021-01-11 21:19   ` Tom Tromey
@ 2021-01-11 21:32     ` Simon Marchi
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-01-11 21:32 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches



On 2021-01-11 4:19 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> This is only compile-tested.
> Simon> gdb/ChangeLog:
> 
> Simon> 	* solib-aix.c (solib_aix_debug_printf): New, use throughout
> Simon> 	file.
> 
> Looks good to me.
> Thank you.
> 
> Tom
> 

Thanks, pushed.

Simon

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

* Re: [PATCH 3/5] gdb: convert aarch64 to new-style debug macros
  2021-01-11 21:30       ` Simon Marchi
@ 2021-01-11 21:50         ` Luis Machado
  2021-01-11 21:54           ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Luis Machado @ 2021-01-11 21:50 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Gdb-patches

Hi,

On 1/11/21 6:30 PM, Simon Marchi wrote:
> 
> 
> On 2021-01-11 8:18 a.m., Luis Machado wrote:
>> Hi,
>>
>> On 1/9/21 1:31 AM, Simon Marchi wrote:
>>> Hi Luis,
>>>
>>> Would you mind giving this patch a quick look?
>>>
>>> Thanks!
>>>
>>> Simon
>>>
>>> On 2021-01-08 11:28 p.m., Simon Marchi wrote:
>>>> I haven't tried this on an actual aarch64 machine, but I am able to
>>>> exercise it like this:
>>>>
>>>>       (gdb) set debug aarch64
>>>>       (gdb) maintenance selftest aa
>>>>       Running selftest aarch64-analyze-prologue.
>>>>       [aarch64] aarch64_analyze_prologue: prologue analysis gave up addr=0x14 opcode=0xf94013e0
>>>>       Running selftest aarch64-process-record.
>>>>       Ran 2 unit tests, 0 failed
>>>>
>>>> gdb/ChangeLog:
>>>>
>>>>      * arch/aarch64-insn.h (aarch64_debug_printf): New.
>>>>      * arch/aarch64-insn.c: Use aarch64_debug_printf.
>>>>      * aarch64-tdep.c: Use aarch64_debug_printf.
>>>>
>>>> Change-Id: Ifdb40e2816ab8e55a9aabb066d1833d9b5a46094
>>>> ---
>>>>    gdb/aarch64-tdep.c      | 89 ++++++++++++++++-------------------------
>>>>    gdb/arch/aarch64-insn.c |  9 ++---
>>>>    gdb/arch/aarch64-insn.h |  5 +++
>>>>    3 files changed, 42 insertions(+), 61 deletions(-)
>>>>
>>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>>> index 77e6ad700fcd..a51b3341e789 100644
>>>> --- a/gdb/aarch64-tdep.c
>>>> +++ b/gdb/aarch64-tdep.c
>>>> @@ -388,12 +388,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>>>            regs[rd] = regs[rm];
>>>>          else
>>>>            {
>>>> -          if (aarch64_debug)
>>>> -        {
>>>> -          debug_printf ("aarch64: prologue analysis gave up "
>>>> -                "addr=%s opcode=0x%x (orr x register)\n",
>>>> -                core_addr_to_string_nz (start), insn);
>>>> -        }
>>>> +          aarch64_debug_printf ("prologue analysis gave up "
>>>> +                    "addr=%s opcode=0x%x (orr x register)",
>>>> +                    core_addr_to_string_nz (start), insn);
>>>> +
>>>>              break;
>>>>            }
>>>>        }
>>>> @@ -513,10 +511,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>>>            }
>>>>          else
>>>>            {
>>>> -          if (aarch64_debug)
>>>> -        debug_printf ("aarch64: prologue analysis gave up addr=%s"
>>>> -                  " opcode=0x%x (iclass)\n",
>>>> -                  core_addr_to_string_nz (start), insn);
>>>> +          aarch64_debug_printf ("prologue analysis gave up addr=%s"
>>>> +                    " opcode=0x%x (iclass)",
>>>> +                    core_addr_to_string_nz (start), insn);
>>>>              break;
>>>>            }
>>>>    @@ -527,12 +524,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>>>>        }
>>>>          else
>>>>        {
>>>> -      if (aarch64_debug)
>>>> -        {
>>>> -          debug_printf ("aarch64: prologue analysis gave up addr=%s"
>>>> -                " opcode=0x%x\n",
>>>> -                core_addr_to_string_nz (start), insn);
>>>> -        }
>>>> +      aarch64_debug_printf ("prologue analysis gave up addr=%s"
>>>> +                " opcode=0x%x",
>>>> +                core_addr_to_string_nz (start), insn);
>>>> +
>>>>          break;
>>>>        }
>>>>        }
>>>> @@ -1606,12 +1601,10 @@ pass_in_x (struct gdbarch *gdbarch, struct regcache *regcache,
>>>>          && (typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION))
>>>>        regval <<= ((X_REGISTER_SIZE - partial_len) * TARGET_CHAR_BIT);
>>>>    -      if (aarch64_debug)
>>>> -    {
>>>> -      debug_printf ("arg %d in %s = 0x%s\n", info->argnum,
>>>> -            gdbarch_register_name (gdbarch, regnum),
>>>> -            phex (regval, X_REGISTER_SIZE));
>>>> -    }
>>>> +      aarch64_debug_printf ("arg %d in %s = 0x%s", info->argnum,
>>>> +                gdbarch_register_name (gdbarch, regnum),
>>>> +                phex (regval, X_REGISTER_SIZE));
>>>> +
>>>>          regcache_cooked_write_unsigned (regcache, regnum, regval);
>>>>          len -= partial_len;
>>>>          buf += partial_len;
>>>> @@ -1646,11 +1639,9 @@ pass_in_v (struct gdbarch *gdbarch,
>>>>          memcpy (reg, buf, len);
>>>>          regcache->cooked_write (regnum, reg);
>>>>    -      if (aarch64_debug)
>>>> -    {
>>>> -      debug_printf ("arg %d in %s\n", info->argnum,
>>>> -            gdbarch_register_name (gdbarch, regnum));
>>>> -    }
>>>> +      aarch64_debug_printf ("arg %d in %s", info->argnum,
>>>> +                gdbarch_register_name (gdbarch, regnum));
>>>> +
>>>>          return 1;
>>>>        }
>>>>      info->nsrn = 8;
>>>> @@ -1680,11 +1671,8 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
>>>>      if (align > 16)
>>>>        align = 16;
>>>>    -  if (aarch64_debug)
>>>> -    {
>>>> -      debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
>>>> -            info->nsaa);
>>>> -    }
>>>> +  aarch64_debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
>>>> +            info->nsaa);
>>>>        item.len = len;
>>>>      item.data = buf;
>>>> @@ -1833,13 +1821,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>>>>      /* The struct_return pointer occupies X8.  */
>>>>      if (return_method != return_method_normal)
>>>>        {
>>>> -      if (aarch64_debug)
>>>> -    {
>>>> -      debug_printf ("struct return in %s = 0x%s\n",
>>>> -            gdbarch_register_name (gdbarch,
>>>> -                           AARCH64_STRUCT_RETURN_REGNUM),
>>>> -            paddress (gdbarch, struct_addr));
>>>> -    }
>>>> +      aarch64_debug_printf
>>>> +    ("struct return in %s = 0x%s",
>>>> +     gdbarch_register_name (gdbarch, AARCH64_STRUCT_RETURN_REGNUM),
>>>> +     paddress (gdbarch, struct_addr));
>>>> +
>>
>> Would it be possible to split this up differently, while still keeping the opening parenthesis on the same line as the call?
> 
> This doesn't work, the "AARCH64_STRUCT_RETURN_REGNUM" line is too long:
> 
>        aarch64_debug_printf ("struct return in %s = 0x%s",
> 			    gdbarch_register_name (gdbarch,
> 						   AARCH64_STRUCT_RETURN_REGNUM),
> 			    paddress (gdbarch, struct_addr));
> 
> but I could do:
> 
>        aarch64_debug_printf ("struct return in %s = 0x%s",
> 			    gdbarch_register_name
> 			      (gdbarch, AARCH64_STRUCT_RETURN_REGNUM),
> 			    paddress (gdbarch, struct_addr));
> 
> aarch64_debug_printf's parenthesis are on the same line, but gdbarch_register_name's
> are not, it's a trade-off.
> 
>>
>>>>          regcache_cooked_write_unsigned (regcache, AARCH64_STRUCT_RETURN_REGNUM,
>>>>                          struct_addr);
>>>>        }
>>>> @@ -2246,12 +2232,10 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
>>>>          gdb_byte buf[register_size (gdbarch, regno)];
>>>>          gdb_assert (len <= sizeof (buf));
>>>>    -      if (aarch64_debug)
>>>> -        {
>>>> -          debug_printf ("read HFA or HVA return value element %d from %s\n",
>>>> -                i + 1,
>>>> -                gdbarch_register_name (gdbarch, regno));
>>>> -        }
>>>> +      aarch64_debug_printf
>>>> +        ("read HFA or HVA return value element %d from %s",
>>>> +         i + 1, gdbarch_register_name (gdbarch, regno));
>>>> +
>>
>> Same here.
>>
>> Otherwise, this looks OK. Thanks for converting it.
> 
> That would require splitting the string in two though, are you ok with that?
> 
> 	  aarch64_debug_printf ("read HFA or HVA return value element %d "
> 				"from %s",
> 				i + 1, gdbarch_register_name (gdbarch, regno));
> 
> Splitting literal strings is higher on my list of things to try to avoid.  As
> you saw, when space gets tight, I like to move the opening parenthesis to the
> next line.  That makes it so the space we have for the arguments does not depend
> on the length of the called function's name.  And since I like long, descriptive
> function names, I use it often :).
This is not a big deal. Either way is a bit ugly to be honest. The 
problem lies on the limit we currently have. Maybe we can discuss that 
in the future.

Meanwhile, your original patch is fine, so I'd go with it.

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

* Re: [PATCH 3/5] gdb: convert aarch64 to new-style debug macros
  2021-01-11 21:50         ` Luis Machado
@ 2021-01-11 21:54           ` Simon Marchi
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-01-11 21:54 UTC (permalink / raw)
  To: Luis Machado; +Cc: Gdb-patches

> This is not a big deal. Either way is a bit ugly to be honest. The problem lies on the limit we currently have. Maybe we can discuss that in the future.
> 
> Meanwhile, your original patch is fine, so I'd go with it.

Ok, I pushed it with the first one modified but left the second as-is.

Simon

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

* Re: [PATCH 1/5] gdb: change jit_debug to a bool
  2021-01-09  4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
                   ` (5 preceding siblings ...)
  2021-01-11 21:17 ` Tom Tromey
@ 2021-01-13 14:32 ` Shahab Vahedi
  2021-01-13 15:49   ` Simon Marchi
  6 siblings, 1 reply; 21+ messages in thread
From: Shahab Vahedi @ 2021-01-13 14:32 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Shahab Vahedi

Hi Simon,

This patch looks fine to me with a minor comment.

Simon Marchi wrote:
>  static void
>  show_jit_debug (struct ui_file *file, int from_tty,
> @@ -1251,13 +1251,13 @@ _initialize_jit ()
>  {
>    jit_reader_dir = relocate_gdb_directory (JIT_READER_DIR,
>  					   JIT_READER_DIR_RELOCATABLE);
> -  add_setshow_zuinteger_cmd ("jit", class_maintenance, &jit_debug,
> -			     _("Set JIT debugging."),
> -			     _("Show JIT debugging."),
> -			     _("When non-zero, JIT debugging is enabled."),
> -			     NULL,
> -			     show_jit_debug,
> -			     &setdebuglist, &showdebuglist);
> +  add_setshow_boolean_cmd ("jit", class_maintenance, &jit_debug,
> +			   _("Set JIT debugging."),
> +			   _("Show JIT debugging."),
> +			   _("When non-zero, JIT debugging is enabled."),
"When set, JIT debugging is enabled."


Cheers,
Shahab

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

* Re: [PATCH 5/5] gdb: convert arc to new-style debug macros
  2021-01-09  4:37   ` Simon Marchi
@ 2021-01-13 15:35     ` Shahab Vahedi
  0 siblings, 0 replies; 21+ messages in thread
From: Shahab Vahedi @ 2021-01-13 15:35 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Shahab Vahedi

Hi Simon,

Thank you for the patch.  I have 2 remarks:

On Fri, Jan 08, 2021 at 11:37:03PM -0500, Simon Marchi wrote:
> Hi Shahab.
> 
> Would you mind giving a look at this patch?
> 
> Thanks!
> 
> Simon
> 
> On 2021-01-08 11:28 p.m., Simon Marchi via Gdb-patches wrote:
> > Add the standard arc_debug_printf, but also arc_debug_printf_v
> > (verbose), for when arc_debug is >= 2.  This is similar to the existing
> > dwarf_read_debug_print_v.
> > 
> > gdb/ChangeLog:
> > 
> > 	* arc-tdep.h (arc_debug_printf, arc_debug_printf_v): New.
> > 	* arc-tdep.c: Use arc_debug_printf instead of arc_debug +
> > 	debug_printf.
> > 	* arc-linux-nat.c: Likewise.
> > 	* arc-linux-tdep.c: Likewise.
> > 	* arc-newlib-tdep.c: Likewise.
> > 
> > Change-Id: I5d937566ed7a1925f7982e8809802c8f0560d8c6
> > ---
> >  gdb/arc-linux-nat.c   |  11 +---
> >  gdb/arc-linux-tdep.c  |  30 +++-------
> >  gdb/arc-newlib-tdep.c |   6 +-
> >  gdb/arc-tdep.c        | 135 ++++++++++++++++--------------------------
> >  gdb/arc-tdep.h        |  10 ++++
> >  5 files changed, 76 insertions(+), 116 deletions(-)
> > 
> > diff --git a/gdb/arc-tdep.h b/gdb/arc-tdep.h
> > index 70fc3d95c483..43a801471772 100644
> > --- a/gdb/arc-tdep.h
> > +++ b/gdb/arc-tdep.h
> > @@ -114,6 +114,16 @@ enum arc_regnum
> >  
> >  extern int arc_debug;
> >  
> > +/* Print an "arc" debug statement if arc_debug is >= 1.  */
> > +
> > +#define arc_debug_printf(fmt, ...) \
> > +  debug_prefixed_printf_cond (arc_debug >= 1, "arc", fmt, ##__VA_ARGS__)
> > +
> > +/* Print an "arc" debug statement if arc_debug is >= 2.  */
> > +
> > +#define arc_debug_printf_v(fmt, ...) \
> > +  debug_prefixed_printf_cond (arc_debug >= 2, "arc", fmt, ##__VA_ARGS__)
> > +

Please have the macro defined per module (file).  That way we can use
a more meaningful module name than "arc", such as "arc-linux-nat" in
arc-linux-nat.c file or "arc-linux-tdep" in arc-linux-tdep.c file, etc.

> > diff --git a/gdb/arc-linux-nat.c b/gdb/arc-linux-nat.c
> > index db5d44312a68..d55623e0e9cf 100644
> > --- a/gdb/arc-linux-nat.c
> > +++ b/gdb/arc-linux-nat.c
> > @@ -295,8 +291,7 @@ ps_err_e
> >  ps_get_thread_area (struct ps_prochandle *ph, lwpid_t lwpid, int idx,
> >  		    void **base)
> >  {
> > -  if (arc_debug >= 2)
> > -    debug_printf ("arc-linux-nat: ps_get_thread_area called");
> > +  arc_debug_printf_v ("called");

A mere arc_debug_printf() would suffice.  In other words, forget about
the verbosity level for this logging.  This will also reduce the amount
of debug macros defined in this file.


Cheers,
Shahab

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

* Re: [PATCH 1/5] gdb: change jit_debug to a bool
  2021-01-13 14:32 ` Shahab Vahedi
@ 2021-01-13 15:49   ` Simon Marchi
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-01-13 15:49 UTC (permalink / raw)
  To: Shahab Vahedi; +Cc: gdb-patches

On 2021-01-13 9:32 a.m., Shahab Vahedi wrote:
> Hi Simon,
> 
> This patch looks fine to me with a minor comment.
> 
> Simon Marchi wrote:
>>  static void
>>  show_jit_debug (struct ui_file *file, int from_tty,
>> @@ -1251,13 +1251,13 @@ _initialize_jit ()
>>  {
>>    jit_reader_dir = relocate_gdb_directory (JIT_READER_DIR,
>>  					   JIT_READER_DIR_RELOCATABLE);
>> -  add_setshow_zuinteger_cmd ("jit", class_maintenance, &jit_debug,
>> -			     _("Set JIT debugging."),
>> -			     _("Show JIT debugging."),
>> -			     _("When non-zero, JIT debugging is enabled."),
>> -			     NULL,
>> -			     show_jit_debug,
>> -			     &setdebuglist, &showdebuglist);
>> +  add_setshow_boolean_cmd ("jit", class_maintenance, &jit_debug,
>> +			   _("Set JIT debugging."),
>> +			   _("Show JIT debugging."),
>> +			   _("When non-zero, JIT debugging is enabled."),
> "When set, JIT debugging is enabled."

Good point, fixed.  I'll push the patch with that fixed, it's probably
not too controversial anyway.

Thanks!

Simon

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

end of thread, other threads:[~2021-01-13 15:50 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-09  4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
2021-01-09  4:28 ` [PATCH 2/5] gdb: convert jit to new-style debug macros Simon Marchi
2021-01-11 21:18   ` Tom Tromey
2021-01-11 21:20     ` Simon Marchi
2021-01-09  4:28 ` [PATCH 3/5] gdb: convert aarch64 " Simon Marchi
2021-01-09  4:31   ` Simon Marchi
2021-01-11 13:18     ` Luis Machado
2021-01-11 21:30       ` Simon Marchi
2021-01-11 21:50         ` Luis Machado
2021-01-11 21:54           ` Simon Marchi
2021-01-09  4:28 ` [PATCH 4/5] gdb: convert solib-aix " Simon Marchi
2021-01-11 21:19   ` Tom Tromey
2021-01-11 21:32     ` Simon Marchi
2021-01-09  4:28 ` [PATCH 5/5] gdb: convert arc " Simon Marchi
2021-01-09  4:37   ` Simon Marchi
2021-01-13 15:35     ` Shahab Vahedi
2021-01-09  4:29 ` [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi
2021-01-11 21:17 ` Tom Tromey
2021-01-11 21:19   ` Simon Marchi
2021-01-13 14:32 ` Shahab Vahedi
2021-01-13 15:49   ` Simon Marchi

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