This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 5/5] Add support for Intel PKRU register to GDB and GDBserver.
- From: Pedro Alves <palves at redhat dot com>
- To: Michael Sturm <michael dot sturm at intel dot com>, mark dot kettenis at xs4all dot nl, eliz at gnu dot org
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 26 Jan 2017 15:38:15 +0000
- Subject: Re: [PATCH v3 5/5] Add support for Intel PKRU register to GDB and GDBserver.
- Authentication-results: sourceware.org; auth=none
- References: <1481021894-29471-1-git-send-email-michael.sturm@intel.com> <1481021894-29471-6-git-send-email-michael.sturm@intel.com>
On 12/06/2016 10:58 AM, Michael Sturm wrote:
> diff --git a/gdb/features/i386/32bit-pkeys.xml b/gdb/features/i386/32bit-pkeys.xml
> new file mode 100644
> index 0000000..84fff51
> --- /dev/null
> +++ b/gdb/features/i386/32bit-pkeys.xml
> @@ -0,0 +1,13 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2015-2016 Free Software Foundation, Inc.
2017.
> +
> +/* PKRU register? */
> +
> +int
> +i386_pkru_regnum_p (struct gdbarch *gdbarch, int regnum)
We should start using bool.
> +{
> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> + int pkru_regnum = tdep->pkru_regnum;
> +
> + if (pkru_regnum < 0)
> + return 0;
return false;
> +
> + regnum -= pkru_regnum;
> + return regnum >= 0 && regnum < I387_NUM_PKEYS_REGS;
> +}
> +
> + Copyright 2015-2016 Free Software Foundation, Inc.
> +
> + Contributed by Intel Corp. <michael.sturm@intel.com>
Please see:
https://sourceware.org/gdb/wiki/ContributionChecklist#Attribution
In the test:
> +#include <stdio.h>
This is not needed, right?
> +#include "x86-cpuid.h"
> +# Read values from pseudo registers.
> +gdb_breakpoint [ gdb_get_line_number "break here 1" ]
> +gdb_continue_to_breakpoint "break here 1" ".*break here 1.*"
> +
> +# set test_string ".*0x12345678.*"
> +gdb_test "info register pkru" ".*pkru.*0x12345678.*" "read pkru register"
> +
> +# set test_string ".*0x44444444.*"
> +gdb_test "print /x \$pkru = 0x44444444" "= 0x44444444" "set pkru value"
> +gdb_test "info register pkru" ".*pkru.*0x44444444.*" "read value after setting value"
> +
> +gdb_breakpoint [ gdb_get_line_number "break here 2" ]
> +gdb_continue_to_breakpoint "break here 2" ".*break here 2.*"
> +
> +gdb_test "print /x rd_value" ".*" "variable after reading pkru"
Is this last test too lax? It'll basically accept anything
but a crash, including an error.
Otherwise it LGTM.
Thanks,
Pedro Alves