[v1,3/5] ld: fix segfault on discarded input sections not fitting in memory regions

Message ID 20250918150606.1630916-4-matthieu.longo@arm.com
State New
Headers
Series ld: fix segfaults when non-contiguous memory support is enabled |

Commit Message

Matthieu Longo Sept. 18, 2025, 3:06 p.m. UTC
  In the case of non-contiguous memory regions, if an input section did not
fit in any of the designated memory regions, the linker marked it as
discarded, and warn_non_contiguous_discards() would only issue warning on
it, relying on later unresolved symbol errors to terminate the process
before a crash occur. This approach was insufficient, and crashes did occur
on AArch64.

This patch renames warn_non_contiguous_discards () to a name that does not
contain "discard" as it created some confusion with the /DISCARD/ output
section. It also promotes the warnings to errors, and ensures that the
link process terminates cleanly if any input section is not allocated to
an output section.
It also updates an AArch32 test expectations to match the corrected
behavior. Tests for the crash cases are added in a subsequent patch.
---
 ld/ldlang.c                                 | 29 ++++++++++++++-------
 ld/testsuite/ld-arm/non-contiguous-arm7.err |  9 +++----
 2 files changed, 23 insertions(+), 15 deletions(-)
  

Comments

Jan Beulich Sept. 26, 2025, 1:18 p.m. UTC | #1
On 18.09.2025 17:06, Matthieu Longo wrote:
> In the case of non-contiguous memory regions, if an input section did not
> fit in any of the designated memory regions, the linker marked it as
> discarded, and warn_non_contiguous_discards() would only issue warning on
> it, relying on later unresolved symbol errors to terminate the process
> before a crash occur. This approach was insufficient, and crashes did occur
> on AArch64.
> 
> This patch renames warn_non_contiguous_discards () to a name that does not
> contain "discard" as it created some confusion with the /DISCARD/ output
> section. It also promotes the warnings to errors, and ensures that the
> link process terminates cleanly if any input section is not allocated to
> an output section.
> It also updates an AArch32 test expectations to match the corrected
> behavior. Tests for the crash cases are added in a subsequent patch.

Looks okay to me, just one style nit:

> @@ -8242,10 +8247,15 @@ warn_non_contiguous_discards (void)
>  
>        for (asection *s = file->the_bfd->sections; s != NULL; s = s->next)
>  	if (s->output_section == NULL && !s->veneer)
> -	  einfo (_("%P: warning: --enable-non-contiguous-regions "
> -		   "discards section `%pA' from `%pB'\n"),
> -		 s, file->the_bfd);
> +	  {
> +	    einfo (_("%P: error: --enable-non-contiguous-regions was not able "
> +		     "to allocate the input section `%pA' (%pB) to an output "
> +		     "section\n"), s, file->the_bfd);

Please retain original line splitting (no new arguments on the same line as
an already split argument).

Jan
  
Matthieu Longo Oct. 13, 2025, 3:24 p.m. UTC | #2
On 2025-09-26 14:18, Jan Beulich wrote:
> On 18.09.2025 17:06, Matthieu Longo wrote:
>> In the case of non-contiguous memory regions, if an input section did not
>> fit in any of the designated memory regions, the linker marked it as
>> discarded, and warn_non_contiguous_discards() would only issue warning on
>> it, relying on later unresolved symbol errors to terminate the process
>> before a crash occur. This approach was insufficient, and crashes did occur
>> on AArch64.
>>
>> This patch renames warn_non_contiguous_discards () to a name that does not
>> contain "discard" as it created some confusion with the /DISCARD/ output
>> section. It also promotes the warnings to errors, and ensures that the
>> link process terminates cleanly if any input section is not allocated to
>> an output section.
>> It also updates an AArch32 test expectations to match the corrected
>> behavior. Tests for the crash cases are added in a subsequent patch.
> 
> Looks okay to me, just one style nit:
> 
>> @@ -8242,10 +8247,15 @@ warn_non_contiguous_discards (void)
>>   
>>         for (asection *s = file->the_bfd->sections; s != NULL; s = s->next)
>>   	if (s->output_section == NULL && !s->veneer)
>> -	  einfo (_("%P: warning: --enable-non-contiguous-regions "
>> -		   "discards section `%pA' from `%pB'\n"),
>> -		 s, file->the_bfd);
>> +	  {
>> +	    einfo (_("%P: error: --enable-non-contiguous-regions was not able "
>> +		     "to allocate the input section `%pA' (%pB) to an output "
>> +		     "section\n"), s, file->the_bfd);
> 
> Please retain original line splitting (no new arguments on the same line as
> an already split argument).
> 
> Jan

Fixed in the next revision.

Matthieu
  

Patch

diff --git a/ld/ldlang.c b/ld/ldlang.c
index bdda599a7a4..454887d71f4 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -5576,7 +5576,7 @@  size_input_section
   lang_input_section_type *is = &((*this_ptr)->input_section);
   asection *i = is->section;
   asection *o = output_section_statement->bfd_section;
-  *removed = 0;
+  *removed = false;
 
   if (link_info.non_contiguous_regions)
     {
@@ -5587,7 +5587,7 @@  size_input_section
 	 have reinitialized its size.  */
       if (i->already_assigned && i->already_assigned != o)
 	{
-	  *removed = 1;
+	  *removed = true;
 	  return dot;
 	}
     }
@@ -5650,7 +5650,7 @@  size_input_section
 			     "would overflow `%pA' after it changed size).\n"),
 			   i, i->output_section);
 
