RFC: branching for GDB 7.11 soon? (possibly Wed)

RFC: branching for GDB 7.11 soon? (possibly Wed)

[Joel Brobecker]

Hello everyone,

According to the GDB 7.11 release cycle wiki page
(https://sourceware.org/gdb/wiki/GDB_7.11_Release),

there no longer any items that are explicitly marked as being
blocking for branching.  We do have a few "maybe"-s. There are also
3 PRs filed with a target-milestone of GDB 7.11. From experience,
if we want them to make a given release, they need to have a champion.

Currently unassigned, we have:

PR19474 "break LINE_NUM" set breakpoint on file other than current source file
(suggest Keith?)

PR19503 internal-error: linux_nat_resume: Assertion `lp != NULL' failed.
(Pedro seems to be on it already)

In addition, we have the following PR, which Keith already sent
a patch:
PR19506 Regression with gdb.Breakpoint("*<addr>")

At the moment, I would be hesitant to branch before we first
analyze what would be required to fix PR19474 ("break LINE_NUM").
But if we're confident that it can be fairly safely fixed on
the branch, I don't see any other issue blocking for branching,
which I would then propose to be doing sometime early Wed.

Sergio - would you be able to give us a rough description of how
good the results are in the buildbots? Anything we should be
aware of for this release? (Thanks!)

Thoughts?

-- 
Joel

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Pedro Alves]

On 02/01/2016 03:06 AM, Joel Brobecker wrote:

> PR19503 internal-error: linux_nat_resume: Assertion `lp != NULL' failed.
> (Pedro seems to be on it already)

I've been looking at it today.

The assertion in question should be fixed, so I've closed this one.

However, I've opened Bug 19548, for the breakpoint re-set issues
discovered in that bug:

 [1] - https://sourceware.org/bugzilla/show_bug.cgi?id=19548

Unfortunately looks like a regression...  I'm still looking.

Thanks,
Pedro Alves

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Pedro Alves]

On 02/01/2016 01:34 PM, Pedro Alves wrote:
> On 02/01/2016 03:06 AM, Joel Brobecker wrote:
> 
>> PR19503 internal-error: linux_nat_resume: Assertion `lp != NULL' failed.
>> (Pedro seems to be on it already)
> 
> I've been looking at it today.
> 
> The assertion in question should be fixed, so I've closed this one.
> 
> However, I've opened Bug 19548, for the breakpoint re-set issues
> discovered in that bug:
> 
>  [1] - https://sourceware.org/bugzilla/show_bug.cgi?id=19548
> 
> Unfortunately looks like a regression...  I'm still looking.
> 

Fix sent:

 [PATCH] Fix 19548: Breakpoint re-set inserts breakpoints when it shouldn't
 https://sourceware.org/ml/gdb-patches/2016-02/msg00014.html

Thanks,
Pedro Alves

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Keith Seitz]

On 01/31/2016 07:06 PM, Joel Brobecker wrote:
> 
> PR19474 "break LINE_NUM" set breakpoint on file other than current source file
> (suggest Keith?)

> At the moment, I would be hesitant to branch before we first
> analyze what would be required to fix PR19474 ("break LINE_NUM").
> But if we're confident that it can be fairly safely fixed on
> the branch, I don't see any other issue blocking for branching,
> which I would then propose to be doing sometime early Wed.

I started looking at this last week, and I have an analysis of the
problems. It all basically boils down to: we cannot (reliably) win in
this situation.

Reminder of the situation from the bug: We have the two symtabs
/home/keiths/tmp/f.c and /home/keiths/tmp/foo/f.c. This later symtab
exists on the disk, though, as /home/keiths/tmp/bar/f.c.

In create_sals_line_offset, we get the current source symtab and then
search for all symtabs matching that name. This is required because we
may have multiple symtabs for the same source file. [See
expand-psymtabs.exp for an example.]

When we go looking for the psymtab, we call psymtab_to_fullname, which
uses find_and_open_source to search the source path, which includes
/home/keiths/tmp/foo (the CDIR) and CWD.

This last bit is the problem. If the user's CWD is /home/keiths/tmp,
openp will return that /home/keiths/tmp/f.c is a match for the requested
symtab filename, /home/keiths/tmp/foo/f.c. This is clearly incorrect.
The two source files are /not/ the same, but find_and_open_source
returns that it found the foo/f.c symtab even when it didn't. [When sal
conversion is complete, gdb will look up the symtab name by PC, it then
finds the "correct" symtab for reporting during "info break."]

So, yes, this bug can be worked around by NOT having your CWD set to
/home/keiths/tmp. A simple "cd .." will "fix" things. Ouch!

Aside: It seems to me that instead of doing basename searches and using
CWD, this whole thing should really do/support directory mappings, e.g.,
the user can say, "whenever you see /home/keiths/tmp/foo, look instead
in /home/keiths/tmp/bar." But I don't think that really addresses the
problem at hand.

The only solution I've come up with is changing the whole API so that in
this specific case (searching for a known symtab), we reject searching
the source path. As you can imagine, that could open up an entirely new
"can of worms." [And that change would be quite intrusive.]

Like Yao, I'm not entirely sure what to do here yet.

Keith

[RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]

[Keith Seitz]

On 02/01/2016 11:51 AM, Keith Seitz wrote:

> I started looking at this last week, and I have an analysis of the
> problems. It all basically boils down to: we cannot (reliably) win in
> this situation.

Well, it turns out I'm wrong. Or at least as not-wrong as the test suite
demonstrates.

> Aside: It seems to me that instead of doing basename searches and using
> CWD, this whole thing should really do/support directory mappings, e.g.,
> the user can say, "whenever you see /home/keiths/tmp/foo, look instead
> in /home/keiths/tmp/bar." But I don't think that really addresses the
> problem at hand.

And the astute reader knows that we already have this! "set
substitute-path" Does the trick.

> The only solution I've come up with is changing the whole API so that in
> this specific case (searching for a known symtab), we reject searching
> the source path. As you can imagine, that could open up an entirely new
> "can of worms." [And that change would be quite intrusive.]

At Pedro's urging, I found another, simpler solution. The big problem
with just about all solutions I was attempting to implement is
[p]symtab_to_fululname/find_and_open_source/openp. These functions are
just completely unaware of what the caller is attempting to do.

So first I want to point out that this "bug" has existed for quite some
time. I've only test back to 7.7 (earlier releases do not build on my
machine without non-trivial intervention), but all suffer from this problem.

Having said that, the scheme I've come up with restricts this "hack" to
create_sals_line_offset, where the problem is observed.

It is also here where we know we are attempting to fetch a list of
symtabs for the default source file, meaning all symtabs with the "same"
file name as default symtab.

So the simple (but apparently working?) algorithm simply compares the
default symtab's fullname vs a reconstruction of the SYMTAB_DIRNAME and
basename of the "collected" symtab. [It also considers substitute-path.]

I would consider this fairly risky for inclusion into a release without
sufficient testing in HEAD, but when it comes to releases, I tend to be
quite conservative.

I've appended the patch below.  This patch causes no regressions in the
test suite, and it fixes 19474.

Before:

$ ./gdb ~/tmp/f
Reading symbols from /home/keiths/tmp/f...done.
(gdb) cd ~/tmp
Working directory /home/keiths/tmp.
(gdb) start
Temporary breakpoint 1 at 0x4006e6: file f.c, line 6.
Starting program: /home/keiths/tmp/f

Temporary breakpoint 1, main () at f.c:6
6	  return foo () + bar_in_main ();
(gdb) b 12
Breakpoint 2 at 0x4006ff: /home/keiths/tmp/f.c:12. (2 locations)


After:
$ ./gdb ~/tmp/f
Reading symbols from /home/keiths/tmp/f...done.
(gdb) cd ~/tmp
Working directory /home/keiths/tmp.
(gdb) start
Temporary breakpoint 1 at 0x4006e6: file f.c, line 6.
Starting program: /home/keiths/tmp/f

Temporary breakpoint 1, main () at f.c:6
6	  return foo () + bar_in_main ();
(gdb) b 12
Breakpoint 2 at 0x4006ff: file f.c, line 12.

I'm sure there are corner cases and a whole bunch of other problems with
this approach, but at least it is isolated to one place (for better or
worse).

Anyone have a better idea?

Keith

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 2360cc1..c0f7020 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1816,6 +1816,66 @@ canonicalize_linespec (struct linespec_state
*state, const linespec_p ls)
     }
 }

+/* Filter the list of SYMTABS with the name of the default symtab given
+   by FULLNAME.
+
+   When collecting symtabs from the fullname of the default symtab,
+   it is possible for openp to get it wrong (because it often searches
+   the current working directory using the basename of the file) if files
+   with the same base name exist.
+
+   This function attempts to ascertain if symtabs in the given list
+   really do match the given fullname of the default symtab.  */
+
+static VEC (symtab_ptr) *
+filter_default_symtabs (const char *fullname, VEC (symtab_ptr) *symtabs)
+{
+  int ix;
+  struct symtab *symtab;
+  VEC (symtab_ptr) *filtered_symtabs = NULL;
+  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
+
+  /* Iterate through the symtabs, searching for matches to FULLNAME.  */
+  for (ix = 0; VEC_iterate (symtab_ptr, symtabs, ix, symtab); ++ix)
+    {
+      const char *basename = lbasename (fullname);
+      char *symtab_with_dir;
+
+      if (SYMTAB_DIRNAME (symtab) == NULL)
+	continue;
+
+      symtab_with_dir = concat (SYMTAB_DIRNAME (symtab), SLASH_STRING,
+				basename, NULL);
+      make_cleanup (xfree, symtab_with_dir);
+      if (streq (fullname, symtab_with_dir))
+	VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
+      else
+	{
+	  /* Now try any path substitution rules.  */
+	  symtab_with_dir = rewrite_source_path (symtab_with_dir);
+	  if (symtab_with_dir != NULL)
+	    {
+	      make_cleanup (xfree, symtab_with_dir);
+	      if (streq (fullname, symtab_with_dir))
+		VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
+	    }
+	}
+    }
+
+  /* If we found no matches, use whatever symtabs were originally
+     "collected."  */
+  if (filtered_symtabs == NULL)
+    {
+      /* Found no exact matches -- use the original list.  */
+      filtered_symtabs = symtabs;
+    }
+  else
+    VEC_free (symtab_ptr, symtabs);
+
+  do_cleanups (back_to);
+  return filtered_symtabs;
+}
+
 /* Given a line offset in LS, construct the relevant SALs.  */

 static struct symtabs_and_lines
@@ -1851,6 +1911,7 @@ create_sals_line_offset (struct linespec_state *self,
       VEC_free (symtab_ptr, ls->file_symtabs);
       ls->file_symtabs = collect_symtabs_from_filename (fullname,
 							self->search_pspace);
+      ls->file_symtabs = filter_default_symtabs (fullname,
ls->file_symtabs);
       use_default = 1;
     }

Re: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]

[Keith Seitz]

On 02/03/2016 04:45 PM, Keith Seitz wrote:
> Anyone have a better idea?

Well, same idea, but slightly faster...

> +  /* Iterate through the symtabs, searching for matches to FULLNAME.  */
> +  for (ix = 0; VEC_iterate (symtab_ptr, symtabs, ix, symtab); ++ix)
> +    {
> +      const char *basename = lbasename (fullname);
> +      char *symtab_with_dir;
> +
> +      if (SYMTAB_DIRNAME (symtab) == NULL)
> +	continue;
> +
> +      symtab_with_dir = concat (SYMTAB_DIRNAME (symtab), SLASH_STRING,
> +				basename, NULL);

I think that the base name of the default symtab will be the same as the
base name of any collected symtabs, so we need only really compare
directory names here.

Any other ideas?

Keith

Re: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]

[Yao Qi]

[I am back from vacation...]

Hi Keith,
Thanks for looking at this bug.

On 04/02/16 00:45, Keith Seitz wrote:
> It is also here where we know we are attempting to fetch a list of
> symtabs for the default source file, meaning all symtabs with the "same"
> file name as default symtab.
>
> So the simple (but apparently working?) algorithm simply compares the
> default symtab's fullname vs a reconstruction of the SYMTAB_DIRNAME and
> basename of the "collected" symtab. [It also considers substitute-path.]

The algorithm/rationale sounds good to me.  I had a similar fix in my
side, but it causes regressions in the following test cases, in which
DW_AT_comp_dir is empty or incorrect.

  gdb.arch/amd64-entry-value-param.exp
  gdb.cp/namelessclass.exp
  gdb.dwarf2/dw2-common-block.exp
  gdb.dwarf2/dw2-single-line-discriminators.exp

but your patch looks better.

>
> I would consider this fairly risky for inclusion into a release without
> sufficient testing in HEAD, but when it comes to releases, I tend to be
> quite conservative.
>

Yes, I agree.  The problem itself can't be fixed properly without a big
structural change.  We don't have to ship this patch to 7.11, IMO.  It
is OK for mainline, however.

> I've appended the patch below.  This patch causes no regressions in the
> test suite, and it fixes 19474.
>

>
> I'm sure there are corner cases and a whole bunch of other problems with
> this approach, but at least it is isolated to one place (for better or
> worse).
>
> Anyone have a better idea?

I don't have any.  I agree that the directory name comparison in you
next mail is better.

-- 
Yao (齐尧)

Re: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]

[Pedro Alves]

On 02/04/2016 12:45 AM, Keith Seitz wrote:

> At Pedro's urging, I found another, simpler solution. The big problem
> with just about all solutions I was attempting to implement is
> [p]symtab_to_fululname/find_and_open_source/openp. These functions are
> just completely unaware of what the caller is attempting to do.

Thanks!


> I'm sure there are corner cases and a whole bunch of other problems with
> this approach, but at least it is isolated to one place (for better or
> worse).

Things to watch out for, out of the blue:

- relative SYMTAB_DIRNAMEs -- is there such a thing?

- symlinks.  Say, what happens if foo/f.c is a symlink to f.c (perhaps
  each is compiled differently, with -DWHATNOT, for example).

> +static VEC (symtab_ptr) *
> +filter_default_symtabs (const char *fullname, VEC (symtab_ptr) *symtabs)
> +{
> +  int ix;
> +  struct symtab *symtab;
> +  VEC (symtab_ptr) *filtered_symtabs = NULL;
> +  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);

I think filtered_symtabs should be guarded with a cleanup as well.

> +
> +  /* Iterate through the symtabs, searching for matches to FULLNAME.  */
> +  for (ix = 0; VEC_iterate (symtab_ptr, symtabs, ix, symtab); ++ix)
> +    {
> +      const char *basename = lbasename (fullname);
> +      char *symtab_with_dir;
> +
> +      if (SYMTAB_DIRNAME (symtab) == NULL)
> +	continue;
> +
> +      symtab_with_dir = concat (SYMTAB_DIRNAME (symtab), SLASH_STRING,
> +				basename, NULL);
> +      make_cleanup (xfree, symtab_with_dir);
> +      if (streq (fullname, symtab_with_dir))
> +	VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
> +      else
> +	{
> +	  /* Now try any path substitution rules.  */
> +	  symtab_with_dir = rewrite_source_path (symtab_with_dir);
> +	  if (symtab_with_dir != NULL)
> +	    {
> +	      make_cleanup (xfree, symtab_with_dir);
> +	      if (streq (fullname, symtab_with_dir))
> +		VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
> +	    }
> +	}

This creates two cleanups per iteration here.  Only two for the whole
loop would suffice, if you used free_current_contents instead of xfree.

> +    }
> +
> +  /* If we found no matches, use whatever symtabs were originally
> +     "collected."  */
> +  if (filtered_symtabs == NULL)

Pedantically, checking VEC_empty instead would be better, as an initial

  VEC_reserve (symtab_ptr, filtered_symtabs, VEC_size (symtab_ptr, symtab));

would make a NULL check wrong.  That reserve might make sense if most of
the time we'll match all symtabs.

> +    {
> +      /* Found no exact matches -- use the original list.  */
> +      filtered_symtabs = symtabs;
> +    }
> +  else
> +    VEC_free (symtab_ptr, symtabs);
> +
> +  do_cleanups (back_to);
> +  return filtered_symtabs;
> +}

Thanks,
Pedro Alves

Re: [RFC] breakpoints/19474 [was Re: RFC: branching for GDB 7.11 soon? (possibly Wed)]

[Yao Qi]

Hi Keith,
Are you still working on this patch?  This problem bites me again in the 7.12
testing, and that leads me digging out this patch.  I don't think we need to
pick it up in 7.12, but it's better to pick it in master after 7.12
release, so that
we have a plenty of time to see how good (or bad) it is.

On Thu, Feb 4, 2016 at 12:31 PM, Pedro Alves <palves@redhat.com> wrote:
> On 02/04/2016 12:45 AM, Keith Seitz wrote:
>
>> At Pedro's urging, I found another, simpler solution. The big problem
>> with just about all solutions I was attempting to implement is
>> [p]symtab_to_fululname/find_and_open_source/openp. These functions are
>> just completely unaware of what the caller is attempting to do.
>
> Thanks!
>
>
>> I'm sure there are corner cases and a whole bunch of other problems with
>> this approach, but at least it is isolated to one place (for better or
>> worse).
>
> Things to watch out for, out of the blue:
>
> - relative SYMTAB_DIRNAMEs -- is there such a thing?
>
> - symlinks.  Say, what happens if foo/f.c is a symlink to f.c (perhaps
>   each is compiled differently, with -DWHATNOT, for example).
>
>> +static VEC (symtab_ptr) *
>> +filter_default_symtabs (const char *fullname, VEC (symtab_ptr) *symtabs)
>> +{
>> +  int ix;
>> +  struct symtab *symtab;
>> +  VEC (symtab_ptr) *filtered_symtabs = NULL;
>> +  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
>
> I think filtered_symtabs should be guarded with a cleanup as well.
>
>> +
>> +  /* Iterate through the symtabs, searching for matches to FULLNAME.  */
>> +  for (ix = 0; VEC_iterate (symtab_ptr, symtabs, ix, symtab); ++ix)
>> +    {
>> +      const char *basename = lbasename (fullname);
>> +      char *symtab_with_dir;
>> +
>> +      if (SYMTAB_DIRNAME (symtab) == NULL)
>> +     continue;
>> +
>> +      symtab_with_dir = concat (SYMTAB_DIRNAME (symtab), SLASH_STRING,
>> +                             basename, NULL);
>> +      make_cleanup (xfree, symtab_with_dir);
>> +      if (streq (fullname, symtab_with_dir))
>> +     VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
>> +      else
>> +     {
>> +       /* Now try any path substitution rules.  */
>> +       symtab_with_dir = rewrite_source_path (symtab_with_dir);
>> +       if (symtab_with_dir != NULL)
>> +         {
>> +           make_cleanup (xfree, symtab_with_dir);
>> +           if (streq (fullname, symtab_with_dir))
>> +             VEC_safe_push (symtab_ptr, filtered_symtabs, symtab);
>> +         }
>> +     }
>
> This creates two cleanups per iteration here.  Only two for the whole
> loop would suffice, if you used free_current_contents instead of xfree.
>
>> +    }
>> +
>> +  /* If we found no matches, use whatever symtabs were originally
>> +     "collected."  */
>> +  if (filtered_symtabs == NULL)
>
> Pedantically, checking VEC_empty instead would be better, as an initial
>
>   VEC_reserve (symtab_ptr, filtered_symtabs, VEC_size (symtab_ptr, symtab));
>
> would make a NULL check wrong.  That reserve might make sense if most of
> the time we'll match all symtabs.
>
>> +    {
>> +      /* Found no exact matches -- use the original list.  */
>> +      filtered_symtabs = symtabs;
>> +    }
>> +  else
>> +    VEC_free (symtab_ptr, symtabs);
>> +
>> +  do_cleanups (back_to);
>> +  return filtered_symtabs;
>> +}
>
> Thanks,
> Pedro Alves
>

-- 
Yao (齐尧)

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Joel Brobecker]

Hello again,

So, quick recap of where we are with respect to branching:

> PR19474 "break LINE_NUM" set breakpoint on file other than current source file

Analyzed, tricky to fix, not blocking for 7.11

> PR19503 internal-error: linux_nat_resume: Assertion `lp != NULL' failed.

Fixed by Pedro.

> PR19506 Regression with gdb.Breakpoint("*<addr>")

This lead to a wider fix:
[PATCH V2 0/4] Add support for "legacy" linespecs
https://www.sourceware.org/ml/gdb-patches/2016-02/msg00024.html

(I will try to take a look at them this weekend)

PR 19548 - breakpoint re-set inserts breakpoints when it shouldn't
Pedro sent a patch:
https://sourceware.org/ml/gdb-patches/2016-02/msg00014.html

There is also a crash (regression):

PR 19546 - gdb crash calling exec in the inferior
Initial guestimate from Pedro:
| Looks like a regression of the explicit locations work.
Still in Pedro's court, or could Keith help?

Sergio - would you be able to give us a rough description of how
good the results are in the buildbots? Anything we should be
aware of for this release? (Thanks!)

Thanks!
-- 
Joel

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Sergio Durigan Junior]

On Sunday, February 07 2016, Joel Brobecker wrote:

> Sergio - would you be able to give us a rough description of how
> good the results are in the buildbots? Anything we should be
> aware of for this release? (Thanks!)

Hey Joel,

Sorry for taking a long time to respond.

I have not been following the BuildBot results close; there are too many
racy testcases and the signal-to-noise ratio is unfortunately bad.  In
fact, I'm working on something that should improve this...

