[PATCH] Remove only use of current_inferior ()->gdbarch outside of gdbarch.*

[PATCH] Remove only use of current_inferior ()->gdbarch outside of gdbarch.*

[Doug Evans]

Hi.

target_gdbarch is a magical thing, there are different kinds of arches:
for multiarch systems (e.g., call) which arch you use for certain
struct gdbarch API calls is important.

E.g.,
https://sourceware.org/ml/gdb-patches/2015-08/msg00789.html

Until this is cleaned up it'd be best to only use target_gdbarch
and not its underlying implementation.
Fortunately there is only one place that needed changing
(further reinforcing my sense that only target_gdbarch should be
used and never current_inferior ()->gdbarch).

Alas, short of an API reorg (which I think is reasonable), I can't think
of a way to prevent this from recurring.


2015-08-27  Doug Evans  <xdje42@gmail.com>

	* ravenscar-thread.c (ravenscar_inferior_created): Replace
	current_inferior ()->gdbarch with its wrapper target_gdbarch.

diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index ef82507..fe36c96 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -341,7 +341,7 @@ ravenscar_inferior_created (struct target_ops *target, int from_tty)
   struct ravenscar_arch_ops *ops;
 
   if (!ravenscar_task_support
-      || gdbarch_ravenscar_ops (current_inferior ()->gdbarch) == NULL
+      || gdbarch_ravenscar_ops (target_gdbarch ()) == NULL
       || !has_ravenscar_runtime ())
     return;
 

Re: [PATCH] Remove only use of current_inferior ()->gdbarch outside of gdbarch.*

[Doug Evans]

On Thu, Aug 27, 2015 at 10:15 PM, Doug Evans <xdje42@gmail.com> wrote:
> Hi.
>
> target_gdbarch is a magical thing, there are different kinds of arches:
> for multiarch systems (e.g., call) which arch you use for certain
> struct gdbarch API calls is important.
>
> E.g.,
> https://sourceware.org/ml/gdb-patches/2015-08/msg00789.html
>
> Until this is cleaned up it'd be best to only use target_gdbarch
> and not its underlying implementation.
> Fortunately there is only one place that needed changing
> (further reinforcing my sense that only target_gdbarch should be
> used and never current_inferior ()->gdbarch).
>
> Alas, short of an API reorg (which I think is reasonable), I can't think
> of a way to prevent this from recurring.
>
>
> 2015-08-27  Doug Evans  <xdje42@gmail.com>
>
>         * ravenscar-thread.c (ravenscar_inferior_created): Replace
>         current_inferior ()->gdbarch with its wrapper target_gdbarch.
>
> diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
> index ef82507..fe36c96 100644
> --- a/gdb/ravenscar-thread.c
> +++ b/gdb/ravenscar-thread.c
> @@ -341,7 +341,7 @@ ravenscar_inferior_created (struct target_ops *target, int from_tty)
>    struct ravenscar_arch_ops *ops;
>
>    if (!ravenscar_task_support
> -      || gdbarch_ravenscar_ops (current_inferior ()->gdbarch) == NULL
> +      || gdbarch_ravenscar_ops (target_gdbarch ()) == NULL
>        || !has_ravenscar_runtime ())
>      return;
>

Hmmm, forgot about ARI, bleah.
We could use that to at least catch recurrences.

Re: [PATCH] Remove only use of current_inferior ()->gdbarch outside of gdbarch.*

[Ulrich Weigand]

Doug Evans wrote:
> On Thu, Aug 27, 2015 at 10:15 PM, Doug Evans <xdje42@gmail.com> wrote:
> > 2015-08-27  Doug Evans  <xdje42@gmail.com>
> >
> >         * ravenscar-thread.c (ravenscar_inferior_created): Replace
> >         current_inferior ()->gdbarch with its wrapper target_gdbarch.

Just as a quick comment: this goes exactly into the opposite direction
from what we discussed in the other thread.  I think we should replace
target_gdbarch () with current_inferior ()->gdbarch *everywhere*,
instead of reverting that here ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Remove only use of current_inferior ()->gdbarch outside of gdbarch.*

