gmane.comp.gdb.patches

Re: [commit/windows] Add thread ID in SuspendThread error warning message.

Joel Brobecker — 2013-06-18T23:43:49

That's interesting. I have the following patch in AdaCore's tree
which I have been uhming and ahming about. Would it apply to your
situation as well?

-                   warning (_("SuspendThread failed. (winerr %u)"),
-                            (unsigned) err);
-                   return NULL;
+                   /* If SuspendThread failed with error 5 (access
+                      denied), then ignore the error.  It's unclear
+                      where this comes from and how to prevent it.
+                      But in the meantime, ignoring it seems to allow
+                      us to inspect the thread (including fetching
+                      registers) without apparent ill effect.  */
+                   if (err != 5)
+                     {
+                       warning (_("SuspendThread (tid=0x%x) failed."
+                                  " (winerr %d)"),
+                                (unsigned) id, (unsigned) err);
+                       return NULL;
+                     }

I thi

Re: [RFA/DWARF] do not use dwarf2_per_objfile in dwarf2_per_objfile_free.

Joel Brobecker — 2013-06-18T23:37:16

Thanks, Tom. Applied.


I certainly could, if you'd like. I am a little afraid of what might
happen if I do, though ;-).

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

Joel Brobecker — 2013-06-18T21:34:58

And also sometimes ChangeLogs help review the change itself as well.

Re: [patch] ChangeLog width 74 -> 80

Joel Brobecker — 2013-06-18T21:09:10

FWIW, I find 80 characters to be too long, and more difficult to read.
I thought 74 characters was a good compromise, so why not standardize
on that?

[PATCH, gdbsim] Avoid silly crash when no binary is loaded

Luis Machado — 2013-06-18T20:49:39

Hi,

This patch prevents the long-standing crash scenario where we start 
gdbsim and "run" without any binaries. Warnings are issued, but those 
don't prevent the simulator from proceeding with garbage data.

Replacing those warnings with error calls seems to be the most 
appropriate here.

Ok?

Luis

Re: [patch] Fix CTRL-C for remote.c (PR remote/15297)

Tom Tromey — 2013-06-18T20:31:42

Jan> http://sourceware.org/bugzilla/show_bug.cgi?id=15297

Jan> With "set debug remote 1" hitting CTRL-C usually gets ignored,
Jan> occasionally it even locks-up GDB by calling unsafe functions from
Jan> a signal handler.

FWIW I read through this and it seems ok to me.

I wonder if the changes to the serial code mean we can remove the
immediate_quit changes in remote.c.

Tom

Re: [PATCH] Fix PR cli/15603

Tom Tromey — 2013-06-18T19:51:21

Jan> Yes, I am fine with this patch.

Thanks Jan.
I am going to check it in now.

Tom

[PATCH 3/3] fix solib-som.c

Tom Tromey — 2013-06-18T19:44:29

som_open_symbol_file_object was leaking a cleanup.

* solib-som.c (som_open_symbol_file_object): Call do_cleanups.
---
 gdb/solib-som.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/solib-som.c b/gdb/solib-som.c
index f88b539..457e464 100644
--- a/gdb/solib-som.c
+++ b/gdb/solib-som.c
< at >< at > -698,6 +698,7 < at >< at > som_open_symbol_file_object (void *from_ttyp)
   int errcode;
   int from_tty = *(int *)from_ttyp;
   gdb_byte buf[4];
+  struct cleanup *cleanup;
 
   if (symfile_objfile)
     if (!query (_("Attempt to reload symbols from process? ")))
< at >< at > -727,10 +728,11 < at >< at > som_open_symbol_file_object (void *from_ttyp)
       return 0;
     }
 
-  make_cleanup (xfree, filename);
+  cleanup = make_cleanup (xfree, filename);
   /* Have a pathname: read the symbol file.  */
   symbol_file_add_main (filename, from_tty);
 
+  do_cleanups (cleanup);
   return 1;
 }

[PATCH 2/3] fix init_cutu_and_read_dies

Tom Tromey — 2013-06-18T19:44:28

init_cutu_and_read_dies creates an outer cleanup and uses its
immediately nested inner cleanup conditionally.  This seems overly
complicated to me; simpler is to just deal with the outer cleanup and
drop the inner one.  That is what this patch implements.

* dwarf2read.c (init_cutu_and_read_dies): Remove 'free_cu_cleanup'.
Simplify cleanup handling.
---
 gdb/dwarf2read.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bbb8584..484a307 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
