Bad behavior with explicit phdrs and loadable orphans

[David Heine]

Nick Clifton wrote:
> Hi David,
> 
> 
>>Thanks for taking a look.  My initial thoughts on this were to find a
>>better memory region to put the orphan section like you've
>>implemented. However, the problem that caused the initial bad behavior
>>was that an assembly file had mis-spelled a section name.  While the
>>proposed patch keeps the system from generating a large file or
>>causing the SEEK overflow in this case, the problem would recur with
>>any orphan that can not be placed by this default placement algorithm
>>for some linker script.
>>   Because the patch manages to place the orphan section, it also
>>removes the warning that indicated the actual problem:
>>
>>	warning: no memory region specified for section
>>
>>For our target when using explicit memory regions, it is usually an
>>error to have orphan sections that are loadable and allocatable for
>>non-relocatable links.   I'm not sure if this is generally true for
>>other targets, but the existence of the original warning indicates
>>that it might be.
> 
> 
> Yes, I think that in general a loadable/allocatable section is intended
> to be used and hence ought to be explicitly handled by the linker
> script.
> 
> 
>>   It might be better to keep the old placement algorithm and convert
>>the warning: "no memory region specified for section" into a failure
>>that will avoid the massive file generation.
> 
> 
> Well I have not applied my suggested patch, since I was waiting for
> feedback from you, so there is no problem with keeping the old
> algorithm.
> 
> 
>>If you think the new placement algorithm makes more sense, it would
>>probably be less intrusive to other targets if it were modified to
>>first look for regions that match one of the positive attributes, then
>>look for ones with no attributes so that the behavior only changes for
>>sections that were previously unplaced.
> 
> 
> Basically I am in favour of the "if it ain't broke, don't fix it"
> approach.  So if we can leave the current placement algorithm in
> place, and just update the warning to an error, then I am in favour of
> doing that.  Please could you try out this alternative patch and let
> me know what you think.

This alternate patch works well.

-David

> Cheers
>         Nick
> 
> 2003-05-02  Nick Clifton  <nickc@redhat.com>
> 
> 	* ldlang.c (lang_size_sections_1): Default to generating a
> 	fatal error message if a loadable section is not allocated
> 	to a memory region when regions are defined.
> 
> Index: ld/ldlang.c
> ===================================================================
> RCS file: /cvs/src/src/ld/ldlang.c,v
> retrieving revision 1.112
> diff -c -3 -p -w -r1.112 ldlang.c
> *** ld/ldlang.c	29 Apr 2003 01:53:46 -0000	1.112
> --- ld/ldlang.c	2 May 2003 09:22:46 -0000
> *************** lang_size_sections_1 (s, output_section_
> *** 3050,3056 ****
>   
>   		    /* If a loadable section is using the default memory
>   		       region, and some non default memory regions were
> ! 		       defined, issue a warning.  */
>   		    if ((bfd_get_section_flags (output_bfd, os->bfd_section)
>   			 & (SEC_ALLOC | SEC_LOAD)) != 0
>   			&& (bfd_get_section_flags (output_bfd, os->bfd_section)
> --- 3050,3056 ----
>   
>   		    /* If a loadable section is using the default memory
>   		       region, and some non default memory regions were
> ! 		       defined, issue an error message.  */
>   		    if ((bfd_get_section_flags (output_bfd, os->bfd_section)
>   			 & (SEC_ALLOC | SEC_LOAD)) != 0
>   			&& (bfd_get_section_flags (output_bfd, os->bfd_section)
> *************** lang_size_sections_1 (s, output_section_
> *** 3062,3070 ****
>   			&& (strcmp (lang_memory_region_list->name,
>   				    "*default*") != 0
>   			    || lang_memory_region_list->next != NULL))
> ! 		      einfo (_("%P: warning: no memory region specified for section `%s'\n"),
>   			     bfd_get_section_name (output_bfd,
>   						   os->bfd_section));
>   
>   		    dot = os->region->current;
>   
> --- 3062,3086 ----
>   			&& (strcmp (lang_memory_region_list->name,
>   				    "*default*") != 0
>   			    || lang_memory_region_list->next != NULL))
> ! 		      {
> ! 			/* By default this is an error rather than just a
> ! 			   warning because if we allocate the section to the
> ! 			   default memory region we can end up creating an
> ! 			   excessivly large binary, or even seg faulting when
> ! 			   attmepting to perform a negative seek.  See
> ! 			     http://sources.redhat.com/ml/binutils/2003-04/msg00423.html			 
> ! 			   for an example of this.  This behaviour can be
> ! 			   overridden by the using the --no-check-sections
> ! 			   switch.  */
> ! 			if (command_line.check_section_addresses)
> ! 			  einfo (_("%P%F: error: no memory region specified for loadable section `%s'\n"),
>   				 bfd_get_section_name (output_bfd,
>   						       os->bfd_section));
> + 			else
> + 			  einfo (_("%P: warning: no memory region specified for loadable section `%s'\n"),
> + 				 bfd_get_section_name (output_bfd,
> + 						       os->bfd_section));
> + 		      }
>   
>   		    dot = os->region->current;
>   
>