Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
@ 2026-07-01 14:28 Tankut Baris Aktemur
  2026-07-01 15:16 ` Lancelot SIX
  2026-07-03 11:03 ` [PATCH v2] gdb, amd-dbgapi-target: use pulongest to print id handles Tankut Baris Aktemur
  0 siblings, 2 replies; 11+ messages in thread
From: Tankut Baris Aktemur @ 2026-07-01 14:28 UTC (permalink / raw)
  To: gdb-patches; +Cc: lancelot.six

In amd-dbgapi-target, there are a number of cases where uint64_t and
uint32_t types are being printed.  The uint64_t cases are in
particular about identifier handles obtained from the debug API.  For
portability, use PRIu64 for uint64_t and PRIu32 for uint32_t values.

In one case, `ptid.tid ()` was being printed for the wave id.  Change
this to use `wave_id.handle` for consistency with the other cases.
---
 gdb/amd-dbgapi-target.c | 58 ++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
index d0424044e0c..14523fcefb3 100644
--- a/gdb/amd-dbgapi-target.c
+++ b/gdb/amd-dbgapi-target.c
@@ -392,26 +392,26 @@ wave_coordinates::to_string () const
   std::string str = "AMDGPU Wave";
 
   str += (agent_id != AMD_DBGAPI_AGENT_NONE
-	  ? string_printf (" %ld", agent_id.handle)
+	  ? string_printf (" %" PRIu64, agent_id.handle)
 	  : " ?");
 
   str += (queue_id != AMD_DBGAPI_QUEUE_NONE
-	  ? string_printf (":%ld", queue_id.handle)
+	  ? string_printf (":%" PRIu64, queue_id.handle)
 	  : ":?");
 
   str += (dispatch_id != AMD_DBGAPI_DISPATCH_NONE
-	  ? string_printf (":%ld", dispatch_id.handle)
+	  ? string_printf (":%" PRIu64, dispatch_id.handle)
 	  : ":?");
 
-  str += string_printf (":%ld", wave_id.handle);
+  str += string_printf (":%" PRIu64, wave_id.handle);
 
   str += (group_ids[0] != UINT32_MAX
-	  ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
-			   group_ids[2])
+	  ? string_printf (" (%" PRIu32 ",%" PRIu32 ",%" PRIu32 ")",
+			   group_ids[0], group_ids[1], group_ids[2])
 	  : " (?,?,?)");
 
   str += (wave_in_group != UINT32_MAX
-	  ? string_printf ("/%d", wave_in_group)
+	  ? string_printf ("/%" PRIu32, wave_in_group)
 	  : "/?");
 
   return str;
@@ -591,8 +591,8 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
      &action);
 
   if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %ld "
-	     "at %s (%s)"),
+    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %" PRIu64
+	     " at %s (%s)"),
 	   breakpoint_id.handle, paddress (inf->arch (), bs->bp_location_at->address),
 	   get_status_string (status));
 
@@ -624,7 +624,7 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
      for our breakpoint_id.  */
   if (resume_breakpoint_id != breakpoint_id)
     error (_("breakpoint resume event is not for this breakpoint. "
-	      "Expected breakpoint_%ld, got breakpoint_%ld"),
+	      "Expected breakpoint_%" PRIu64 ", got breakpoint_%" PRIu64),
 	   breakpoint_id.handle, resume_breakpoint_id.handle);
 
   amd_dbgapi_event_processed (resume_event_id);
@@ -1055,8 +1055,8 @@ amd_dbgapi_target::resume (ptid_t scope_ptid, int step, enum gdb_signal signo)
 	     indicate that the process has exited.  GDB treats resuming a
 	     thread that no longer exists as being successful.  */
 	  && status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
-	error (_("wave_resume for wave_%ld failed (%s)"), wave_id.handle,
-	       get_status_string (status));
+	error (_("wave_resume for wave_%" PRIu64 " failed (%s)"),
+	       wave_id.handle, get_status_string (status));
 
       wi.stopping = false;
     }
@@ -1130,12 +1130,12 @@ amd_dbgapi_target::stop (ptid_t ptid)
 	    }
 
 	  if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
-	    error (_("wave_stop for wave_%ld failed (%s)"), wave_id.handle,
-		   get_status_string (status));
+	    error (_("wave_stop for wave_%" PRIu64 " failed (%s)"),
+		   wave_id.handle, get_status_string (status));
 	}
       else if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
-	error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
-	       get_status_string (status));
+	error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
+	       wave_id.handle, get_status_string (status));
 
       /* The status is AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID.  The wave
 	 could have terminated since the last time the wave list was
@@ -1145,7 +1145,7 @@ amd_dbgapi_target::stop (ptid_t ptid)
       wi.stopping = true;
 
       amd_dbgapi_debug_printf ("got AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID "
-			       "for wave_%ld, last_resume_mode=%s, "
+			       "for wave_%" PRIu64 ", last_resume_mode=%s, "
 			       "report_thread_events=%d",
 			       wave_id.handle,
 			       resume_mode_to_string (wi.last_resume_mode),
@@ -1268,7 +1268,7 @@ dbgapi_notifier_handler (int err, gdb_client_data client_data)
 					  sizeof (runtime_state),
 					  &runtime_state);
       if (status != AMD_DBGAPI_STATUS_SUCCESS)
-	error (_("event_get_info for event_%ld failed (%s)"),
+	error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
 	       event_id.handle, get_status_string (status));
 
       switch (runtime_state)
@@ -1404,7 +1404,7 @@ add_gpu_thread (inferior *inf, ptid_t wave_ptid)
 
   if (!info.wave_info_map.try_emplace (wave_id.handle,
 					wave_info (wave_id)).second)
-    internal_error ("wave ID %ld already in map", wave_id.handle);
+    internal_error ("wave ID %" PRIu64 " already in map", wave_id.handle);
 
   /* Create new GPU threads silently to avoid spamming the terminal
      with thousands of "[New Thread ...]" messages.  */
@@ -1436,7 +1436,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
 	  = amd_dbgapi_event_get_info (event_id, AMD_DBGAPI_EVENT_INFO_WAVE,
 				       sizeof (wave_id), &wave_id);
 	if (status != AMD_DBGAPI_STATUS_SUCCESS)
-	  error (_("event_get_info for event_%ld failed (%s)"),
+	  error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
 		 event_id.handle, get_status_string (status));
 
 	ptid_t event_ptid = make_gpu_ptid (info.inf->pid, wave_id);
@@ -1502,7 +1502,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
 	      }
 	  }
 	else
-	  error (_("wave_get_info for wave_%ld failed (%s)"),
+	  error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
 		 wave_id.handle, get_status_string (status));
 
 	info.wave_events.emplace_back (event_ptid, ws);
@@ -1542,7 +1542,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
 				       AMD_DBGAPI_EVENT_INFO_RUNTIME_STATE,
 				       sizeof (runtime_state), &runtime_state);
 	if (status != AMD_DBGAPI_STATUS_SUCCESS)
-	  error (_("event_get_info for event_%ld failed (%s)"),
+	  error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
 		 event_id.handle, get_status_string (status));
 
 	gdb_assert (runtime_state == AMD_DBGAPI_RUNTIME_STATE_UNLOADED);
@@ -2069,14 +2069,14 @@ amd_dbgapi_target::thread_architecture (ptid_t ptid)
 				     sizeof (architecture_id),
 				     &architecture_id);
   if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    error (_("Couldn't get architecture for wave_%ld"), ptid.tid ());
+    error (_("Couldn't get architecture for wave_%" PRIu64), wave_id.handle);
 
   uint32_t elf_amdgpu_machine;
   status = amd_dbgapi_architecture_get_info
     (architecture_id, AMD_DBGAPI_ARCHITECTURE_INFO_ELF_AMDGPU_MACHINE,
      sizeof (elf_amdgpu_machine), &elf_amdgpu_machine);
   if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    error (_("Couldn't get elf_amdgpu_machine for architecture_%ld"),
+    error (_("Couldn't get elf_amdgpu_machine for architecture_%" PRIu64),
 	   architecture_id.handle);
 
   struct gdbarch_info info;
@@ -2154,7 +2154,7 @@ amd_dbgapi_target::update_thread_list ()
 		    || wi.stopping)
 		  {
 		    amd_dbgapi_debug_printf
-		      ("wave_%ld disappeared, keeping it"
+		      ("wave_%" PRIu64 " disappeared, keeping it"
 		       " (last_resume_mode=%s, stopping=%d)",
 		       wave_id.handle,
 		       resume_mode_to_string (wi.last_resume_mode),
@@ -2162,8 +2162,8 @@ amd_dbgapi_target::update_thread_list ()
 		  }
 		else
 		  {
-		    amd_dbgapi_debug_printf ("wave_%ld disappeared, deleting it",
-					     wave_id.handle);
+		    amd_dbgapi_debug_printf ("wave_%" PRIu64 " disappeared, "
+					     "deleting it", wave_id.handle);
 		    delete_thread_silent (&tp);
 		  }
 	      }
@@ -2283,8 +2283,8 @@ amd_dbgapi_target::displaced_step_finish (thread_info *thread,
 				sizeof (stop_reason), &stop_reason);
 
   if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
-	   get_status_string (status));
+    error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
+	   wave_id.handle, get_status_string (status));
 
   status = amd_dbgapi_displaced_stepping_complete (wave_id, stepping_id);
 
