(Moved review to correct thread)
Thanks for the reviews.
On 7 Jun 2018, at 21:18, Simon Marchi <simon.marchi@ericsson.com>
wrote:
Hi Alan,
Just some quick comments.
I get this when building on x86-64 with --enable-targets=all:
Hmm.. I had lost that flag from my build script. I Re-added it, and
reproduced the issues.
CXX aarch64-tdep.o
In file included from
/home/emaisin/src/binutils-gdb/gdb/nat/aarch64-sve-linux-ptrace.h:29:0,
from
/home/emaisin/src/binutils-gdb/gdb/aarch64-tdep.c:61:
/home/emaisin/src/binutils-gdb/gdb/nat/aarch64-linux-sigcontext.h:19:22:
error: field ‘head’ has incomplete type ‘_aarch64_ctx’
struct _aarch64_ctx head;
^
/home/emaisin/src/binutils-gdb/gdb/nat/aarch64-linux-sigcontext.h:19:9:
note: forward declaration of ‘struct _aarch64_ctx’
struct _aarch64_ctx head;
^
First, we should not include "nat/aarch64-sve-linux-ptrace.h" (a file
that only makes
sense when building on AArch64) in aarch64-tdep.c, a file built on all
architecture
when including the support for AArch64 debugging. It looks like
aarch64-tdep.c
needs sve_vq_from_vl. Maybe that definition could be moved to arch/,
which can be
included in aarch64-tdep.c.
I had put it in there because I wanted to try and make it a complete
block
copied from Linux. The issue makes sense, so I’ve updated to restore
sve_vq_from_vl/sve_vl_from_vq back to arch/aarch64.h and removed it
from
nat/aarch64-linux-sigcontext.h
Then, is the _aarch64_ctx structure guaranteed to be defined on older
AArch64 kernels
or should we include it too?
_aarch64_ctx is part of the standard aarch64 signal handling. A quick
git blame gives
me 2012 - which is roughly the age of aarch64. So, it should always be
defined.
Updated patch below. Checked it builds (with other sve patches) on:
X86 all-targets
Aarch64 Linux 4.10 (pre sve headers) ubuntu 16.04
Aarch64 Linux 4.15 (with sve headers) ubuntu 18.04
Are you ok with the new version?