Several regressions and we branch soon.

Several regressions and we branch soon.

[Doug Evans]

Hi.

fyi, I'm seeing regressions in the following tests on amd64-linux.

FAIL: gdb.threads/attach-stopped.exp: nonthreaded: attach2 to stopped bt
FAIL: gdb.threads/attach-stopped.exp: threaded: attach2 to stopped bt
FAIL: gdb.base/attach-pie-noexec.exp: attach
FAIL: gdb.base/attach-pie-noexec.exp: info shared
FAIL: gdb.base/attach-twice.exp: attach
FAIL: gdb.base/attach.exp: attach1, after setting file
FAIL: gdb.base/attach.exp: attach1 detach (the program is no longer running)
FAIL: gdb.base/attach.exp: attach2, with no file
FAIL: gdb.base/attach.exp: after attach2, set should_exit
FAIL: gdb.base/attach.exp: continue to breakpoint: postloop (the
program is no longer running)
FAIL: gdb.base/attach.exp: continue until exit at after attach2, exit
(the program is no longer running)
FAIL: gdb.base/attach.exp: attach when process' a.out not in cwd
FAIL: gdb.base/attach.exp: continue until exit (the program is no
longer running)
FAIL: gdb.base/attach.exp: starting with --pid (timeout)
FAIL: gdb.base/attach.exp: cmdline attach run: run to prompt
UNRESOLVED: gdb.base/attach.exp: cmdline attach run: run to main
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO:
BINprelinkNOdebugNOpieNO: attach: attach main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO:
BINprelinkNOdebugNOpieYES: attach: seen displacement message as
NONZERO
... plus lots more for break-interp.exp ...
FAIL: gdb.base/default.exp: info set
FAIL: gdb.base/default.exp: show
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=off: re-attach
to inferior
FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs
FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs (the
program is no longer running)
FAIL: gdb.base/gnu_vector.exp: set vector return value
FAIL: gdb.multi/multi-attach.exp: backtrace 1
FAIL: gdb.multi/multi-attach.exp: backtrace 2
FAIL: gdb.python/py-sync-interp.exp: attach and where
FAIL: gdb.server/ext-attach.exp: backtrace 1
FAIL: gdb.server/ext-attach.exp: detach (the program is no longer running)
FAIL: gdb.server/ext-attach.exp: backtrace 2
FAIL: gdb.server/ext-attach.exp: monitor exit
FAIL: gdb.threads/attach-into-signal.exp: nonthreaded: detach (the
program is no longer running)
UNRESOLVED: gdb.threads/attach-into-signal.exp: nonthreaded: attach
(pass 1), pending signal catch
FAIL: gdb.threads/attach-into-signal.exp: threaded: thread apply 2
print $_siginfo.si_signo
FAIL: gdb.threads/attach-into-signal.exp: threaded: detach (the
program is no longer running)
UNRESOLVED: gdb.threads/attach-into-signal.exp: threaded: attach (pass
1), pending signal catch

I haven't root-caused any of them yet.
Is anyone else seeing these on amd64-linux?

Re: Several regressions and we branch soon.

[Patrick Palka]

On Tue, Jun 23, 2015 at 2:31 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> fyi, I'm seeing regressions in the following tests on amd64-linux.
>
> FAIL: gdb.threads/attach-stopped.exp: nonthreaded: attach2 to stopped bt
> FAIL: gdb.threads/attach-stopped.exp: threaded: attach2 to stopped bt
> FAIL: gdb.base/attach-pie-noexec.exp: attach
> FAIL: gdb.base/attach-pie-noexec.exp: info shared
> FAIL: gdb.base/attach-twice.exp: attach
> FAIL: gdb.base/attach.exp: attach1, after setting file
> FAIL: gdb.base/attach.exp: attach1 detach (the program is no longer running)
> FAIL: gdb.base/attach.exp: attach2, with no file
> FAIL: gdb.base/attach.exp: after attach2, set should_exit
> FAIL: gdb.base/attach.exp: continue to breakpoint: postloop (the
> program is no longer running)
> FAIL: gdb.base/attach.exp: continue until exit at after attach2, exit
> (the program is no longer running)
> FAIL: gdb.base/attach.exp: attach when process' a.out not in cwd
> FAIL: gdb.base/attach.exp: continue until exit (the program is no
> longer running)
> FAIL: gdb.base/attach.exp: starting with --pid (timeout)
> FAIL: gdb.base/attach.exp: cmdline attach run: run to prompt
> UNRESOLVED: gdb.base/attach.exp: cmdline attach run: run to main
> FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO:
> BINprelinkNOdebugNOpieNO: attach: attach main bt
> FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO:
> BINprelinkNOdebugNOpieYES: attach: seen displacement message as
> NONZERO
> ... plus lots more for break-interp.exp ...
> FAIL: gdb.base/default.exp: info set
> FAIL: gdb.base/default.exp: show
> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=on: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=off: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=on: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=off: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=on: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=off: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=on: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=off: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=on: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=off: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=on: re-attach to inferior
> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=off: re-attach
> to inferior
> FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
> FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs
> FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs (the
> program is no longer running)
> FAIL: gdb.base/gnu_vector.exp: set vector return value
> FAIL: gdb.multi/multi-attach.exp: backtrace 1
> FAIL: gdb.multi/multi-attach.exp: backtrace 2
> FAIL: gdb.python/py-sync-interp.exp: attach and where
> FAIL: gdb.server/ext-attach.exp: backtrace 1
> FAIL: gdb.server/ext-attach.exp: detach (the program is no longer running)
> FAIL: gdb.server/ext-attach.exp: backtrace 2
> FAIL: gdb.server/ext-attach.exp: monitor exit
> FAIL: gdb.threads/attach-into-signal.exp: nonthreaded: detach (the
> program is no longer running)
> UNRESOLVED: gdb.threads/attach-into-signal.exp: nonthreaded: attach
> (pass 1), pending signal catch
> FAIL: gdb.threads/attach-into-signal.exp: threaded: thread apply 2
> print $_siginfo.si_signo
> FAIL: gdb.threads/attach-into-signal.exp: threaded: detach (the
> program is no longer running)
> UNRESOLVED: gdb.threads/attach-into-signal.exp: threaded: attach (pass
> 1), pending signal catch
>
> I haven't root-caused any of them yet.
> Is anyone else seeing these on amd64-linux?

GDB has not been clean of FAILs for a while right? IIRC I've been
seeing dozens of FAILs since I started submitting patches to GDB
(around June 2014).  So I wonder how many of these are long-standing
FAILs not introduced in this development cycle.

Re: Several regressions and we branch soon.

[Doug Evans]