Having said that, I will take a look at the last results and check if
there's anything that deserves attention.  I'll do that today, so I
should have something to say by tomorrow.

Cheers,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Keith Seitz]

On 02/07/2016 12:12 AM, Joel Brobecker wrote:
> There is also a crash (regression):
> 
> PR 19546 - gdb crash calling exec in the inferior
> Initial guestimate from Pedro:
> | Looks like a regression of the explicit locations work.
> Still in Pedro's court, or could Keith help?

I looked at this over the weekend and updated the bz. It seems pretty
trivial, and the patch is very safe to use.

In short, update_breakpoints_after_exec is charged with deleting any
breakpoint with no location. momentary_breakpoint types were implemented
to rely on this behavior. So b->location can be NULL, contrary to what I
previously assumed.

Simply handling NULL in location_empty_p fixes this.

Writing a test for this now.

Keith

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Pedro Alves]

On 02/08/2016 07:28 PM, Keith Seitz wrote:

> I looked at this over the weekend and updated the bz. It seems pretty
> trivial, and the patch is very safe to use.
> 
> In short, update_breakpoints_after_exec is charged with deleting any
> breakpoint with no location. momentary_breakpoint types were implemented
> to rely on this behavior. So b->location can be NULL, contrary to what I
> previously assumed.
> 
> Simply handling NULL in location_empty_p fixes this.
> 
> Writing a test for this now.

