This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC-v5] Fix .text section offset for windows DLL (was Calling __stdcall functions in the inferior)
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Pierre Muller <pierre dot muller at ics-cnrs dot unistra dot fr>
- Cc: 'asmwarrior' <asmwarrior at gmail dot com>, 'Eli Zaretskii' <eliz at gnu dot org>, gdb-patches at sourceware dot org
- Date: Fri, 7 Dec 2012 11:10:35 +0400
- Subject: Re: [RFC-v5] Fix .text section offset for windows DLL (was Calling __stdcall functions in the inferior)
- References: <20121024194517.GK3555@adacore.com> <011901cdb2ab$48076b90$d81642b0$@muller@ics-cnrs.unistra.fr> <20121105171121.GA2972@adacore.com> <50991f5f.8382440a.1100.ffff82abSMTPIN_ADDED@mx.google.com> <509ABA17.30507@redhat.com> <000301cdbd96$f5cd9f10$e168dd30$@muller@ics-cnrs.unistra.fr> <20121122173019.GF9964@adacore.com> <15690.5992342674$1353883881@news.gmane.org> <87624si9ur.fsf@fleche.redhat.com> <001501cdccaf$ad85e9b0$0891bd10$@muller@ics-cnrs.unistra.fr>
> 2012-11-25 Pierre Muller <muller@sourceware.org>
>
> * coff-pe-read.h (pe_text_section_offset): Declare new function.
> * coff-pe-read.c (debug_coff_pe_read): New static variable.
> (struct read_pe_section_data): Add section_name field.
> (IMAGE_SCN_CNT_CODE): New macro, if not already defined.
> (IMAGE_SCN_CNT_INITIALIZED_DATA): Ditto.
> (IMAGE_SCN_CNT_UNINITIALIZED_DATA): Ditto.
> (get_pe_section_index): New function.
> (struct pe_sections_info): New type.
> (get_section_vmas): Use new struct pe_sections_info.
> (add_pe_exported_sym): Handle unnamed exported function.
> (add_pe_forwarded_sym): New function.
> (read_pe_truncate_name): Truncate at last dot.
> (pe_as16): New function.
> (read_pe_exported_syms): Use ordinal of function to
> retrieve correct RVA address of function and handle
> forwarded symbol.
> (pe_text_section_offset): New function.
> (show_debug_coff_pe_read): New function.
> (_initialize_coff_pe_read): New function adding
> 'set/show debug coff_pe_read' commands.
>
> * windows-tdep.c (windows_xfer_shared_library): Use
> pe_text_section_offset function instead of possibly wrong
> 0x1000 constant for .text sextion offset.
Looks good - OK to commit after the following minor corrections
have been applied. For the record, I have tested this patch on
x86-windows against AdaCore's GDB testsuite, no regression.
Thank you!
> +/* Get the index of the named section in our own full arrayi.
Small typo at the end if "array".
> +get_pe_section_index (const char *section_name,
> + struct read_pe_section_data *sections,
> + int nb_sections)
> +{
> + int i;
> + for (i = 0; i < nb_sections; i++)
Missing empty line after variable declarations...
> + DLL_NAME is the internal name of the DLL file,
> + OBJFILE is the objfile struct of DLL_NAME. */
> +
> +
> +static int
> +add_pe_forwarded_sym (const char *sym_name, const char *forward_dll_name,
Can you delete the second empty line?
> + char * last_point = strrchr (dll_name, '.');
No space between '*' and 'last_point'.
> + otherix++;
> + section_data = xrealloc (section_data, otherix
> + * sizeof (struct read_pe_section_data));
> + name = xstrdup (sec_name);
> + section_data[otherix - 1].section_name = name;
> + make_cleanup (xfree, name);
> + section_data[otherix - 1].rva_start = vaddr;
> + section_data[otherix - 1].rva_end = vaddr + vsize;
> + section_data[otherix - 1].vma_offset = 0;
> + if (characteristics & IMAGE_SCN_CNT_CODE)
> + section_data[otherix - 1].ms_type = mst_text;
> + else if (characteristics & IMAGE_SCN_CNT_INITIALIZED_DATA)
> + section_data[otherix - 1].ms_type = mst_data;
> + else if (characteristics & IMAGE_SCN_CNT_UNINITIALIZED_DATA)
> + section_data[otherix - 1].ms_type = mst_bss;
> + else
> + section_data[otherix - 1].ms_type = mst_unknown;
A possible suggestion: It seems simpler to increment otherix
at the end rather than at the beginning, and thus have:
section_data = xrealloc (section_data, (otherix + 1) [...]);
[...]
section_data[otherix].rva_end = vaddr + vsize;
section_data[otherix].vma_offset = 0;
[...]
otherix++;
> + /* First handle forward cases. */
> + if ((func_rva >= export_rva)
> + && (func_rva < export_rva + export_size))
You don't need the extra parentheses...
--
Joel