Re: V3 [PATCH 1/2] Switch to a new section if the SECTION_RETAIN bit doesn't match

[Jeff Law <law@redhat.com>]

On 12/8/20 5:51 AM, H.J. Lu wrote:
> When definitions marked with used attribute and unmarked definitions are
> placed in the section with the same name, switch to a new section if the
> SECTION_RETAIN bit doesn't match.
>
> gcc/
>
> 	PR target/98146
> 	* output.h (switch_to_section): Add a tree argument, default to
> 	nullptr.
> 	* varasm.c (get_section): If the SECTION_RETAIN bit doesn't match,
> 	return and switch to a new section later.
> 	(assemble_start_function): Pass decl to switch_to_section.
> 	(assemble_variable): Likewise.
> 	(switch_to_section): If the SECTION_RETAIN bit doesn't match,
> 	switch to a new section.
>
> gcc/testsuite/
>
> 	PR target/98146
> 	* c-c++-common/attr-used-5.c: New test.
> 	* c-c++-common/attr-used-6.c: Likewise.
> 	* c-c++-common/attr-used-7.c: Likewise.
> 	* c-c++-common/attr-used-8.c: Likewise.
> 	* c-c++-common/attr-used-9.c: Likewise.
> ---
>  gcc/output.h                             |  2 +-
>  gcc/testsuite/c-c++-common/attr-used-5.c | 26 ++++++++++++++++++++
>  gcc/testsuite/c-c++-common/attr-used-6.c | 26 ++++++++++++++++++++
>  gcc/testsuite/c-c++-common/attr-used-7.c |  8 +++++++
>  gcc/testsuite/c-c++-common/attr-used-8.c |  8 +++++++
>  gcc/testsuite/c-c++-common/attr-used-9.c | 28 ++++++++++++++++++++++
>  gcc/varasm.c                             | 30 ++++++++++++++++++++----
>  7 files changed, 123 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/attr-used-5.c
>  create mode 100644 gcc/testsuite/c-c++-common/attr-used-6.c
>  create mode 100644 gcc/testsuite/c-c++-common/attr-used-7.c
>  create mode 100644 gcc/testsuite/c-c++-common/attr-used-8.c
>  create mode 100644 gcc/testsuite/c-c++-common/attr-used-9.c
>
>
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index 0fac3688828..c5ea29c4e4c 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -342,6 +342,11 @@ get_section (const char *name, unsigned int flags, tree decl,
>  	      sect->common.flags |= (SECTION_WRITE | SECTION_RELRO);
>  	      return sect;
>  	    }
> +	  /* If the SECTION_RETAIN bit doesn't match, return and switch
> +	     to a new section later.  */
> +	  if ((sect->common.flags & SECTION_RETAIN)
> +	      != (flags & SECTION_RETAIN))
> +	    return sect;
At the least you need to fix the function comment as it would no longer
match the behavior in this case.  It also seems to me that having
SECTION_RETAIN be a special case like this just seems wrong.   In my
mind there isn't anything fundamentally different with SECTION_RETAIN vs
other symbols that it deserves to be special cased like this.

Clearly we need to fix something as these bits are causing significant
fallout, but I'm just not sure what the right fix ought to be.

jeff