Re: [PATCH] Orphan sections and NOLOAD output section

[Alan Modra <amodra@gmail.com>]

On Thu, Apr 21, 2022 at 05:31:58PM +0200, Eric Botcazou via Binutils wrote:
> Hi,
> 
> we recently got a report about a counter-intuitive behavior of the GNU linker
> for orphan sections matched by a NOLOAD output section, which turned out to be
> an exact duplicate of:
>   https://stackoverflow.com/questions/48764136/gcc-noload-directive-cause-wrong-memory-mapping
> 
> What happens is that the effect of the command:
> 
>   .foo (NOLOAD) : {}
> 
> is not equivalent to that of:
> 
>   .foo (NOLOAD) : { *(.foo) }
> 
> when there is more than 1 object file containing a .foo section: the former
> will output two .foo sections, the first with PROGBITS and the second with
> NOBITS, where the latter will output only one with NOBITS.

Yes, the first orphan .foo will be placed in the empty noload output
section but the second orphan .foo will find its SEC_LOAD and
SEC_ALLOC flags don't match the now non-empty output section.

> I think that the commands are essentially equivalent so the linker ought to yield
> the same outcome, namely the single output section with NOBITS.
> 
> Tested on x86-64/Linux, OK for the mainline?
> 
> 
> 2022-04-21  Eric Botcazou  <ebotcazou@adacore.com>
> 
> ld/
> 	* ldelf.c (ldelf_place_orphan): Match only by name for a NOLOAD
> 	output section present in the script.
> 	* testsuite/ld-elf/orphan-13.d: New file.
> 	* testsuite/ld-elf/orphan-13.ld: Likewise.
> 	* testsuite/ld-elf/orphan-13a.s: Likewise.
> 	* testsuite/ld-elf/orphan-13b.s: Likewise.
> 	* testsuite/ld-elf/orphan-13c.s: Likewise.
> 
> -- 
> Eric Botcazou

> diff --git a/ld/ldelf.c b/ld/ldelf.c
> index 4094640b3f7..35718a9113b 100644
> --- a/ld/ldelf.c
> +++ b/ld/ldelf.c
> @@ -2098,13 +2098,17 @@ ldelf_place_orphan (asection *s, const char *secname, int constraint)
>  	   lang_insert_orphan to create a new output section.  */
>  	constraint = SPECIAL;
>  
> -	/* Check to see if we already have an output section statement
> -	   with this name, and its bfd section has compatible flags.
> +	/* Check to see if we already have an output section statement with
> +	   this name, and it was either present in the script with a special
> +	   type (knowing that lang_insert_orphan only creates normal_section
> +	   output sections) or its BFD section has compatible flags.
> +
>  	   If the section already exists but does not have any flags
>  	   set, then it has been created by the linker, possibly as a
>  	   result of a --section-start command line switch.  */
>  	if (os->bfd_section != NULL
> -	    && (os->bfd_section->flags == 0
> +	    && (os->sectype == noload_section

I think this test should be os->sectype >= noload_section, so that
noalloc_section and the typed section variants behave the same as
noload_section.  OK with that change.

> +		|| os->bfd_section->flags == 0
>  		|| (((s->flags ^ os->bfd_section->flags)
>  		     & (SEC_LOAD | SEC_ALLOC)) == 0
>  		    && (!elfinput
> diff --git a/ld/testsuite/ld-elf/orphan-13.d b/ld/testsuite/ld-elf/orphan-13.d
> new file mode 100644
> index 00000000000..8f867dc8938
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/orphan-13.d
> @@ -0,0 +1,12 @@
> +#source: orphan-13a.s
> +#source: orphan-13b.s
> +#source: orphan-13c.s
> +#ld: -T orphan-13.ld
> +#readelf: -S --wide
> +#xfail: [uses_genelf]
> +#xfail: xstormy16-*-*
> +
> +#...
> +  \[[ 0-9]+\] \.foo +NOBITS +[0-9a-f]+ +[0-9a-f]+ +0+30 +0+ +A +0 +0 +[0-9]+
> +  \[[ 0-9]+\] [._][^f].*
> +#pass
> diff --git a/ld/testsuite/ld-elf/orphan-13.ld b/ld/testsuite/ld-elf/orphan-13.ld
> new file mode 100644
> index 00000000000..face613f8bd
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/orphan-13.ld
> @@ -0,0 +1,7 @@
> +SECTIONS
> +{
> +  . = SIZEOF_HEADERS;
> +  .text : { *(.text) }
> +  .data : { *(.data) }
> +  .foo (NOLOAD) : {}
> +}
> diff --git a/ld/testsuite/ld-elf/orphan-13a.s b/ld/testsuite/ld-elf/orphan-13a.s
> new file mode 100644
> index 00000000000..a8a6f364455
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/orphan-13a.s
> @@ -0,0 +1,14 @@
> + .globl main
> + .globl _main
> + .globl start
> + .globl _start
> + .globl __start
> + .text
> +main:
> +_main:
> +start:
> +_start:
> +__start:
> +
> + .section .foo,"a",%progbits
> + .long 1,1,1,1
> diff --git a/ld/testsuite/ld-elf/orphan-13b.s b/ld/testsuite/ld-elf/orphan-13b.s
> new file mode 100644
> index 00000000000..c475eb11d1b
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/orphan-13b.s
> @@ -0,0 +1,2 @@
> + .section .foo,"a",%progbits
> + .long 1,1,1,1
> diff --git a/ld/testsuite/ld-elf/orphan-13c.s b/ld/testsuite/ld-elf/orphan-13c.s
> new file mode 100644
> index 00000000000..c475eb11d1b
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/orphan-13c.s
> @@ -0,0 +1,2 @@
> + .section .foo,"a",%progbits
> + .long 1,1,1,1


-- 
Alan Modra
Australia Development Lab, IBM