This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH] Fix length calculation in aarch64_linux_set_debug_regs
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Pedro Alves <palves at redhat dot com>, <gdb-patches at sourceware dot org>
- Cc: <qiyaoltc at gmail dot com>
- Date: Mon, 2 Nov 2015 12:09:16 -0500
- Subject: Re: [PATCH] Fix length calculation in aarch64_linux_set_debug_regs
- Authentication-results: sourceware.org; auth=none
- References: <1446475684-31936-1-git-send-email-simon dot marchi at ericsson dot com> <56378884 dot 70001 at redhat dot com>
I looked a bit more into the issue and did some testing, and it appears the
current code is correct (as Yao mentioned). It's probably not as clear as it
could be though. I think it would be nicer if expressed as
(size of fixed part) + (size of variable part)
On 15-11-02 11:00 AM, Pedro Alves wrote:
> IIUYC, you're pointing out two issues:
> #1 - the offsetof that doesn't work in C++.
This actually appears to be a bug in g++.
See this, especially towards the end: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14932
The expression is accepted by gcc and clang++, but not g++ (test with 4.8 and 5.2).
> #2 - an off-by-one.
Never mind, I thought wrongly at first that sizeof (regs.dbg_regs [count - 1])
would give the size for "count - 1" elements, but it's actually the size of a
single element. So scratch that, the current code works fine.
So the only reason to change the code would be to circumvent a bug in g++, which
is probably a valid one (otherwise we can't build).
> 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:
> I think it's a little nicer to hide away the offsetof+sizeof.
You mean hide in in a function? This expression is only used at one place and I think it's
reasonably straightforward if expressed correctly, but if you think it will make the code
clearer I don't mind.