diff mbox series

[3/3] bfd: populate delay import directory in PE header.

Message ID 346348f7-5f9e-de25-6ada-a8c9e59df702@jdrake.com
State New
Headers
Series PE delay import fixes |

Checks

Context Check Description
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_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jeremy Drake April 15, 2025, 5:09 a.m. UTC 
  Date: Sat, 8 Mar 2025 15:42:07 -0800

Previously, the delay import table was constructed but its rva and size
were never put into the PE optional header.

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

Comments

Jan Beulich May 2, 2025, 1:21 p.m. UTC | #1 
On 15.04.2025 07:09, Jeremy Drake wrote:
> Date: Sat, 8 Mar 2025 15:42:07 -0800
> 
> Previously, the delay import table was constructed but its rva and size
> were never put into the PE optional header.
> 
> Signed-off-by: Jeremy Drake <sourceware-bugzilla@jdrake.com>

Looks okay to me, just one nit:

> @@ -4544,6 +4546,54 @@ _bfd_XXi_final_link_postscript (bfd * abfd, struct coff_final_link_info *pfinfo)
>  	}
>      }
> 
> +  /* The delay import directory.  This is .didat$2 */
> +  h1 = coff_link_hash_lookup (coff_hash_table (info),
> +			      "__DELAY_IMPORT_DIRECTORY_start__", false, false,
> +			      true);
> +  if (h1 != NULL)
> +    {
> +      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)

Can the two if()s please be combined, to ...

> +	{
> +	  bfd_vma delay_va;
> +
> +	  delay_va =
> +	    (h1->root.u.def.value
> +	     + h1->root.u.def.section->output_section->vma
> +	     + h1->root.u.def.section->output_offset);
> +
> +	  h1 = coff_link_hash_lookup (coff_hash_table (info),
> +				      "__DELAY_IMPORT_DIRECTORY_end__", false,
> +				      false, true);
> +	  if (h1 != NULL
> +	      && (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_DELAY_IMPORT_DESCRIPTOR].Size =
> +		((h1->root.u.def.value
> +		  + h1->root.u.def.section->output_section->vma
> +		  + h1->root.u.def.section->output_offset)
> +		 - delay_va);
> +	      if (pe_data (abfd)->pe_opthdr.DataDirectory[PE_DELAY_IMPORT_DESCRIPTOR].Size
> +		  != 0)
> +		pe_data (abfd)->pe_opthdr.DataDirectory[PE_DELAY_IMPORT_DESCRIPTOR].VirtualAddress =
> +		  delay_va - pe_data (abfd)->pe_opthdr.ImageBase;
> +	    }
> +	  else
> +	    {
> +	      _bfd_error_handler
> +		(_("%pB: unable to fill in DataDirectory[%d]: %s not defined correctly"),
> +		 abfd, PE_DELAY_IMPORT_DESCRIPTOR,
> +		 "__DELAY_IMPORT_DIRECTORY_end__");
> +	      result = false;
> +	    }
> +	}
> +    }

... reduce indentation depth by one level for the entire body?

Jan
Jeremy Drake May 2, 2025, 5:04 p.m. UTC | #2 
On Fri, 2 May 2025, Jan Beulich wrote:

> On 15.04.2025 07:09, Jeremy Drake wrote:
> > Date: Sat, 8 Mar 2025 15:42:07 -0800
> >
> > Previously, the delay import table was constructed but its rva and size
> > were never put into the PE optional header.
> >
> > Signed-off-by: Jeremy Drake <sourceware-bugzilla@jdrake.com>
>
> Looks okay to me, just one nit:
>
> > @@ -4544,6 +4546,54 @@ _bfd_XXi_final_link_postscript (bfd * abfd, struct coff_final_link_info *pfinfo)
> >  	}
> >      }
> >
> > +  /* The delay import directory.  This is .didat$2 */
> > +  h1 = coff_link_hash_lookup (coff_hash_table (info),
> > +			      "__DELAY_IMPORT_DIRECTORY_start__", false, false,
> > +			      true);
> > +  if (h1 != NULL)
> > +    {
> > +      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)
>
> Can the two if()s please be combined, to ...
>
> > +	{
> > +	  bfd_vma delay_va;
> > +
> > +	  delay_va =
> > +	    (h1->root.u.def.value
> > +	     + h1->root.u.def.section->output_section->vma
> > +	     + h1->root.u.def.section->output_offset);
> > +
> > +	  h1 = coff_link_hash_lookup (coff_hash_table (info),
> > +				      "__DELAY_IMPORT_DIRECTORY_end__", false,
> > +				      false, true);
> > +	  if (h1 != NULL
> > +	      && (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_DELAY_IMPORT_DESCRIPTOR].Size =
> > +		((h1->root.u.def.value
> > +		  + h1->root.u.def.section->output_section->vma
> > +		  + h1->root.u.def.section->output_offset)
> > +		 - delay_va);
> > +	      if (pe_data (abfd)->pe_opthdr.DataDirectory[PE_DELAY_IMPORT_DESCRIPTOR].Size
> > +		  != 0)
> > +		pe_data (abfd)->pe_opthdr.DataDirectory[PE_DELAY_IMPORT_DESCRIPTOR].VirtualAddress =
> > +		  delay_va - pe_data (abfd)->pe_opthdr.ImageBase;
> > +	    }
> > +	  else
> > +	    {
> > +	      _bfd_error_handler
> > +		(_("%pB: unable to fill in DataDirectory[%d]: %s not defined correctly"),
> > +		 abfd, PE_DELAY_IMPORT_DESCRIPTOR,
> > +		 "__DELAY_IMPORT_DIRECTORY_end__");
> > +	      result = false;
> > +	    }
> > +	}
> > +    }
>
> .... reduce indentation depth by one level for the entire body?

