From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-136527-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 121451 invoked by alias); 26 Jan 2017 15:38:40 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Received: (qmail 121416 invoked by uid 89); 26 Jan 2017 15:38:37 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=corp, Corp, sk:contrib, contributionchecklist
X-HELO: mx1.redhat.com
Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Jan 2017 15:38:27 +0000
Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26])	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))	(No client certificate requested)	by mx1.redhat.com (Postfix) with ESMTPS id 4F5F78048C;	Thu, 26 Jan 2017 15:38:27 +0000 (UTC)
Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4])	by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0QFcGDP008210;	Thu, 26 Jan 2017 10:38:26 -0500
Subject: Re: [PATCH v3 5/5] Add support for Intel PKRU register to GDB and GDBserver.
To: Michael Sturm <michael.sturm@intel.com>, mark.kettenis@xs4all.nl,        eliz@gnu.org
References: <1481021894-29471-1-git-send-email-michael.sturm@intel.com> <1481021894-29471-6-git-send-email-michael.sturm@intel.com>
Cc: gdb-patches@sourceware.org
From: Pedro Alves <palves@redhat.com>
Message-ID: <c160cf95-ddf6-2c62-963b-588558f6c1d2@redhat.com>
Date: Thu, 26 Jan 2017 15:38:00 -0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <1481021894-29471-6-git-send-email-michael.sturm@intel.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
X-SW-Source: 2017-01/txt/msg00583.txt.bz2

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