Re: [RFC-v5] Fix .text section offset for windows DLL (was Calling __stdcall functions in the inferior)

Joel Brobecker
2012-12-07T07:10:59Z

> 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