OK.
diff mbox series

Patch

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index 7dfa82ee43e..6fe0c0186e7 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, loadcfg;
+  IMAGE_DATA_DIRECTORY idata2, idata5, didat2, tls, loadcfg;

   sa = extra->SectionAlignment;
   fa = extra->FileAlignment;
@@ -601,6 +601,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];
+  didat2 = pe->pe_opthdr.DataDirectory[PE_DELAY_IMPORT_DESCRIPTOR];
   tls = pe->pe_opthdr.DataDirectory[PE_TLS_TABLE];
   loadcfg = pe->pe_opthdr.DataDirectory[PE_LOAD_CONFIG_TABLE];

@@ -651,6 +652,7 @@  _bfd_XXi_swap_aouthdr_out (bfd * abfd, void * in, void * out)
      a final link is going to be performed, it can overwrite them.  */
   extra->DataDirectory[PE_IMPORT_TABLE]  = idata2;
   extra->DataDirectory[PE_IMPORT_ADDRESS_TABLE] = idata5;
+  extra->DataDirectory[PE_DELAY_IMPORT_DESCRIPTOR] = didat2;
   extra->DataDirectory[PE_TLS_TABLE] = tls;
   extra->DataDirectory[PE_LOAD_CONFIG_TABLE] = loadcfg;

@@ -4544,6 +4546,54 @@  _bfd_XXi_final_link_postscript (bfd * abfd, struct coff_final_link_info *pfinfo)
 	}
     }

+  /* The delay import directory.  This is .didat$2 */
+  h1 = coff_link_hash_lookup (coff_hash_table (info),
+			      "__DELAY_IMPORT_DIRECTORY_start__", false, false,
+			      true);
+  if (h1 != NULL)
+    {
+      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)
+	{
+	  bfd_vma delay_va;
+
+	  delay_va =
+	    (h1->root.u.def.value
+	     + h1->root.u.def.section->output_section->vma
+	     + h1->root.u.def.section->output_offset);
+
+	  h1 = coff_link_hash_lookup (coff_hash_table (info),
+				      "__DELAY_IMPORT_DIRECTORY_end__", false,
+				      false, true);
+	  if (h1 != NULL
+	      && (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_DELAY_IMPORT_DESCRIPTOR].Size =
+		((h1->root.u.def.value
+		  + h1->root.u.def.section->output_section->vma
+		  + h1->root.u.def.section->output_offset)
+		 - delay_va);
+	      if (pe_data (abfd)->pe_opthdr.DataDirectory[PE_DELAY_IMPORT_DESCRIPTOR].Size
+		  != 0)
+		pe_data (abfd)->pe_opthdr.DataDirectory[PE_DELAY_IMPORT_DESCRIPTOR].VirtualAddress =
+		  delay_va - pe_data (abfd)->pe_opthdr.ImageBase;
+	    }
+	  else
+	    {
+	      _bfd_error_handler
+		(_("%pB: unable to fill in DataDirectory[%d]: %s not defined correctly"),
+		 abfd, PE_DELAY_IMPORT_DESCRIPTOR,
+		 "__DELAY_IMPORT_DIRECTORY_end__");
+	      result = false;
+	    }
+	}
+    }
+
   name[0] = bfd_get_symbol_leading_char (abfd);
   strcpy (name + !!name[0], "_tls_used");
   h1 = coff_link_hash_lookup (coff_hash_table (info), name, false, false, true);