Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Yao Qi <qiyaoltc@gmail.com>
To: Patrick Palka <patrick@parcs.ath.cx>
Cc: Doug Evans <dje@google.com>,  Keith Seitz <keiths@redhat.com>,
	 gdb-patches <gdb-patches@sourceware.org>
Subject: Re: Several regressions and we branch soon.
Date: Wed, 24 Jun 2015 11:55:00 -0000	[thread overview]
Message-ID: <86mvzpqq1z.fsf@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.20.8.1506231742590.4322@idea> (Patrick Palka's	message of "Tue, 23 Jun 2015 17:45:02 -0400 (EDT)")

Patrick Palka <patrick@parcs.ath.cx> writes:

> The problem here seems to be that the "show" function corresponding to
> the "mpx bound" command calls error("Intel(R) Memory Protection
> Extensions not supported on this target."); which throws an exception
> that causes the entire "info set"/"show" loop to exit early.  Should

Yes, I think your analysis is correct.

> "show foo" ever throw an exception?  Should the "info set"/"show" loop
> expect an exception to be thrown from a show function?

IMO, "show foo" shouldn't throw an exception.

>
> This diff fixes the default.exp FAILs as far as I can tell:
>

Thanks for the fix, but I feel that the original code has some drawbacks,

> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index 42d0346..d11efa1 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -8777,11 +8777,17 @@ i386_mpx_info_bounds (char *args, int from_tty)
>    struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
>
>    if (!i386_mpx_enabled ())
> -    error (_("Intel(R) Memory Protection Extensions not\
> - supported on this target."));
> +    {
> +      printf_unfiltered (_("Intel(R) Memory Protection Extensions not "
> +			   "supported on this target.\n"));
> +      return;
> +    }
>
>    if (args == NULL)
> -    error (_("Address of pointer variable expected."));
> +    {
> +      printf_unfiltered (_("Address of pointer variable expected.\n"));
> +      return;
> +    }
>
>    addr = parse_and_eval_address (args);

It is odd that command "info mpx bounds" requires an argument, which is
an address.  The set/show command pair works in a way that command set
modify or update some state of GDB, and command info just display the
state of GDB.  That is to say, "set mpx bounds ADDRESS" should set
address, which is stored somewhere in GDB (in a global variable or a
per-inferior variable), and "info mpx bounds" shows the current state or
setting in GDB, no argument is needed.

If this is a right direction, I can give a patch.

b.t.w, is it a good (or bad) idea to register this command if mpx
is supported on the target?

-- 
Yao (齐尧)


  reply	other threads:[~2015-06-24 11:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-23 18:31 Doug Evans
2015-06-23 18:55 ` Patrick Palka
2015-06-23 19:03   ` Doug Evans
2015-06-23 20:17     ` Keith Seitz
2015-06-23 20:53       ` Doug Evans
2015-06-23 21:45         ` Patrick Palka
2015-06-24 11:55           ` Yao Qi [this message]
2015-06-25 16:35             ` Tedeschi, Walfred
2015-07-01  8:49               ` Yao Qi
     [not found]                 ` <AC542571535E904D8E8ADAE745D60B1944445D44@IRSMSX104.ger.corp.intel.com>
2015-07-01  9:30                   ` Walfred Tedeschi
2015-07-02 10:09                     ` Yao Qi
2015-07-02 15:34           ` Yao Qi
2015-07-02 16:19             ` [PATCH] Don't throw an error in "show mpx bound" implementation Patrick Palka
2015-07-06  9:31               ` Yao Qi
2015-06-24 10:21 ` Several regressions and we branch soon Yao Qi
     [not found]   ` <87lhf8yz90.fsf@br87z6lw.de.ibm.com>
2015-06-25 13:34     ` Doug Evans
2015-06-25 18:00       ` Andreas Arnez
2015-06-30 15:21         ` Yao Qi
2015-06-30 18:09           ` Andreas Arnez
2015-07-01  8:01             ` Yao Qi
2015-07-10  9:33             ` Yao Qi
2015-07-10 16:12               ` Andreas Arnez
2015-07-10 16:23                 ` Ulrich Weigand
2015-07-20 15:08                   ` Andreas Arnez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86mvzpqq1z.fsf@gmail.com \
    --to=qiyaoltc@gmail.com \
    --cc=dje@google.com \
    --cc=gdb-patches@sourceware.org \
    --cc=keiths@redhat.com \
    --cc=patrick@parcs.ath.cx \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox