From: Tom Tromey <tromey@redhat.com>
To: Pedro Alves <palves@redhat.com>
Cc: Joel Brobecker <brobecker@adacore.com>, gdb-patches@sourceware.org
Subject: Re: RFC: bfd_section should not be NULL in call to prim_record_minimal_*
Date: Tue, 17 Apr 2012 14:21:00 -0000 [thread overview]
Message-ID: <87hawiwijp.fsf@fleche.redhat.com> (raw)
In-Reply-To: <4F8D5EEC.30908@redhat.com> (Pedro Alves's message of "Tue, 17 Apr 2012 13:15:40 +0100")
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> [In my mind, symbols ideally wouldn't need back pointers to thei
Pedro> "containers" (symtabs, object files, etc), because that wastes
Pedro> space; the lookup routines instead should bubble up the "container"
Pedro> the symbol was found in if necessary. But that may be very
Pedro> hard to do.]
I agree, both that this would be preferable and that it might be hard.
In fact, on my current "split objfile" branch
(archer-tromey-multi-inferior-3), I had to break this link --
on the branch, the symbols are not relocated and may be shared
by multiple objfiles. (I sent a WIP version of this branch to
gdb-patches a while ago.)
I broke the linkage in a somewhat hacky way, though. The branch is big
enough that I didn't want to also try to fix this problem at the same
time.
Pedro> The "section" and "obj_section" are one of those sources of difference
Pedro> and confusion that it'd be nice to get rid of:
No kidding. I have often wondered about this myself.
Pedro> OTOH, this stuff is space sensitive, and in the long term,
Pedro> it could prove better to only hold a "short" instead of a
Pedro> pointer (8 bytes on 64-bit hosts).
It seems like we could perhaps use the BFD index everywhere.
This gets into a fair amount of ugly business.
Pedro> Like in the patch below. When testing it, I ran linespec.exp
Pedro> first, and that revealed that msymbol_objfile had a bug in that
Pedro> it didn't look at all the pspaces, only the current (the test
Pedro> adds a second inferior, therefore a second pspace). Not good
Pedro> when you want to look up the objfile because you want to know the
Pedro> msyms' pspace to begin with.
This seems odd to me. And, with the split objfile stuff, it would be
actively wrong, since a given msymbol could appear in multiple pspaces.
Now, normally I wouldn't mention a patch that isn't checked in yet,
but...
I think it would be better to pass the pspace around with the minimal
symbol in linespec.c. We certainly know it at the time we actually find
the minsym, so it is just a matter of not losing the information.
Ok, I just looked at linespec.c again, and this is why there is an
objfile in minsym_and_objfile.
Here's a totally untested patch based on this idea.
Note that compare_msymbols, while not exactly wrong right now, is
definitely weird in that it makes an assumption about the layout of
minsym_and_objfile.
Tom
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 228214b..7cbf1bf 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1899,7 +1899,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls)
VEC_iterate (minsym_and_objfile_d, ls->minimal_symbols, i, elem);
++i)
{
- pspace = SYMBOL_OBJ_SECTION (elem->minsym)->objfile->pspace;
+ pspace = elem->objfile->pspace;
set_current_program_space (pspace);
minsym_found (state, elem->objfile, elem->minsym, &sals);
}
@@ -2584,20 +2584,20 @@ compare_symbols (const void *a, const void *b)
static int
compare_msymbols (const void *a, const void *b)
{
- struct minimal_symbol * const *sa = a;
- struct minimal_symbol * const *sb = b;
+ const struct minsym_and_objfile *sa = a;
+ const struct minsym_and_objfile *sb = b;
uintptr_t uia, uib;
- uia = (uintptr_t) SYMBOL_OBJ_SECTION (*sa)->objfile->pspace;
- uib = (uintptr_t) SYMBOL_OBJ_SECTION (*sb)->objfile->pspace;
+ uia = (uintptr_t) sa->objfile->pspace;
+ uib = (uintptr_t) sa->objfile->pspace;
if (uia < uib)
return -1;
if (uia > uib)
return 1;
- uia = (uintptr_t) *sa;
- uib = (uintptr_t) *sb;
+ uia = (uintptr_t) sa->minsym;
+ uib = (uintptr_t) sb->minsym;
if (uia < uib)
return -1;
next prev parent reply other threads:[~2012-04-17 14:16 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-16 21:14 Joel Brobecker
2012-04-16 21:14 ` [RFA/commit 1/2] Unused local variables in xcoffread.c:read_xcoff_symtab Joel Brobecker
2012-04-16 21:19 ` [RFA/commit 2/2] pspace != NULL failed assertion on ppc-aix Joel Brobecker
2012-04-18 0:43 ` Joel Brobecker
2012-04-18 10:17 ` Pedro Alves
2012-04-18 14:57 ` Joel Brobecker
2012-04-18 15:13 ` Pedro Alves
2012-04-18 0:32 ` checked in: [RFA/commit 1/2] Unused local variables in xcoffread.c:read_xcoff_symtab Joel Brobecker
2012-04-17 12:03 ` RFC: bfd_section should not be NULL in call to prim_record_minimal_* Tom Tromey
2012-04-17 15:17 ` Joel Brobecker
2012-04-17 15:23 ` Tom Tromey
2012-04-17 23:26 ` Joel Brobecker
2012-04-18 15:01 ` Tom Tromey
2012-04-18 0:24 ` Joel Brobecker
2012-04-17 12:45 ` Pedro Alves
2012-04-17 14:21 ` Tom Tromey [this message]
2012-04-17 14:29 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87hawiwijp.fsf@fleche.redhat.com \
--to=tromey@redhat.com \
--cc=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox