headers
Daniel Jacobowitz | 1 Feb 2009 18:53

Re: MI solib notification

On Fri, Jan 30, 2009 at 11:51:11PM +0200, Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir <at> codesourcery.com>
> > Date: Sat, 31 Jan 2009 00:10:46 +0300
> > 
> > Are doc changes OK?
> 
> Yes, thanks.  But I have a few comments:
> 
> > + <at> item =library-loaded,id=" <at> var{id}",target-name=" <at> var{target-name}",host-name=" <at> var{host-name}",low-address=" <at> var{low}",high-address=" <at> var{high}",symbols-loaded=" <at> var{loaded}"
> 
> This is a very long line, and the  <at> table that it's part of has  <at> code
> markup, which means TeX will not break this line.  We need to make it
> shorter.  One idea is this:
> 
>    <at> item =library-loaded, <at> var{info}
> 
> and then describe the contents of  <at> var{info} below, perhaps as a
> separate  <at> table.  WDYT?

MI output comes all on one line, but what we've done elsewhere in the
GDB/MI chapter is insert line breaks after some of the commas.  How
about that here too?  This form is easy to visually parse, once you're
used to the MI syntax.

--

-- 
Daniel Jacobowitz
CodeSourcery
Permalink | Reply | 16 comments |
headers
Daniel Jacobowitz | 1 Feb 2009 18:56

Re: [RFA] fixes to building gdb w/gdbtk and w/o tcl/tk

On Fri, Jan 30, 2009 at 12:36:15PM -0800, Doug Evans wrote:
> +    if test -z "${no_tcl}" -a -z "${no_tk}"; then

Please use "&& test" instead of "-a"; it's more portable that way.
> -    BUILD_TCLSH=${TCL_BIN_DIR}/tclsh
> +    BUILD_TCLSH="${TCL_BIN_DIR}/tclsh"

For the record I'm pretty sure you don't need quotes after '='.

Otherwise seems OK.  When you're reindenting a lot, also attaching a
whitespace-insensitive diff makes things a lot easier, though.

--

-- 
Daniel Jacobowitz
CodeSourcery
Permalink | Reply | 0 comments |
headers
Daniel Jacobowitz | 1 Feb 2009 19:04

Re: MI solib notification

On Sat, Jan 31, 2009 at 12:10:46AM +0300, Vladimir Prus wrote:
> +static void mi_solib_loaded (struct so_list *solib)
> +{
> +  struct mi_interp *mi = top_level_interpreter_data ();
> +  target_terminal_ours ();
> +  fprintf_unfiltered (mi->event_channel, 
> +		     
"library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",low-address=\"0x%s\",high-address=\"0x%s\",symbols-loaded=\"%d\"", 
> +		      solib->so_original_name, solib->so_original_name, 
> +		      solib->so_name, 
> +		      paddr (solib->addr_low), paddr (solib->addr_high), 
> +		      solib->symbols_loaded);
> +  gdb_flush (mi->event_channel);
> +}

Do existing clients use addr_low / addr_high from "info shared"?
If so, do you know what they use it for?

These fields make sense for SVR4 models, like Linux and BSD shared
libraries, where shared libraries get a single chunk of address space.
But they don't make sense for some DLL systems which load the text and
data separately, or for kernel modules where each section can get a
different load offset.  We should either report the boundaries of
the first contiguous piece, which will not cover the whole library,
or else the highest and lowest address, which may cover bits of
some other library.

> diff --git a/gdb/solib.c b/gdb/solib.c
> index cce4f7f..5a28292 100644
> --- a/gdb/solib.c
(Continue reading)

Permalink | Reply | 14 comments |
headers
Eli Zaretskii | 1 Feb 2009 19:22
Picon

Re: MI solib notification

> Date: Sun, 1 Feb 2009 12:53:09 -0500
> From: Daniel Jacobowitz <drow <at> false.org>
> Cc: Vladimir Prus <vladimir <at> codesourcery.com>,
> 	gdb-patches <at> sources.redhat.com, nickrob <at> snap.net.nz
> 
> > > + <at> item =library-loaded,id=" <at> var{id}",target-name=" <at> var{target-name}",host-name=" <at> var{host-name}",low-address=" <at> var{low}",high-address=" <at> var{high}",symbols-loaded=" <at> var{loaded}"
> > 
> > This is a very long line, and the  <at> table that it's part of has  <at> code
> > markup, which means TeX will not break this line.  We need to make it
> > shorter.  One idea is this:
> > 
> >    <at> item =library-loaded, <at> var{info}
> > 
> > and then describe the contents of  <at> var{info} below, perhaps as a
> > separate  <at> table.  WDYT?
> 
> MI output comes all on one line, but what we've done elsewhere in the
> GDB/MI chapter is insert line breaks after some of the commas.  How
> about that here too?

That'd be okay as well, although I don't really understand why it is
better than my suggestion.
Permalink | Reply | 15 comments |
headers
Daniel Jacobowitz | 1 Feb 2009 19:23

