This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RE: [PATCH 1/3] Added command remove-symbol-file.
- From: "Blanc, Nicolas" <nicolas dot blanc at intel dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Mon, 29 Apr 2013 17:30:01 +0000
- Subject: RE: [PATCH 1/3] Added command remove-symbol-file.
- References: <1366098721-18302-1-git-send-email-nicolas dot blanc at intel dot com> <1366098721-18302-2-git-send-email-nicolas dot blanc at intel dot com> <517ABA6A dot 5070400 at redhat dot com>
Hi Pedro,
>> +/* Upon notification of FREE_OBJFILE remove any reference
>> + to any user-added file that is about to be freed. */
>
> Why only user-added files?
I choose to restrict the scope of the command to user-added files in order to
limit potential unforeseen side-effects in a first time. This restriction
can be lifted in the future if needed.
>> +static void
>> +remove_user_added_objfile (struct objfile *objfile) {
>> + struct so_list *gdb;
>> +
>> + if (!objfile)
>> + return;
>> +
>> + if (!(objfile->flags & OBJF_USERLOADED)
>> + || !(objfile->flags & OBJF_SHARED))
>> + return;
>> +
>> +
>> + gdb = so_list_head;
>> + while (gdb)
>> + {
>> + if (gdb->objfile == objfile)
>> + gdb->objfile = NULL;
>> + gdb = gdb->next;
>> + }
>
> Or rather/also, this looks a bit weird to me.
> Can we ever really ever find a user-loaded file in the so_list_head list? What would that mean?
> IIRC, the only way to get a OBJF_USERLOADED|OBJF_SHARED objfile is through "dll-symbols" (dll_symbol_command), but that doesn't create any entry in the shared library list.
This is a good question. I added this function because update_solib_list handles the case of user-added files:
/* Unless the user loaded it explicitly, free SO's objfile. */
if (gdb->objfile && ! (gdb->objfile->flags & OBJF_USERLOADED)
&& !solib_used (gdb))
free_objfile (gdb->objfile);
If the user-loaded check above is needed then remove_user_added_objfile() is also needed.
But I don't think it is currently possible that user-loaded files end up in so_list_head either.
>> +static void
>> +disable_breakpoints_in_free_objfile (struct objfile * objfile)
>
> This is clearly mirroring the naming of
> disable_breakpoints_in_unloaded_shlib. Should be "in freed objfile".
> "in free objfile" would mean something else.
Ok
> + error (_("USAGE: remove-symbol-file <text_low_address>"));
> I'd s/low// here. text_address is clear and common enough that having "low" there makes me go "what does low mean here?" Or just <address> even.
Ok, I am using text_addr now.
> + if (objf->flags & OBJF_USERLOADED && objf->addr_low == addr)
> As I mentioned, the .text address may not be the lower address in the object at all, so this "addr_low" confused me.
> I'd be happier with naming the field for what ig really is, something like "add_symbol_file_addr", with a comment indicating this is related to "add-symbol-file", but I see you're reusing an existing variable. At the very least,
> the variable's definition should gain a comment explaining its overloading for add-symbol-file.
I understand your point. GDB's implementation\doc assume that addr_low is the text address.
It' not only the case for add-symbol-file but also for info shared.
So I've added the following comment to struct objfile for clarification:
/* The base address of the object file, which by default is assumed to be
the address of the text section. The remove-symbol-file command uses
this field to identify the object file to remove. */
CORE_ADDR addr_low;
I hope this helps.
Thanks,
Nicolas
--
Pedro Alves
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052