[PATCH] Update the core file architecture if a target description is present

[Simon Marchi]

On 2021-05-18 4:20 p.m., Luis Machado via Gdb-patches wrote:
> At the moment, the core target has its own gdbarch (m_core_gdbarch), and that
> gets set from the core_bfd on the core target's constructor.
> 
> That gdbarch doesn't contain a target description because it is constructed
> before we get a chance to fetch the target description.
> 
> As a result, some hooks that depend on the target description being set are
> not set, and that leads to problems. One of the examples is
> gdbarch_report_signal_info, which is used to show AArch64 tag violation
> information.
> 
> Fix this by updating the core file gdbarch after fetching the target
> description.

It's a bit strange that the core target first determines a gdbarch from
the core bfd, in the constructor.  But then it's the target that is
responsible for providing the target description, also found in a
section of the core bfd, that is ultimately used to determine a new
gdbarch.  Couldn't the target determine the right gdbarch from the
start, in the constructor, from the target description stored in the
core?

Perhaps the answer is obvious, but I don't have a core file with such a
section to test and I'm not sure how to generate one.

> +static void
> +core_on_new_gdbarch (struct gdbarch *newarch)
> +{
> +  /* The target architecture has changed.  Attempt to update the core
> +     target's architecture with NEWARCH's target description.  */
> +  struct gdbarch_info info;
> +
> +  gdbarch_info_init (&info);
> +  info.abfd = core_bfd;
> +  info.target_desc = gdbarch_target_desc (newarch);
> +
> +  struct gdbarch *new_core_arch = gdbarch_find_by_info (info);
> +
> +  /* If we've found a valid architecture, update the core target.  */
> +  if (new_core_arch != nullptr)
> +    {
> +      /* Update the core file architecture.  */
> +      struct core_target *target = get_current_core_target ();
> +
> +      if (target != nullptr)
> +	target->set_core_gdbarch (new_core_arch);
> +    }
> +}

Just remember that core_on_new_gdbarch will be called every time
set_target_arch is set.  That is pretty much every time an exec file is
loaded in an inferior, and every time an inferior is ran.  For this
reason, I think it would be good to put the quick and cheap check "does
the current inferior even have a core target as its process target"
first.  That would just mean doing this first:

  core_target *target = get_current_core_target ();
  if (target == nullptr)
    {
      /* Current inferior does not use a core target.  */
      return;
    }

You can then re-use target in the following code.

> +
>  void _initialize_corelow ();
>  void
>  _initialize_corelow ()
> @@ -1194,4 +1225,8 @@ _initialize_corelow ()
>  	   maintenance_print_core_file_backed_mappings,
>  	   _("Print core file's file-backed mappings."),
>  	   &maintenanceprintlist);
> +
> +  /* Register observer to listen to architecture changes.  */
> +  gdb::observers::architecture_changed.attach (core_on_new_gdbarch,
> +					       "py-unwind");

Replace "py-unwind" with something appropriate, core, core-target,
corelow, etc.

Simon