Re: [PATCH/WIP] C/C++ wchar_t/Unicode printing support

On Fri, Jan 30, 2009 at 04:41:35PM -0700, Tom Tromey wrote:
> Joel> Do you think we could have a configure option that allows us to
> Joel> deactivate this feature in order to avoid the dependency?
> 
> I don't think it would be very easy to make this an optional feature,
> or to make it fall back to the currently existing code.
> 
> I suppose one option would be to have a degraded mode where we require
> that the host charset and the target charset be the same.  Then maybe
> we could make it work by redefining iswprint and wchar_t.

I don't see the connection between the iconv dependency and iswprint /
wchar_t.  Are there portability issues for those too?  They don't come
from libiconv.

It seems like a dummy version of iconv_open which only succeeds if the
two character sets are the same, plus a pass-through version of iconv,
would be enough to remove the iconv dependency.  That degraded mode
covers all local debugging.  There'd need to be a little additional
logic too, to allow you to set all the charset variables at once;
otherwise you'd be stuck since validate would fail when you tried to
change any of them.  Or just remove the settings in that case.

> I don't think it would be very easy to preserve the current
> functionality and make the iconv stuff an add-on.  The current charset
> API is not well suited to variable length encodings.

In my opinion, the current non-iconv, non-identity conversions
are not a major loss.  But if someone was motivated to preserve them,
it seems like that wouldn't be hard; add them to the fallback
(Continue reading)

Permalink | Reply | 5 comments |
headers
Daniel Jacobowitz | 1 Feb 2009 19:28

Re: [RFC] Use untested for macscp.exp if no macro information generated

On Wed, Jan 28, 2009 at 02:23:11PM +0100, Pierre Muller wrote:
> 
> I tried to simplify my patch for macro support
> in order to get less uninteresting failures.

I think your earlier patch had the right idea.  Why did it leave some
failures?  We should have none if we're going to skip the test.

>  if {[list_and_check_macro main WHERE {macscp1.c {before macscp1_3}}]} {
> -    return 0
> +    global verbose
> +    set macro_support "unknown"
> +    send_gdb "info source\n"
> +    gdb_expect 10 {

General note, please use gdb_test_multiple.  No need for a timeout in
most cases; use the default setting.

--

-- 
Daniel Jacobowitz
CodeSourcery
Permalink | Reply | 7 comments |
headers
Daniel Jacobowitz | 1 Feb 2009 19:29

Re: MI solib notification

On Sun, Feb 01, 2009 at 08:22:28PM +0200, Eli Zaretskii wrote:
> That'd be okay as well, although I don't really understand why it is
> better than my suggestion.

It's not a big difference; I find it more natural to have the output
all grouped together and looking similar to GDB's expected output.

--

-- 
Daniel Jacobowitz
CodeSourcery
Permalink | Reply | 14 comments |
headers
Daniel Jacobowitz | 1 Feb 2009 19:34

Re: info frame ADDR internal error

On Sun, Jan 25, 2009 at 07:44:32PM +0000, Pedro Alves wrote:
> The new frame that create_new_frame created, isn't linked in the
> regular ( current_frame->... ) frame chain, it lives in its own chain,
> so this frame_find_by_id call isn't going to find it, unless you
> get lucky.
> 
> This reinforces the bad things I was saying about create_new_frame.
> 
> Any suggestions on how this could be fixed?

This is just gross.  But yes, I see a way we could "fix" it.  This
code is only reachable from select_frame and info frame.  In the
former case, if we create a new frame we could clear the old frame
chain and set both current and selected frame to the new one;
in the latter case we'd have to clear the frame chain after info
frame was done, and restore the saved selected frame.

I do think this is somewhat useful, although being able to set
registers while debugging a core file would eliminate a lot of the
use...

--

-- 
Daniel Jacobowitz
CodeSourcery
Permalink | Reply | 0 comments |
headers
Daniel Jacobowitz | 1 Feb 2009 19:37

Re: small get_prev_frame cleanup (don't allow a NULL this_frame anymore), plus caller adjustment

On Mon, Jan 26, 2009 at 12:26:58AM +0000, Pedro Alves wrote:
> To clean that up, the best seems to be to export ``has_stack_frames'',
> which is also the predicate used by deprecated_get_selected_frame and
> get_selected_frame, and use it at the get_prev_frame's and
> deprecated_get_seleted_frame caller (varobj_create).  (This is not to
> say that checks couldn't move yet closer to the top-level, but I'm
> leaving that for another time --- focusing on get_prev_frame.)
> 
> WDYT?

I think this is OK.  Another choice would be to push
find_frame_addr_in_frame_chain into frame.c... but we do not want
anything else to use it.

--

-- 
Daniel Jacobowitz
CodeSourcery
Permalink | Reply | 1 comment |
headers
Daniel Jacobowitz | 1 Feb 2009 19:47

Re: MI solib notification

One more question: should this show up in -list-features?

--

-- 
Daniel Jacobowitz
CodeSourcery
Permalink | Reply | 0 comments |

Gmane