Kevin Pouget | 1 Feb 2012 09:31
Picon
Gravatar

Re: RFA: fix PR python/13599

On Wed, Feb 1, 2012 at 12:22 AM, Doug Evans <dje <at> google.com> wrote:
> On Tue, Jan 31, 2012 at 2:01 PM, Tom Tromey <tromey <at> redhat.com> wrote:
>> This fixes PR python/13599.
>>
>> The bug is that there is no way to get the line number in the source at
>> which a symbol was defined, even though gdb tracks this information.
>>
>> This patch changes gdb.Symbol to add a new 'line' attribute.
>>
>> This requires a doc review.
>>
>> Built and regtested on x86-64 Fedora 15.
>>
>> Tom
>>
>> 2012-01-31  Tom Tromey  <tromey <at> redhat.com>
>>
>>        PR python/13599:
>>        * python/py-symbol.c (sympy_line): New function.
>>        (symbol_object_getset): Add "line".
>
> 'tis ok with me.
>
> One comment though.
> The first thing that come to mind, as a user, is "Well, if I can get
> the line number from the symbol, why do I have to look elsewhere to
> get the file name?"
> The reason is an internal gdb implementation detail that we expose in
> python. IWBN if symbol tables were more nebulous entities - we might
> want the freedom to change them a bit.  [Not that we necessarily can
(Continue reading)

Tristan Gingold | 1 Feb 2012 10:27
Favicon

Re: [PATCH] Fix Darwin build error


On Jan 31, 2012, at 9:18 PM, Josh Matthews wrote:

> Darwin is busted again. Here's a fix.

Ok (and obvious).

Tristan.

> 
> Cheers,
> Josh
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index daf970c..105e6e5 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
>  <at>  <at>  -1,3 +1,8  <at>  <at> 
> +2012-01-31  Josh Matthews  <josh <at> joshmatthews.net>
> +
> +       Fix build error in Darwin port.
> +       * i386-darwin-nat.c: Include i386-nat.h.
> +
> 2012-01-31  Tom Tromey  <tromey <at> redhat.com>
> 
>        * language.h (struct language_defn) <la_iterate_over_symbols>: Fix
> diff --git a/gdb/i386-darwin-nat.c b/gdb/i386-darwin-nat.c
> index 12e2e65..a4f11e0 100644
> --- a/gdb/i386-darwin-nat.c
> +++ b/gdb/i386-darwin-nat.c
(Continue reading)

Joel Brobecker | 1 Feb 2012 11:21
Favicon

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

Hi Tom,

> Jan> I have noticed nightly regression testing to start randomly timing
> Jan> out a lot for gdb.ada/* testcases, narrowed it down to this
> Jan> check-in.
> 
> I looked into this.

I missed this message! You should have told me and I would have looked
into it.

> 2012-01-31  Tom Tromey  <tromey <at> redhat.com>
> 
> 	* ada-lang.c (ada_lookup_symbol_list_full): New function, from
> 	ada_lookup_symbol_list.  Add 'full_search' argument.
> 	(ada_lookup_symbol_list): Rewrite in terms of
> 	ada_lookup_symbol_list_full.
> 	(ada_iterate_over_symbols): Use ada_lookup_symbol_list_full.

Thanks a lot for the patch. I agree that the patch looks correct,
but I'm a little confused by the name of the new function (the _full
suffix suggests that the lookup is always a full search).

Traditionally, we've used a _1 suffix instead, which is very
unimaginative and yet so convenient.  I was going to change
your patch this way, but in the end, I'm just thinking I should
just add the parameter and update all callers. There are not that
many. I suspect you might have thought about this too, only to
just try the minimum change.

(Continue reading)

Pedro Alves | 1 Feb 2012 11:41
Picon
Favicon

Re: RFA: fix PR python/13351

On 01/31/2012 09:59 PM, Tom Tromey wrote:
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
>  <at>  <at>  -292,8 +292,9  <at>  <at>  gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
>  
>        TRY_CATCH (except, RETURN_MASK_ALL)
>  	{
> -	  selected_frame = get_selected_frame (_("No frame selected."));
> -	  block = get_frame_block (selected_frame, NULL);
> +	  selected_frame = get_selected_frame_if_set ();
> +	  if (selected_frame)
> +	    block = get_frame_block (selected_frame, NULL);
>  	}

I'm getting a bit nervous about get_selected_frame_if_set usages getting spread
around.  (It feels a bit like we're going back in time, to a time before the
always-a-frame work.)  The selected frame may not be set, while we still may have
a current frame, so get_selected_frame would lazily select it, and so the lookup
would still start on a frame block.  If you don't want an error, you can pass NULL
to get_selected_frame.

--

-- 
Pedro Alves

Joel Brobecker | 1 Feb 2012 11:55
Favicon

Re: [PATCH] Fix Darwin build error

Thank you, Josh.

> On Jan 31, 2012, at 9:18 PM, Josh Matthews wrote:
> 
> > Darwin is busted again. Here's a fix.
> 
> Ok (and obvious).

I applied the patch for you.

I noticed that you do not have a copyright assignment on file
with the FSF. For this change, it's OK because it is tiny and
obvious.  But if you would like to continue contributing to
the GDB project, you'll need one. The process is a little bit
on the long-ish side, so you might want to start it sooner
rather than later, if you're interested.

--

-- 
Joel

Phil Muldoon | 1 Feb 2012 12:49
Picon
Favicon

Re: RFA: fix PR python/13599

On 01/31/2012 11:22 PM, Doug Evans wrote:
> On Tue, Jan 31, 2012 at 2:01 PM, Tom Tromey<tromey <at> redhat.com>  wrote:
>> This fixes PR python/13599.
>>
>> The bug is that there is no way to get the line number in the source at
>> which a symbol was defined, even though gdb tracks this information.
>>
>> This patch changes gdb.Symbol to add a new 'line' attribute.
>>
>> This requires a doc review.
>>
>> Built and regtested on x86-64 Fedora 15.
>>
>> Tom
>>
>> 2012-01-31  Tom Tromey<tromey <at> redhat.com>
>>
>>         PR python/13599:
>>         * python/py-symbol.c (sympy_line): New function.
>>         (symbol_object_getset): Add "line".
> 'tis ok with me.
>
> One comment though.
> The first thing that come to mind, as a user, is "Well, if I can get
> the line number from the symbol, why do I have to look elsewhere to
> get the file name?"
> The reason is an internal gdb implementation detail that we expose in
> python. IWBN if symbol tables were more nebulous entities - we might
> want the freedom to change them a bit.  [Not that we necessarily can
> change things now, but I think it's something we should keep in mind.]
(Continue reading)

Phil Muldoon | 1 Feb 2012 12:51
Picon
Favicon

Re: RFA: fix PR python/13351

On 01/31/2012 11:13 PM, Doug Evans wrote:
> On Tue, Jan 31, 2012 at 1:59 PM, Tom Tromey<tromey <at> redhat.com>  wrote:
>> This fixes PR python/13351.
>>
>> The bug is that gdb.lookup_symbol required a current frame, but this
>> restriction doesn't really make sense.  It is ok to look up global
>> symbols without a current frame.
>>
>> This patch just lifts the restriction.
>>
>> This requires a doc review.
>>
>> Built and regtested on x86-64 Fedora 15.
>>
>> Tom
>>
>> 2012-01-31  Tom Tromey<tromey <at> redhat.com>
>>
>>         PR python/13351:
>>         * python/py-symbol.c (gdbpy_lookup_symbol): Work even when there
>>         is no frame.
> I think a bit of discussion is needed. [Sorry!]
>
> There is also lookup_global_symbol.
> Are we sure we want lookup_symbol to work when there is no frame?
> [it doesn't feel very clean to me, but maybe I just need to look at it
> differently]

I agree that two APIs for the same functionality are a pain.  However we 
have to keep them for backwards compatibility.
(Continue reading)

Tristan Gingold | 1 Feb 2012 15:06
Favicon

RFA: display an error message if libunwind-XX cannot be load

Hi,

no message was reported if libunwind cannot be loaded, which serially impairs debugging.  Hence this
patchlet to fill the gap.

The message is not highly user friendly (it appears before the banner), but this is not unlike sol-thread.c
I haven't handle the case where a symbol couldn't be resolved, but in practice this happens less frequently.

Ok for trunk ?

Manually tested only.

Tristan.

2012-02-01  Tristan Gingold  <gingold <at> adacore.com>

	* libunwind-frame.c (libunwind_load): Display message if dlopen failed.

diff --git a/gdb/libunwind-frame.c b/gdb/libunwind-frame.c
index 53ce1e2..360a28e 100644
--- a/gdb/libunwind-frame.c
+++ b/gdb/libunwind-frame.c
 <at>  <at>  -509,7 +509,11  <at>  <at>  libunwind_load (void)

   handle = dlopen (LIBUNWIND_SO, RTLD_NOW);
   if (handle == NULL)
-    return 0;
+    {
+      fprintf_unfiltered (gdb_stderr, "\
+[GDB failed to load %s: %s]\n", LIBUNWIND_SO, dlerror ());
(Continue reading)

Tom Tromey | 1 Feb 2012 16:03
Picon
Favicon

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

>>>>> "Joel" == Joel Brobecker <brobecker <at> adacore.com> writes:

Joel> I missed this message! You should have told me and I would have looked
Joel> into it.

Oh, sure, now you tell me ;-)

Joel> Thanks a lot for the patch. I agree that the patch looks correct,
Joel> but I'm a little confused by the name of the new function (the _full
Joel> suffix suggests that the lookup is always a full search).

Joel> Traditionally, we've used a _1 suffix instead, which is very
Joel> unimaginative and yet so convenient.  I was going to change
Joel> your patch this way, but in the end, I'm just thinking I should
Joel> just add the parameter and update all callers. There are not that
Joel> many. I suspect you might have thought about this too, only to
Joel> just try the minimum change.

Joel> Would it be OK with you if I made that change instead?

I will do it, it is no big deal.

Tom

Tom Tromey | 1 Feb 2012 16:07
Picon
Favicon

Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs

>>>>> "Joel" == Joel Brobecker <brobecker <at> adacore.com> writes:

Joel> [...], but in the end, I'm just thinking I should just add the
Joel> parameter and update all callers.

Here's the patch.  Let me know what you think.  I'm running regression
tests on it now, but I don't expect any problems.

Tom

2012-02-01  Tom Tromey  <tromey <at> redhat.com>

	* ada-lang.c (resolve_subexp): Update.
	(ada_lookup_symbol_list): Add 'full_search' argument.
	(ada_iterate_over_symbols): Pass 0 as full_search argument to
	ada_lookup_symbol_list.
	(ada_lookup_encoded_symbol): Update.
	(get_var_value): Update.
	* ada-exp.y (block_lookup): Update.
	(write_var_or_type): Update.
	(write_name_assoc): Update.
	* ada-lang.h (ada_lookup_symbol_list): Update.

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index d8c21a4..d1c7367 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
 <at>  <at>  -1021,7 +1021,7  <at>  <at>  block_lookup (struct block *context, char *raw_name)
   else
     name = ada_encode (raw_name);
(Continue reading)


Gmane