On Tue, Jun 23, 2015 at 1:55 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Tue, Jun 23, 2015 at 2:31 PM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> fyi, I'm seeing regressions in the following tests on amd64-linux.
>>
>> FAIL: gdb.threads/attach-stopped.exp: nonthreaded: attach2 to stopped bt
>> FAIL: gdb.threads/attach-stopped.exp: threaded: attach2 to stopped bt
>> FAIL: gdb.base/attach-pie-noexec.exp: attach
>> FAIL: gdb.base/attach-pie-noexec.exp: info shared
>> FAIL: gdb.base/attach-twice.exp: attach
>> FAIL: gdb.base/attach.exp: attach1, after setting file
>> FAIL: gdb.base/attach.exp: attach1 detach (the program is no longer running)
>> FAIL: gdb.base/attach.exp: attach2, with no file
>> FAIL: gdb.base/attach.exp: after attach2, set should_exit
>> FAIL: gdb.base/attach.exp: continue to breakpoint: postloop (the
>> program is no longer running)
>> FAIL: gdb.base/attach.exp: continue until exit at after attach2, exit
>> (the program is no longer running)
>> FAIL: gdb.base/attach.exp: attach when process' a.out not in cwd
>> FAIL: gdb.base/attach.exp: continue until exit (the program is no
>> longer running)
>> FAIL: gdb.base/attach.exp: starting with --pid (timeout)
>> FAIL: gdb.base/attach.exp: cmdline attach run: run to prompt
>> UNRESOLVED: gdb.base/attach.exp: cmdline attach run: run to main
>> FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO:
>> BINprelinkNOdebugNOpieNO: attach: attach main bt
>> FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO:
>> BINprelinkNOdebugNOpieYES: attach: seen displacement message as
>> NONZERO
>> ... plus lots more for break-interp.exp ...
>> FAIL: gdb.base/default.exp: info set
>> FAIL: gdb.base/default.exp: show
>> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=on: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=off: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=on: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=off: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=on: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=off: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=on: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=off: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=on: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=off: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=on: re-attach to inferior
>> FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=off: re-attach
>> to inferior
>> FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
>> FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs
>> FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs (the
>> program is no longer running)
>> FAIL: gdb.base/gnu_vector.exp: set vector return value
>> FAIL: gdb.multi/multi-attach.exp: backtrace 1
>> FAIL: gdb.multi/multi-attach.exp: backtrace 2
>> FAIL: gdb.python/py-sync-interp.exp: attach and where
>> FAIL: gdb.server/ext-attach.exp: backtrace 1
>> FAIL: gdb.server/ext-attach.exp: detach (the program is no longer running)
>> FAIL: gdb.server/ext-attach.exp: backtrace 2
>> FAIL: gdb.server/ext-attach.exp: monitor exit
>> FAIL: gdb.threads/attach-into-signal.exp: nonthreaded: detach (the
>> program is no longer running)
>> UNRESOLVED: gdb.threads/attach-into-signal.exp: nonthreaded: attach
>> (pass 1), pending signal catch
>> FAIL: gdb.threads/attach-into-signal.exp: threaded: thread apply 2
>> print $_siginfo.si_signo
>> FAIL: gdb.threads/attach-into-signal.exp: threaded: detach (the
>> program is no longer running)
>> UNRESOLVED: gdb.threads/attach-into-signal.exp: threaded: attach (pass
>> 1), pending signal catch
>>
>> I haven't root-caused any of them yet.
>> Is anyone else seeing these on amd64-linux?
>
> GDB has not been clean of FAILs for a while right? IIRC I've been
> seeing dozens of FAILs since I started submitting patches to GDB
> (around June 2014).  So I wonder how many of these are long-standing
> FAILs not introduced in this development cycle.

GDB has not been clean "since forever".
However, my testing harness takes that into account.
I have lists of tests I expect to see fail for each of my test configs
(fission, no-fission, and so on), and my harness post-processes
gdb.sum to produce a better report of pass/fail.
(using a version of validate_failures.py from gcc)
When something shows up that I haven't seen before,
and I run these tests A LOT, I can tell.

Every one of the above is new(ish - at least new in the
last few months - I've been sidelined for a few reasons
and haven't been running the tests as often as I normally do).

Re: Several regressions and we branch soon.

[Keith Seitz]

FWIW, I updated my virgin sandbox earlier, and I don't see a bunch of
the failures that you are/have. I've listed the ones I do see below.
FWIW, I have another branch that was last updated the first week of May
which shows none of these problems.

I'll start looking at these failures immediately.

Keith

On 06/23/2015 12:03 PM, Doug Evans wrote:

>>> FAIL: gdb.base/default.exp: info set
>>> FAIL: gdb.base/default.exp: show
>>> FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
>>> FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs
>>> FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs (the
>>> program is no longer running)
>>> FAIL: gdb.base/gnu_vector.exp: set vector return value

Re: Several regressions and we branch soon.

[Doug Evans]

On Tue, Jun 23, 2015 at 3:17 PM, Keith Seitz <keiths@redhat.com> wrote:
> FWIW, I updated my virgin sandbox earlier, and I don't see a bunch of
> the failures that you are/have. I've listed the ones I do see below.
> FWIW, I have another branch that was last updated the first week of May
> which shows none of these problems.
>
> I'll start looking at these failures immediately.
>
> Keith
>
> On 06/23/2015 12:03 PM, Doug Evans wrote:
>
>>>> FAIL: gdb.base/default.exp: info set
>>>> FAIL: gdb.base/default.exp: show
>>>> FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
>>>> FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs
>>>> FAIL: gdb.base/gnu_vector.exp: continue to add_some_intvecs (the
>>>> program is no longer running)
>>>> FAIL: gdb.base/gnu_vector.exp: set vector return value

Stupid yama.
My machine rebooted and I lost my disabling of it.
sudo sysctl -w kernel.yama.ptrace_scope=0
Another pre-check to add to my harness, bleah.

My list now matches yours.

Re: Several regressions and we branch soon.

[Patrick Palka]

On Tue, 23 Jun 2015, Doug Evans wrote:

> On Tue, Jun 23, 2015 at 3:17 PM, Keith Seitz <keiths@redhat.com> wrote:
>> FWIW, I updated my virgin sandbox earlier, and I don't see a bunch of
>> the failures that you are/have. I've listed the ones I do see below.
>> FWIW, I have another branch that was last updated the first week of May
>> which shows none of these problems.
>>
>> I'll start looking at these failures immediately.
>>
>> Keith
>>
>> On 06/23/2015 12:03 PM, Doug Evans wrote:
>>
>>>>> FAIL: gdb.base/default.exp: info set
>>>>> FAIL: gdb.base/default.exp: show

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
"show foo" ever throw an exception?  Should the "info set"/"show" loop
expect an exception to be thrown from a show function?

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

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);

