Re: [PATCH] gdb: ensure bp_location::section is set correct to avoid an assert

["Sébastien Darche" <sdarche@efficios.com>]

On 9/25/25 17:40, Andrew Burgess wrote:
> Maybe the answer is as simple as moving the .section assignment into the
> earlier if block, something like:
> 
>    if (is_function && want_start_sal)
>      {
>        sal = find_function_start_sal (func_addr, NULL, self->funfirstline);
> 
>        /* This breakpoint is for the ifunc case, FUNC_ADDR is can be
>           anywhere, in a completely different section to MSYMBOL, or even
>           in a different objfile!
> 
>           TODO: I haven't checked, maybe find_function_start_sal already
>           fills this stuff in for us?  Or maybe it could be made too?
>           For now I'm assuming all we have is an address, but this needs
>           checking.  */
>        sal.section = find_pc_overlay (func_addr);
>        if (sal.section == nullptr)
>          sal.section = find_pc_section (func_addr);
>      }
>    else
>      {
>        sal.objfile = objfile;
>        sal.msymbol = msymbol;
>        /* Store func_addr, not the minsym's address in case this was an
> 	 ifunc that hasn't been resolved yet.  */
>        if (is_function)
> 	sal.pc = func_addr;
>        else
> 	sal.pc = msymbol->value_address (objfile);
>        sal.pspace = current_program_space;
> 
>        /* We can assign the section based on MSYMBOL here because the
>           breakpoint is actually being placed at (or near) MSYMBOL.  */
>        sal.section = msymbol->obj_section (objfile);
>      }
> 

To answer your question on whether find_function_start_sal does fill 
this for us : it depends. It manages to do it on amd64 but not on amdgpu.

By default, the sal does not contain a valid section. It's only when we 
try to adjust the pc past the prologue (skip_prologue_sal) that a 
section is computed for the pc at the start of the function. If we do 
have a prologue, then we assign that section (symtab.c:3914). If not 
(and that is the case on amdgpu), then we're left with an empty 
sal.section. I would say the behavior is not really consistent.

I would agree it could be made to.
> Does this look like a valid path forward maybe?

Your solution seems to work for the gnu-ifunc test and fixes the 
regression for gdb.rocm/displaced-stepping.exp - so I'd say it's a good 
aproach. I am not familiar with overlays, so I can't really judge if the 
change would impact how they are handled.

I think it would be best to ensure find_function_start_sal has a 
consistent behavior across architectures. I'll submit a small patch 
which should address this. This would also at least reduce the chance 
for another bug like this to appear somewhere else :

 > So then you'll have to add a find_pc_section in _another_ place....


Sébastien