diff mbox series

[v5,2/3] bfd: fill in PE load config directory entry.

Message ID 4bfc1e76-3936-3f79-96a0-9b315ef0b4a6@jdrake.com
State New
Headers
Series [v5,1/3] bfd: properly use bfd_get_symbol_leading_char in peXXigen. |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jeremy Drake April 2, 2025, 10:53 p.m. UTC 
  This is filled in with the rva of _load_config_used if defined (much
like _tls_used), and the size is the first 32-bit value at that symbol.

Signed-off-by: Jeremy Drake <sourceware-bugzilla@jdrake.com>
---
 bfd/peXXigen.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

Comments

Jan Beulich April 3, 2025, 8:50 a.m. UTC | #1 
On 03.04.2025 00:53, Jeremy Drake wrote:
> @@ -4573,6 +4575,71 @@ _bfd_XXi_final_link_postscript (bfd * abfd, struct coff_final_link_info *pfinfo)
>  #endif
>      }
> 
> +  name[0] = bfd_get_symbol_leading_char (abfd);
> +  strcpy (name + !!name[0], "_load_config_used");
> +  h1 = coff_link_hash_lookup (coff_hash_table (info), name, false, false, true);
> +  if (h1 != NULL)
> +    {
> +      char data[4];
> +      if ((h1->root.type == bfd_link_hash_defined
> +	   || h1->root.type == bfd_link_hash_defweak)
> +	  && h1->root.u.def.section != NULL
> +	  && h1->root.u.def.section->output_section != NULL)
> +	{
> +	  pe_data (abfd)->pe_opthdr.DataDirectory[PE_LOAD_CONFIG_TABLE].VirtualAddress =
> +	    (h1->root.u.def.value
> +	     + h1->root.u.def.section->output_section->vma
> +	     + h1->root.u.def.section->output_offset
> +	     - pe_data (abfd)->pe_opthdr.ImageBase);
> +
> +	  if (pe_data (abfd)->pe_opthdr.DataDirectory[PE_LOAD_CONFIG_TABLE].VirtualAddress &
> +	      ((bfd_arch_bits_per_address (abfd) >> 3) - 1))

Instead of shifting by 3, wouldn't you better divide by bfd_arch_bits_per_byte()?
Okay with this adjustment and the message text adjustments requested in reply to
patch 1.

Jan
Jeremy Drake April 3, 2025, 6:09 p.m. UTC | #2 
On Thu, 3 Apr 2025, Jan Beulich wrote:

> Instead of shifting by 3, wouldn't you better divide by bfd_arch_bits_per_byte()?
> Okay with this adjustment and the message text adjustments requested in reply to
> patch 1.

I grepped through the binutils-gdb checkout and it appears this will be
the only caller of bfd_arch_bits_per_byte.
diff mbox series

Patch

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index 29831d4caaa..0dec04fcc3e 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -593,7 +593,7 @@  _bfd_XXi_swap_aouthdr_out (bfd * abfd, void * in, void * out)
   struct internal_extra_pe_aouthdr *extra = &pe->pe_opthdr;
   PEAOUTHDR *aouthdr_out = (PEAOUTHDR *) out;
   bfd_vma sa, fa, ib;
-  IMAGE_DATA_DIRECTORY idata2, idata5, tls;
+  IMAGE_DATA_DIRECTORY idata2, idata5, tls, loadcfg;

   sa = extra->SectionAlignment;
   fa = extra->FileAlignment;
@@ -602,6 +602,7 @@  _bfd_XXi_swap_aouthdr_out (bfd * abfd, void * in, void * out)
   idata2 = pe->pe_opthdr.DataDirectory[PE_IMPORT_TABLE];
   idata5 = pe->pe_opthdr.DataDirectory[PE_IMPORT_ADDRESS_TABLE];
   tls = pe->pe_opthdr.DataDirectory[PE_TLS_TABLE];
+  loadcfg = pe->pe_opthdr.DataDirectory[PE_LOAD_CONFIG_TABLE];

   if (aouthdr_in->tsize)
     {
@@ -651,6 +652,7 @@  _bfd_XXi_swap_aouthdr_out (bfd * abfd, void * in, void * out)
   extra->DataDirectory[PE_IMPORT_TABLE]  = idata2;
   extra->DataDirectory[PE_IMPORT_ADDRESS_TABLE] = idata5;
   extra->DataDirectory[PE_TLS_TABLE] = tls;
+  extra->DataDirectory[PE_LOAD_CONFIG_TABLE] = loadcfg;

   if (extra->DataDirectory[PE_IMPORT_TABLE].VirtualAddress == 0)
     /* Until other .idata fixes are made (pending patch), the entry for
@@ -4573,6 +4575,71 @@  _bfd_XXi_final_link_postscript (bfd * abfd, struct coff_final_link_info *pfinfo)
 #endif
     }

+  name[0] = bfd_get_symbol_leading_char (abfd);
+  strcpy (name + !!name[0], "_load_config_used");
+  h1 = coff_link_hash_lookup (coff_hash_table (info), name, false, false, true);
+  if (h1 != NULL)
+    {
+      char data[4];
+      if ((h1->root.type == bfd_link_hash_defined
+	   || h1->root.type == bfd_link_hash_defweak)
+	  && h1->root.u.def.section != NULL
+	  && h1->root.u.def.section->output_section != NULL)
+	{
+	  pe_data (abfd)->pe_opthdr.DataDirectory[PE_LOAD_CONFIG_TABLE].VirtualAddress =
+	    (h1->root.u.def.value
+	     + h1->root.u.def.section->output_section->vma
+	     + h1->root.u.def.section->output_offset
+	     - pe_data (abfd)->pe_opthdr.ImageBase);
+
+	  if (pe_data (abfd)->pe_opthdr.DataDirectory[PE_LOAD_CONFIG_TABLE].VirtualAddress &
+	      ((bfd_arch_bits_per_address (abfd) >> 3) - 1))
+	    {
+	      _bfd_error_handler
+		(_("%pB: unable to fill in DataDictionary[%d]: %s not properly aligned"),
+		 abfd, PE_LOAD_CONFIG_TABLE, name);
+	      result = false;
+	    }
+
+	  /* The size is stored as the first 4 bytes at _load_config_used.
+	     The Microsoft PE format documentation says for compatibility with
+	     Windows XP and earlier, the size must be 64 for x86 images.  If
+	     anyone cares about those versions, the size should be overridden
+	     for i386.  */
+	  if (bfd_get_section_contents (abfd,
+		h1->root.u.def.section->output_section, data,
+		h1->root.u.def.section->output_offset + h1->root.u.def.value,
+		4))
+	    {
+	      pe_data (abfd)->pe_opthdr.DataDirectory[PE_LOAD_CONFIG_TABLE].Size =
+		bfd_get_32 (abfd, data);
+
+	      if (pe_data (abfd)->pe_opthdr.DataDirectory[PE_LOAD_CONFIG_TABLE].Size >
+		  h1->root.u.def.section->size - h1->root.u.def.value)
+		{
+		  _bfd_error_handler
+		    (_("%pB: unable to fill in DataDictionary[%d]: size too large for the containing section"),
+		     abfd, PE_LOAD_CONFIG_TABLE);
+		  result = false;
+		}
+	    }
+	  else
+	    {
+	      _bfd_error_handler
+		(_("%pB: unable to fill in DataDictionary[%d]: size can't be read"),
+		 abfd, PE_LOAD_CONFIG_TABLE);
+	      result = false;
+	    }
+	}
+      else
+	{
+	  _bfd_error_handler
+	    (_("%pB: unable to fill in DataDictionary[%d]: %s not defined correctly"),
+	     abfd, PE_LOAD_CONFIG_TABLE, name);
+	  result = false;
+	}
+    }
+
 /* If there is a .pdata section and we have linked pdata finally, we
      need to sort the entries ascending.  */
 #if !defined(COFF_WITH_pep) && (defined(COFF_WITH_pex64) || defined(COFF_WITH_peAArch64) || defined(COFF_WITH_peLoongArch64) || defined (COFF_WITH_peRiscV64))