@@ -8856,7 +8862,7 @@ static struct cmd_list_element *mpx_set_cmdlist, *mpx_show_cmdlist;
  static void
  set_mpx_cmd (char *args, int from_tty)
  {
-  help_list (mpx_set_cmdlist, "set mpx", all_commands, gdb_stdout);
+  help_list (mpx_set_cmdlist, "set mpx ", all_commands, gdb_stdout);
  }

  /* Helper function for the CLI commands.  */
@@ -8901,7 +8907,7 @@ is \"default\"."),

    add_prefix_cmd ("mpx", class_support, set_mpx_cmd, _("\
  Set Intel(R) Memory Protection Extensions specific variables."),
-		  &mpx_set_cmdlist, "set tdesc ",
+		  &mpx_set_cmdlist, "set mpx ",
  		  0 /* allow-unknown */, &setlist);

    /* Add "mpx" prefix for the show commands.  */

Re: Several regressions and we branch soon.

[Yao Qi]

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 (齐尧)

RE: Several regressions and we branch soon.

[Tedeschi, Walfred]

Hi Patrick,

This command was proposed initially as an standalone like:

Set-mpx-bound and show-mpx-bound.

Recommendation was to introduce it in the sets and shows, which I have agreed.
Also because "set" is also used to set values of variables when used alone.
Which is similar to what "set mpx bound" is doing, In this sense it can be considered as the right category to have it, as Joel indicated.

About the show, well that is the natural counterpart of the set, right?

Also, I agree with Yao patch. I would use a "warning" instead.

Initialization of the command can be placed in a different location. I could think of adding them at the validation of the tdesc, i.e.
I386_validate_tdesc_p and amd64_validate_tdesc_p.  Would you agree with that?

Open questions are:
1. Command call. Should they still be called "set mpx bound" / "show mpx bound"
2. Should initialization move to the validation routine?


Thanks a lot and regards,
-Fred

-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Yao Qi
Sent: Wednesday, June 24, 2015 1:55 PM
To: Patrick Palka
Cc: Doug Evans; Keith Seitz; gdb-patches
Subject: Re: Several regressions and we branch soon.

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 (齐尧)
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: Several regressions and we branch soon.

[Yao Qi]

"Tedeschi, Walfred" <walfred.tedeschi@intel.com> writes:

> This command was proposed initially as an standalone like:
>
> Set-mpx-bound and show-mpx-bound.
>
> Recommendation was to introduce it in the sets and shows, which I have agreed.
> Also because "set" is also used to set values of variables when used alone.
> Which is similar to what "set mpx bound" is doing, In this sense it
> can be considered as the right category to have it, as Joel indicated.
>
> About the show, well that is the natural counterpart of the set, right?
>
> Also, I agree with Yao patch. I would use a "warning" instead.
>
> Initialization of the command can be placed in a different location. I
> could think of adding them at the validation of the tdesc, i.e.
> I386_validate_tdesc_p and amd64_validate_tdesc_p.  Would you agree with that?
>
> Open questions are:
> 1. Command call. Should they still be called "set mpx bound" / "show
> mpx bound"

Command "show mpx bound" expects an argument, which is an address.  Is
this argument mandatory?  In other words, can gdb scan bound directory
and bound table from inferior memory and print all entries? this would
be slow, but "show mpx bound" doesn't need an argument.

After I read intel mpx doc and the patch, I have more questions in my
mind,

 - if program doesn't set mpx bounds at all, GDB attaches to the program,
   and set mpx bounds, when GDB detaches from this program, does GDB
   need to clear these mpx bounds it sets?

 - if program does set mpx bounds too (through mpx instructions or
   compiler builtins), do we expect GDB to show these mpx bounds too?

 - If program sets mpx bounds through mxp instructions and GDB sets mpx
   bounds too, does this interfere each other? or program's mxp bounds
   setting is stored in bnd0-bnd3, but GDB's mpx bound setting is bound
   directory and bound table, so this doesn't interfere each other?

> 2. Should initialization move to the validation routine?

If we do so, commands are not shown up on the target doesn't meet the
requirements of commands.  After some thinking, I prefer registering
mpx commands unconditionally even target doesn't support mpx.  The
"show" command still can tell user that this command doesn't work on
this target.  Otherwise, it should be confusing that some commands
disappear silently.

-- 
Yao (齐尧)

Re: Several regressions and we branch soon.

[Walfred Tedeschi]

Hello Yao,

Thank you for your feedback!

See below:

> This command was proposed initially as an standalone like: 
> Set-mpx-bound and show-mpx-bound. Recommendation was to introduce it 
> in the sets and shows, which I have agreed. Also because "set" is also 
> used to set values of variables when used alone. Which is similar to 
> what "set mpx bound" is doing, In this sense it can be considered as 
> the right category to have it, as Joel indicated. About the show, well 
> that is the natural counterpart of the set, right? Also, I agree with 
> Yao patch. I would use a "warning" instead. Initialization of the 
> command can be placed in a different location. I could think of adding 
> them at the validation of the tdesc, i.e. I386_validate_tdesc_p and 
> amd64_validate_tdesc_p. Would you agree with that? Open questions are: 
> 1. Command call. Should they still be called "set mpx bound" / "show 
> mpx bound"
> Command "show mpx bound" expects an argument, which is an address.  Is this argument mandatory?  In other words, can gdb scan bound directory and bound table from inferior memory and print all entries? this would be slow, but "show mpx bound" doesn't need an argument.
Though slow i think it could be done. In case of displaying all entries 
there will be no context for the user. It means he will se a big table 
of numbers.  Therefore we decided to display only for the pointer desired.
In case you guys think that it might be also interesting we could spand 
the command work in that way.
> After I read intel mpx doc and the patch, I have more questions in my mind,
>
>   - if program doesn't set mpx bounds at all, GDB attaches to the program,
>     and set mpx bounds, when GDB detaches from this program, does GDB
>     need to clear these mpx bounds it sets?
In case program does not set bounds GDB will also not able to set 
bounds. Basically idea is to have bounds as variables.
Once user has modified its done.
>   - if program does set mpx bounds too (through mpx instructions or
>     compiler builtins), do we expect GDB to show these mpx bounds too?
No. Same as above.
>   - If program sets mpx bounds through mxp instructions and GDB sets mpx
>     bounds too, does this interfere each other? or program's mxp bounds
>     setting is stored in bnd0-bnd3, but GDB's mpx bound setting is bound
>     directory and bound table, so this doesn't interfere each other?

