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 03:34, Firmiana wrote:
> Hi,
> 
> Thanks for the review. Given the amount of changes needed, I plan to prepare a v2 that
> supersedes this version. Before preparing v2, I would like to clarify a few questions.
> 
> First, regarding copyright assignment: I do not currently have an FSF
> copyright assignment on file.  I noticed the 2022 binutils announcement
> that DCO signed contributions are accepted:
> 
>   https://sourceware.org/pipermail/binutils/2022-October/123680.html <https://sourceware.org/pipermail/binutils/2022-October/123680.html>
> 
> and binutils/MAINTAINERS documents DCO as an alternative to FSF copyright
> assignment.  Does this DCO path also apply to GDB patches touching gdb/?
> If so, I will add:
> 
>   Signed-off-by: Jielun Wu <firmiana402@gmail.com <mailto:firmiana402@gmail.com>>
> 
> to the v2 commit.  If GDB still requires FSF copyright assignment for
> this patch, I am willing to start that process.

Thanks a lot.

No it doesn't apply to GDB.  There are ongoing discussions to maybe use
DCO for GDB too but we haven't reached a consensus yet.

> On the technical comments, I agree with most of them and plan to address
> them in v2:
> 
>   - make the helper comments more precise;
>   - change the `buf > buf_end` cases in safe_skip_bytes() to assertions;
>   - switch the new fixed-width integer helpers to reference/template
>     style, unless you prefer keeping them closer to the existing
>     safe_read_uleb128/safe_read_sleb128 style;
>   - add assertions that LEN fits in the destination type;
>   - remove the unrelated whitespace-only changes;
>   - change the dwarf_block_to_dwarf_reg_deref guard to the
>     `gdb_assert (buf <= buf_end); if (buf == buf_end) return -1;`
>     form;
>   - add selftests for dwarf_block_to_dwarf_reg and
>     dwarf_block_to_dwarf_reg_deref;
>   - rewrite the DW_OP_deref* branch using the explicit if/else form you
>     suggested.
> 
> For the helper style: in v1 I mirrored the nearby safe_read_uleb128 and
> safe_read_sleb128 helpers, which use output pointer parameters and short
> "or throw an error" comments.  But I agree that these new helpers can be
> clearer, so unless you prefer otherwise I will use the reference/template
> style you suggested for v2.

Ack.  It's good to leave the place a little nicer than how you found it
:).

Simon