public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: Binutils <binutils@sourceware.org>
Subject: V2 [PATCH] gas: Generate a new section for SHF_GNU_RETAIN
Date: Fri, 4 Dec 2020 08:43:13 -0800	[thread overview]
Message-ID: <CAMe9rOqh0O0wdeirryZ27eBQGwGN2E+dB3cxHQ4O7WY6RF+EzA@mail.gmail.com> (raw)
In-Reply-To: <20201204152907.mhls5ilpv47bj5fq@jozef-acer-manjaro>

[-- Attachment #1: Type: text/plain, Size: 3903 bytes --]

On Fri, Dec 4, 2020 at 7:29 AM Jozef Lawrynowicz
<jozef.l@mittosystems.com> wrote:
>
> On Fri, Dec 04, 2020 at 05:52:54AM -0800, H.J. Lu wrote:
> > For
> >       .globl  foo2
> >       .section        .data.foo,"aR"
> >       .align 4
> >       .type   foo2, @object
> >       .size   foo2, 4
> > foo2:
> >       .long   2
> >       .globl  foo1
> >       .section        .data.foo
> >       .align 4
> >       .type   foo1, @object
> >       .size   foo1, 4
> > foo1:
> >       .long   1
> >
> > generate a new section if the SHF_GNU_RETAIN bit doesn't match.
> >
> >       * config/obj-elf.c (SEC_ASSEMBLER_SHF_MASK): New.
> >       (get_section_by_match): Also check if SEC_ASSEMBLER_SHF_MASK of
> >       sh_flags matches.  Rename info to sh_info.
> >       (obj_elf_change_section): Rename info to sh_info.
> >       (obj_elf_section): Rename info to sh_info.  Set sh_flags for
> >       SHF_GNU_RETAIN.
> >       * config/obj-elf.h (elf_section_match): Rename info to sh_info.
> >       Add sh_flags.
> >       * testsuite/gas/elf/elf.exp: Run section27.
> >       * testsuite/gas/elf/section24b.d: Updated.
> >       * testsuite/gas/elf/section27.d: New file.
> >       * testsuite/gas/elf/section27.s: Likewise.
> > ---
> >  gas/config/obj-elf.c               | 21 ++++++++++++------
> >  gas/config/obj-elf.h               |  3 ++-
> >  gas/testsuite/gas/elf/elf.exp      |  1 +
> >  gas/testsuite/gas/elf/section24b.d | 10 ++++++---
> >  gas/testsuite/gas/elf/section27.d  | 14 ++++++++++++
> >  gas/testsuite/gas/elf/section27.s  | 34 ++++++++++++++++++++++++++++++
> >  6 files changed, 73 insertions(+), 10 deletions(-)
> >  create mode 100644 gas/testsuite/gas/elf/section27.d
> >  create mode 100644 gas/testsuite/gas/elf/section27.s
>
> I just want to check that we are OK with the fact that a .section
> directive without any flags will always use the default flags, even if
> the section previously had the 'R' flag set. I suppose this is just
> standard behavior and how the .section directive has always behaved.

Correct.

> $ cat asm-tester.s
> .section .data.foo,"awR"
> .word 0
> .section .data.foo
> .word 0
> $ as asm-tester.s -o tester.o
> $ readelf --wide -S tester.o
> ...
>   [ 4] .data.foo         PROGBITS        0000000000000000 000040 000002 00 WAR  0   0  1
>   [ 5] .data.foo         PROGBITS        0000000000000000 000042 000002 00  WA  0   0  1
> ...
>
> Also, I think the following change should be added, since the OR'ing of
> SHF_GNU_RETAIN flag between sections, from the original patch, is now
> redundant.
>
> diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
> index 54d42d9ecb..9ac53e4d0e 100644
> --- a/gas/config/obj-elf.c
> +++ b/gas/config/obj-elf.c
> @@ -806,17 +806,9 @@ obj_elf_change_section (const char *name,
>                 as_bad (_("changed section attributes for %s"), name);
>             }
>           else
> -           {
> -             /* Don't overwrite a previously set SHF_GNU_RETAIN flag for the
> -                section.  The entire section must be marked retained.  */
> -             if ((elf_tdata (stdoutput)->has_gnu_osabi & elf_gnu_osabi_retain)
> -                 && ((elf_section_flags (old_sec) & SHF_GNU_RETAIN)))
> -               attr |= SHF_GNU_RETAIN;
> -
> -             /* FIXME: Maybe we should consider removing a previously set
> -                processor or application specific attribute as suspicious ?  */
> -             elf_section_flags (sec) = attr;
> -           }
> +           /* FIXME: Maybe we should consider removing a previously set
> +              processor or application specific attribute as suspicious ?  */
> +           elf_section_flags (sec) = attr;
>
>           if ((flags & SEC_MERGE) && old_sec->entsize != (unsigned) entsize)
>             as_bad (_("changed section entity size for %s"), name);
>

Fixed.

Here is the updated patch.  OK for master?

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-gas-Generate-a-new-section-for-SHF_GNU_RETAIN.patch --]
[-- Type: text/x-patch, Size: 7927 bytes --]

From ac8a6311162292ede4cd2da96ad62f74c0af5b7f Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 24 Sep 2020 05:45:50 -0700
Subject: [PATCH] gas: Generate a new section for SHF_GNU_RETAIN

For
	.globl	foo2
	.section	.data.foo,"aR"
	.align 4
	.type	foo2, @object
	.size	foo2, 4
foo2:
	.long	2
	.globl	foo1
	.section	.data.foo
	.align 4
	.type	foo1, @object
	.size	foo1, 4
foo1:
	.long	1

generate a new section if the SHF_GNU_RETAIN bit doesn't match.

	* config/obj-elf.c (SEC_ASSEMBLER_SHF_MASK): New.
	(get_section_by_match): Also check if SEC_ASSEMBLER_SHF_MASK of
	sh_flags matches.  Rename info to sh_info.
	(obj_elf_change_section): Don't check previous SHF_GNU_RETAIN.
	Rename info to sh_info.
	(obj_elf_section): Rename info to sh_info.  Set sh_flags for
	SHF_GNU_RETAIN.
	* config/obj-elf.h (elf_section_match): Rename info to sh_info.
	Add sh_flags.
	* testsuite/gas/elf/elf.exp: Run section27.
	* testsuite/gas/elf/section24b.d: Updated.
	* testsuite/gas/elf/section27.d: New file.
	* testsuite/gas/elf/section27.s: Likewise.
---
 gas/config/obj-elf.c               | 35 +++++++++++++++---------------
 gas/config/obj-elf.h               |  3 ++-
 gas/testsuite/gas/elf/elf.exp      |  1 +
 gas/testsuite/gas/elf/section24b.d | 10 ++++++---
 gas/testsuite/gas/elf/section27.d  | 14 ++++++++++++
 gas/testsuite/gas/elf/section27.s  | 34 +++++++++++++++++++++++++++++
 6 files changed, 76 insertions(+), 21 deletions(-)
 create mode 100644 gas/testsuite/gas/elf/section27.d
 create mode 100644 gas/testsuite/gas/elf/section27.s

diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index 54d42d9ecb..5d3b1a0edc 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -519,6 +519,9 @@ struct section_stack
 
 static struct section_stack *section_stack;
 
+/* ELF section flags for unique sections.  */
+#define SEC_ASSEMBLER_SHF_MASK SHF_GNU_RETAIN
+
 /* Return TRUE iff SEC matches the section info INF.  */
 
 static bfd_boolean
@@ -529,9 +532,12 @@ get_section_by_match (bfd *abfd ATTRIBUTE_UNUSED, asection *sec, void *inf)
   const char *group_name = elf_group_name (sec);
   const char *linked_to_symbol_name
     = sec->map_head.linked_to_symbol_name;