Yes. Like it happens with variables location dependent variables.
To be able to to set bounds also on registers and tables and make them 
sync we need debug information.
The solution by now is without. In this sense user has to know a bit of 
assembler to know where to set the bounds, for the case it being debugged.

>> 2. Should initialization move to the validation routine?
> If we do so, commands are not shown up on the target doesn't meet the requirements of commands.  After some thinking, I prefer registering mpx commands unconditionally even target doesn't support mpx.  The "show" command still can tell user that this command doesn't work on this target.  Otherwise, it should be confusing that some commands disappear silently
> --
> Yao (齐尧)

Hope I have answered your questions.

Thanks a lot and best regards,
-Fred
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: Several regressions and we branch soon.

[Yao Qi]

Walfred Tedeschi <walfred.tedeschi@intel.com> writes:

>>   - if program doesn't set mpx bounds at all, GDB attaches to the program,
>>     and set mpx bounds, when GDB detaches from this program, does GDB
>>     need to clear these mpx bounds it sets?
> In case program does not set bounds GDB will also not able to set
> bounds. Basically idea is to have bounds as variables.
> Once user has modified its done.

so GDB can only update and show the mpx bounds set in the program, is it correct?

>>   - if program does set mpx bounds too (through mpx instructions or
>>     compiler builtins), do we expect GDB to show these mpx bounds too?
> No. Same as above.

Your answer to Q1 is contradictive to it.

>>   - If program sets mpx bounds through mxp instructions and GDB sets mpx
>>     bounds too, does this interfere each other? or program's mxp bounds
>>     setting is stored in bnd0-bnd3, but GDB's mpx bound setting is bound
>>     directory and bound table, so this doesn't interfere each other?
>
> Yes. Like it happens with variables location dependent variables.

Sorry, I don't understand what does "Yes" mean to my alternative
question.  What are "variables location dependent variables"?

> To be able to to set bounds also on registers and tables and make them
> sync we need debug information.

Can you elaborate on this?  Why do we need debug information?  AFAIK,
bounds are stored in either registers and tables, GDB can read them from
registers and tables, and show them.

> The solution by now is without. In this sense user has to know a bit
> of assembler to know where to set the bounds, for the case it being
> debugged.

-- 
Yao (齐尧)

Re: Several regressions and we branch soon.

[Yao Qi]

Hi Patrick,
After discussing with Walfred Tedeschi on intel mpx, I think command
"show mpx bound" still needs an input argument, otherwise, GDB
will display many bound table entries, which is less useful to
users.

I don't want to rename the command to "show-mpx-bound", because GDB
doesn't have any other "show-*" commands.  So looks your fix is the
best one I can think of.

On 23/06/15 22:45, Patrick Palka wrote:
> 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"));

The indentation looks wrong to me.

> +      return;
> +    }

Could you add the changelog entry in your patch, and post it again?
Then, it can be reviewed properly.

-- 
Yao (齐尧)

[PATCH] Don't throw an error in "show mpx bound" implementation

[Patrick Palka]

"show" functions should not throw an exception in part because it causes
the output of the commands "info set" and "show" to get truncated.

This fixes the following fails:

    FAIL: gdb.base/default.exp: info set
    FAIL: gdb.base/default.exp: show

gdb/ChangeLog:

	* i386-tdep.c (i386_mpx_info_bounds): Don't call error, instead
	use printf_unfiltered.
	(set_mpx_cmd): Add missing trailing space to command string
	literal.
	(_initialize_i386_tdep): Give the "mpx" prefix command its
	correct name.
---
 gdb/i386-tdep.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 85e433e..371a282 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8775,11 +8775,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);
 
@@ -8854,7 +8860,7 @@ static struct cmd_list_element *mpx_set_cmdlist, *mpx_show_cmdlist;
 static void
 set_mpx_cmd (char *args, int from_tty)
 {
-  help_list (mpx_set_cmdlist, "set mpx", all_commands, gdb_stdout);
+  help_list (mpx_set_cmdlist, "set mpx ", all_commands, gdb_stdout);
 }
 
 /* Helper function for the CLI commands.  */
@@ -8899,7 +8905,7 @@ is \"default\"."),
 
   add_prefix_cmd ("mpx", class_support, set_mpx_cmd, _("\
 Set Intel(R) Memory Protection Extensions specific variables."),
-		  &mpx_set_cmdlist, "set tdesc ",
+		  &mpx_set_cmdlist, "set mpx ",
 		  0 /* allow-unknown */, &setlist);
 
   /* Add "mpx" prefix for the show commands.  */
-- 
2.5.0.rc0.5.g91e10c5.dirty

Re: [PATCH] Don't throw an error in "show mpx bound" implementation

[Yao Qi]

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

> "show" functions should not throw an exception in part because it causes
> the output of the commands "info set" and "show" to get truncated.
>
> This fixes the following fails:
>
>     FAIL: gdb.base/default.exp: info set
>     FAIL: gdb.base/default.exp: show
>
> gdb/ChangeLog:
>
> 	* i386-tdep.c (i386_mpx_info_bounds): Don't call error, instead
> 	use printf_unfiltered.
> 	(set_mpx_cmd): Add missing trailing space to command string
> 	literal.
> 	(_initialize_i386_tdep): Give the "mpx" prefix command its
> 	correct name.

It looks good to me.  Thanks.

-- 
Yao (齐尧)

Re: Several regressions and we branch soon.

[Yao Qi]

Doug Evans <dje@google.com> writes:

> ... plus lots more for break-interp.exp ...
> FAIL: gdb.base/default.exp: info set
> FAIL: gdb.base/default.exp: show

I can see these two fails in my machine too.

> FAIL: gdb.base/gnu_vector.exp: call add_singlevecs

I see this fail too,
print add_singlevecs((char1) {6}, (int1) {12}, (double1) {24})^M
^M
Program received signal SIGSEGV, Segmentation fault.^M
0x00000000004008ec in add_singlevecs (a=..., b=..., c=...) at ../../../../../binutils-gdb/gdb/testsuite/gdb.base/gnu_vector.c:132^M
132       return (double1) {a[0] + b[0] + c[0]};^M
The program being debugged was signaled while in a function called from GDB.^M
GDB remains in the frame where the signal was received.^M
To change this behavior use "set unwindonsignal on".^M
Evaluation of the expression containing the function^M
(add_singlevecs) will be abandoned.^M
When the function is done executing, GDB will silently stop.^M
(gdb) FAIL: gdb.base/gnu_vector.exp: call add_singlevecs

-- 
Yao (齐尧)

Re: Several regressions and we branch soon.

[Andreas Arnez]

On Wed, Jun 24 2015, Yao Qi wrote:

> Doug Evans <dje@google.com> writes:
>
>> FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
>
> I see this fail too,
> print add_singlevecs((char1) {6}, (int1) {12}, (double1) {24})^M
> ^M
> Program received signal SIGSEGV, Segmentation fault.^M
> 0x00000000004008ec in add_singlevecs (a=..., b=..., c=...) at ../../../../../binutils-gdb/gdb/testsuite/gdb.base/gnu_vector.c:132^M
> 132       return (double1) {a[0] + b[0] + c[0]};^M
> The program being debugged was signaled while in a function called from GDB.^M
> GDB remains in the frame where the signal was received.^M
> To change this behavior use "set unwindonsignal on".^M
> Evaluation of the expression containing the function^M
> (add_singlevecs) will be abandoned.^M
> When the function is done executing, GDB will silently stop.^M
> (gdb) FAIL: gdb.base/gnu_vector.exp: call add_singlevecs

This is not a regression in GDB, but a new test uncovering GDB's missing
vector ABI support on x86_64.  So for the branch I suggest to suppress
these new FAILs with gnu_vector.exp -- either in the test case or in the
test harness.

Re: Several regressions and we branch soon.

[Doug Evans]

On Thu, Jun 25, 2015 at 3:21 AM, Andreas Arnez <arnez@linux.vnet.ibm.com> wrote:
> On Wed, Jun 24 2015, Yao Qi wrote:
>
>> Doug Evans <dje@google.com> writes:
>>
>>> FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
>>
>> I see this fail too,
>> print add_singlevecs((char1) {6}, (int1) {12}, (double1) {24})^M
>> ^M
>> Program received signal SIGSEGV, Segmentation fault.^M
>> 0x00000000004008ec in add_singlevecs (a=..., b=..., c=...) at ../../../../../binutils-gdb/gdb/testsuite/gdb.base/gnu_vector.c:132^M
>> 132       return (double1) {a[0] + b[0] + c[0]};^M
>> The program being debugged was signaled while in a function called from GDB.^M
>> GDB remains in the frame where the signal was received.^M
>> To change this behavior use "set unwindonsignal on".^M
>> Evaluation of the expression containing the function^M
>> (add_singlevecs) will be abandoned.^M
>> When the function is done executing, GDB will silently stop.^M
>> (gdb) FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
>
> This is not a regression in GDB, but a new test uncovering GDB's missing
> vector ABI support on x86_64.  So for the branch I suggest to suppress
> these new FAILs with gnu_vector.exp -- either in the test case or in the
> test harness.

Any reason to not mark them as K/XFAILs in the master branch?
We should avoid adding new tests the we know are going to FAIL (*1).
And indeed I see discussion in the submission about that.
https://sourceware.org/ml/gdb-patches/2015-05/msg00319.html
So we're good there.
The question remains though: why are these failing?

---
(*1):  It should be a rule in the contribution checklist.
https://sourceware.org/gdb/wiki/ContributionChecklist

It's not, but if I dig a bit deeper, it is:
"Known failing new testcases must produce KFAIL (GDB problem) or XFAIL
(environment problem)."
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook

I added a bit more text to the checklist so one doesn't have to dig to see it.

Re: Several regressions and we branch soon.

[Andreas Arnez]

On Thu, Jun 25 2015, Doug Evans wrote:

> On Thu, Jun 25, 2015 at 3:21 AM, Andreas Arnez <arnez@linux.vnet.ibm.com> wrote:
>> On Wed, Jun 24 2015, Yao Qi wrote:
>>
>>> Doug Evans <dje@google.com> writes:
>>>
>>>> FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
>>>
>>> I see this fail too,
>>> print add_singlevecs((char1) {6}, (int1) {12}, (double1) {24})^M
>>> ^M
>>> Program received signal SIGSEGV, Segmentation fault.^M
>>> 0x00000000004008ec in add_singlevecs (a=..., b=..., c=...) at ../../../../../binutils-gdb/gdb/testsuite/gdb.base/gnu_vector.c:132^M
>>> 132       return (double1) {a[0] + b[0] + c[0]};^M
>>> The program being debugged was signaled while in a function called from GDB.^M
>>> GDB remains in the frame where the signal was received.^M
>>> To change this behavior use "set unwindonsignal on".^M
>>> Evaluation of the expression containing the function^M
>>> (add_singlevecs) will be abandoned.^M
>>> When the function is done executing, GDB will silently stop.^M
>>> (gdb) FAIL: gdb.base/gnu_vector.exp: call add_singlevecs
>>
>> This is not a regression in GDB, but a new test uncovering GDB's missing
>> vector ABI support on x86_64.  So for the branch I suggest to suppress
>> these new FAILs with gnu_vector.exp -- either in the test case or in the
>> test harness.
>
> Any reason to not mark them as K/XFAILs in the master branch?
> We should avoid adding new tests the we know are going to FAIL (*1).
> And indeed I see discussion in the submission about that.
> https://sourceware.org/ml/gdb-patches/2015-05/msg00319.html
> So we're good there.

Right, this was considered in the patch.  But only what I've actually
seen fail in my testing is marked as KFAIL.  Since more FAILs are
observed now, maybe we could mark them as KFAIL as well.  Or skip them
altogether, like in the patch below.  WDYT?

> The question remains though: why are these failing?

Good question.  Apparently there are various problems with inferior
function calls on x86_64 when vector arguments and/or vector return
values are involved.  I haven't analyzed this, and I'd rather leave that
to the x86_64 maintainers.


-- >8 --
Subject: [PATCH] gnu_vector.exp: Skip tests known to crash inferior on x86(_64)
From: Andreas Arnez <arnez@linux.vnet.ibm.com>

diff --git a/gdb/testsuite/gdb.base/gnu_vector.exp b/gdb/testsuite/gdb.base/gnu_vector.exp
index cf91fbb..885e2f2 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.exp
+++ b/gdb/testsuite/gdb.base/gnu_vector.exp
@@ -189,13 +189,18 @@ gdb_test "print add_various_floatvecs(2, f4a, f4b)" "= \\{3, 6, 16, 20\\}" \
 setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "print add_structvecs(i2, (struct just_int2)\{2*i2\}, (struct two_int2)\{3*i2, 4*i2\})" \
     "= \\{i = \\{10, 20\\}\\}" "call add_structvecs"
+if { [istarget "i?86-*-*"] || [istarget "x86_64-*-*" ] } {
+    # The next test has been seen to crash the inferior on x86_64, so
+    # just stop here.
+    kfail gdb/18537 "skip remaining vector ABI tests on this arch"
+    return
+}
 gdb_test "print add_singlevecs((char1) \{6\}, (int1) \{12\}, (double1) \{24\})" "= \\{42\\}" \
     "call add_singlevecs"
 
 # Test vector return value handling with "finish" and "return".
 gdb_breakpoint "add_some_intvecs"
 gdb_continue "add_some_intvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "finish" "Value returned is .* = \\{10, 20, 48, 72\\}" \
     "finish shows vector return value"
 gdb_continue "add_some_intvecs"