-- 
2.34.1


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

* Re: [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
  2026-07-01 14:28 [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32 Tankut Baris Aktemur
@ 2026-07-01 15:16 ` Lancelot SIX
  2026-07-01 16:56   ` Aktemur, Baris
  2026-07-02 20:15   ` Tom Tromey
  2026-07-03 11:03 ` [PATCH v2] gdb, amd-dbgapi-target: use pulongest to print id handles Tankut Baris Aktemur
  1 sibling, 2 replies; 11+ messages in thread
From: Lancelot SIX @ 2026-07-01 15:16 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches

Hi,

Doesn't GDB have an implicit habit/policy not to use the PRI??? macros?

I do not mind them in amd-dbgapi-target.c, but want to make sure global 
maintainers are fine with this.

I guess the alternative is to use pulongest + %s instead of PRIu64.

i.e. something like this:

     str += (agent_id != AMD_DBGAPI_AGENT_NONE
         ? string_printf (" %s", pulongest (agent_id.handle))
         : "?");

Lancelot.

On 01/07/2026 15:28, Tankut Baris Aktemur wrote:
> In amd-dbgapi-target, there are a number of cases where uint64_t and
> uint32_t types are being printed.  The uint64_t cases are in
> particular about identifier handles obtained from the debug API.  For
> portability, use PRIu64 for uint64_t and PRIu32 for uint32_t values.
> 
> In one case, `ptid.tid ()` was being printed for the wave id.  Change
> this to use `wave_id.handle` for consistency with the other cases.
> ---
>   gdb/amd-dbgapi-target.c | 58 ++++++++++++++++++++---------------------
>   1 file changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> index d0424044e0c..14523fcefb3 100644
> --- a/gdb/amd-dbgapi-target.c
> +++ b/gdb/amd-dbgapi-target.c
> @@ -392,26 +392,26 @@ wave_coordinates::to_string () const
>     std::string str = "AMDGPU Wave";
>   
>     str += (agent_id != AMD_DBGAPI_AGENT_NONE
> -	  ? string_printf (" %ld", agent_id.handle)
> +	  ? string_printf (" %" PRIu64, agent_id.handle)
>   	  : " ?");
>   
>     str += (queue_id != AMD_DBGAPI_QUEUE_NONE
> -	  ? string_printf (":%ld", queue_id.handle)
> +	  ? string_printf (":%" PRIu64, queue_id.handle)
>   	  : ":?");
>   
>     str += (dispatch_id != AMD_DBGAPI_DISPATCH_NONE
> -	  ? string_printf (":%ld", dispatch_id.handle)
> +	  ? string_printf (":%" PRIu64, dispatch_id.handle)
>   	  : ":?");
>   
> -  str += string_printf (":%ld", wave_id.handle);
> +  str += string_printf (":%" PRIu64, wave_id.handle);
>   
>     str += (group_ids[0] != UINT32_MAX
> -	  ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
> -			   group_ids[2])
> +	  ? string_printf (" (%" PRIu32 ",%" PRIu32 ",%" PRIu32 ")",
> +			   group_ids[0], group_ids[1], group_ids[2])
>   	  : " (?,?,?)");
>   
>     str += (wave_in_group != UINT32_MAX
> -	  ? string_printf ("/%d", wave_in_group)
> +	  ? string_printf ("/%" PRIu32, wave_in_group)
>   	  : "/?");
>   
>     return str;
> @@ -591,8 +591,8 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
>        &action);
>   
>     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %ld "
> -	     "at %s (%s)"),
> +    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %" PRIu64
> +	     " at %s (%s)"),
>   	   breakpoint_id.handle, paddress (inf->arch (), bs->bp_location_at->address),
>   	   get_status_string (status));
>   
> @@ -624,7 +624,7 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
>        for our breakpoint_id.  */
>     if (resume_breakpoint_id != breakpoint_id)
>       error (_("breakpoint resume event is not for this breakpoint. "
> -	      "Expected breakpoint_%ld, got breakpoint_%ld"),
> +	      "Expected breakpoint_%" PRIu64 ", got breakpoint_%" PRIu64),
>   	   breakpoint_id.handle, resume_breakpoint_id.handle);
>   
>     amd_dbgapi_event_processed (resume_event_id);
> @@ -1055,8 +1055,8 @@ amd_dbgapi_target::resume (ptid_t scope_ptid, int step, enum gdb_signal signo)
>   	     indicate that the process has exited.  GDB treats resuming a
>   	     thread that no longer exists as being successful.  */
>   	  && status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> -	error (_("wave_resume for wave_%ld failed (%s)"), wave_id.handle,
> -	       get_status_string (status));
> +	error (_("wave_resume for wave_%" PRIu64 " failed (%s)"),
> +	       wave_id.handle, get_status_string (status));
>   
>         wi.stopping = false;
>       }
> @@ -1130,12 +1130,12 @@ amd_dbgapi_target::stop (ptid_t ptid)
>   	    }
>   
>   	  if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> -	    error (_("wave_stop for wave_%ld failed (%s)"), wave_id.handle,
> -		   get_status_string (status));
> +	    error (_("wave_stop for wave_%" PRIu64 " failed (%s)"),
> +		   wave_id.handle, get_status_string (status));
>   	}
>         else if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> -	error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
> -	       get_status_string (status));
> +	error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
> +	       wave_id.handle, get_status_string (status));
>   
>         /* The status is AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID.  The wave
>   	 could have terminated since the last time the wave list was
> @@ -1145,7 +1145,7 @@ amd_dbgapi_target::stop (ptid_t ptid)
>         wi.stopping = true;
>   
>         amd_dbgapi_debug_printf ("got AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID "
> -			       "for wave_%ld, last_resume_mode=%s, "
> +			       "for wave_%" PRIu64 ", last_resume_mode=%s, "
>   			       "report_thread_events=%d",
>   			       wave_id.handle,
>   			       resume_mode_to_string (wi.last_resume_mode),
> @@ -1268,7 +1268,7 @@ dbgapi_notifier_handler (int err, gdb_client_data client_data)
>   					  sizeof (runtime_state),
>   					  &runtime_state);
>         if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -	error (_("event_get_info for event_%ld failed (%s)"),
> +	error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
>   	       event_id.handle, get_status_string (status));
>   
>         switch (runtime_state)
> @@ -1404,7 +1404,7 @@ add_gpu_thread (inferior *inf, ptid_t wave_ptid)
>   
>     if (!info.wave_info_map.try_emplace (wave_id.handle,
>   					wave_info (wave_id)).second)
> -    internal_error ("wave ID %ld already in map", wave_id.handle);
> +    internal_error ("wave ID %" PRIu64 " already in map", wave_id.handle);
>   
>     /* Create new GPU threads silently to avoid spamming the terminal
>        with thousands of "[New Thread ...]" messages.  */
> @@ -1436,7 +1436,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
>   	  = amd_dbgapi_event_get_info (event_id, AMD_DBGAPI_EVENT_INFO_WAVE,
>   				       sizeof (wave_id), &wave_id);
>   	if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -	  error (_("event_get_info for event_%ld failed (%s)"),
> +	  error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
>   		 event_id.handle, get_status_string (status));
>   
>   	ptid_t event_ptid = make_gpu_ptid (info.inf->pid, wave_id);
> @@ -1502,7 +1502,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
>   	      }
>   	  }
>   	else
> -	  error (_("wave_get_info for wave_%ld failed (%s)"),
> +	  error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
>   		 wave_id.handle, get_status_string (status));
>   
>   	info.wave_events.emplace_back (event_ptid, ws);
> @@ -1542,7 +1542,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
>   				       AMD_DBGAPI_EVENT_INFO_RUNTIME_STATE,
>   				       sizeof (runtime_state), &runtime_state);
>   	if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -	  error (_("event_get_info for event_%ld failed (%s)"),
> +	  error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
>   		 event_id.handle, get_status_string (status));
>   
>   	gdb_assert (runtime_state == AMD_DBGAPI_RUNTIME_STATE_UNLOADED);
> @@ -2069,14 +2069,14 @@ amd_dbgapi_target::thread_architecture (ptid_t ptid)
>   				     sizeof (architecture_id),
>   				     &architecture_id);
>     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -    error (_("Couldn't get architecture for wave_%ld"), ptid.tid ());
> +    error (_("Couldn't get architecture for wave_%" PRIu64), wave_id.handle);
>   
>     uint32_t elf_amdgpu_machine;
>     status = amd_dbgapi_architecture_get_info
>       (architecture_id, AMD_DBGAPI_ARCHITECTURE_INFO_ELF_AMDGPU_MACHINE,
>        sizeof (elf_amdgpu_machine), &elf_amdgpu_machine);
>     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -    error (_("Couldn't get elf_amdgpu_machine for architecture_%ld"),
> +    error (_("Couldn't get elf_amdgpu_machine for architecture_%" PRIu64),
>   	   architecture_id.handle);
>   
>     struct gdbarch_info info;
> @@ -2154,7 +2154,7 @@ amd_dbgapi_target::update_thread_list ()
>   		    || wi.stopping)
>   		  {
>   		    amd_dbgapi_debug_printf
> -		      ("wave_%ld disappeared, keeping it"
> +		      ("wave_%" PRIu64 " disappeared, keeping it"
>   		       " (last_resume_mode=%s, stopping=%d)",
>   		       wave_id.handle,
>   		       resume_mode_to_string (wi.last_resume_mode),
> @@ -2162,8 +2162,8 @@ amd_dbgapi_target::update_thread_list ()
>   		  }
>   		else
>   		  {
> -		    amd_dbgapi_debug_printf ("wave_%ld disappeared, deleting it",
> -					     wave_id.handle);
> +		    amd_dbgapi_debug_printf ("wave_%" PRIu64 " disappeared, "
> +					     "deleting it", wave_id.handle);
>   		    delete_thread_silent (&tp);
>   		  }
>   	      }
> @@ -2283,8 +2283,8 @@ amd_dbgapi_target::displaced_step_finish (thread_info *thread,
>   				sizeof (stop_reason), &stop_reason);
>   
>     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -    error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
> -	   get_status_string (status));
> +    error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
> +	   wave_id.handle, get_status_string (status));
>   
>     status = amd_dbgapi_displaced_stepping_complete (wave_id, stepping_id);
>   


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

