Re: [PATCH 01/43] Replace the symbol needs evaluator with a parser

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-04-28 6:17 a.m., Zoran Zaric wrote:
> 
>>> Some concerns were raised that a linear scan of the expression byte
>>> stream would have issues if a DWARF producer would try to hide some
>>> non DWARF related data after a control flow operation. Although the
>>> testsuite doesn't show this case, it is a valid concern, so one of
>>> the later patches in this series will address it by switching back to
>>> the then redesigned DWARF expression evaluator.
>>
>> While re-reading your exchange with Tom, I was under the impression that
>> traversing the "control flow graph" of the expression, visiting each
>> node only once, would be a good solution.  It would avoid the infinite
>> loop problem, the "two branches" problem, and even the corner cases
>> where you have garbage in the middle of the expression, or if the
>> expression jumps in the middle of an instruction to re-use the operand
>> of an instruction as an instruction.
>>
>> Patch 39 changes this back to an evaluator, so I'm not sure if this is
>> what you implemented or something else, I'll see when I get there.
> 
> There is no concept of a "control flow graph" in the  existing gdb evaluator, there is just a byte stream that gdb either fully traverse and evaluates or it "fakes" the target access.

When I meant "control flow graph", I didn't mean to have a proper
control flow graph representation with classes and all (unless we think
it would be useful), but just navigate it conceptually.

You maintain a queue of "to visit" instructions and a set of "already
visited" instructions.  You by pushing the first instruction in the
queue and work until the queue is empty.  For all instructions but
conditional jumps, you push the instruction after it in the queue.  For
conditional jumps, you also push the jump target in the queue.  You
maintain the "already visited" set on the side to avoid visiting the
same instruction twice, in case there are loops.

> 
> The solution that Tom talked about was just an idea that he used in some other tool and would need to be implemented from scratch.
> 
> The patch 39 switches the evaluator design to throwing the missing context exception idea which re-enables using the same evaluator for both purposes.

I'll see and understand better when I get there.  But from what I
understood of past dicussions, using the evaluator may give wrong
results, because one branch may require only registers while another
branch may require a frame (that's the point of your patch).  So I don't
really understand how going back to using an evaluator would work.

Although that depends on the semantic we want symbol_needs to have:

 - find what's needed to evaluate the expression in the current context,
   only in the branch that would be taken right now?
 - find what's needed to evaluate the expression in any possible
   context, considering all branches?

Simon