@@ -203,5 +208,4 @@ gdb_test "return (int4) \{4, 2, 7, 6\}" \
     "#0 .* main .*" \
     "set vector return value" \
     "Make add_some_intvecs return now. .y or n.*" "y"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "continue" "4 2 7 6\r\n.*" "verify vector return value"

Re: Several regressions and we branch soon.

[Yao Qi]

Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

> Right, this was considered in the patch.  But only what I've actually
> seen fail in my testing is marked as KFAIL.  Since more FAILs are
> observed now, maybe we could mark them as KFAIL as well.  Or skip them
> altogether, like in the patch below.  WDYT?

I am inclined to skip them altogether, but I think we need skip more.
With your patch applied,  I still see them in gdb.sum

 KFAIL: gdb.base/gnu_vector.exp: call add_some_intvecs (PRMS: gdb/18537)
 KPASS: gdb.base/gnu_vector.exp: call add_many_charvecs (PRMS gdb/18537)
 KFAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs (PRMS: gdb/18537)
 KFAIL: gdb.base/gnu_vector.exp: call add_structvecs (PRMS: gdb/18537)
 KFAIL: gdb.base/gnu_vector.exp: skip remaining vector ABI tests on this arch (PRMS: gdb/18537)

KPASS is confusing here.  I'd like to skip all of them on x86 and emit
UNSUPPORTED in gdb.sum, because we've already know that vector infcall
doesn't support on x86, UNSUPPORTED is better than KFAIL, IMO.

-- 
Yao (齐尧)

Re: Several regressions and we branch soon.

[Andreas Arnez]

On Tue, Jun 30 2015, Yao Qi wrote:

> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
>> Right, this was considered in the patch.  But only what I've actually
>> seen fail in my testing is marked as KFAIL.  Since more FAILs are
>> observed now, maybe we could mark them as KFAIL as well.  Or skip them
>> altogether, like in the patch below.  WDYT?
>
> I am inclined to skip them altogether, but I think we need skip more.
> With your patch applied,  I still see them in gdb.sum
>
>  KFAIL: gdb.base/gnu_vector.exp: call add_some_intvecs (PRMS: gdb/18537)
>  KPASS: gdb.base/gnu_vector.exp: call add_many_charvecs (PRMS gdb/18537)
>  KFAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs (PRMS: gdb/18537)
>  KFAIL: gdb.base/gnu_vector.exp: call add_structvecs (PRMS: gdb/18537)
>  KFAIL: gdb.base/gnu_vector.exp: skip remaining vector ABI tests on this arch (PRMS: gdb/18537)
>
> KPASS is confusing here.  I'd like to skip all of them on x86 and emit
> UNSUPPORTED in gdb.sum, because we've already know that vector infcall
> doesn't support on x86, UNSUPPORTED is better than KFAIL, IMO.

You're probably right.  I chose KFAIL because GDB doesn't provide any
indication that this is not expected to work correctly, but yields a
wrong result instead.  Anyway, the KFAILs obviously bring more trouble
than benefit, and UNSUPPORTED looks good enough to me.

So how about this?

-- >8 --
Subject: [PATCH] gnu_vector.exp: Skip infcall tests on x86/x86_64

Since the new KFAILs/KPASSs for the infcall tests on x86 and x86_64
targets generated unnecessary noise, this change skips them with
UNSUPPORTED instead.

gdb/testsuite/ChangeLog:

	* gdb.base/gnu_vector.exp: On x86 and x86_64 targets, skip the
	infcall tests instead of setting up for KFAIL.
---
 gdb/testsuite/gdb.base/gnu_vector.exp |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.base/gnu_vector.exp b/gdb/testsuite/gdb.base/gnu_vector.exp
index cf91fbb..173da4d 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.exp
+++ b/gdb/testsuite/gdb.base/gnu_vector.exp
@@ -177,16 +177,19 @@ gdb_test "ptype struct_with_vector_1" "type = struct {\r\n\[\t \]+int i;\r\n\[\t
 
 # Test inferior function calls with vector arguments and/or vector
 # return values.
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
+if { [istarget "i?86-*-*"] || [istarget "x86_64-*-*" ] } {
+    # These platforms don't support infcalls with vector arguments
+    # and/or vector return values, so skip the remaining tests.
+    # See also PR exp/18537.
+    unsupported "skip remaining vector ABI tests on this arch"
+    return
+}
 gdb_test "print add_some_intvecs(i4a, i4b, 3 * i4a)" "= \\{17, 34, 72, 132\\}" \
     "call add_some_intvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "print add_many_charvecs(c4, c4, c4, c4, c4, c4, c4, c4, c4, c4)" \
     "= \\{10, 20, 30, 40\\}" "call add_many_charvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "print add_various_floatvecs(2, f4a, f4b)" "= \\{3, 6, 16, 20\\}" \
     "call add_various_floatvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "print add_structvecs(i2, (struct just_int2)\{2*i2\}, (struct two_int2)\{3*i2, 4*i2\})" \
     "= \\{i = \\{10, 20\\}\\}" "call add_structvecs"
 gdb_test "print add_singlevecs((char1) \{6\}, (int1) \{12\}, (double1) \{24\})" "= \\{42\\}" \
@@ -195,7 +198,6 @@ gdb_test "print add_singlevecs((char1) \{6\}, (int1) \{12\}, (double1) \{24\})"
 # Test vector return value handling with "finish" and "return".
 gdb_breakpoint "add_some_intvecs"
 gdb_continue "add_some_intvecs"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "finish" "Value returned is .* = \\{10, 20, 48, 72\\}" \
     "finish shows vector return value"
 gdb_continue "add_some_intvecs"
@@ -203,5 +205,4 @@ gdb_test "return (int4) \{4, 2, 7, 6\}" \
     "#0 .* main .*" \
     "set vector return value" \
     "Make add_some_intvecs return now. .y or n.*" "y"
-setup_kfail gdb/18537 "i?86-*-*" "x86_64-*-*"
 gdb_test "continue" "4 2 7 6\r\n.*" "verify vector return value"
-- 
1.7.9.5

Re: Several regressions and we branch soon.

[Yao Qi]