Thanks Keith.  Having the testsuite cover this would be great.

BTW, calling both b->location and the b->loc things "locations"
is ambiguous and confusing, IMO.  How about we start calling
the new b->location, the "location spec" ?

Thanks,
Pedro Alves

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Keith Seitz]

On 02/08/2016 12:06 PM, Pedro Alves wrote:
>> Writing a test for this now.
> 
> Thanks Keith.  Having the testsuite cover this would be great.

Coming right up!

> BTW, calling both b->location and the b->loc things "locations"
> is ambiguous and confusing, IMO.  How about we start calling
> the new b->location, the "location spec" ?

That's an excellent idea. I will see about updating some header files
with this.

Keith

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Sergio Durigan Junior]

On Sunday, February 07 2016, Joel Brobecker wrote:

> Sergio - would you be able to give us a rough description of how
> good the results are in the buildbots? Anything we should be
> aware of for this release? (Thanks!)

Hi guys,

There's a breakage on the C++ builder caused by:

  <http://gdb-build.sergiodj.net/builders/Fedora-x86_64-cxx-build-m64/builds/1869>

This will be fixed by Simon's patch here:

  <https://sourceware.org/ml/gdb-patches/2016-02/msg00200.html>


Today, Simon pushed a commit that caused a few regressions on Ada, on
some builders.  This is:

  <https://sourceware.org/ml/gdb-testers/2016-q1/msg04420.html>


