Re: [PATCH v2 4/5] gdbstub: Add Xfer:siginfo:read stub

[Gustavo Romero]

Hi Richard!

On 3/7/24 6:13 PM, Richard Henderson wrote:
> On 3/7/24 08:26, Gustavo Romero wrote:
> > +void gdb_handle_query_xfer_siginfo(GArray *params, void *user_ctx)
> > +{
> > +    unsigned long offset, len;
> > +    uint8_t *siginfo_offset;
> > +
> > +    offset = get_param(params, 0)->val_ul;
> > +    len = get_param(params, 1)->val_ul;
> > +
> > +    if (offset + len > sizeof(target_siginfo_t)) {
>
> If you save the siginfo_len from gdb_handlesig, you can place this in user.c
Shouldn't all user-only stubs be placed in user-target.c? Like
gdb_handle_query_xfer_auxv and gdb_handle_query_xfer_exec_file, and since
what controls the inclusion in the build of user-target.c is CONFIG_USER_ONLY?


> Is it really correct to reject (offset == 0) + (len == large), rather than 
> truncate len?

I think this is correct. GDB mentions briefly that an invalid offset
should be treated as an error. Thus, I think that a valid offset but
a non-existing/invalid (large) length should be treated the same,
cause in the end data on invalid offsets are being requested anyways.


> > +    /* Reply */
> > +    g_string_assign(gdbserver_state.str_buf, "l");
> > +    gdb_memtox(gdbserver_state.str_buf, (const char *)siginfo_offset, len);
>
> It seems easy enough to reply with the exact length remaining...

I think the correct is to reply an error in case GDB asks a data
we don't have rather than returning anything else to satisfy GDB.
If offset+len is inside target_siginfo_t, than that's ok, otherwise
that's an error.


Cheers,
Gustavo