Re: [PATCH] Fix length calculation in aarch64_linux_set_debug_regs

[Pedro Alves <palves@redhat.com>]

On 11/02/2015 02:48 PM, Simon Marchi wrote:
> There is this build failure when building in C++:
> 
> In file included from build-gnulib/import/stddef.h:45:0,
>                  from /usr/include/time.h:37,
>                  from build-gnulib/import/time.h:41,
>                  from build-gnulib/import/sys/stat.h:44,
>                  from ../bfd/bfd.h:44,
>                  from /home/simark/src/binutils-gdb/gdb/common/common-types.h:35,
>                  from /home/simark/src/binutils-gdb/gdb/common/common-defs.h:44,
>                  from /home/simark/src/binutils-gdb/gdb/nat/aarch64-linux-hw-point.c:19:
> /home/simark/src/binutils-gdb/gdb/nat/aarch64-linux-hw-point.c: In function ‘void aarch64_linux_set_debug_regs(const aarch64_debug_reg_state*, int, int)’:
> /home/simark/src/binutils-gdb/gdb/nat/aarch64-linux-hw-point.c:564:64: error: ‘count’ cannot appear in a constant-expression
>    iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs[count - 1])
>                                                                 ^
> 
> I don't really understand the length computation done here.
> 
> From what I understand, the dbg_regs array in the user_hwdebug_state structure
> is 16 elements long, but we don't use all of them.  We want iov_len to reflect
> only the used bytes.  If that's true, I don't think that the current code is
> correct.
> Instead, it can be computed simply with:
> 
>   offsetof (struct user_hwdebug_state, dbg_regs) + count * sizeof (regs.dbg_reg[0]);
> 
> Does it make sense?

IIUYC, you're pointing out two issues:

#1 - the offsetof that doesn't work in C++.

#2 - an off-by-one.

I don't know enough about Aarch64 to judge #1, but it does sound right to me.

On #2, I saw the same on x86.  See my fix here:

  https://sourceware.org/ml/gdb-patches/2015-02/msg00213.html

I think it's a little nicer to hide away the offsetof+sizeof.

Thanks,
Pedro Alves