-		  *removed = 1;
+		  *removed = true;
 		  dot = end;
 		  i->output_section = NULL;
 		  return dot;
@@ -8231,9 +8231,14 @@  lang_propagate_lma_regions (void)
     }
 }
 
+/* Checks whether any input section was not allocated to an output section.
+   If such a case is found, emits an error for the corresponding input section
+   and stops the link process.  */
+
 static void
-warn_non_contiguous_discards (void)
+error_non_contiguous_unallocated_sections (void)
 {
+  bool removed_section = false;
   LANG_FOR_EACH_INPUT_STATEMENT (file)
     {
       if ((file->the_bfd->flags & (BFD_LINKER_CREATED | DYNAMIC)) != 0
@@ -8242,10 +8247,15 @@  warn_non_contiguous_discards (void)
 
       for (asection *s = file->the_bfd->sections; s != NULL; s = s->next)
 	if (s->output_section == NULL && !s->veneer)
-	  einfo (_("%P: warning: --enable-non-contiguous-regions "
-		   "discards section `%pA' from `%pB'\n"),
-		 s, file->the_bfd);
+	  {
+	    einfo (_("%P: error: --enable-non-contiguous-regions was not able "
+		     "to allocate the input section `%pA' (%pB) to an output "
+		     "section\n"), s, file->the_bfd);
+	    removed_section = true;
+	  }
     }
+  if (removed_section)
+    fatal (_("%P: final link failed\n"));
 }
 
 static void
@@ -8663,9 +8673,8 @@  lang_process (void)
   if (command_line.check_section_addresses)
     lang_check_section_addresses ();
 
-  if (link_info.non_contiguous_regions
-      && link_info.non_contiguous_regions_warnings)
-    warn_non_contiguous_discards ();
+  if (link_info.non_contiguous_regions)
+    error_non_contiguous_unallocated_sections ();
 
   /* Check any required symbols are known.  */
   ldlang_check_require_defined_symbols ();
diff --git a/ld/testsuite/ld-arm/non-contiguous-arm7.err b/ld/testsuite/ld-arm/non-contiguous-arm7.err
index 7b3a3d8c3c9..21f71898eb1 100644
--- a/ld/testsuite/ld-arm/non-contiguous-arm7.err
+++ b/ld/testsuite/ld-arm/non-contiguous-arm7.err
@@ -1,5 +1,4 @@ 
-.* may change behaviour for section .?\.bss.? from .*
-.* may change behaviour for section .?\.bss\.MY_BUF.? from .*
-.* discards section .?\.bss\.MY_BUF.? from .*
-.* unresolvable R_ARM_ABS32 relocation against symbol .?MY_BUF.?
-.* final link failed
+.*: warning: --enable-non-contiguous-regions may change behaviour for section `\.bss' from `.*non-contiguous-arm7\.o' \(assigned to \.bss, but additional match: \.bss_ram2\)
+.*: warning: --enable-non-contiguous-regions may change behaviour for section `\.bss\.MY_BUF' from `.*non-contiguous-arm7\.o' \(assigned to \.bss, but additional match: \.bss_ram2\)
+.*: error: --enable-non-contiguous-regions was not able to allocate the input section `\.bss\.MY_BUF' \(.*non-contiguous-arm7\.o\) to an output section
+.*: final link failed