On 30/06/15 19:09, Andreas Arnez wrote:
> So how about this?
>
> -- >8 --
> Subject: [PATCH] gnu_vector.exp: Skip infcall tests on x86/x86_64
>
> Since the new KFAILs/KPASSs for the infcall tests on x86 and x86_64
> targets generated unnecessary noise, this change skips them with
> UNSUPPORTED instead.
>
> gdb/testsuite/ChangeLog:
>
> 	* gdb.base/gnu_vector.exp: On x86 and x86_64 targets, skip the
> 	infcall tests instead of setting up for KFAIL.

The patch is OK to me.

-- 
Yao (齐尧)

Re: Several regressions and we branch soon.

[Yao Qi]

Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

> Subject: [PATCH] gnu_vector.exp: Skip infcall tests on x86/x86_64
>
> Since the new KFAILs/KPASSs for the infcall tests on x86 and x86_64
> targets generated unnecessary noise, this change skips them with
> UNSUPPORTED instead.

Hi Andreas,
I still see some fails in gnu_vector.exp in various architectures,

Here are some fails on ppc64be-m64, as I found from buildbot
https://www.sourceware.org/ml/gdb-testers/2015-q3/msg01198.html

new FAIL: gdb.base/gnu_vector.exp: call add_some_intvecs
new FAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs
new FAIL: gdb.base/gnu_vector.exp: finish shows vector return value
new FAIL: gdb.base/gnu_vector.exp: verify vector return value (the program exited)

Does GDB support vector infcall on ppc64be?  I checked the test result
on ppc64le https://www.sourceware.org/ml/gdb-testers/2015-q3/msg01201.html
but gnu_vector.exp isn't compiled successfully (due to old gcc?)  this
case isn't compiled successfully on aix buildslave either.

I also see two fails on s390x from buildbot
https://www.sourceware.org/ml/gdb-testers/2015-q3/msg00957.html

FAIL: gdb.base/gnu_vector.exp: finish shows vector return value
FAIL: gdb.base/gnu_vector.exp: verify vector return value (the program exited)

IIUC, vector infcall should be supported on s390 GDB, right?

I also see some vector infcall fails on both arm and aarch64 too.  What
GDB targets should support vector infcall?  ppc64 (le and be) and s390?
I am wondering we should only do the vector infcall tests on the
supported GDB targets, and skip for the rest of them.

-- 
Yao (齐尧)

Re: Several regressions and we branch soon.

[Andreas Arnez]

On Fri, Jul 10 2015, Yao Qi wrote:

> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
>> Subject: [PATCH] gnu_vector.exp: Skip infcall tests on x86/x86_64
>>
>> Since the new KFAILs/KPASSs for the infcall tests on x86 and x86_64
>> targets generated unnecessary noise, this change skips them with
>> UNSUPPORTED instead.
>
> Hi Andreas,
> I still see some fails in gnu_vector.exp in various architectures,
>
> Here are some fails on ppc64be-m64, as I found from buildbot
> https://www.sourceware.org/ml/gdb-testers/2015-q3/msg01198.html
>
> new FAIL: gdb.base/gnu_vector.exp: call add_some_intvecs
> new FAIL: gdb.base/gnu_vector.exp: call add_various_floatvecs
> new FAIL: gdb.base/gnu_vector.exp: finish shows vector return value
> new FAIL: gdb.base/gnu_vector.exp: verify vector return value (the program exited)
>
> Does GDB support vector infcall on ppc64be?

AFAIK, it should.  However, on that system the compilation with
"-mcpu=native" fails because GCC5 emits an ABI warning:

  gnu_vector.c:62:1: note: the layout of aggregates containing vectors with 4-byte alignment has changed in GCC 5

Then the test case falls back to compiling without an "-mcpu=" flag, so
the FAILs occur with GCC's default machine options.  I am not sure
whether that is supposed to work with GDB.  (Does anybody know?)

Anyway, maybe we should add "-Wno-psabi" to the compile options.  This
will likely get rid of the (probably unimportant) FAILs above.

> I checked the test result
> on ppc64le https://www.sourceware.org/ml/gdb-testers/2015-q3/msg01201.html
> but gnu_vector.exp isn't compiled successfully (due to old gcc?)  this
> case isn't compiled successfully on aix buildslave either.

GCC5 on ppc64le emits the ABI warning even in absence of "-mcpu=".
Again it should help to add "-Wno-psabi".

>
> I also see two fails on s390x from buildbot
> https://www.sourceware.org/ml/gdb-testers/2015-q3/msg00957.html
>
> FAIL: gdb.base/gnu_vector.exp: finish shows vector return value
> FAIL: gdb.base/gnu_vector.exp: verify vector return value (the program exited)
>
> IIUC, vector infcall should be supported on s390 GDB, right?

Yes.  But in this case no vector ABI is used, because that test machine
does not have a vector facility and because -march=native is not
supported by GCC (yet).  Thus vector return values are not passed in
vector registers, but according to RETURN_VALUE_STRUCT_CONVENTION.  And
then we hit the problem that displaying such return values is not
supported by GDB: https://sourceware.org/bugzilla/show_bug.cgi?id=8549

Note that various other testsuite FAILs on s390 are due to
non-displayable structure return values as well, e.g.:

  FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Small

Thus I have already been working on fixing that.

> I also see some vector infcall fails on both arm and aarch64 too.  What
> GDB targets should support vector infcall?  ppc64 (le and be) and s390?

Sorry, I do not know.  This question should better be addressed to the
various architecture maintainers.  Note that many architectures have
*multiple* vector ABIs, depending on the level of hardware support
available.  So a complete answer to your question would be a filled-out
table like this (where the examples are obviously completely made up):

  | architecture | vector ABI | infcall | "finish" |
  |--------------+------------+---------+----------|
  | foo          | no HW      | OK      | bad ret  |
  |              | VX16       | broken  | broken   |
  |              | VY32       | OK      | OK       |
  |--------------+------------+---------+----------|
  | bar          | no HW      | OK      | OK       |
  |              | VBLURB     | OK      | OK       |
  |              | V-ng       | unsupp  | no ret   |

The s390 part currently looks like this:

  | architecture | vector ABI          | infcall | "finish" |
  |--------------+---------------------+---------+----------|
  | s390x        | no HW               | OK      | no ret   |
  |              | S390_VECTOR_ABI_128 | OK      | OK       |

(1) Assuming that the vector return value fits in a vector register.

> I am wondering we should only do the vector infcall tests on the
> supported GDB targets, and skip for the rest of them.

We could.  On the other hand there is a difference from the usual
"lacking support" case: Normally GDB tells the user about the lacking
support.  Here, GDB performs a bogus inferior function call instead,
shows a wrong return value, or even crashes the inferior.  This seems
more like a bug than a missing feature to me.  In my view, targets that
can not perform vector ABI infcalls correctly should at least suppress
the infcall and emit an appropriate error message.

--
Andreas