* RE: [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
  2026-07-01 15:16 ` Lancelot SIX
@ 2026-07-01 16:56   ` Aktemur, Baris
  2026-07-02 20:15   ` Tom Tromey
  1 sibling, 0 replies; 11+ messages in thread
From: Aktemur, Baris @ 2026-07-01 16:56 UTC (permalink / raw)
  To: Six, Lancelot, gdb-patches

AMD General

On Wednesday, July 1, 2026 5:17 PM, Six, Lancelot wrote:
> Hi,
>
> Doesn't GDB have an implicit habit/policy not to use the PRI??? macros?
>
> I do not mind them in amd-dbgapi-target.c, but want to make sure global
> maintainers are fine with this.

I wasn't aware of a policy.  There are existing uses in GDB, including
the amd-dbgapi-target.c file.  It would be great to clarify.

-Baris


> I guess the alternative is to use pulongest + %s instead of PRIu64.
>
> i.e. something like this:
>
>      str += (agent_id != AMD_DBGAPI_AGENT_NONE
>          ? string_printf (" %s", pulongest (agent_id.handle))
>          : "?");
>
> Lancelot.
>
> On 01/07/2026 15:28, Tankut Baris Aktemur wrote:
> > In amd-dbgapi-target, there are a number of cases where uint64_t and
> > uint32_t types are being printed.  The uint64_t cases are in
> > particular about identifier handles obtained from the debug API.  For
> > portability, use PRIu64 for uint64_t and PRIu32 for uint32_t values.
> >
> > In one case, `ptid.tid ()` was being printed for the wave id.  Change
> > this to use `wave_id.handle` for consistency with the other cases.
> > ---
> >   gdb/amd-dbgapi-target.c | 58 ++++++++++++++++++++---------------------
> >   1 file changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> > index d0424044e0c..14523fcefb3 100644
> > --- a/gdb/amd-dbgapi-target.c
> > +++ b/gdb/amd-dbgapi-target.c
> > @@ -392,26 +392,26 @@ wave_coordinates::to_string () const
> >     std::string str = "AMDGPU Wave";
> >
> >     str += (agent_id != AMD_DBGAPI_AGENT_NONE
> > -     ? string_printf (" %ld", agent_id.handle)
> > +     ? string_printf (" %" PRIu64, agent_id.handle)
> >       : " ?");
> >
> >     str += (queue_id != AMD_DBGAPI_QUEUE_NONE
> > -     ? string_printf (":%ld", queue_id.handle)
> > +     ? string_printf (":%" PRIu64, queue_id.handle)
> >       : ":?");
> >
> >     str += (dispatch_id != AMD_DBGAPI_DISPATCH_NONE
> > -     ? string_printf (":%ld", dispatch_id.handle)
> > +     ? string_printf (":%" PRIu64, dispatch_id.handle)
> >       : ":?");
> >
> > -  str += string_printf (":%ld", wave_id.handle);
> > +  str += string_printf (":%" PRIu64, wave_id.handle);
> >
> >     str += (group_ids[0] != UINT32_MAX
> > -     ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
> > -                      group_ids[2])
> > +     ? string_printf (" (%" PRIu32 ",%" PRIu32 ",%" PRIu32 ")",
> > +                      group_ids[0], group_ids[1], group_ids[2])
> >       : " (?,?,?)");
> >
> >     str += (wave_in_group != UINT32_MAX
> > -     ? string_printf ("/%d", wave_in_group)
> > +     ? string_printf ("/%" PRIu32, wave_in_group)
> >       : "/?");
> >
> >     return str;
> > @@ -591,8 +591,8 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
> >        &action);
> >
> >     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> > -    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %ld "
> > -        "at %s (%s)"),
> > +    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %" PRIu64
> > +        " at %s (%s)"),
> >        breakpoint_id.handle, paddress (inf->arch (), bs->bp_location_at->address),
> >        get_status_string (status));
> >
> > @@ -624,7 +624,7 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
> >        for our breakpoint_id.  */
> >     if (resume_breakpoint_id != breakpoint_id)
> >       error (_("breakpoint resume event is not for this breakpoint. "
> > -         "Expected breakpoint_%ld, got breakpoint_%ld"),
> > +         "Expected breakpoint_%" PRIu64 ", got breakpoint_%" PRIu64),
> >        breakpoint_id.handle, resume_breakpoint_id.handle);
> >
> >     amd_dbgapi_event_processed (resume_event_id);
> > @@ -1055,8 +1055,8 @@ amd_dbgapi_target::resume (ptid_t scope_ptid, int step, enum
> gdb_signal signo)
> >          indicate that the process has exited.  GDB treats resuming a
> >          thread that no longer exists as being successful.  */
> >       && status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> > -   error (_("wave_resume for wave_%ld failed (%s)"), wave_id.handle,
> > -          get_status_string (status));
> > +   error (_("wave_resume for wave_%" PRIu64 " failed (%s)"),
> > +          wave_id.handle, get_status_string (status));
> >
> >         wi.stopping = false;
> >       }
> > @@ -1130,12 +1130,12 @@ amd_dbgapi_target::stop (ptid_t ptid)
> >         }
> >
> >       if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> > -       error (_("wave_stop for wave_%ld failed (%s)"), wave_id.handle,
> > -              get_status_string (status));
> > +       error (_("wave_stop for wave_%" PRIu64 " failed (%s)"),
> > +              wave_id.handle, get_status_string (status));
> >     }
> >         else if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> > -   error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
> > -          get_status_string (status));
> > +   error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
> > +          wave_id.handle, get_status_string (status));
> >
> >         /* The status is AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID.  The wave
> >      could have terminated since the last time the wave list was
> > @@ -1145,7 +1145,7 @@ amd_dbgapi_target::stop (ptid_t ptid)
> >         wi.stopping = true;
> >
> >         amd_dbgapi_debug_printf ("got AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID "
> > -                          "for wave_%ld, last_resume_mode=%s, "
> > +                          "for wave_%" PRIu64 ", last_resume_mode=%s, "
> >                            "report_thread_events=%d",
> >                            wave_id.handle,
> >                            resume_mode_to_string (wi.last_resume_mode),
> > @@ -1268,7 +1268,7 @@ dbgapi_notifier_handler (int err, gdb_client_data client_data)
> >                                       sizeof (runtime_state),
> >                                       &runtime_state);
> >         if (status != AMD_DBGAPI_STATUS_SUCCESS)
> > -   error (_("event_get_info for event_%ld failed (%s)"),
> > +   error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
> >            event_id.handle, get_status_string (status));
> >
> >         switch (runtime_state)
> > @@ -1404,7 +1404,7 @@ add_gpu_thread (inferior *inf, ptid_t wave_ptid)
> >
> >     if (!info.wave_info_map.try_emplace (wave_id.handle,
> >                                     wave_info (wave_id)).second)
> > -    internal_error ("wave ID %ld already in map", wave_id.handle);
> > +    internal_error ("wave ID %" PRIu64 " already in map", wave_id.handle);
> >
> >     /* Create new GPU threads silently to avoid spamming the terminal
> >        with thousands of "[New Thread ...]" messages.  */
> > @@ -1436,7 +1436,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
> >       = amd_dbgapi_event_get_info (event_id, AMD_DBGAPI_EVENT_INFO_WAVE,
> >                                    sizeof (wave_id), &wave_id);
> >     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> > -     error (_("event_get_info for event_%ld failed (%s)"),
> > +     error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
> >              event_id.handle, get_status_string (status));
> >
> >     ptid_t event_ptid = make_gpu_ptid (info.inf->pid, wave_id);
> > @@ -1502,7 +1502,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
> >           }
> >       }
> >     else
> > -     error (_("wave_get_info for wave_%ld failed (%s)"),
> > +     error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
> >              wave_id.handle, get_status_string (status));
> >
> >     info.wave_events.emplace_back (event_ptid, ws);
> > @@ -1542,7 +1542,7 @@ process_one_event (amd_dbgapi_inferior_info &info,
> >                                    AMD_DBGAPI_EVENT_INFO_RUNTIME_STATE,
> >                                    sizeof (runtime_state), &runtime_state);
> >     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> > -     error (_("event_get_info for event_%ld failed (%s)"),
> > +     error (_("event_get_info for event_%" PRIu64 " failed (%s)"),
> >              event_id.handle, get_status_string (status));
> >
> >     gdb_assert (runtime_state == AMD_DBGAPI_RUNTIME_STATE_UNLOADED);
> > @@ -2069,14 +2069,14 @@ amd_dbgapi_target::thread_architecture (ptid_t ptid)
> >                                  sizeof (architecture_id),
> >                                  &architecture_id);
> >     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> > -    error (_("Couldn't get architecture for wave_%ld"), ptid.tid ());
> > +    error (_("Couldn't get architecture for wave_%" PRIu64), wave_id.handle);
> >
> >     uint32_t elf_amdgpu_machine;
> >     status = amd_dbgapi_architecture_get_info
> >       (architecture_id, AMD_DBGAPI_ARCHITECTURE_INFO_ELF_AMDGPU_MACHINE,
> >        sizeof (elf_amdgpu_machine), &elf_amdgpu_machine);
> >     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> > -    error (_("Couldn't get elf_amdgpu_machine for architecture_%ld"),
> > +    error (_("Couldn't get elf_amdgpu_machine for architecture_%" PRIu64),
> >        architecture_id.handle);
> >
> >     struct gdbarch_info info;
> > @@ -2154,7 +2154,7 @@ amd_dbgapi_target::update_thread_list ()
> >                 || wi.stopping)
> >               {
> >                 amd_dbgapi_debug_printf
> > -                 ("wave_%ld disappeared, keeping it"
> > +                 ("wave_%" PRIu64 " disappeared, keeping it"
> >                    " (last_resume_mode=%s, stopping=%d)",
> >                    wave_id.handle,
> >                    resume_mode_to_string (wi.last_resume_mode),
> > @@ -2162,8 +2162,8 @@ amd_dbgapi_target::update_thread_list ()
> >               }
> >             else
> >               {
> > -               amd_dbgapi_debug_printf ("wave_%ld disappeared, deleting it",
> > -                                        wave_id.handle);
> > +               amd_dbgapi_debug_printf ("wave_%" PRIu64 " disappeared, "
> > +                                        "deleting it", wave_id.handle);
> >                 delete_thread_silent (&tp);
> >               }
> >           }
> > @@ -2283,8 +2283,8 @@ amd_dbgapi_target::displaced_step_finish (thread_info *thread,
> >                             sizeof (stop_reason), &stop_reason);
> >
> >     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> > -    error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
> > -      get_status_string (status));
> > +    error (_("wave_get_info for wave_%" PRIu64 " failed (%s)"),
> > +      wave_id.handle, get_status_string (status));
> >
> >     status = amd_dbgapi_displaced_stepping_complete (wave_id, stepping_id);
> >


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

* Re: [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
  2026-07-01 15:16 ` Lancelot SIX
  2026-07-01 16:56   ` Aktemur, Baris
@ 2026-07-02 20:15   ` Tom Tromey
  2026-07-03 12:59     ` Simon Marchi
  2026-07-03 13:47     ` Pedro Alves
  1 sibling, 2 replies; 11+ messages in thread
From: Tom Tromey @ 2026-07-02 20:15 UTC (permalink / raw)
  To: Lancelot SIX; +Cc: Tankut Baris Aktemur, gdb-patches

>>>>> Lancelot SIX <Lancelot.Six@amd.com> writes:

> Hi,
> Doesn't GDB have an implicit habit/policy not to use the PRI??? macros?

> I do not mind them in amd-dbgapi-target.c, but want to make sure
> global maintainers are fine with this.

> I guess the alternative is to use pulongest + %s instead of PRIu64.

> i.e. something like this:

>     str += (agent_id != AMD_DBGAPI_AGENT_NONE
>         ? string_printf (" %s", pulongest (agent_id.handle))
>         : "?");

I don't know if it's a policy exactly, but personally I prefer pulongest
et al, since I find the PRI* macros pretty ugly and unreadable.

Tom

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

* [PATCH v2] gdb, amd-dbgapi-target: use pulongest to print id handles
  2026-07-01 14:28 [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32 Tankut Baris Aktemur
  2026-07-01 15:16 ` Lancelot SIX
@ 2026-07-03 11:03 ` Tankut Baris Aktemur
  2026-07-03 11:22   ` Lancelot SIX
  1 sibling, 1 reply; 11+ messages in thread
From: Tankut Baris Aktemur @ 2026-07-03 11:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: lancelot.six

In amd-dbgapi-target, there are a number of cases where identifier
handles that are obtained from dbgapi are being printed.  Those
handles are defined to have type uint64_t.  Use pulongest when
printing them for improved portability.

In one case, `ptid.tid ()` was being printed for the wave id.  Change
this to use `wave_id.handle` for consistency with the other cases.
---
 gdb/amd-dbgapi-target.c | 87 +++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 42 deletions(-)

diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
index d0424044e0c..caae9a98501 100644
--- a/gdb/amd-dbgapi-target.c
+++ b/gdb/amd-dbgapi-target.c
@@ -392,18 +392,18 @@ wave_coordinates::to_string () const
   std::string str = "AMDGPU Wave";
 
   str += (agent_id != AMD_DBGAPI_AGENT_NONE
-	  ? string_printf (" %ld", agent_id.handle)
+	  ? string_printf (" %s", pulongest (agent_id.handle))
 	  : " ?");
 
   str += (queue_id != AMD_DBGAPI_QUEUE_NONE
-	  ? string_printf (":%ld", queue_id.handle)
+	  ? string_printf (":%s", pulongest (queue_id.handle))
 	  : ":?");
 
   str += (dispatch_id != AMD_DBGAPI_DISPATCH_NONE
-	  ? string_printf (":%ld", dispatch_id.handle)
+	  ? string_printf (":%s", pulongest (dispatch_id.handle))
 	  : ":?");
 
-  str += string_printf (":%ld", wave_id.handle);
+  str += string_printf (":%s", pulongest (wave_id.handle));
 
   str += (group_ids[0] != UINT32_MAX
 	  ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
@@ -591,9 +591,10 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
      &action);
 
   if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %ld "
+    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %s "
 	     "at %s (%s)"),
-	   breakpoint_id.handle, paddress (inf->arch (), bs->bp_location_at->address),
+	   pulongest (breakpoint_id.handle),
+	   paddress (inf->arch (), bs->bp_location_at->address),
 	   get_status_string (status));
 
   if (action == AMD_DBGAPI_BREAKPOINT_ACTION_RESUME)
@@ -624,8 +625,9 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
      for our breakpoint_id.  */
   if (resume_breakpoint_id != breakpoint_id)
     error (_("breakpoint resume event is not for this breakpoint. "
-	      "Expected breakpoint_%ld, got breakpoint_%ld"),
-	   breakpoint_id.handle, resume_breakpoint_id.handle);
+	      "Expected breakpoint_%s, got breakpoint_%s"),
+	   pulongest (breakpoint_id.handle),
+	   pulongest (resume_breakpoint_id.handle));
 
   amd_dbgapi_event_processed (resume_event_id);
 }
@@ -1055,8 +1057,8 @@ amd_dbgapi_target::resume (ptid_t scope_ptid, int step, enum gdb_signal signo)
 	     indicate that the process has exited.  GDB treats resuming a
 	     thread that no longer exists as being successful.  */
 	  && status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
-	error (_("wave_resume for wave_%ld failed (%s)"), wave_id.handle,
-	       get_status_string (status));
+	error (_("wave_resume for wave_%s failed (%s)"),
+	       pulongest (wave_id.handle), get_status_string (status));
 
       wi.stopping = false;
     }
@@ -1130,12 +1132,12 @@ amd_dbgapi_target::stop (ptid_t ptid)
 	    }
 
 	  if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
-	    error (_("wave_stop for wave_%ld failed (%s)"), wave_id.handle,
-		   get_status_string (status));
+	    error (_("wave_stop for wave_%s failed (%s)"),
+		   pulongest (wave_id.handle), get_status_string (status));
 	}
       else if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
-	error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
-	       get_status_string (status));
+	error (_("wave_get_info for wave_%s failed (%s)"),
+	       pulongest (wave_id.handle), get_status_string (status));
 
       /* The status is AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID.  The wave
 	 could have terminated since the last time the wave list was
@@ -1145,9 +1147,9 @@ amd_dbgapi_target::stop (ptid_t ptid)
       wi.stopping = true;
 
       amd_dbgapi_debug_printf ("got AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID "
-			       "for wave_%ld, last_resume_mode=%s, "
+			       "for wave_%s, last_resume_mode=%s, "
 			       "report_thread_events=%d",
-			       wave_id.handle,
+			       pulongest (wave_id.handle),
 			       resume_mode_to_string (wi.last_resume_mode),
 			       m_report_thread_events);
 
@@ -1212,8 +1214,8 @@ struct scoped_amd_dbgapi_event_processed
   {
     amd_dbgapi_status_t status = amd_dbgapi_event_processed (m_event_id);
     if (status != AMD_DBGAPI_STATUS_SUCCESS)
-      warning (_("Failed to acknowledge amd-dbgapi event %" PRIu64),
-	       m_event_id.handle);
+      warning (_("Failed to acknowledge amd-dbgapi event %s"),
+	       pulongest (m_event_id.handle));
   }
 
   DISABLE_COPY_AND_ASSIGN (scoped_amd_dbgapi_event_processed);
@@ -1268,8 +1270,8 @@ dbgapi_notifier_handler (int err, gdb_client_data client_data)
 					  sizeof (runtime_state),
 					  &runtime_state);
       if (status != AMD_DBGAPI_STATUS_SUCCESS)
-	error (_("event_get_info for event_%ld failed (%s)"),
-	       event_id.handle, get_status_string (status));
+	error (_("event_get_info for event_%s failed (%s)"),
+	       pulongest (event_id.handle), get_status_string (status));
 
       switch (runtime_state)
 	{
@@ -1404,7 +1406,7 @@ add_gpu_thread (inferior *inf, ptid_t wave_ptid)
 
   if (!info.wave_info_map.try_emplace (wave_id.handle,
 					wave_info (wave_id)).second)
-    internal_error ("wave ID %ld already in map", wave_id.handle);
+    internal_error ("wave ID %s already in map", pulongest (wave_id.handle));
 
   /* Create new GPU threads silently to avoid spamming the terminal
      with thousands of "[New Thread ...]" messages.  */
@@ -1436,8 +1438,8 @@ process_one_event (amd_dbgapi_inferior_info &info,
 	  = amd_dbgapi_event_get_info (event_id, AMD_DBGAPI_EVENT_INFO_WAVE,
 				       sizeof (wave_id), &wave_id);
 	if (status != AMD_DBGAPI_STATUS_SUCCESS)
-	  error (_("event_get_info for event_%ld failed (%s)"),
-		 event_id.handle, get_status_string (status));
+	  error (_("event_get_info for event_%s failed (%s)"),
+		 pulongest (event_id.handle), get_status_string (status));
 
 	ptid_t event_ptid = make_gpu_ptid (info.inf->pid, wave_id);
 	target_waitstatus ws;
@@ -1502,8 +1504,8 @@ process_one_event (amd_dbgapi_inferior_info &info,
 	      }
 	  }
 	else
-	  error (_("wave_get_info for wave_%ld failed (%s)"),
-		 wave_id.handle, get_status_string (status));
+	  error (_("wave_get_info for wave_%s failed (%s)"),
+		 pulongest (wave_id.handle), get_status_string (status));
 
 	info.wave_events.emplace_back (event_ptid, ws);
 	break;
@@ -1542,8 +1544,8 @@ process_one_event (amd_dbgapi_inferior_info &info,
 				       AMD_DBGAPI_EVENT_INFO_RUNTIME_STATE,
 				       sizeof (runtime_state), &runtime_state);
 	if (status != AMD_DBGAPI_STATUS_SUCCESS)
-	  error (_("event_get_info for event_%ld failed (%s)"),
-		 event_id.handle, get_status_string (status));
+	  error (_("event_get_info for event_%s failed (%s)"),
+		 pulongest (event_id.handle), get_status_string (status));
 
 	gdb_assert (runtime_state == AMD_DBGAPI_RUNTIME_STATE_UNLOADED);
 	gdb_assert
@@ -1620,9 +1622,9 @@ process_event_queue (amd_dbgapi_inferior_info &info,
 
       if (event_kind != AMD_DBGAPI_EVENT_KIND_NONE)
 	amd_dbgapi_debug_printf ("Pulled event from dbgapi: "
-				 "event_id.handle = %" PRIu64 ", "
+				 "event_id.handle = %s, "
 				 "event_kind = %s",
-				 event_id.handle,
+				 pulongest (event_id.handle),
 				 event_kind_str (event_kind));
 
       if (event_id == AMD_DBGAPI_EVENT_NONE || event_kind == until_event_kind)
@@ -1860,8 +1862,8 @@ attach_amd_dbgapi (inferior *inf)
   /* Are we already attached?  */
   if (info.process_id != AMD_DBGAPI_PROCESS_NONE)
     {
-      amd_dbgapi_debug_printf
-	("already attached: process_id = %" PRIu64, info.process_id.handle);
+      amd_dbgapi_debug_printf ("already attached: process_id = %s",
+			       pulongest (info.process_id.handle));
       return;
     }
 
@@ -1895,8 +1897,8 @@ attach_amd_dbgapi (inferior *inf)
       return;
     }
 
-  amd_dbgapi_debug_printf ("process_id = %" PRIu64 ", notifier fd = %d",
-			   info.process_id.handle,
+  amd_dbgapi_debug_printf ("process_id = %s, notifier fd = %d",
+			   pulongest (info.process_id.handle),
 			   amd_dbgapi_notifier_get_fd (info.notifier));
 
   set_process_memory_precision (info);
@@ -2069,15 +2071,16 @@ amd_dbgapi_target::thread_architecture (ptid_t ptid)
 				     sizeof (architecture_id),
 				     &architecture_id);
   if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    error (_("Couldn't get architecture for wave_%ld"), ptid.tid ());
+    error (_("Couldn't get architecture for wave_%s"),
+	   pulongest (wave_id.handle));
 
   uint32_t elf_amdgpu_machine;
   status = amd_dbgapi_architecture_get_info
     (architecture_id, AMD_DBGAPI_ARCHITECTURE_INFO_ELF_AMDGPU_MACHINE,
      sizeof (elf_amdgpu_machine), &elf_amdgpu_machine);
   if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    error (_("Couldn't get elf_amdgpu_machine for architecture_%ld"),
-	   architecture_id.handle);
+    error (_("Couldn't get elf_amdgpu_machine for architecture_%s"),
+	   pulongest (architecture_id.handle));
 
   struct gdbarch_info info;
   info.bfd_arch_info = bfd_lookup_arch (bfd_arch_amdgcn, elf_amdgpu_machine);
@@ -2154,16 +2157,16 @@ amd_dbgapi_target::update_thread_list ()
 		    || wi.stopping)
 		  {
 		    amd_dbgapi_debug_printf
-		      ("wave_%ld disappeared, keeping it"
+		      ("wave_%s disappeared, keeping it"
 		       " (last_resume_mode=%s, stopping=%d)",
-		       wave_id.handle,
+		       pulongest (wave_id.handle),
 		       resume_mode_to_string (wi.last_resume_mode),
 		       wi.stopping);
 		  }
 		else
 		  {
-		    amd_dbgapi_debug_printf ("wave_%ld disappeared, deleting it",
-					     wave_id.handle);
+		    amd_dbgapi_debug_printf ("wave_%s disappeared, deleting it",
+					     pulongest (wave_id.handle));
 		    delete_thread_silent (&tp);
 		  }
 	      }
@@ -2283,8 +2286,8 @@ amd_dbgapi_target::displaced_step_finish (thread_info *thread,
 				sizeof (stop_reason), &stop_reason);
 
   if (status != AMD_DBGAPI_STATUS_SUCCESS)
-    error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
-	   get_status_string (status));
+    error (_("wave_get_info for wave_%s failed (%s)"),
+	   pulongest (wave_id.handle), get_status_string (status));
 
   status = amd_dbgapi_displaced_stepping_complete (wave_id, stepping_id);
 
-- 
2.34.1


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

* Re: [PATCH v2] gdb, amd-dbgapi-target: use pulongest to print id handles
  2026-07-03 11:03 ` [PATCH v2] gdb, amd-dbgapi-target: use pulongest to print id handles Tankut Baris Aktemur
@ 2026-07-03 11:22   ` Lancelot SIX
  0 siblings, 0 replies; 11+ messages in thread
From: Lancelot SIX @ 2026-07-03 11:22 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches

On 03/07/2026 12:03, Tankut Baris Aktemur wrote:
> In amd-dbgapi-target, there are a number of cases where identifier
> handles that are obtained from dbgapi are being printed.  Those
> handles are defined to have type uint64_t.  Use pulongest when
> printing them for improved portability.
> 
> In one case, `ptid.tid ()` was being printed for the wave id.  Change
> this to use `wave_id.handle` for consistency with the other cases.

Hi Baris,

This looks good to me, thanks.

Approved-by: Lancelot Six <lancelot.six@amd.com> (amdgpu)

Best,
Lancelot.

> ---
>   gdb/amd-dbgapi-target.c | 87 +++++++++++++++++++++--------------------
>   1 file changed, 45 insertions(+), 42 deletions(-)
> 
> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> index d0424044e0c..caae9a98501 100644
> --- a/gdb/amd-dbgapi-target.c
> +++ b/gdb/amd-dbgapi-target.c
> @@ -392,18 +392,18 @@ wave_coordinates::to_string () const
>     std::string str = "AMDGPU Wave";
>   
>     str += (agent_id != AMD_DBGAPI_AGENT_NONE
> -	  ? string_printf (" %ld", agent_id.handle)
> +	  ? string_printf (" %s", pulongest (agent_id.handle))
>   	  : " ?");
>   
>     str += (queue_id != AMD_DBGAPI_QUEUE_NONE
> -	  ? string_printf (":%ld", queue_id.handle)
> +	  ? string_printf (":%s", pulongest (queue_id.handle))
>   	  : ":?");
>   
>     str += (dispatch_id != AMD_DBGAPI_DISPATCH_NONE
> -	  ? string_printf (":%ld", dispatch_id.handle)
> +	  ? string_printf (":%s", pulongest (dispatch_id.handle))
>   	  : ":?");
>   
> -  str += string_printf (":%ld", wave_id.handle);
> +  str += string_printf (":%s", pulongest (wave_id.handle));
>   
>     str += (group_ids[0] != UINT32_MAX
>   	  ? string_printf (" (%d,%d,%d)", group_ids[0], group_ids[1],
> @@ -591,9 +591,10 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
>        &action);
>   
>     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %ld "
> +    error (_("amd_dbgapi_report_breakpoint_hit failed for breakpoint %s "
>   	     "at %s (%s)"),
> -	   breakpoint_id.handle, paddress (inf->arch (), bs->bp_location_at->address),
> +	   pulongest (breakpoint_id.handle),
> +	   paddress (inf->arch (), bs->bp_location_at->address),
>   	   get_status_string (status));
>   
>     if (action == AMD_DBGAPI_BREAKPOINT_ACTION_RESUME)
> @@ -624,8 +625,9 @@ amd_dbgapi_target_breakpoint::check_status (struct bpstat *bs)
>        for our breakpoint_id.  */
>     if (resume_breakpoint_id != breakpoint_id)
>       error (_("breakpoint resume event is not for this breakpoint. "
> -	      "Expected breakpoint_%ld, got breakpoint_%ld"),
> -	   breakpoint_id.handle, resume_breakpoint_id.handle);
> +	      "Expected breakpoint_%s, got breakpoint_%s"),
> +	   pulongest (breakpoint_id.handle),
> +	   pulongest (resume_breakpoint_id.handle));
>   
>     amd_dbgapi_event_processed (resume_event_id);
>   }
> @@ -1055,8 +1057,8 @@ amd_dbgapi_target::resume (ptid_t scope_ptid, int step, enum gdb_signal signo)
>   	     indicate that the process has exited.  GDB treats resuming a
>   	     thread that no longer exists as being successful.  */
>   	  && status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> -	error (_("wave_resume for wave_%ld failed (%s)"), wave_id.handle,
> -	       get_status_string (status));
> +	error (_("wave_resume for wave_%s failed (%s)"),
> +	       pulongest (wave_id.handle), get_status_string (status));
>   
>         wi.stopping = false;
>       }
> @@ -1130,12 +1132,12 @@ amd_dbgapi_target::stop (ptid_t ptid)
>   	    }
>   
>   	  if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> -	    error (_("wave_stop for wave_%ld failed (%s)"), wave_id.handle,
> -		   get_status_string (status));
> +	    error (_("wave_stop for wave_%s failed (%s)"),
> +		   pulongest (wave_id.handle), get_status_string (status));
>   	}
>         else if (status != AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID)
> -	error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
> -	       get_status_string (status));
> +	error (_("wave_get_info for wave_%s failed (%s)"),
> +	       pulongest (wave_id.handle), get_status_string (status));
>   
>         /* The status is AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID.  The wave
>   	 could have terminated since the last time the wave list was
> @@ -1145,9 +1147,9 @@ amd_dbgapi_target::stop (ptid_t ptid)
>         wi.stopping = true;
>   
>         amd_dbgapi_debug_printf ("got AMD_DBGAPI_STATUS_ERROR_INVALID_WAVE_ID "
> -			       "for wave_%ld, last_resume_mode=%s, "
> +			       "for wave_%s, last_resume_mode=%s, "
>   			       "report_thread_events=%d",
> -			       wave_id.handle,
> +			       pulongest (wave_id.handle),
>   			       resume_mode_to_string (wi.last_resume_mode),
>   			       m_report_thread_events);
>   
> @@ -1212,8 +1214,8 @@ struct scoped_amd_dbgapi_event_processed
>     {
>       amd_dbgapi_status_t status = amd_dbgapi_event_processed (m_event_id);
>       if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -      warning (_("Failed to acknowledge amd-dbgapi event %" PRIu64),
> -	       m_event_id.handle);
> +      warning (_("Failed to acknowledge amd-dbgapi event %s"),
> +	       pulongest (m_event_id.handle));
>     }
>   
>     DISABLE_COPY_AND_ASSIGN (scoped_amd_dbgapi_event_processed);
> @@ -1268,8 +1270,8 @@ dbgapi_notifier_handler (int err, gdb_client_data client_data)
>   					  sizeof (runtime_state),
>   					  &runtime_state);
>         if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -	error (_("event_get_info for event_%ld failed (%s)"),
> -	       event_id.handle, get_status_string (status));
> +	error (_("event_get_info for event_%s failed (%s)"),
> +	       pulongest (event_id.handle), get_status_string (status));
>   
>         switch (runtime_state)
>   	{
> @@ -1404,7 +1406,7 @@ add_gpu_thread (inferior *inf, ptid_t wave_ptid)
>   
>     if (!info.wave_info_map.try_emplace (wave_id.handle,
>   					wave_info (wave_id)).second)
> -    internal_error ("wave ID %ld already in map", wave_id.handle);
> +    internal_error ("wave ID %s already in map", pulongest (wave_id.handle));
>   
>     /* Create new GPU threads silently to avoid spamming the terminal
>        with thousands of "[New Thread ...]" messages.  */
> @@ -1436,8 +1438,8 @@ process_one_event (amd_dbgapi_inferior_info &info,
>   	  = amd_dbgapi_event_get_info (event_id, AMD_DBGAPI_EVENT_INFO_WAVE,
>   				       sizeof (wave_id), &wave_id);
>   	if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -	  error (_("event_get_info for event_%ld failed (%s)"),
> -		 event_id.handle, get_status_string (status));
> +	  error (_("event_get_info for event_%s failed (%s)"),
> +		 pulongest (event_id.handle), get_status_string (status));
>   
>   	ptid_t event_ptid = make_gpu_ptid (info.inf->pid, wave_id);
>   	target_waitstatus ws;
> @@ -1502,8 +1504,8 @@ process_one_event (amd_dbgapi_inferior_info &info,
>   	      }
>   	  }
>   	else
> -	  error (_("wave_get_info for wave_%ld failed (%s)"),
> -		 wave_id.handle, get_status_string (status));
> +	  error (_("wave_get_info for wave_%s failed (%s)"),
> +		 pulongest (wave_id.handle), get_status_string (status));
>   
>   	info.wave_events.emplace_back (event_ptid, ws);
>   	break;
> @@ -1542,8 +1544,8 @@ process_one_event (amd_dbgapi_inferior_info &info,
>   				       AMD_DBGAPI_EVENT_INFO_RUNTIME_STATE,
>   				       sizeof (runtime_state), &runtime_state);
>   	if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -	  error (_("event_get_info for event_%ld failed (%s)"),
> -		 event_id.handle, get_status_string (status));
> +	  error (_("event_get_info for event_%s failed (%s)"),
> +		 pulongest (event_id.handle), get_status_string (status));
>   
>   	gdb_assert (runtime_state == AMD_DBGAPI_RUNTIME_STATE_UNLOADED);
>   	gdb_assert
> @@ -1620,9 +1622,9 @@ process_event_queue (amd_dbgapi_inferior_info &info,
>   
>         if (event_kind != AMD_DBGAPI_EVENT_KIND_NONE)
>   	amd_dbgapi_debug_printf ("Pulled event from dbgapi: "
> -				 "event_id.handle = %" PRIu64 ", "
> +				 "event_id.handle = %s, "
>   				 "event_kind = %s",
> -				 event_id.handle,
> +				 pulongest (event_id.handle),
>   				 event_kind_str (event_kind));
>   
>         if (event_id == AMD_DBGAPI_EVENT_NONE || event_kind == until_event_kind)
> @@ -1860,8 +1862,8 @@ attach_amd_dbgapi (inferior *inf)
>     /* Are we already attached?  */
>     if (info.process_id != AMD_DBGAPI_PROCESS_NONE)
>       {
> -      amd_dbgapi_debug_printf
> -	("already attached: process_id = %" PRIu64, info.process_id.handle);
> +      amd_dbgapi_debug_printf ("already attached: process_id = %s",
> +			       pulongest (info.process_id.handle));
>         return;
>       }
>   
> @@ -1895,8 +1897,8 @@ attach_amd_dbgapi (inferior *inf)
>         return;
>       }
>   
> -  amd_dbgapi_debug_printf ("process_id = %" PRIu64 ", notifier fd = %d",
> -			   info.process_id.handle,
> +  amd_dbgapi_debug_printf ("process_id = %s, notifier fd = %d",
> +			   pulongest (info.process_id.handle),
>   			   amd_dbgapi_notifier_get_fd (info.notifier));
>   
>     set_process_memory_precision (info);
> @@ -2069,15 +2071,16 @@ amd_dbgapi_target::thread_architecture (ptid_t ptid)
>   				     sizeof (architecture_id),
>   				     &architecture_id);
>     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -    error (_("Couldn't get architecture for wave_%ld"), ptid.tid ());
> +    error (_("Couldn't get architecture for wave_%s"),
> +	   pulongest (wave_id.handle));
>   
>     uint32_t elf_amdgpu_machine;
>     status = amd_dbgapi_architecture_get_info
>       (architecture_id, AMD_DBGAPI_ARCHITECTURE_INFO_ELF_AMDGPU_MACHINE,
>        sizeof (elf_amdgpu_machine), &elf_amdgpu_machine);
>     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -    error (_("Couldn't get elf_amdgpu_machine for architecture_%ld"),
> -	   architecture_id.handle);
> +    error (_("Couldn't get elf_amdgpu_machine for architecture_%s"),
> +	   pulongest (architecture_id.handle));
>   
>     struct gdbarch_info info;
>     info.bfd_arch_info = bfd_lookup_arch (bfd_arch_amdgcn, elf_amdgpu_machine);
> @@ -2154,16 +2157,16 @@ amd_dbgapi_target::update_thread_list ()
>   		    || wi.stopping)
>   		  {
>   		    amd_dbgapi_debug_printf
> -		      ("wave_%ld disappeared, keeping it"
> +		      ("wave_%s disappeared, keeping it"
>   		       " (last_resume_mode=%s, stopping=%d)",
> -		       wave_id.handle,
> +		       pulongest (wave_id.handle),
>   		       resume_mode_to_string (wi.last_resume_mode),
>   		       wi.stopping);
>   		  }
>   		else
>   		  {
> -		    amd_dbgapi_debug_printf ("wave_%ld disappeared, deleting it",
> -					     wave_id.handle);
> +		    amd_dbgapi_debug_printf ("wave_%s disappeared, deleting it",
> +					     pulongest (wave_id.handle));
>   		    delete_thread_silent (&tp);
>   		  }
>   	      }
> @@ -2283,8 +2286,8 @@ amd_dbgapi_target::displaced_step_finish (thread_info *thread,
>   				sizeof (stop_reason), &stop_reason);
>   
>     if (status != AMD_DBGAPI_STATUS_SUCCESS)
> -    error (_("wave_get_info for wave_%ld failed (%s)"), wave_id.handle,
> -	   get_status_string (status));
> +    error (_("wave_get_info for wave_%s failed (%s)"),
> +	   pulongest (wave_id.handle), get_status_string (status));
>   
>     status = amd_dbgapi_displaced_stepping_complete (wave_id, stepping_id);
>   


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

* Re: [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
  2026-07-02 20:15   ` Tom Tromey
@ 2026-07-03 12:59     ` Simon Marchi
  2026-07-03 13:47     ` Pedro Alves
  1 sibling, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2026-07-03 12:59 UTC (permalink / raw)
  To: Tom Tromey, Lancelot SIX; +Cc: Tankut Baris Aktemur, gdb-patches

On 7/2/26 4:15 PM, Tom Tromey wrote:
>>>>>> Lancelot SIX <Lancelot.Six@amd.com> writes:
> 
>> Hi,
>> Doesn't GDB have an implicit habit/policy not to use the PRI??? macros?
> 
>> I do not mind them in amd-dbgapi-target.c, but want to make sure
>> global maintainers are fine with this.
> 
>> I guess the alternative is to use pulongest + %s instead of PRIu64.
> 
>> i.e. something like this:
> 
>>     str += (agent_id != AMD_DBGAPI_AGENT_NONE
>>         ? string_printf (" %s", pulongest (agent_id.handle))
>>         : "?");
> 
> I don't know if it's a policy exactly, but personally I prefer pulongest
> et al, since I find the PRI* macros pretty ugly and unreadable.
> 
> Tom

On my side I have no problem with the PRI macros, I like to use the
standard things where possible.

Alternatively, we could start using fmt :)

Simon

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

* Re: [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
  2026-07-02 20:15   ` Tom Tromey
  2026-07-03 12:59     ` Simon Marchi
@ 2026-07-03 13:47     ` Pedro Alves
  2026-07-03 18:15       ` Tom Tromey
  1 sibling, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2026-07-03 13:47 UTC (permalink / raw)
  To: Tom Tromey, Lancelot SIX; +Cc: Tankut Baris Aktemur, gdb-patches

On 2026-07-02 21:15, Tom Tromey wrote:
>>>>>> Lancelot SIX <Lancelot.Six@amd.com> writes:
> 
>> Hi,
>> Doesn't GDB have an implicit habit/policy not to use the PRI??? macros?
> 
>> I do not mind them in amd-dbgapi-target.c, but want to make sure
>> global maintainers are fine with this.
> 
>> I guess the alternative is to use pulongest + %s instead of PRIu64.
> 
>> i.e. something like this:
> 
>>     str += (agent_id != AMD_DBGAPI_AGENT_NONE
>>         ? string_printf (" %s", pulongest (agent_id.handle))
>>         : "?");
> 
> I don't know if it's a policy exactly, but personally I prefer pulongest
> et al, since I find the PRI* macros pretty ugly and unreadable.

Agreed.  And it's also a portability hazard, like other printf formats, as you have
to match the printf format to the type of the variable.  For typedefs, you have
to either hardcode the underlying type (as here) when it's supposed to be an implementation
detail, or add new PRI-like macros for the typedefs.  The pulongest (and paddress, etc.)
pattern mostly eliminates that, as the printf format is always "%s", at a small runtime cost.

(std::cout << ... << ... tends to be unreadable too IMHO, but we don't use that, thankfully).

{fmt} would be better, but that's either a new external dependency, or 
bump to c++20 for std::format, or c++23 for widely available std::print.

Pedro Alves


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

* Re: [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
  2026-07-03 13:47     ` Pedro Alves
@ 2026-07-03 18:15       ` Tom Tromey
  2026-07-03 19:18         ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2026-07-03 18:15 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, Lancelot SIX, Tankut Baris Aktemur, gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> Agreed.  And it's also a portability hazard, like other printf
Pedro> formats, as you have to match the printf format to the type of
Pedro> the variable.

Yeah, I meant to point this out as well.

For pulongest you really only need to know whether the type is signed;
and we could easily fix that if we cared to.

Pedro> (std::cout << ... << ... tends to be unreadable too IMHO, but we don't use that, thankfully).

I thought about this for ui_file but meh.  It's kind of i18n-unfriendly
as well.

Pedro> {fmt} would be better, but that's either a new external dependency, or 
Pedro> bump to c++20 for std::format, or c++23 for widely available std::print.

I looked into this but it is also i18n-unfriendly, since as far as I can
tell it requires you to either give up gettext or give up any sort of
format checking.  For printf this isn't an issue because GCC understands
that _() is "transparent".

Also of course switching to fmt would be a colossal effort.

Tom

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

* Re: [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
  2026-07-03 18:15       ` Tom Tromey
@ 2026-07-03 19:18         ` Pedro Alves
  2026-07-03 19:22           ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2026-07-03 19:18 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Lancelot SIX, Tankut Baris Aktemur, gdb-patches

On 2026-07-03 19:15, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> Agreed.  And it's also a portability hazard, like other printf
> Pedro> formats, as you have to match the printf format to the type of
> Pedro> the variable.
> 
> Yeah, I meant to point this out as well.
> 
> For pulongest you really only need to know whether the type is signed;
> and we could easily fix that if we cared to.
> 
> Pedro> (std::cout << ... << ... tends to be unreadable too IMHO, but we don't use that, thankfully).
> 
> I thought about this for ui_file but meh.  It's kind of i18n-unfriendly
> as well.

Yeah, like, very unfriendly, similar to how ui_out is unfriendly, with the message split
over several calls.

BTW, we should look into actually doing i18n, like send the .po files to a translation team.
Maybe we can convince one of the new release managers to integrate the flow into the
release process.  :-)

I think Ubuntu had GDB translations at some point.  Haven't checked in a long while.

> 
> Pedro> {fmt} would be better, but that's either a new external dependency, or 
> Pedro> bump to c++20 for std::format, or c++23 for widely available std::print.
> 
> I looked into this but it is also i18n-unfriendly, since as far as I can
> tell it requires you to either give up gettext or give up any sort of
> format checking.  For printf this isn't an issue because GCC understands
> that _() is "transparent".
> 

Ah, yeah, if you run the format string via gettext, that returns a runtime string, so
we'd need to use std::vformat, which as you say, loses compile-time checking:

 std::vformat (_("Hello, {0}!"), std::make_format_args (name));

But I think we could make it work, with a wrapper, which is what we'd do anyhow to
work with ui_file.  But we make the wrapper a template, which itself does the
format checking at compile time.  Like:

  template<typename... Args>
  void
  gdb_format (std::format_string<Args...> f, Args&&... args)
  {
    std::string s
      = std::vformat (gettext (f.get ()), std::make_format_args (args...));
    gdb_stdout->puts (s.c_str ());
  }

then with:

  gdb_printf ("stopped at {}\n", addr);

the literal "stopped at {}\n" gets bound to the format_string<Args...> parameter.
format_string's ctor parses the literal and checks placeholders against Args..., at compile
time, and a mismatch results in a compile error.  If that compiles successfully, then we
call gettext and use std::vformat without the checking, which we no longer need as it's
already validated when we get there.

Note, written in email client, completely untested.

> Also of course switching to fmt would be a colossal effort.
No doubt.  Could be incremental, though.


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

* Re: [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32
  2026-07-03 19:18         ` Pedro Alves
@ 2026-07-03 19:22           ` Pedro Alves
  0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2026-07-03 19:22 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Lancelot SIX, Tankut Baris Aktemur, gdb-patches



On 2026-07-03 20:18, Pedro Alves wrote:
> On 2026-07-03 19:15, Tom Tromey wrote:
>>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>>
>> Pedro> Agreed.  And it's also a portability hazard, like other printf
>> Pedro> formats, as you have to match the printf format to the type of
>> Pedro> the variable.
>>
>> Yeah, I meant to point this out as well.
>>
>> For pulongest you really only need to know whether the type is signed;
>> and we could easily fix that if we cared to.
>>
>> Pedro> (std::cout << ... << ... tends to be unreadable too IMHO, but we don't use that, thankfully).
>>
>> I thought about this for ui_file but meh.  It's kind of i18n-unfriendly
>> as well.
> 
> Yeah, like, very unfriendly, similar to how ui_out is unfriendly, with the message split
> over several calls.
> 
> BTW, we should look into actually doing i18n, like send the .po files to a translation team.
> Maybe we can convince one of the new release managers to integrate the flow into the
> release process.  :-)
> 
> I think Ubuntu had GDB translations at some point.  Haven't checked in a long while.
> 
>>
>> Pedro> {fmt} would be better, but that's either a new external dependency, or 
>> Pedro> bump to c++20 for std::format, or c++23 for widely available std::print.
>>
>> I looked into this but it is also i18n-unfriendly, since as far as I can
>> tell it requires you to either give up gettext or give up any sort of
>> format checking.  For printf this isn't an issue because GCC understands
>> that _() is "transparent".
>>
> 
> Ah, yeah, if you run the format string via gettext, that returns a runtime string, so
> we'd need to use std::vformat, which as you say, loses compile-time checking:
> 
>  std::vformat (_("Hello, {0}!"), std::make_format_args (name));
> 
> But I think we could make it work, with a wrapper, which is what we'd do anyhow to
> work with ui_file.  But we make the wrapper a template, which itself does the
> format checking at compile time.  Like:
> 
>   template<typename... Args>
>   void
>   gdb_format (std::format_string<Args...> f, Args&&... args)
>   {
>     std::string s
>       = std::vformat (gettext (f.get ()), std::make_format_args (args...));
>     gdb_stdout->puts (s.c_str ());
>   }
> 
> then with:
> 
>   gdb_printf ("stopped at {}\n", addr);

To be clear, I meant:

  gdb_format ("stopped at {}\n", addr);

Pedro Alves

> 
> the literal "stopped at {}\n" gets bound to the format_string<Args...> parameter.
> format_string's ctor parses the literal and checks placeholders against Args..., at compile
> time, and a mismatch results in a compile error.  If that compiles successfully, then we
> call gettext and use std::vformat without the checking, which we no longer need as it's
> already validated when we get there.
> 
> Note, written in email client, completely untested.
> 
>> Also of course switching to fmt would be a colossal effort.
> No doubt.  Could be incremental, though.
> 


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

end of thread, other threads:[~2026-07-03 19:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-01 14:28 [PATCH] gdb, amd-dbgapi-target: use PRIu64 and PRIu32 Tankut Baris Aktemur
2026-07-01 15:16 ` Lancelot SIX
2026-07-01 16:56   ` Aktemur, Baris
2026-07-02 20:15   ` Tom Tromey
2026-07-03 12:59     ` Simon Marchi
2026-07-03 13:47     ` Pedro Alves
2026-07-03 18:15       ` Tom Tromey
2026-07-03 19:18         ` Pedro Alves
2026-07-03 19:22           ` Pedro Alves
2026-07-03 11:03 ` [PATCH v2] gdb, amd-dbgapi-target: use pulongest to print id handles Tankut Baris Aktemur
2026-07-03 11:22   ` Lancelot SIX

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox