Re: RFC: remove gdbarch from struct breakpoint

["Ulrich Weigand" <uweigand@de.ibm.com>]

Pedro Alves wrote:
> On Monday 07 November 2011 15:20:58, Joel Brobecker wrote:
> > I think that makes sense. I am trying to figure out how a breakpoint
> > could have a gdbarch that made some sort of sense when the breakpoint
> > has two locations and each location had a different gdbarch from
> > the other....
> 
> History behind the fields:
>  http://sourceware.org/ml/gdb-patches/2009-06/msg00215.html
> and:
>  http://sourceware.org/ml/gdb-patches/2009-07/msg00075.html
> 
> Reading the first url, I was wondering if we'd indeed need the
> breakpoint's gdbarch for reparsing conditions and watchpoint
> expressions (or anything that uses expressions instead of linespecs),
> but I can't find such dependency in the code.  Maybe Ulrich can
> take a look at this.  The Cell combined debugger can maybe
> reveal hidden dependencies with the gdbarch fallbacks we do.

Ahh.  That seems to be one of the "open ends" of multi-arch support
that has never been addressed so far.  Currently, expression parsing
always takes place in the "current" architecture context; see the
get_current_arch call in parse_exp_in_context.  This is really a bug;
the parse routines probably ought to take a gdbarch parameter instead.

(The reasons why expression parsing is dependent on the architecture
are basically those Stan described; things like how to interpret register
names, struct layout ABI, and predefined types like "long".)

Having a gdbarch field in "struct breakpoint" was in part a preparation
for this: once expression parsing is -properly- done specifically in an
architecture context, we need to decide which arch to pass when parsing
(and re-parsing) breakpoint conditions and expressions.  My thought was
that the right way to do that would be to remember and re-use the
architecture that was current at the time the expression was initially
entered; just like is currently done with the language and input radix
used when reparsing conditions and watchpoint expressions.

Most other architecture-dependent things in breakpoint.c etc really
should use a per-location architecture.  However, the way the per-
location arch is currently chosen is to either determine it from
the location --but this works only if we find a symtab!-- or else
fall back to the main breakpoint architecture.  If we remove that
fall back, there will be cases where breakpoints are set at locations
with no symtab (e.g. in generated code, stack trampolines, etc.)
and we will not find any associated architecture.

Now maybe it is not really correct to use the same field both for the
architecture to parse a breakpoint expression, and as the fall-back
location arch for locations without symtab.  Maybe the fall-back could
instead be a per-program_space default gdbarch ... or we could add a
gdbarch field to the SAL, and require all users to fill it ...

> > In most cases, it is sufficient to replace the use of this field with
> > the location's gdbarch instead.  In fact, I think the cases in
> > tracepoint.c where this is not done are probably latent bugs.
> 
> Yeah.

Agreed about the tracepoint.c uses.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com