Re: [PATCH] gdb: Implement stop-on-solib-events for GPU code objects

[Simon Marchi <simark@simark.ca>]

On 9/24/26 9:04 AM, Bratislav Filipovic wrote:
> GDB's "set stop-on-solib-events 1" setting allows users to stop execution
> when shared libraries are loaded or unloaded, enabling inspection and
> breakpoint placement before library code executes. This feature works for
> CPU shared libraries but is not working for GPU code objects loaded by the
> AMD ROCm runtime.
> 
> This commit implements stop-on-solib-events support for GPU code objects,
> making GPU code object load/unload events behave consistently with CPU
> shared library events.
> 
> The root cause is in amd_dbgapi_target_breakpoint::check_status(), which
> unconditionally sets bs->stop = 0 and bs->print_it = print_it_noop,
> regardless of the stop_on_solib_events setting. This is in contrast to
> internal_breakpoint::check_status() for CPU shared libraries, which
> respects the setting.
> 
> The fix includes:
> 
> 1. Generalize print_solib_event() function to eliminate code duplication
>    between CPU and GPU event printing. The function now accepts parameters
>    for event description, field names, and plural forms. This reduces
>    ~50 lines of duplicated code.
> 
> 2. Add code_object_list_updated flag to amd_dbgapi_inferior_info to track
>    when AMD_DBGAPI_EVENT_KIND_CODE_OBJECT_LIST_UPDATED events occur during
>    process_event_queue().

This flag does not need to live in amd_dbgapi_inferior_info, since it's

only needed within the duration of one

amd_dbgapi_target_breakpoint::check_status call.  It's not a state that

needs to be persisted.  Perhaps have process_event_queue return that

value, so check_status can use it?  process_event_queue already returns

a value (amd_dbgapi_event_id_t), it could return a small struct instead.

> +enum print_stop_action
> +amd_dbgapi_target_breakpoint::print_it (const bpstat *bs) const
> +{
> +  /* We only reach here when check_status set bs->print_it to print_it_normal,
> +     which happens only for GPU code object events when stop_on_solib_events
> +     is enabled.  */
> +  bool any_deleted = !current_program_space->deleted_solibs.empty ();
> +  bool any_added = !current_program_space->added_solibs.empty ();
> +
> +  if (any_added || any_deleted)
> +    current_uiout->text (_("Stopped due to GPU code object event:\n"));
> +  else
> +    current_uiout->text (_("Stopped due to GPU code object event (no "
> +			   "code objects added or removed)\n"));
> +
> +  if (current_uiout->is_mi_like_p ())
> +    {
> +      current_uiout->field_string
> +	("reason", async_reason_lookup (EXEC_ASYNC_SOLIB_EVENT));
> +      current_uiout->field_string ("object-kind", gpu_code_object_kind);
> +    }

This (and the change in print_solib_event) adds a new field tot he stops
of kind "solib-event".  I think it should be documented where
"solib-event" is described, in section "GDB/MI Async Records" of the
documentation.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index a24f67cb8de..2c948737720 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -22513,14 +22513,15 @@ The surrounding square brackets are optional.
>  @item set stop-on-solib-events
>  @kindex set stop-on-solib-events
>  This command controls whether @value{GDBN} should give you control
> -when the dynamic linker notifies it about some shared library event.
> -The most common event of interest is loading or unloading of a new
> -shared library.
> +when the dynamic linker notifies it about some shared library event,
> +or when GPU code objects are loaded or unloaded (AMD ROCm targets).
> +The most common events of interest are loading or unloading of a new
> +shared library or code object.
>  
>  @item show stop-on-solib-events
>  @kindex show stop-on-solib-events
>  Show whether @value{GDBN} stops and gives you control when shared
> -library events happen.
> +library events or GPU code object events happen.
>  @end table
>  
>  Shared libraries are also supported in many cross or remote debugging
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 92b21017b03..5e00f9d653c 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -10849,8 +10849,9 @@ leave it stopped or free to run as needed."),
>  Set stopping for shared library events."), _("\
>  Show stopping for shared library events."), _("\
>  If nonzero, gdb will give control to the user when the dynamic linker\n\
> -notifies gdb of shared library events.  The most common event of interest\n\
> -to the user would be loading/unloading of a new library."),
> +notifies gdb of shared library events, or when GPU code objects are loaded\n\
> +or unloaded (AMD ROCm targets).  The most common events of interest to the\n\

As a GDB developer, I find this documentation change a bit unnecessary,
because I know that GPU code objects are (in GDB) the same as shared
libraries, just with another name.  But I guess it's good to be
explicit, because that might not be clear to end users.  But I would not
mention "(AMD ROCm targets)" part (same for the documentation part
above), because we want to keep the documentation of these commands
fairly target-agnostic.

Will the term "GPU code objects" be applicable for the Intel GPU target
too?

> +proc test_enabled {} {
> +    with_rocm_gpu_lock {
> +	clean_restart $::testfile
> +
> +	if {![runto_main -inferior-args $::hipmodule_path]} {
> +	    return
> +	}
> +
> +	gdb_breakpoint [gdb_get_line_number "Enable SOLIB events here"] -temporary
> +	gdb_continue_to_breakpoint "at enable solib-event"
> +
> +	gdb_test_no_output "set stop-on-solib-events 1"
> +
> +	with_test_prefix "file://" {
> +	    gdb_breakpoint "test_file_load" -temporary
> +	    gdb_continue_to_breakpoint "at test_file_load"
> +
> +	    # Test 1: file:// load event (hipModuleLoad).
> +	    gdb_test "continue" \
> +		"Stopped due to GPU code object event.*Inferior loaded file://\[^\r\n\]+" \
> +		"load"
> +
> +	    # Test 2: file:// unload event (hipModuleUnload).
> +	    gdb_test "continue" \
> +		"Stopped due to GPU code object event.*Inferior unloaded file://\[^\r\n\]+" \
> +		"unload"
> +	}
> +
> +	with_test_prefix "memory://" {
> +	    gdb_breakpoint "test_memory_load" -temporary
> +	    gdb_continue_to_breakpoint "at test_memory_load"
> +
> +	    # Test 3: memory:// load event (hipModuleLoadData).
> +	    gdb_test "continue" \
> +		"Stopped due to GPU code object event.*Inferior loaded memory://\[^\r\n\]+" \
> +		"load"
> +
> +	    # Test 4: memory:// unload event (hipModuleUnload).
> +	    gdb_test "continue" \
> +		"Stopped due to GPU code object event.*Inferior unloaded memory://.*" \

This last line should use \[^\r\n\]+ like the other ones I guess.

Simon