Re: [PATCH 6/7] gdb: select "Cygwin" OS ABI for Cygwin binaries

[Pedro Alves <palves@redhat.com>]

On 4/1/20 10:53 PM, Simon Marchi via Gdb-patches wrote:
> On 2020-04-01 5:36 p.m., Pedro Alves wrote:
>> On 3/16/20 5:08 PM, Simon Marchi via Gdb-patches wrote:
>>> +bool
>>> +is_linked_with_cygwin_dll (bfd *abfd)
>>> +{
>>> +  /* The list of DLLs a PE is linked to is in the .idata section.  See:
>>> +
>>> +     https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#the-idata-section
>>> +   */
>>> +  asection *idata_section = bfd_get_section_by_name (abfd, ".idata");
>>> +  if (idata_section == nullptr)
>>> +    return false;
>>> +
>>> +  /* Find the virtual address of the .idata section.  We must subtract this
>>> +     from the RVAs (relative virtual addresses) to obtain an offset in the
>>> +     section. */
>>> +  bfd_vma idata_addr =
>>> +    pe_data (abfd)->pe_opthdr.DataDirectory[PE_IMPORT_TABLE].VirtualAddress;
>>
>> = on next line.  Unless it doesn't fit, then let's ignore.
> 
> Yes it fits.
>>> +
>>> +  /* Map the section's data.  */
>>> +  bfd_size_type idata_size;
>>> +  const gdb_byte *const idata_contents
>>> +    = gdb_bfd_map_section (idata_section, &idata_size);
>>> +  if (idata_contents == nullptr)
>>> +    {
>>> +      warning (_("Failed to get content of .idata section."));
>>> +      return false;
>>> +    }
>>> +
>>> +  const gdb_byte *iter = idata_contents;
>>> +  const gdb_byte *end = idata_contents + idata_size;
>>> +  const pe_import_directory_entry null_dir_entry = { 0 };
>>> +
>>> +  /* Iterate through all directory entries.  */
>>> +  while (true)
>>> +    {
>>> +      /* Is there enough space left in the section for another entry?  */
>>> +      if (iter + sizeof (pe_import_directory_entry) > end)
>>> +	{
>>> +	  warning (_("Failed to parse .idata section: unexpected end of "
>>> +		     ".idata section."));
>>> +	  break;
>>> +	}
>>> +
>>> +      pe_import_directory_entry *dir_entry = (pe_import_directory_entry *) iter;
>>
>> Is 'iter' guaranteed to be sufficiently aligned?  Recall that this is
>> host-independent code.
> 
> I admit I haven't thought about that.  Within the PE file, the data of sections is
> aligned on at least 512 bytes.  See "FileAlignment" here:
> 
> https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#optional-header-windows-specific-fields-image-only

Ah, good.  Sounds like the data structures were designed for
memory mapping.  Which isn't that surprising.

> 
> However, when BFD maps the file/section data in memory for GDB to read, is that mapping
> guaranteed to be sufficiently aligned as well?

I think so.  If bfd heap allocates, then memory will be aligned to the word size at
least.  If bfd mmaps, then memory will be aligned to page size.  And then if the
data structures are aligned, we should be good.

Thanks,
Pedro Alves