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

Zoran Zaric via Gdb-patches
2021-04-28T10:18:11Z

>> 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.

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.

Having a separate control flow representation is still a good idea for
the future because it removes the need for byte stream parsing code
duplication that is present in couple of places.

>
>> -class symbol_needs_eval_context : public dwarf_expr_context
>> +static enum symbol_needs_kind
>> +dwarf2_get_symbol_read_needs (gdb::array_view<const gdb_byte> expr,
>> + dwarf2_per_cu_data *per_cu,
>> + dwarf2_per_objfile *per_objfile,
>> + bfd_endian byte_order,
>> + int addr_size,
>> + int ref_addr_size,
>> + int depth = 0)
>
> The wrapped lines above are missing one column of indent.
>
> Simon
>

Thanks, I will fix this in the next iteration.

Zoran