[1/2] RISC-V: Don't assume the priv attributes are in order when handling them.

Message ID 1592362991-19087-2-git-send-email-nelson.chu@sifive.com
State New
Headers show
Series
  • RISC-V: Update linker behavior when merging the elf priv spec attributes
Related show

Commit Message

Nelson Chu June 17, 2020, 3:03 a.m.
There is no guarantee that the priv attributes should be defined in order.
Therefore, we shouldn't have the order assumption when handling them in the
riscv_merge_attributes.  Set priv_attrs_merged to TRUE if we have handled
all of the priv attributes.

	bfd/
	* elfnn-riscv.c (riscv_merge_attributes): Once we meet one of the
	priv attributes, we will check the conflicts for all of them (major,
	minor and revision), and then set the priv_attrs_merged to TRUE to
	indicate that we have handled all of the priv attributes.  Remove
	the unused boolean priv_may_conflict, in_priv_zero and out_priv_zero.
---
 bfd/elfnn-riscv.c | 73 +++++++++++++++++++++++++++----------------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

-- 
2.7.4

Comments

Jim Wilson June 21, 2020, 12:42 a.m. | #1
On Tue, Jun 16, 2020 at 8:03 PM Nelson Chu <nelson.chu@sifive.com> wrote:
>         bfd/

>         * elfnn-riscv.c (riscv_merge_attributes): Once we meet one of the

>         priv attributes, we will check the conflicts for all of them (major,

>         minor and revision), and then set the priv_attrs_merged to TRUE to

>         indicate that we have handled all of the priv attributes.  Remove

>         the unused boolean priv_may_conflict, in_priv_zero and out_priv_zero.


Looks good to me.

Jim
Nelson Chu June 22, 2020, 2:07 a.m. | #2
Committed.  Thanks.

Nelson

On Sun, Jun 21, 2020 at 8:42 AM Jim Wilson <jimw@sifive.com> wrote:
>

> On Tue, Jun 16, 2020 at 8:03 PM Nelson Chu <nelson.chu@sifive.com> wrote:

> >         bfd/

> >         * elfnn-riscv.c (riscv_merge_attributes): Once we meet one of the

> >         priv attributes, we will check the conflicts for all of them (major,

> >         minor and revision), and then set the priv_attrs_merged to TRUE to

> >         indicate that we have handled all of the priv attributes.  Remove

> >         the unused boolean priv_may_conflict, in_priv_zero and out_priv_zero.

>

> Looks good to me.

>

> Jim

Patch

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 986e717..2804459 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -2987,9 +2987,7 @@  riscv_merge_attributes (bfd *ibfd, struct bfd_link_info *info)
   obj_attribute *in_attr;
   obj_attribute *out_attr;
   bfd_boolean result = TRUE;
-  bfd_boolean priv_may_conflict = FALSE;
-  bfd_boolean in_priv_zero = TRUE;
-  bfd_boolean out_priv_zero = TRUE;
+  bfd_boolean priv_attrs_merged = FALSE;
   const char *sec_name = get_elf_backend_data (ibfd)->obj_attrs_section;
   unsigned int i;
 
@@ -3048,41 +3046,42 @@  riscv_merge_attributes (bfd *ibfd, struct bfd_link_info *info)
       case Tag_RISCV_priv_spec:
       case Tag_RISCV_priv_spec_minor:
       case Tag_RISCV_priv_spec_revision:
-	if (in_attr[i].i != 0)
-	  in_priv_zero = FALSE;
-	if (out_attr[i].i != 0)
-	  out_priv_zero = FALSE;
-	if (out_attr[i].i != in_attr[i].i)
-	  priv_may_conflict = TRUE;
-
-	/* We check the priv version conflict when parsing the
-	   revision version.  */
-	if (i != Tag_RISCV_priv_spec_revision)
-	  break;
-
-	/* Allow to link the object without the priv setting.  */
-	if (out_priv_zero)
-	  {
-	    out_attr[i].i = in_attr[i].i;
-	    out_attr[Tag_RISCV_priv_spec].i =
-		in_attr[Tag_RISCV_priv_spec].i;
-	    out_attr[Tag_RISCV_priv_spec_minor].i =
-		in_attr[Tag_RISCV_priv_spec_minor].i;
-	  }
-	else if (!in_priv_zero
-		 && priv_may_conflict)
+	/* If we have handled the priv attributes, then skip it.  */
+	if (!priv_attrs_merged)
 	  {
-	    _bfd_error_handler
-	      (_("error: %pB use privilege spec version %u.%u.%u but "
-		 "the output use version %u.%u.%u."),
-	       ibfd,
-	       in_attr[Tag_RISCV_priv_spec].i,
-	       in_attr[Tag_RISCV_priv_spec_minor].i,
-	       in_attr[i].i,
-	       out_attr[Tag_RISCV_priv_spec].i,
-	       out_attr[Tag_RISCV_priv_spec_minor].i,
-	       out_attr[i].i);
-	    result = FALSE;
+	    unsigned int Tag_a = Tag_RISCV_priv_spec;
+	    unsigned int Tag_b = Tag_RISCV_priv_spec_minor;
+	    unsigned int Tag_c = Tag_RISCV_priv_spec_revision;
+
+	    /* Allow to link the object without the priv specs.  */
+	    if (out_attr[Tag_a].i == 0
+		&& out_attr[Tag_b].i == 0
+		&& out_attr[Tag_c].i == 0)
+	      {
+		out_attr[Tag_a].i = in_attr[Tag_a].i;
+		out_attr[Tag_b].i = in_attr[Tag_b].i;
+		out_attr[Tag_c].i = in_attr[Tag_c].i;
+	      }
+	    else if ((in_attr[Tag_a].i != 0
+		      || in_attr[Tag_b].i != 0
+		      || in_attr[Tag_c].i != 0)
+		     && (out_attr[Tag_a].i != in_attr[Tag_a].i
+			 || out_attr[Tag_b].i != in_attr[Tag_b].i
+			 || out_attr[Tag_c].i != in_attr[Tag_c].i))
+	      {
+		_bfd_error_handler
+		  (_("error: %pB use privilege spec version %u.%u.%u but "
+		     "the output use version %u.%u.%u."),
+		   ibfd,
+		   in_attr[Tag_a].i,
+		   in_attr[Tag_b].i,
+		   in_attr[Tag_c].i,
+		   out_attr[Tag_a].i,
+		   out_attr[Tag_b].i,
+		   out_attr[Tag_c].i);
+		result = FALSE;
+	      }
+	    priv_attrs_merged = TRUE;
 	  }
 	break;