From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28130 invoked by alias); 25 Jun 2009 14:40:16 -0000 Received: (qmail 28117 invoked by uid 22791); 25 Jun 2009 14:40:14 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 25 Jun 2009 14:40:05 +0000 Received: (qmail 11703 invoked from network); 25 Jun 2009 14:40:03 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 25 Jun 2009 14:40:03 -0000 From: Pedro Alves To: gdb-patches@sourceware.org, tromey@redhat.com Subject: Re: [RFC] [1/2] First cut at multi-executable support. Date: Thu, 25 Jun 2009 14:40:00 -0000 User-Agent: KMail/1.9.10 References: <200906151621.57104.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200906251541.04243.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-06/txt/msg00668.txt.bz2 On Wednesday 17 June 2009 19:19:23, Tom Tromey wrote: > >>>>> "Pedro" == Pedro Alves writes: > > Pedro> All-in-all, what do you think of this, and the patch below? > > I think it is unbelievably awesome. > > Pedro> There's certainly scope of a lot of goodies to be added on top > Pedro> of this, like extending thread control support (threads groups, itsets > Pedro> or whatever); better support for symbol scoping a-la HPD's ## syntax > Pedro> would be great too; Making the breakpoints module smarter about > Pedro> that also would be nice. > > ... sharing objfiles across inferiors would be nice. It seems to me > that this would be important for capping memory use when there are > many inferiors. I wonder if this makes sense with the aspace/sspace > stuff, given that an objfile now has a symbol space. We could split an objfile in two, with the shared parts not having a reference to a symbol space. I can see us sharing the type stuff as a first step. The min|p|symbol,blocks,etc (everything that holds an address) is a different story due to the fact that objfiles are relocated in place. Ideally, I think we would stop relocating objfiles' symbols etc. in place (objfile_relocate), and instead store unrelocated (with VMAs == link addresses) addresses in the symbols, and then be able to given a symbol and a sspace/inferior, request its "cooked" address, that is, we'd store section relocation offsets on a separate structure, and apply then on demand for the space we're interested in. E.g:. struct objfile { minsyms; psymtabs/psymbols /* (while we have them) */ symtab/symbols; section_offsets; /* link time offsets (or unique offsets arranged by GDB if this is a relocatable object */ num_sections; }; struct cooked_objfile { struct objfile *raw_obfile; struct symbol_space *sspace; section_offsets; /* load time offsets (relocation offsets), array the size of raw_objfile->num_sections */ }; Then you have the reverse side of the story, that is, in many, many places, you have a PC, and you want the matching symbol. Here, I think we should look for the matching cooked section for the PC, uncook this PC (unrelocate it, given it section offset), and then do the symbol lookup with this uncooked PC. At a high distance, this sounds feasible to me, but, looking at the code, this looks like a huge effort, and I'm sure I'm missing a lot of complications. OTOH, having this would help make the breakpoints module much smarter and leaner. E.g., a new inferior loaded the same objfile where you already have a breakpoint set? Given the breakpoint symbol, get the cooked address in this new address space, and set a new raw breakpoint location there, no need to re-evalutate the breakpoint's string expression in the new space. > > When gdb sees a new inferior, does it immediately read its debuginfo? About the same as if you had started the new inferior with 'gdb newinferiorfile -ex "run"'. > This seems like another possible performance problem; lazily reading > it would be friendlier. In a scenario like the "make" case, I would > assume that most inferiors will not actually require any user > attention, and time and memory spent on their debuginfo is just > wasted. Yeah, I know you have patches for this. ;-) In the "make" case, you're not likely to have debug info for "make", "sed", "cut", "awk", "true", "false", whatnot, though. But yeah, there's still a lot that can be done. > > > I saw this in the output: > > [Thread debugging using libthread_db enabled] > process 16875 is executing new program: /bin/true > (no debugging symbols found) > (no debugging symbols found) > (no debugging symbols found) > > Program exited normally. > make[2]: Leaving directory `/home/pedro/gdb/sspaces/build/gdb/gdbserver' > > Program exited normally. > > Program exited normally. > > Program exited normally. > make[1]: Leaving directory `/home/pedro/gdb/sspaces/build/gdb' > > Program exited normally. > > I think the "(no debugging symbols found)" thing has been addressed. > At least, there was a patch. > > "Program exited normally." could also use some love... at least some > info about the program, and maybe removing the excess newlines? For my own testing, I've tweaked those messages to include the process id. It's just mad otherwise. However, it has been one of my goals to not change much how the single-inferior case works/outputs as a first incremental step. I'm sure there will be different opinions on to what GDB should output, so this avoids such discussions for now :-). In this case, in truth, I deferred proposing to change such outputs until afterwards, to avoid having to go and do a bunch of mechanical testsuite patching. I certainly would like to have better messages! This reasoning holds also for other things I think we'll want to change in the future, like for example, the follow-fork model (new inferior only pops up after resuming, not when the fork is caught). > > > I see in many places that we pass an address_space and a CORE_ADDR > together, e.g.: > > +insert_single_step_breakpoint (struct address_space *aspace, CORE_ADDR next_pc) > > Does a CORE_ADDR make sense without an address_space? Sure does. In many places, you only need to know the offset into the symbol/address space (that's a CORE_ADDR), while the space is implied from context. > I'm curious whether you considered putting the address_space into > CORE_ADDR, For 5 minutes only. This is C, not C++. :-) >grep CORE_ADDR * -rn | grep -v ChangeLog | wc -l 4470 What we have is an address or symbol space (the new entities), and an offset into them (CORE_ADDR). It just happens that GDB is (mostly anyway, and also discarding the unused bit hacks) prepared currently for a single linear address space, with CORE_ADDRess representing offsets into this single address space. I do think there's scope in the future (particularly if we add sharing of objfiles like I mentioned above), for an object representing an address like: sspace | objfile | section | offset_in_section index ^ scalar > or making a new type holding both. I just didn't find a use for that, that's all. > > > +#define exec_bfd current_symbol_space->ebfd > +#define exec_bfd_mtime current_symbol_space->ebfd_mtime > > +#define so_list_head current_symbol_space->so_list > > +#define symfile_objfile current_symbol_space->symfile_objfile_1 > > +#define object_files current_symbol_space->objfiles > > +#define current_target_sections (¤t_symbol_space->target_sections) > > I don't mind this kind of thing as a transitional change, to make it > so a patch doesn't completely explode. But I think I'd prefer, in the > long run, to replace these macros with their expansions (or maybe > accessor macros taking current_symbol_space as an argument) > everywhere. What do you think? Agreed. > Also, I am curious about the relationship between the current inferior > and the current symbol space. I guess whenever the current inferior > changes, the current inferior space must as well? Yes, they go hand in hand, most of the times. We switch the current space whenever GDB switches current thread (switch_to_thread, for example). > > + ui_out_table_header (uiout, 1, ui_left, "current", ""); > > For some reason I did not think of leaving the header empty when I did > this for "info inferiors". I like this better... since you're also > touching print_inferior, how about just making that change? But I had already. :-) Or, do you mean to split that change out of the patch? I can do that. I think that would be good idea. I'm now working on cleaning up a bit the patch and fixing the random crashes I mentioned, and I'll be posting an updated patch soon. -- Pedro Alves