Re: [PATCH][PR gdb/34239][PR gdb/34299] gdb: Check bounds before reading DWARF expression operands

[Simon Marchi <simark@simark.ca>]

On 2026-06-26 04:31, Firmiana wrote:
> 
> Hi,
> 
> Thanks for the review and for testing the patch.
> 
> Before preparing v2, I would like to respond to a few points.
> 
> About the safe_read_* helper implementation: yes, your understanding is
> right.  The helper saves the original pointer, advances through
> safe_skip_bytes to validate that the requested byte range is available,
> and only then calls extract_*_integer on the saved pointer.  I agree that
> this was not obvious enough, so I will make the helper comments and/or
> implementation clearer in v2.
> 
> On the question of reading into `uoffset` and then assigning to the real
> destination: that was mainly done to mirror the nearby
> safe_read_uleb128/safe_read_sleb128 helper style, which uses fixed output
> types and output pointer parameters.  Based on your comment and the other
> review, I plan to change the new fixed-width helpers to use references,
> probably templated, so many call sites can read directly into `result`,
> `this->m_len`, or the relevant field.

The GDB code used to be C, and is now organically evolving into C++.
When you find some opportunities to use some C++ constructs to make the
code better, feel free to do so.

> For DW_OP_deref_size / DW_OP_deref_type: the v1 code is intended to be
> equivalent to:
> 
>   addr_size = *op_ptr;
>   op_ptr++;
> 
> but with a bounds check first.  safe_read_unsigned_integer keeps the
> original pointer for extract_unsigned_integer and uses safe_skip_bytes to
> validate and compute the next pointer.  Since this was confusing, I will
> rewrite the branch more explicitly in v2.
> 
> I will also switch the testcase loop to foreach_with_prefix.

Good idea.

Simon