-  unsigned int info = elf_section_data (sec)->this_hdr.sh_info;
+  unsigned int sh_info = elf_section_data (sec)->this_hdr.sh_info;
+  bfd_vma sh_flags = (elf_section_data (sec)->this_hdr.sh_flags
+		      & SEC_ASSEMBLER_SHF_MASK);
 
-  return (info == match->info
+  return (sh_info == match->sh_info
+	  && sh_flags == match->sh_flags
 	  && ((bfd_section_flags (sec) & SEC_ASSEMBLER_SECTION_ID)
 	       == (match->flags & SEC_ASSEMBLER_SECTION_ID))
 	  && sec->section_id == match->section_id
@@ -740,7 +746,7 @@ obj_elf_change_section (const char *name,
 	type = bfd_elf_get_default_section_type (flags);
       elf_section_type (sec) = type;
       elf_section_flags (sec) = attr;
-      elf_section_data (sec)->this_hdr.sh_info = match_p->info;
+      elf_section_data (sec)->this_hdr.sh_info = match_p->sh_info;
 
       /* Prevent SEC_HAS_CONTENTS from being inadvertently set.  */
       if (type == SHT_NOBITS)
@@ -806,17 +812,9 @@ obj_elf_change_section (const char *name,
 		as_bad (_("changed section attributes for %s"), name);
 	    }
 	  else
-	    {
-	      /* Don't overwrite a previously set SHF_GNU_RETAIN flag for the
-		 section.  The entire section must be marked retained.  */
-	      if ((elf_tdata (stdoutput)->has_gnu_osabi & elf_gnu_osabi_retain)
-		  && ((elf_section_flags (old_sec) & SHF_GNU_RETAIN)))
-		attr |= SHF_GNU_RETAIN;
-
-	      /* FIXME: Maybe we should consider removing a previously set
-		 processor or application specific attribute as suspicious ?  */
-	      elf_section_flags (sec) = attr;
-	    }
+	    /* FIXME: Maybe we should consider removing a previously set
+	       processor or application specific attribute as suspicious?  */
+	    elf_section_flags (sec) = attr;
 
 	  if ((flags & SEC_MERGE) && old_sec->entsize != (unsigned) entsize)
 	    as_bad (_("changed section entity size for %s"), name);
@@ -1322,18 +1320,21 @@ obj_elf_section (int push)
 	      if (ISDIGIT (* input_line_pointer))
 		{
 		  char *t = input_line_pointer;
-		  match.info = strtoul (input_line_pointer,
+		  match.sh_info = strtoul (input_line_pointer,
 					&input_line_pointer, 0);
-		  if (match.info == (unsigned int) -1)
+		  if (match.sh_info == (unsigned int) -1)
 		    {
 		      as_warn (_("unsupported mbind section info: %s"), t);
-		      match.info = 0;
+		      match.sh_info = 0;
 		    }
 		}
 	      else
 		input_line_pointer = save;
 	    }
 
+	  if ((gnu_attr & SHF_GNU_RETAIN) != 0)
+	    match.sh_flags |= SHF_GNU_RETAIN;
+
 	  if (*input_line_pointer == ',')
 	    {
 	      char *save = input_line_pointer;
diff --git a/gas/config/obj-elf.h b/gas/config/obj-elf.h
index 4f29572eef..c714ba7a70 100644
--- a/gas/config/obj-elf.h
+++ b/gas/config/obj-elf.h
@@ -106,8 +106,9 @@ struct elf_section_match
 {
   const char *   group_name;
   const char *   linked_to_symbol_name;
-  unsigned int   info;
   unsigned int   section_id;
+  unsigned int   sh_info;		/* ELF section information.  */
+  bfd_vma        sh_flags;		/* ELF section flags.  */
   flagword       flags;
 };
 
diff --git a/gas/testsuite/gas/elf/elf.exp b/gas/testsuite/gas/elf/elf.exp
index 25c40a2810..80dec4effc 100644
--- a/gas/testsuite/gas/elf/elf.exp
+++ b/gas/testsuite/gas/elf/elf.exp
@@ -268,6 +268,7 @@ if { [is_elf_format] } then {
     run_dump_test "section24b"
     run_dump_test "section25"
     run_dump_test "section26"
+    run_dump_test "section27"
     run_dump_test "sh-link-zero"
     run_dump_test "dwarf2-1" $dump_opts
     run_dump_test "dwarf2-2" $dump_opts
diff --git a/gas/testsuite/gas/elf/section24b.d b/gas/testsuite/gas/elf/section24b.d
index 451ec21635..03dd2916ef 100644
--- a/gas/testsuite/gas/elf/section24b.d
+++ b/gas/testsuite/gas/elf/section24b.d
@@ -3,8 +3,12 @@
 #source: section24.s
 #readelf: -S --wide
 
-#failif
 #...
-  \[..\] .(text|data|bss|rodata)[ 	]+PROGBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 [^R] .*
+  \[..\] .text[ 	]+PROGBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 +AX .*
+#...
+  \[..\] .data[ 	]+PROGBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 +WA .*
+#...
+  \[..\] .bss[ 	]+NOBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 +WA .*
+#...
+  \[..\] .rodata[ 	]+PROGBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 +A .*
 #pass
-
diff --git a/gas/testsuite/gas/elf/section27.d b/gas/testsuite/gas/elf/section27.d
new file mode 100644
index 0000000000..5489d98d6e
--- /dev/null
+++ b/gas/testsuite/gas/elf/section27.d
@@ -0,0 +1,14 @@
+#readelf: -h -S --wide
+#name: SHF_GNU_RETAIN sections 27
+#notarget: ![supports_gnu_osabi]
+
+#...
+ +OS/ABI: +UNIX - (GNU|FreeBSD)
+#...
+  \[..\] .text[ 	]+PROGBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00  AX.*
+  \[..\] .data[ 	]+PROGBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00  WA.*
+  \[..\] .bss[ 	]+NOBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00  WA.*
+  \[..\] .bss[ 	]+NOBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 WAR.*
+  \[..\] .data[ 	]+PROGBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 WAR.*
+  \[..\] .text[ 	]+PROGBITS[ 	]+[0-9a-f]+ [0-9a-f]+ [0-9a-f]+ 00 AXR.*
+#pass
diff --git a/gas/testsuite/gas/elf/section27.s b/gas/testsuite/gas/elf/section27.s
new file mode 100644
index 0000000000..78e410ff95
--- /dev/null
+++ b/gas/testsuite/gas/elf/section27.s
@@ -0,0 +1,34 @@
+	.section	.bss,"aw"
+	.global	discard0
+	.type	discard0, %object
+discard0:
+	.zero	2
+
+	.section	.data,"aw"
+	.global	discard1
+	.type	discard1, %object
+discard1:
+	.word	1
+
+	.text
+	.global	discard2
+	.type	discard2, %function
+discard2:
+	.word	0
+
+	.section	.bss,"awR",%nobits
+	.global	retain0
+	.type	retain0, %object
+retain0:
+	.zero	2
+
+	.section	.data,"awR",%progbits
+	.type	retain1, %object
+retain1:
+	.word	1
+
+	.section	.text,"axR",%progbits
+	.global	retain2
+	.type	retain2, %function
+retain2:
+	.word	0
-- 
2.28.0


  reply	other threads:[~2020-12-04 16:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-04 13:52 H.J. Lu
2020-12-04 15:29 ` Jozef Lawrynowicz
2020-12-04 16:43   ` H.J. Lu [this message]
2020-12-08 12:33     ` V2 " H.J. Lu
2020-12-08 17:08       ` Nick Clifton
2020-12-09  0:05         ` V3 " H.J. Lu
2020-12-09  0:37           ` Alan Modra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMe9rOqh0O0wdeirryZ27eBQGwGN2E+dB3cxHQ4O7WY6RF+EzA@mail.gmail.com \
    --to=hjl.tools@gmail.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).