Aside from that, I haven't seen anything that caught my attention.  I
will keep looking, and if I find something I'll let you guys know.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Simon Marchi]

On 16-02-09 12:35 AM, Sergio Durigan Junior wrote:
> Today, Simon pushed a commit that caused a few regressions on Ada, on
> some builders.  This is:
> 
>   <https://sourceware.org/ml/gdb-testers/2016-q1/msg04420.html>

I'll look into it right now.  I did not receive anything from the
buildbot, is it normal?

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Pedro Alves]

On 02/09/2016 02:14 PM, Simon Marchi wrote:
> On 16-02-09 12:35 AM, Sergio Durigan Junior wrote:
>> Today, Simon pushed a commit that caused a few regressions on Ada, on
>> some builders.  This is:
>>
>>   <https://sourceware.org/ml/gdb-testers/2016-q1/msg04420.html>
> 
> I'll look into it right now.  I did not receive anything from the
> buildbot, is it normal?

Yes, only build breakages get you a nag email.  "Normal" testsuite
regressions don't result in personal notifications, because the testsuite
still trips on too many racy failures.  Once the testing infrustructure handles
racy tests more gracefully, and testing is more stable, I think we should
enable personal nags, but not yet.

Thanks,
Pedro Alves

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Simon Marchi]

On 16-02-09 09:18 AM, Pedro Alves wrote:
> On 02/09/2016 02:14 PM, Simon Marchi wrote:
>> On 16-02-09 12:35 AM, Sergio Durigan Junior wrote:
>>> Today, Simon pushed a commit that caused a few regressions on Ada, on
>>> some builders.  This is:
>>>
>>>   <https://sourceware.org/ml/gdb-testers/2016-q1/msg04420.html>
>>
>> I'll look into it right now.  I did not receive anything from the
>> buildbot, is it normal?
> 
> Yes, only build breakages get you a nag email.  "Normal" testsuite
> regressions don't result in personal notifications, because the testsuite
> still trips on too many racy failures.  Once the testing infrustructure handles
> racy tests more gracefully, and testing is more stable, I think we should
> enable personal nags, but not yet.

Ok, my understanding was that I broke some previously passing tests.  Upon
inspection, it seems like before my commit, a lot of Ada tests simply were
not running.  For example:

  FAIL: gdb.ada/aliased_array.exp: compilation foo.adb