[Doug Evans]

On Fri, Aug 28, 2015 at 10:21 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Doug Evans wrote:
>> On Thu, Aug 27, 2015 at 10:15 PM, Doug Evans <xdje42@gmail.com> wrote:
>> > 2015-08-27  Doug Evans  <xdje42@gmail.com>
>> >
>> >         * ravenscar-thread.c (ravenscar_inferior_created): Replace
>> >         current_inferior ()->gdbarch with its wrapper target_gdbarch.
>
> Just as a quick comment: this goes exactly into the opposite direction
> from what we discussed in the other thread.  I think we should replace
> target_gdbarch () with current_inferior ()->gdbarch *everywhere*,
> instead of reverting that here ...

This does nothing to fix the underlying problem, which is the
referencing of global state instead obtaining the needed state
(inferior, gdbarch, or whatever) from the passed in context.
That's what I was talking about in the other thread at any rate.
*In the mean time*, let's be consistent, and this patch is simpler.

When we do go to properly fix this (or at least take the next step
to properly fixing this), *then* we can go through and remove all
the target_gdbarch calls.

Re: [PATCH] Remove only use of current_inferior ()->gdbarch outside of gdbarch.*

[Ulrich Weigand]

Doug Evans wrote:
> On Fri, Aug 28, 2015 at 10:21 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > Doug Evans wrote:
> >> On Thu, Aug 27, 2015 at 10:15 PM, Doug Evans <xdje42@gmail.com> wrote:
> >> > 2015-08-27  Doug Evans  <xdje42@gmail.com>
> >> >
> >> >         * ravenscar-thread.c (ravenscar_inferior_created): Replace
> >> >         current_inferior ()->gdbarch with its wrapper target_gdbarch.
> >
> > Just as a quick comment: this goes exactly into the opposite direction
> > from what we discussed in the other thread.  I think we should replace
> > target_gdbarch () with current_inferior ()->gdbarch *everywhere*,
> > instead of reverting that here ...
> 
> This does nothing to fix the underlying problem, which is the
> referencing of global state instead obtaining the needed state
> (inferior, gdbarch, or whatever) from the passed in context.
> That's what I was talking about in the other thread at any rate.
> *In the mean time*, let's be consistent, and this patch is simpler.
> 
> When we do go to properly fix this (or at least take the next step
> to properly fixing this), *then* we can go through and remove all
> the target_gdbarch calls.

Well, OK.  I guess I can see why we'd want to use target_gdbarch
consistently until it's eliminated completely ...  I don't really
object to this patch, in any case.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Remove only use of current_inferior ()->gdbarch outside of gdbarch.*

[Doug Evans]

"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> Doug Evans wrote:
>> On Fri, Aug 28, 2015 at 10:21 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>> > Doug Evans wrote:
>> >> On Thu, Aug 27, 2015 at 10:15 PM, Doug Evans <xdje42@gmail.com> wrote:
>> >> > 2015-08-27  Doug Evans  <xdje42@gmail.com>
>> >> >
>> >> >         * ravenscar-thread.c (ravenscar_inferior_created): Replace
>> >> >         current_inferior ()->gdbarch with its wrapper target_gdbarch.
>> >
>> > Just as a quick comment: this goes exactly into the opposite direction
>> > from what we discussed in the other thread.  I think we should replace
>> > target_gdbarch () with current_inferior ()->gdbarch *everywhere*,
>> > instead of reverting that here ...
>> 
>> This does nothing to fix the underlying problem, which is the
>> referencing of global state instead obtaining the needed state
>> (inferior, gdbarch, or whatever) from the passed in context.
>> That's what I was talking about in the other thread at any rate.
>> *In the mean time*, let's be consistent, and this patch is simpler.
>> 
>> When we do go to properly fix this (or at least take the next step
>> to properly fixing this), *then* we can go through and remove all
>> the target_gdbarch calls.
>
> Well, OK.  I guess I can see why we'd want to use target_gdbarch
> consistently until it's eliminated completely ...  I don't really
> object to this patch, in any case.

Committed, thanks.