Re: Several regressions and we branch soon.

[Ulrich Weigand]

Andreas Arnez wrote:
> On Fri, Jul 10 2015, Yao Qi wrote:

> GCC5 on ppc64le emits the ABI warning even in absence of "-mcpu=".
> Again it should help to add "-Wno-psabi".

Yes, for ppc64 we definitely should add -Wno-psabi.

> Yes.  But in this case no vector ABI is used, because that test machine
> does not have a vector facility and because -march=native is not
> supported by GCC (yet).  Thus vector return values are not passed in
> vector registers, but according to RETURN_VALUE_STRUCT_CONVENTION.  And
> then we hit the problem that displaying such return values is not
> supported by GDB: https://sourceware.org/bugzilla/show_bug.cgi?id=8549

Hmm.  Since this is a separate problem independent of vector support,
maybe this test case should accept the failure to display struct return
values, and pass the test (or maybe KFAIL?).

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: Several regressions and we branch soon.

[Andreas Arnez]

On Fri, Jul 10 2015, Ulrich Weigand wrote:

> Andreas Arnez wrote:
>> On Fri, Jul 10 2015, Yao Qi wrote:
>
>> GCC5 on ppc64le emits the ABI warning even in absence of "-mcpu=".
>> Again it should help to add "-Wno-psabi".
>
> Yes, for ppc64 we definitely should add -Wno-psabi.

OK.

>> Yes.  But in this case no vector ABI is used, because that test machine
>> does not have a vector facility and because -march=native is not
>> supported by GCC (yet).  Thus vector return values are not passed in
>> vector registers, but according to RETURN_VALUE_STRUCT_CONVENTION.  And
>> then we hit the problem that displaying such return values is not
>> supported by GDB: https://sourceware.org/bugzilla/show_bug.cgi?id=8549
>
> Hmm.  Since this is a separate problem independent of vector support,
> maybe this test case should accept the failure to display struct return
> values, and pass the test (or maybe KFAIL?).

Sure, that's an option.  I prefer KFAIL, such that it's easy to find
(and remove) when the bug is fixed.

Here's a patch for both of the suggested changes.  OK to apply?

-- >8 --
Subject: [PATCH] gnu_vector.exp: Avoid some more known FAILs

This avoids two more types of FAILs with the gnu_vector test case.

First, for POWER targets newer GCCs emit an ABI note when invoked with
"-mcpu=native".  Then the test case fell back to non-native compile,
producing code for a non-vector ABI.  But that is not supported by GDB.
Thus the compiler note is now suppressed with "-Wno-psabi".

Second, on s390 the test case produced FAILs after falling back to a
non-vector ABI when using "finish" or "return" in a vector-valued
function.  This was due to a long-standing known bug (Bug 8549).  This
case is now detected, and KFAILs are emitted instead.

gdb/testsuite/ChangeLog:

	* gdb.base/gnu_vector.exp: Try compilation with "-mcpu=native
	-Wno-psabi" if "-mcpu=native" fails.  For the tests with "finish"
	and "return" use KFAIL when GDB can not read/write the vector
	return value.
---
 gdb/testsuite/gdb.base/gnu_vector.exp | 57 +++++++++++++++++++++++++++--------
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/gdb/testsuite/gdb.base/gnu_vector.exp b/gdb/testsuite/gdb.base/gnu_vector.exp
index 173da4d..0bef3f8 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.exp
+++ b/gdb/testsuite/gdb.base/gnu_vector.exp
@@ -25,11 +25,19 @@ standard_testfile .c
 # without a CPU option.  If all variants fail, assume that the
 # compiler can not handle GNU vectors.
 
-if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" ${binfile} executable {debug quiet additional_flags=-mcpu=native}] != ""
-     && [gdb_compile "${srcdir}/${subdir}/${srcfile}" ${binfile} executable {debug quiet additional_flags=-march=native}] != ""
-     && [gdb_compile "${srcdir}/${subdir}/${srcfile}" ${binfile} executable {debug quiet}] != ""} {
-    untested "compiler can't handle the vector_size attribute?"
-    return -1
+proc do_compile { {opts {}} } {
+    global srcdir subdir srcfile binfile
+    set ccopts {debug quiet}
+    foreach opt $opts {lappend ccopts "additional_flags=$opt"}
+    gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" executable $ccopts
+}
+
+if { [do_compile {-mcpu=native}] != ""
+     && [do_compile {-mcpu=native -Wno-psabi}] != ""
+     && [do_compile {-march=native}] != ""
+     && [do_compile] != ""} {
+	untested "compiler can't handle vector_size attribute?"
+	return -1
 }
 
 clean_restart ${binfile}
@@ -195,14 +203,37 @@ gdb_test "print add_structvecs(i2, (struct just_int2)\{2*i2\}, (struct two_int2)
 gdb_test "print add_singlevecs((char1) \{6\}, (int1) \{12\}, (double1) \{24\})" "= \\{42\\}" \
     "call add_singlevecs"
 
-# Test vector return value handling with "finish" and "return".
+# Test "finish" from vector-valued function.
 gdb_breakpoint "add_some_intvecs"
 gdb_continue "add_some_intvecs"
-gdb_test "finish" "Value returned is .* = \\{10, 20, 48, 72\\}" \
-    "finish shows vector return value"
+set test "finish shows vector return value"
+gdb_test_multiple "finish" $test {
+    -re "Value returned is .* = \\{10, 20, 48, 72\\}.*$gdb_prompt $" {
+	pass $test
+    }
+    -re "Value returned has type: .* Cannot determine contents.*$gdb_prompt $" {
+	kfail "gdb/8549" $test
+    }
+}
+
+# Test "return" from vector-valued function.
 gdb_continue "add_some_intvecs"
-gdb_test "return (int4) \{4, 2, 7, 6\}" \
-    "#0 .* main .*" \
-    "set vector return value" \
-    "Make add_some_intvecs return now. .y or n.*" "y"
-gdb_test "continue" "4 2 7 6\r\n.*" "verify vector return value"
+set test "set vector return value"
+set ok 1
+gdb_test_multiple "return (int4) \{4, 2, 7, 6\}" $test {
+    -re "#0 .* main .*$gdb_prompt $" {
+	if { $ok } {
+	    pass $test
+	    gdb_test "continue" "4 2 7 6\r\n.*" "verify vector return value"
+	}
+    }
+    -re "The location .* is unknown.\r\n.* return value .* will be ignored.\r\n" {
+	kfail "gdb/8549" $test
+	set ok 0
+	exp_continue
+    }
+    -re "Make add_some_intvecs return now. .y or n. $" {
+	send_gdb "y\n"
+	exp_continue
+    }
+}
-- 
2.4.6