< at >< at > -4988,7 +4988,7 < at >< at > init_cutu_and_read_dies (struct dwarf2_per_cu_data *this_cu,
   struct die_info *comp_unit_die;
   int has_children;
   struct attribute *attr;
-  struct cleanup *cleanups, *free_cu_cleanup = NULL;
+  struct cleanup *cleanups;
   struct signatured_type *sig_type = NULL;
   struct dwarf2_section_info *abbrev_section;
   /* Non-zero if CU currently points to a DWO file and we need to
< at >< at > -5047,7 +5047,7 < at >< at > in

[PATCH 1/3] fix dwarf_decode_line_header

Tom Tromey — 2013-06-18T19:44:27

dwarf_decode_line_header has one return that is missing a call to
do_cleanups.  This patch adds it.

This was found using the cleanup checker.

* dwarf2read.c (dwarf_decode_line_header): Call do_cleanups
on all return paths.
---
 gdb/dwarf2read.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 4cd0c0e..bbb8584 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
< at >< at > -15655,6 +15655,7 < at >< at > dwarf_decode_line_header (unsigned int offset, struct dwarf2_cu *cu)
   if (line_ptr + lh->total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
+      do_cleanups (back_to);
       return 0;
     }
   lh->statement_program_end = line_ptr + lh->total_length;

[PATCH 0/3] a few more fixes from the cleanup checker

Tom Tromey — 2013-06-18T19:44:26

This series is a few remaining buglets caught by the cleanup checker.

I built and regtested this series on x86-64 Fedora 18.

Tom

Re: [PATCH v2] Add convenience variable $_exitsignal

Sergio Durigan Junior — 2013-06-18T19:15:52


OK, patch updated, thanks.

Re: [PATCH v2] Add convenience variable $_exitsignal

Eli Zaretskii — 2013-06-18T19:07:14

I'd use "resets", but otherwise this sounds fine, thanks.

Re: [PATCH v2] Add convenience variable $_exitsignal

Sergio Durigan Junior — 2013-06-18T19:02:48


Yes.  I could say "sets $_exitcode to < at >code{void}" instead, WDYT?

Re: [PATCH v2] Add convenience variable $_exitsignal

Eli Zaretskii — 2013-06-18T18:55:20

This is good, but I wonder whether the meaning of "clears $_exitcode"
will be sufficiently clear (pun intended) to the reader.  You mean to
say that the variable will be void, right?

Thanks.

[PATCH v2] Add convenience variable $_exitsignal

Sergio Durigan Junior — 2013-06-18T18:46:48

Hi,

This is the second version of the patch which adds the convenience
variable "$_exitsignal".  You can look at the first version here:

<http://sourceware.org/ml/gdb-patches/2013-06/msg00352.html>

I have made the proposed modifications suggested by Eli, Doug and
Pedro.

Doug, please take a look at tell me what you think.  Eli, I have
rewritten the doc entry for $_exitcode in order to make it explain which
variable is set and which is cleared depending on the situation.  And
Pedro, I've made the test more complete.

Hope it is fine now.  When talking to Pedro on IRC, he proposed that I
make $_exitcode a lazy variable.  I will do it after I get the $_signo
patch in (still working on it).

OK to apply?

Re: RFC: fix ref counting in gdbpy_selected_inferior

Tom Tromey — 2013-06-18T18:42:21

Tom> gdbpy_selected_inferior is double-counting its returned object.
Tom> This is obvious when you realize that inferior_to_inferior_object
Tom> returns a new reference.
Tom> This patch fixes the problem by removing an incref.

Tom> Built and regtested on x86-64 Fedora 18.

Tom> * python/py-inferior.c (gdbpy_selected_inferior): Don't incref.

I'm checking this in now.

Tom

Re: RFC: another Python refcounting fix

Tom Tromey — 2013-06-18T18:32:41

Tom> I had a couple more refcounting fixes on my Python checker branch.
Tom> Here's the first one.  finish_python_initialization does not decref
Tom> 'pythondir' along all the exit paths.

Tom> Built and regtested on x86-64 Fedora 18.

Tom> * python/python.c (finish_python_initialization): Decref
Tom> 'pythondir' on failure path as well.

I'm checking this in now.

Tom

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

Pedro Alves — 2013-06-18T18:19:15

Well, that's not the project's policy.  ChangeLogs are supposed to
be reviewed too.

But let's stop this before it gets into a ChangeLog-are-useless
argument...  I know you write good ChangeLog entries.  ;-)

Re: [PATCH v2] testsuite/gdb.base: Make skip test not rely on undefined behaviour.

Will Newton — 2013-06-18T18:16:49

Applied with this change, thanks!

--
Will Newton
Toolchain Working Group, Linaro

Re: [RFC] PR 15075 dprintf interferes with "next"

Pedro Alves — 2013-06-18T18:09:56


Context is split in multiple lines with '()'s, not ','s:

 (dprintf_create_breakpoints_sal)
 (dprintf_after_condition_true): New functions.



When I tried it last, I didn't find making the dprintf
explicitly silent necessary, given:

      /* Print nothing for this entry if we don't stop or don't
 print.  */
      if (!bs->stop || !bs->print)
bs->print_it = print_it_noop;

So did it turn out really necessary for some reason?  Please
don't leave such changes between revisions unexplained.



(
IMO, we should stop adding these (like glibc has done so too):
http://sourceware.org/ml/gdb-patches/2013-05/msg00253.html
)


That's not exactly true.  It's supported, but testing is racy at the moment.
Write instead:

if [is_remote target] then {
   # Testing with remote/non-stop is racy at the moment.
   unsupported "Testing dprintf with remote/non-stop is not supported."
   return 0
}


Hmm, this still looks racy to me, even on native targets.
"continue &" produces a gdb prompt.  gdb_test_multiple
inside has a