Re: [PATCH v2 07/16] Remove the exec_bfd macro

[Simon Marchi <simark@simark.ca>]

On 2020-10-19 5:44 p.m., Tom Tromey wrote:
> This removes the exec_bfd macro, in favor of new accessors on
> program_space.  In one spot the accessor can't be used; but this is
> still a big improvement over the macro, IMO.


Maybe just check for lines that are too long after the replacement.

A quick way I use is:

    $ git show | sed 's|\t|        |' | less

And when in less, I search for:

    ^\+.{81}

That highlights the lines that are added that are too long.

> @@ -480,18 +482,20 @@ exec_file_attach (const char *filename, int from_tty)
>  		 gdb_bfd_errmsg (bfd_get_error (), matching).c_str ());
>  	}
>
> -      target_section_table sections = build_section_table (exec_bfd);
> +	  target_section_table sections
> +	  = build_section_table (current_program_space->exec_bfd ());
>
> -      current_program_space->ebfd_mtime = bfd_get_mtime (exec_bfd);
> +      current_program_space->ebfd_mtime
> +	= bfd_get_mtime (current_program_space->exec_bfd ());
>
>        validate_files ();
>
> -      set_gdbarch_from_file (exec_bfd);
> +      set_gdbarch_from_file (current_program_space->exec_bfd ());
>
>        /* Add the executable's sections to the current address spaces'
>  	 list of sections.  This possibly pushes the exec_ops
>  	 target.  */
> -      add_target_sections (&exec_bfd, sections);
> +      add_target_sections (&current_program_space->ebfd, sections);

It's odd that the owner of these sections is the address of the ebfd
field in the program_space structure, and not the bfd pointer itself.
Do you know if this is done on purpose?

The only reason I would see is if we wanted a stable "owner" value even
when the exec bfd changes.  But I don't see any comment to that effect.

Looking at the code, it looks like adding and removing the target
sections is tied to setting and unsetting the exec bfd.

 - exec_file_attach sets the exec bfd and adds the corresponding target
   sections
 - program_space::close unsets the exec bfd and removes the
   corresponding target sections

I don't see anything that could change the value of exec bfd in the mean
time, that would make the "owner" value of these sections stale.  So I
believe it would be safe to use `current_program_space->exec_bfd ()` as
the owner.

Otherwise, this patch LGTM.

Simon