This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


  After asmwarrior clarifications
that my patch alone didn't cause any crash,
I finally committed the patch,
after adding the changes suggested by Joel below.

  The final patch is attached.

  Concerning Pedro's comments,
I will answer to them in his last reply.

Thanks to all,

Pierre Muller  


> -----Message d'origine-----
> De?: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé?: vendredi 7 décembre 2012 08:11
> À?: Pierre Muller
> Cc?: 'asmwarrior'; 'Eli Zaretskii'; gdb-patches@sourceware.org
> Objet?: Re: [RFC-v5] Fix .text section offset for windows DLL (was Calling
> __stdcall functions in the inferior)
> 
> > 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

Attachment: fix-dll-offset-v9.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]