becomes

  PASS: gdb.ada/aliased_array.exp: compilation foo.adb
  FAIL: gdb.ada/aliased_array.exp: print bt

Which introduces new FAILs.  But there is no previously passing test that
now fails, I think.  So if that's the case, I don't think it should block
the release.

Also, I don't have all those failures when I run the testsuite on my machine,
so I don't really know what causes them.

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Sergio Durigan Junior]

On Tuesday, February 09 2016, Simon Marchi wrote:

> On 16-02-09 09:18 AM, Pedro Alves wrote:
>> On 02/09/2016 02:14 PM, Simon Marchi wrote:
>>> On 16-02-09 12:35 AM, Sergio Durigan Junior wrote:
>>>> Today, Simon pushed a commit that caused a few regressions on Ada, on
>>>> some builders.  This is:
>>>>
>>>>   <https://sourceware.org/ml/gdb-testers/2016-q1/msg04420.html>
>>>
>>> I'll look into it right now.  I did not receive anything from the
>>> buildbot, is it normal?
>> 
>> Yes, only build breakages get you a nag email.  "Normal" testsuite
>> regressions don't result in personal notifications, because the testsuite
>> still trips on too many racy failures.  Once the testing infrustructure handles
>> racy tests more gracefully, and testing is more stable, I think we should
>> enable personal nags, but not yet.
>
> Ok, my understanding was that I broke some previously passing tests.  Upon
> inspection, it seems like before my commit, a lot of Ada tests simply were
> not running.  For example:
>
>   FAIL: gdb.ada/aliased_array.exp: compilation foo.adb
>
> becomes
>
>   PASS: gdb.ada/aliased_array.exp: compilation foo.adb
>   FAIL: gdb.ada/aliased_array.exp: print bt
>
> Which introduces new FAILs.  But there is no previously passing test that
> now fails, I think.  So if that's the case, I don't think it should block
> the release.

Hm, recently some extra packages were installed on some buildslaves in
order to test more things; one of those packages was gcc-gnat for Ada.
I think that may indeed be the cause.  Sorry if that was a false alarm.

> Also, I don't have all those failures when I run the testsuite on my machine,
> so I don't really know what causes them.

These tests don't seem to be racy, so if you can't reproduce the
failures, I'd say you did not introduce them.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Joel Brobecker]

Hello everyone,

Once again, I am very grateful to everyone who is so responsive
in trying to help us create that branch!

Quick status update again, based on the latest feedback:

> > PR19506 Regression with gdb.Breakpoint("*<addr>")
> 
> This lead to a wider fix:
> [PATCH V2 0/4] Add support for "legacy" linespecs
> https://www.sourceware.org/ml/gdb-patches/2016-02/msg00024.html

I took a look over the weekend, and it seems fairly unintrusive.
I propose we push it now. Otherwise, I think it's safe to create
the branch before pushing this patch, and backport afterwards.

> PR 19548 - breakpoint re-set inserts breakpoints when it shouldn't
> Pedro sent a patch:
> https://sourceware.org/ml/gdb-patches/2016-02/msg00014.html

Time to push?

> There is also a crash (regression):
> 
> PR 19546 - gdb crash calling exec in the inferior
> Initial guestimate from Pedro:
> | Looks like a regression of the explicit locations work.
> Still in Pedro's court, or could Keith help?

Looks like the fix is fairly straightforward.

> Sergio - would you be able to give us a rough description of how
> good the results are in the buildbots? Anything we should be
> aware of for this release? (Thanks!)

In terms of status:

- C++ build detected a build regression: Fixed, AFAIK.

- Some regressions in Ada due to a testsuite patch
  Worse case scenario, we could revert on the branch, if a simple
  fix is not available (I am confident, though).
  I can't see from the URLs provided what the error looks like,
  but it should only affect in-tree build & testing?

So, to summarize, given how easy it can be to break C++ building,
and looking at the issues we want to solve, I can propose the following
plan:

   1. Branch now, hold the pre-release;
   2. Fix the issues above still pending on both master + branch;
   3. Once the issues above are fixed on the branch, issue
      the first pre-release.

What do you guys think?

Thanks!
-- 
Joel

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Pedro Alves]

On 02/09/2016 11:56 AM, Joel Brobecker wrote:
> Hello everyone,
> 
> Once again, I am very grateful to everyone who is so responsive
> in trying to help us create that branch!
> 
> Quick status update again, based on the latest feedback:
> 
>>> PR19506 Regression with gdb.Breakpoint("*<addr>")
>>
>> This lead to a wider fix:
>> [PATCH V2 0/4] Add support for "legacy" linespecs
>> https://www.sourceware.org/ml/gdb-patches/2016-02/msg00024.html
> 
> I took a look over the weekend, and it seems fairly unintrusive.
> I propose we push it now. Otherwise, I think it's safe to create
> the branch before pushing this patch, and backport afterwards.

I took a quick look and it looks fine to me too.

> 
>> PR 19548 - breakpoint re-set inserts breakpoints when it shouldn't
>> Pedro sent a patch:
>> https://sourceware.org/ml/gdb-patches/2016-02/msg00014.html
> 
> Time to push?

Done.

> 
>> There is also a crash (regression):
>>
>> PR 19546 - gdb crash calling exec in the inferior
>> Initial guestimate from Pedro:
>> | Looks like a regression of the explicit locations work.
>> Still in Pedro's court, or could Keith help?
> 
> Looks like the fix is fairly straightforward.
> 
>> Sergio - would you be able to give us a rough description of how
>> good the results are in the buildbots? Anything we should be
>> aware of for this release? (Thanks!)
> 
> In terms of status:
> 
> - C++ build detected a build regression: Fixed, AFAIK.

Yes, fixed.

> 
> - Some regressions in Ada due to a testsuite patch
>   Worse case scenario, we could revert on the branch, if a simple
>   fix is not available (I am confident, though).
>   I can't see from the URLs provided what the error looks like,
>   but it should only affect in-tree build & testing?
> 
> So, to summarize, given how easy it can be to break C++ building,
> and looking at the issues we want to solve, I can propose the following
> plan:
> 
>    1. Branch now, hold the pre-release;
>    2. Fix the issues above still pending on both master + branch;
>    3. Once the issues above are fixed on the branch, issue
>       the first pre-release.
> 
> What do you guys think?

Sounds good to me.

Thanks,
Pedro Alves

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Keith Seitz]

On 02/09/2016 04:37 AM, Pedro Alves wrote:
> On 02/09/2016 11:56 AM, Joel Brobecker wrote:
>> Hello everyone,
>>
>> Once again, I am very grateful to everyone who is so responsive
>> in trying to help us create that branch!
>>
>> Quick status update again, based on the latest feedback:
>>
>>>> PR19506 Regression with gdb.Breakpoint("*<addr>")
>>>
>>> This lead to a wider fix:
>>> [PATCH V2 0/4] Add support for "legacy" linespecs
>>> https://www.sourceware.org/ml/gdb-patches/2016-02/msg00024.html
>>
>> I took a look over the weekend, and it seems fairly unintrusive.
>> I propose we push it now. Otherwise, I think it's safe to create
>> the branch before pushing this patch, and backport afterwards.
> 
> I took a quick look and it looks fine to me too.

Well, that's two global maintainers who think it looks okay; I've pushed
this.

>>> There is also a crash (regression):
>>>
>>> PR 19546 - gdb crash calling exec in the inferior
>>> Initial guestimate from Pedro:
>>> | Looks like a regression of the explicit locations work.
>>> Still in Pedro's court, or could Keith help?
>>
>> Looks like the fix is fairly straightforward.
>>

Patch submitted:

https://sourceware.org/ml/gdb-patches/2016-02/msg00245.html

Keith

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Joel Brobecker]

> So, to summarize, given how easy it can be to break C++ building,
> and looking at the issues we want to solve, I can propose the following
> plan:
> 
>    1. Branch now, hold the pre-release;
>    2. Fix the issues above still pending on both master + branch;
>    3. Once the issues above are fixed on the branch, issue
>       the first pre-release.

Thanks everyone for making all the fixes; I just created the branch,
and since everything in our list seems to have been fixed, I am going
to start working on the first official pre-release right away.

Stay tuned!

-- 
Joel

Re: RFC: branching for GDB 7.11 soon? (possibly Wed)

[Pedro Alves]

On 02/10/2016 03:40 AM, Joel Brobecker wrote:
>> So, to summarize, given how easy it can be to break C++ building,
>> and looking at the issues we want to solve, I can propose the following
>> plan:
>>
>>    1. Branch now, hold the pre-release;
>>    2. Fix the issues above still pending on both master + branch;
>>    3. Once the issues above are fixed on the branch, issue
>>       the first pre-release.
> 
> Thanks everyone for making all the fixes; I just created the branch,
> and since everything in our list seems to have been fixed, I am going
> to start working on the first official pre-release right away.

Hurray!  Thanks everyone.

Thanks,
Pedro Alves