[PATCH 0/2] RISC-V: Fix the conflicting priv spec problems

[PATCH 0/2] RISC-V: Fix the conflicting priv spec problems

[Nelson Chu]

Hi binutils,

The link compatibility problem is found by Jim in the following link,
https://sourceware.org/pipermail/binutils/2020-May/111307.html


There are two patches used to fix the compatibility problem,
* RISC-V: Don't generate the ELF privilege attributes when no CSR are used.
* RISC-V: The object without priv spec attributes can be linked with any object.

For the first patch, the assembler will generate the ELF priv spec attributes
only when,

1. We already set the ELF priv spec attributes in the assembly file.
(.attribute priv_spec/priv_spec_minor/priv_spec_revision number)

2. The CSR is explicitly used by the CSR instruction.  Then assembler will
generate the priv attributes by -mpriv-spec, --with-priv-spec or the default
setting (the newest spec).

The behavior is basically the same as before (the version controling patches
have not been applied), except the case 2 mentioned above.

Therefore, we still need the second patch, which allows to link the object
without any privilege set. That is, an object file without a priv spec
version set should be allowed to link with any other object file.  And when
the first linked object doesn't contain any priv set, then we should copy the
verison according to the next linked object.

I recover to the time before the version controling patches are not applied.
And I get the following ld testsuite result by adding CC_FOR_TARGET to run
the compiler needed cases.

 === ld Summary ===

# of expected passes            483
# of unexpected failures        14
# of expected failures          9
# of unsupported tests          180

Then I use "git pull" to test the upstream binutils (just build binutils and
use the previous compiler), and also get the same conflicting priv spec errors.

=== ld Summary ===

# of expected passes            350
# of expected failures          7
# of untested testcases         16
# of unsupported tests          170

After applying the first patch, the expected passes number is back.
The extra passed one is "map to directory", which is added recently in the
ld/testsuite/ld-scripts/map-address.exp.

=== ld Summary ===

# of expected passes            484
# of unexpected failures        14
# of expected failures          9
# of unsupported tests          180

Ane then I get the same result when applying the second patches with the first one
(extra 8 pass are the new testcases, used to check the second patch),

=== ld Summary ===

# of expected passes            492
# of unexpected failures        14
# of expected failures          9
# of unsupported tests          180


We can only use the second patch to fix the compatibility problem, but it is
hard to write testcases to test it (hard to generate an object without priv set).
So I suggest to apply the first patch too, and the behavior should be more make sence.

Thanks
Nelson

[PATCH 1/2] RISC-V: Don't generate the ELF privilege attributes when no CSR are used.

[Nelson Chu]

	gas/
	* config/tc-riscv.c (explicit_csr): New static boolean.
	Used to indicate CSR are explictly used.
	(riscv_ip): Set explicit_csr to TRUE if any CSR is used.
	(riscv_write_out_attrs): If we already have set elf priv
	attributes, then generate them.  Otherwise, don't generate
	them when no CSR are used.

	* testsuite/gas/riscv/attribute-01.d: Remove the priv attributes.
	* testsuite/gas/riscv/attribute-02.d: Likewise.
	* testsuite/gas/riscv/attribute-03.d: Likewise.
	* testsuite/gas/riscv/attribute-04.d: Likewise.
	* testsuite/gas/riscv/attribute-05.d: Likewise.
	* testsuite/gas/riscv/attribute-06.d: Likewise.
	* testsuite/gas/riscv/attribute-07.d: Likewise.
	* testsuite/gas/riscv/attribute-08.d: Likewise.
	* testsuite/gas/riscv/attribute-09.d: Likewise.
	* testsuite/gas/riscv/attribute-10.d: Likewise.
	* testsuite/gas/riscv/attribute-unknown.d: Likewise.
	* testsuite/gas/riscv/attribute-11.s: New testcase.
	* testsuite/gas/riscv/attribute-11.d: New testcase.  The CSR is
	used, so we should output the ELF priv attributes.
	* testsuite/gas/riscv/attribute-12.d: New testcase.  The CSR is
	used, so output the priv attributes according to the -mpriv-spec.
	* testsuite/gas/riscv/attribute-13.d: New testcase.  The CSR isn't
	used, so ignore the -mpriv-spec setting.

	ld/
	* testsuite/ld-riscv-elf/attr-merge-arch-01.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-arch-02.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-arch-03.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-stack-align.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-strict-align-01.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-strict-align-02.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-strict-align-03.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-strict-align-04.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-strict-align-05.d: Likewise.
	* testsuite/ld-riscv-elf/call-relax.d: Add -mno-arch-attr.
---
 gas/config/tc-riscv.c                                  | 9 +++++++++
 gas/testsuite/gas/riscv/attribute-01.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-02.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-03.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-04.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-06.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-07.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-08.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-09.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-10.d                 | 3 ---
 gas/testsuite/gas/riscv/attribute-11.d                 | 8 ++++++++
 gas/testsuite/gas/riscv/attribute-11.s                 | 1 +
 gas/testsuite/gas/riscv/attribute-12.d                 | 9 +++++++++
 gas/testsuite/gas/riscv/attribute-13.d                 | 6 ++++++
 gas/testsuite/gas/riscv/attribute-unknown.d            | 3 ---
 ld/testsuite/ld-riscv-elf/attr-merge-arch-01.d         | 3 ---
 ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d         | 3 ---
 ld/testsuite/ld-riscv-elf/attr-merge-arch-03.d         | 3 ---
 ld/testsuite/ld-riscv-elf/attr-merge-stack-align.d     | 2 --
 ld/testsuite/ld-riscv-elf/attr-merge-strict-align-01.d | 3 ---
 ld/testsuite/ld-riscv-elf/attr-merge-strict-align-02.d | 3 ---
 ld/testsuite/ld-riscv-elf/attr-merge-strict-align-03.d | 3 ---
 ld/testsuite/ld-riscv-elf/attr-merge-strict-align-04.d | 3 ---
 ld/testsuite/ld-riscv-elf/attr-merge-strict-align-05.d | 3 ---
 ld/testsuite/ld-riscv-elf/call-relax.d                 | 2 +-
 25 files changed, 34 insertions(+), 57 deletions(-)
 create mode 100644 gas/testsuite/gas/riscv/attribute-11.d
 create mode 100644 gas/testsuite/gas/riscv/attribute-11.s
 create mode 100644 gas/testsuite/gas/riscv/attribute-12.d
 create mode 100644 gas/testsuite/gas/riscv/attribute-13.d

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index da94b5b..cc77dbf 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -374,6 +374,9 @@ static bfd_boolean start_assemble = FALSE;
 /* Indicate ELF attributes are explictly set.  */
 static bfd_boolean explicit_attr = FALSE;
 
+/* Indicate CSR are explictly used.  */
+static bfd_boolean explicit_csr = FALSE;
+
 /* Macros for encoding relaxation state for RVC branches and far jumps.  */
 #define RELAX_BRANCH_ENCODE(uncond, rvc, length)	\
   ((relax_substateT) 					\
@@ -2212,6 +2215,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
 
 	    case 'E':		/* Control register.  */
 	      insn_with_csr = TRUE;
+	      explicit_csr = TRUE;
 	      if (reg_lookup (&s, RCLASS_CSR, &regno))
 		INSERT_OPERAND (CSR, *ip, regno);
 	      else
@@ -3605,6 +3609,11 @@ riscv_write_out_attrs (void)
       && !riscv_set_default_priv_spec (NULL))
     return;
 
+  /* If we already have set elf priv attributes, then generate them.
+     Otherwise, don't generate them when no CSR are used.  */
+  if (!explicit_csr)
+    return;
+
   /* Re-write priv attributes by default_priv_spec.  */
   priv_str = riscv_get_priv_spec_name (default_priv_spec);
   p = priv_str;
diff --git a/gas/testsuite/gas/riscv/attribute-01.d b/gas/testsuite/gas/riscv/attribute-01.d
index f027347..2e19e09 100644
--- a/gas/testsuite/gas/riscv/attribute-01.d
+++ b/gas/testsuite/gas/riscv/attribute-01.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p0_m2p0_a2p0_f2p0_d2p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-02.d b/gas/testsuite/gas/riscv/attribute-02.d
index 02b532d..ae0195e 100644
--- a/gas/testsuite/gas/riscv/attribute-02.d
+++ b/gas/testsuite/gas/riscv/attribute-02.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p0_m2p0_a2p0_f2p0_d2p0_xargle0p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-03.d b/gas/testsuite/gas/riscv/attribute-03.d
index ded529a..9916ff6 100644
--- a/gas/testsuite/gas/riscv/attribute-03.d
+++ b/gas/testsuite/gas/riscv/attribute-03.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p0_m2p0_a2p0_f2p0_d2p0_xargle0p0_xfoo0p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-04.d b/gas/testsuite/gas/riscv/attribute-04.d
index df6c818..408464d 100644
--- a/gas/testsuite/gas/riscv/attribute-04.d
+++ b/gas/testsuite/gas/riscv/attribute-04.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p0_m2p0_a2p0_f2p0_d2p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-06.d b/gas/testsuite/gas/riscv/attribute-06.d
index e1d62c4..a2dd9fb 100644
--- a/gas/testsuite/gas/riscv/attribute-06.d
+++ b/gas/testsuite/gas/riscv/attribute-06.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-07.d b/gas/testsuite/gas/riscv/attribute-07.d
index 59f02b4..342a537 100644
--- a/gas/testsuite/gas/riscv/attribute-07.d
+++ b/gas/testsuite/gas/riscv/attribute-07.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv64i2p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-08.d b/gas/testsuite/gas/riscv/attribute-08.d
index 13b82a9..c10ac0c 100644
--- a/gas/testsuite/gas/riscv/attribute-08.d
+++ b/gas/testsuite/gas/riscv/attribute-08.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32e1p9"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-09.d b/gas/testsuite/gas/riscv/attribute-09.d
index 53945a2..cad1713 100644
--- a/gas/testsuite/gas/riscv/attribute-09.d
+++ b/gas/testsuite/gas/riscv/attribute-09.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p1_m2p0_zicsr0p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-10.d b/gas/testsuite/gas/riscv/attribute-10.d
index 91691fd..ba903d1 100644
--- a/gas/testsuite/gas/riscv/attribute-10.d
+++ b/gas/testsuite/gas/riscv/attribute-10.d
@@ -4,6 +4,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/gas/testsuite/gas/riscv/attribute-11.d b/gas/testsuite/gas/riscv/attribute-11.d
new file mode 100644
index 0000000..4bd66f0
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-11.d
@@ -0,0 +1,8 @@
+#as: -march-attr
+#readelf: -A
+#source: attribute-11.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: [0-9_\"].*
+#...
diff --git a/gas/testsuite/gas/riscv/attribute-11.s b/gas/testsuite/gas/riscv/attribute-11.s
new file mode 100644
index 0000000..81099b2
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-11.s
@@ -0,0 +1 @@
+	csrr a0, 0x0
diff --git a/gas/testsuite/gas/riscv/attribute-12.d b/gas/testsuite/gas/riscv/attribute-12.d
new file mode 100644
index 0000000..980b36c
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-12.d
@@ -0,0 +1,9 @@
+#as: -march-attr -mpriv-spec=1.9.1
+#readelf: -A
+#source: attribute-11.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: 1
+  Tag_RISCV_priv_spec_minor: 9
+  Tag_RISCV_priv_spec_revision: 1
diff --git a/gas/testsuite/gas/riscv/attribute-13.d b/gas/testsuite/gas/riscv/attribute-13.d
new file mode 100644
index 0000000..b8dfe3a
--- /dev/null
+++ b/gas/testsuite/gas/riscv/attribute-13.d
@@ -0,0 +1,6 @@
+#as: -march-attr -mpriv-spec=1.9.1
+#readelf: -A
+#source: empty.s
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
diff --git a/gas/testsuite/gas/riscv/attribute-unknown.d b/gas/testsuite/gas/riscv/attribute-unknown.d
index 120e3de..667f21a 100644
--- a/gas/testsuite/gas/riscv/attribute-unknown.d
+++ b/gas/testsuite/gas/riscv/attribute-unknown.d
@@ -4,8 +4,5 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: [a-zA-Z0-9_\"].*
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
   Tag_unknown_255: "test"
   Tag_unknown_256: 123 \(0x7b\)
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-01.d b/ld/testsuite/ld-riscv-elf/attr-merge-arch-01.d
index 032f964..5baaba4 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-01.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-01.d
@@ -7,6 +7,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p0_m2p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
index 54a7621..a7d79a1 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-02.d
@@ -7,6 +7,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p0_m2p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-arch-03.d b/ld/testsuite/ld-riscv-elf/attr-merge-arch-03.d
index 67f0437..d46dee8 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-arch-03.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-arch-03.d
@@ -7,6 +7,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: "rv32i2p0_m2p0_xbar2p0_xfoo2p0"
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-stack-align.d b/ld/testsuite/ld-riscv-elf/attr-merge-stack-align.d
index 5585fac..e4d965a 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-stack-align.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-stack-align.d
@@ -8,6 +8,4 @@ Attribute Section: riscv
 File Attributes
   Tag_RISCV_stack_align: 16-bytes
   Tag_RISCV_arch: [a-zA-Z0-9_\"].*
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
 #...
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-01.d b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-01.d
index 91011a2..1039930 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-01.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-01.d
@@ -8,6 +8,3 @@ Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: [a-zA-Z0-9_\"].*
   Tag_RISCV_unaligned_access: Unaligned access
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-02.d b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-02.d
index 5bdea27..12ca1c4 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-02.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-02.d
@@ -8,6 +8,3 @@ Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: [a-zA-Z0-9_\"].*
   Tag_RISCV_unaligned_access: Unaligned access
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-03.d b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-03.d
index ac886fb..e41351d 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-03.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-03.d
@@ -8,6 +8,3 @@ Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: [a-zA-Z0-9_\"].*
   Tag_RISCV_unaligned_access: Unaligned access
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-04.d b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-04.d
index dd45f76..ac2a766 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-04.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-04.d
@@ -7,6 +7,3 @@
 Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: [a-zA-Z0-9_\"].*
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-05.d b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-05.d
index ef0c154..608c05e 100644
--- a/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-05.d
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-strict-align-05.d
@@ -8,6 +8,3 @@ Attribute Section: riscv
 File Attributes
   Tag_RISCV_arch: [a-zA-Z0-9_\"].*
   Tag_RISCV_unaligned_access: Unaligned access
-  Tag_RISCV_priv_spec: [0-9_\"].*
-  Tag_RISCV_priv_spec_minor: [0-9_\"].*
-#...
diff --git a/ld/testsuite/ld-riscv-elf/call-relax.d b/ld/testsuite/ld-riscv-elf/call-relax.d
index 46d9c84..597ff67 100644
--- a/ld/testsuite/ld-riscv-elf/call-relax.d
+++ b/ld/testsuite/ld-riscv-elf/call-relax.d
@@ -3,7 +3,7 @@
 #source: call-relax-1.s
 #source: call-relax-2.s
 #source: call-relax-3.s
-#as: -march=rv32ic
+#as: -march=rv32ic -mno-arch-attr
 #ld: -melf32lriscv
 #objdump: -d
 #pass
-- 
2.7.4

[PATCH 2/2] RISC-V: The object without priv spec attributes can be linked with any object.

[Nelson Chu]

	bfd/
	* elfnn-riscv.c (riscv_merge_attributes): Add new boolean
	priv_may_conflict, in_priv_zero and out_priv_zero to decide whether
	the object can be linked according to it's priv attributes.  The object
	without any priv spec attributes can be linked with others.  If the first
	input object doesn't contain any priv attributes, then we need to copy
	the setting from the next input one.  Also report more detailed error
	messages to user.

	ld/
	* testsuite/ld-riscv-elf/attr-merge-priv-spec.d: Rename to
	attr-merge-priv-spec-01.d.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-c.s: Set priv spec
	to 1.11.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-d.s: Empty priv spec
	setting.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-02.d: New testcase.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-03.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-01.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-02.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-03.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-04.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-05.d: Likewise.
	* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-06.d: Likewise.
	* testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
---
 bfd/elfnn-riscv.c                                  | 40 ++++++++++++++++++++--
 .../ld-riscv-elf/attr-merge-priv-spec-01.d         | 12 +++++++
 .../ld-riscv-elf/attr-merge-priv-spec-02.d         | 12 +++++++
 .../ld-riscv-elf/attr-merge-priv-spec-03.d         | 12 +++++++
 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-c.s |  2 ++
 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-d.s |  1 +
 .../ld-riscv-elf/attr-merge-priv-spec-failed-01.d  |  5 +++
 .../ld-riscv-elf/attr-merge-priv-spec-failed-02.d  |  5 +++
 .../ld-riscv-elf/attr-merge-priv-spec-failed-03.d  |  6 ++++
 .../ld-riscv-elf/attr-merge-priv-spec-failed-04.d  |  6 ++++
 .../ld-riscv-elf/attr-merge-priv-spec-failed-05.d  |  6 ++++
 .../ld-riscv-elf/attr-merge-priv-spec-failed-06.d  |  6 ++++
 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec.d   | 12 -------
 ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp         | 10 +++++-
 14 files changed, 120 insertions(+), 15 deletions(-)
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-01.d
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-02.d
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-03.d
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-c.s
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-d.s
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-01.d
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-02.d
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-03.d
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-04.d
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-05.d
 create mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-06.d
 delete mode 100644 ld/testsuite/ld-riscv-elf/attr-merge-priv-spec.d

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 5fa6e35..9aed2a0 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -3067,6 +3067,9 @@ 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;
   const char *sec_name = get_elf_backend_data (ibfd)->obj_attrs_section;
   unsigned int i;
 
@@ -3121,20 +3124,52 @@ riscv_merge_attributes (bfd *ibfd, struct bfd_link_info *info)
 	      out_attr[Tag_RISCV_arch].s = merged_arch;
 	  }
 	break;
+
       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 wihtout 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)
 	  {
 	    _bfd_error_handler
-	      (_("error: %pB: conflicting priv spec version "
-		 "(major/minor/revision)."), ibfd);
+	      (_("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;
 	  }
 	break;
+
       case Tag_RISCV_unaligned_access:
 	out_attr[i].i |= in_attr[i].i;
 	break;
+
       case Tag_RISCV_stack_align:
 	if (out_attr[i].i == 0)
 	  out_attr[i].i = in_attr[i].i;
@@ -3149,6 +3184,7 @@ riscv_merge_attributes (bfd *ibfd, struct bfd_link_info *info)
 	    result = FALSE;
 	  }
 	break;
+
       default:
 	result &= _bfd_elf_merge_unknown_attribute_low (ibfd, obfd, i);
       }
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-01.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-01.d
new file mode 100644
index 0000000..0aa6fe0
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-01.d
@@ -0,0 +1,12 @@
+#source: attr-merge-priv-spec-a.s
+#source: attr-merge-priv-spec-b.s
+#as: -march-attr
+#ld: -r
+#readelf: -A
+
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: 1
+  Tag_RISCV_priv_spec_minor: 9
+  Tag_RISCV_priv_spec_revision: 1
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-02.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-02.d
new file mode 100644
index 0000000..0ac4ca7
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-02.d
@@ -0,0 +1,12 @@
+#source: attr-merge-priv-spec-a.s
+#source: attr-merge-priv-spec-d.s
+#as: -march-attr
+#ld: -r
+#readelf: -A
+
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: 1
+  Tag_RISCV_priv_spec_minor: 9
+  Tag_RISCV_priv_spec_revision: 1
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-03.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-03.d
new file mode 100644
index 0000000..6950483
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-03.d
@@ -0,0 +1,12 @@
+#source: attr-merge-priv-spec-d.s
+#source: attr-merge-priv-spec-a.s
+#as: -march-attr
+#ld: -r
+#readelf: -A
+
+Attribute Section: riscv
+File Attributes
+  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
+  Tag_RISCV_priv_spec: 1
+  Tag_RISCV_priv_spec_minor: 9
+  Tag_RISCV_priv_spec_revision: 1
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-c.s b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-c.s
new file mode 100644
index 0000000..7ea3185
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-c.s
@@ -0,0 +1,2 @@
+	.attribute priv_spec, 1
+	.attribute priv_spec_minor, 11
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-d.s b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-d.s
new file mode 100644
index 0000000..37fddd0
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-d.s
@@ -0,0 +1 @@
+# Empty priv attributes setting.
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-01.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-01.d
new file mode 100644
index 0000000..c52ebac
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-01.d
@@ -0,0 +1,5 @@
+#source: attr-merge-priv-spec-a.s
+#source: attr-merge-priv-spec-c.s
+#as:
+#ld: -r
+#error: .*use privilege spec version 1.11.0 but the output use version 1.9.1.
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-02.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-02.d
new file mode 100644
index 0000000..fc00145
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-02.d
@@ -0,0 +1,5 @@
+#source: attr-merge-priv-spec-c.s
+#source: attr-merge-priv-spec-a.s
+#as:
+#ld: -r
+#error: .*use privilege spec version 1.9.1 but the output use version 1.11.0.
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-03.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-03.d
new file mode 100644
index 0000000..1d40e90
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-03.d
@@ -0,0 +1,6 @@
+#source: attr-merge-priv-spec-a.s
+#source: attr-merge-priv-spec-d.s
+#source: attr-merge-priv-spec-c.s
+#as:
+#ld: -r
+#error: .*use privilege spec version 1.11.0 but the output use version 1.9.1.
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-04.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-04.d
new file mode 100644
index 0000000..0efee3c
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-04.d
@@ -0,0 +1,6 @@
+#source: attr-merge-priv-spec-d.s
+#source: attr-merge-priv-spec-a.s
+#source: attr-merge-priv-spec-c.s
+#as:
+#ld: -r
+#error: .*use privilege spec version 1.11.0 but the output use version 1.9.1.
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-05.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-05.d
new file mode 100644
index 0000000..5b9b8d0
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-05.d
@@ -0,0 +1,6 @@
+#source: attr-merge-priv-spec-c.s
+#source: attr-merge-priv-spec-d.s
+#source: attr-merge-priv-spec-a.s
+#as:
+#ld: -r
+#error: .*use privilege spec version 1.9.1 but the output use version 1.11.0.
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-06.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-06.d
new file mode 100644
index 0000000..dab7eb6
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-06.d
@@ -0,0 +1,6 @@
+#source: attr-merge-priv-spec-d.s
+#source: attr-merge-priv-spec-c.s
+#source: attr-merge-priv-spec-a.s
+#as:
+#ld: -r
+#error: .*use privilege spec version 1.9.1 but the output use version 1.11.0.
diff --git a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec.d b/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec.d
deleted file mode 100644
index 0aa6fe0..0000000
--- a/ld/testsuite/ld-riscv-elf/attr-merge-priv-spec.d
+++ /dev/null
@@ -1,12 +0,0 @@
-#source: attr-merge-priv-spec-a.s
-#source: attr-merge-priv-spec-b.s
-#as: -march-attr
-#ld: -r
-#readelf: -A
-
-Attribute Section: riscv
-File Attributes
-  Tag_RISCV_arch: [a-zA-Z0-9_\"].*
-  Tag_RISCV_priv_spec: 1
-  Tag_RISCV_priv_spec_minor: 9
-  Tag_RISCV_priv_spec_revision: 1
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 0e9750e..1a0c68f 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -35,9 +35,17 @@ if [istarget "riscv*-*-*"] {
     run_dump_test "attr-merge-strict-align-04"
     run_dump_test "attr-merge-strict-align-05"
     run_dump_test "attr-merge-stack-align"
-    run_dump_test "attr-merge-priv-spec"
+    run_dump_test "attr-merge-priv-spec-01"
+    run_dump_test "attr-merge-priv-spec-02"
+    run_dump_test "attr-merge-priv-spec-03"
     run_dump_test "attr-merge-arch-failed-01"
     run_dump_test "attr-merge-stack-align-failed"
+    run_dump_test "attr-merge-priv-spec-failed-01"
+    run_dump_test "attr-merge-priv-spec-failed-02"
+    run_dump_test "attr-merge-priv-spec-failed-03"
+    run_dump_test "attr-merge-priv-spec-failed-04"
+    run_dump_test "attr-merge-priv-spec-failed-05"
+    run_dump_test "attr-merge-priv-spec-failed-06"
     run_ld_link_tests {
 	{ "Weak reference 32" "-T weakref.ld -melf32lriscv" ""
 	    "-march=rv32i -mabi=ilp32" {weakref32.s}
-- 
2.7.4

Re: [PATCH 1/2] RISC-V: Don't generate the ELF privilege attributes when no CSR are used.

[Jim Wilson]

On Sat, May 30, 2020 at 1:44 AM Nelson Chu <nelson.chu@sifive.com> wrote:
>         gas/
>         * config/tc-riscv.c (explicit_csr): New static boolean.
>         Used to indicate CSR are explictly used.
>         ...

This looks good.

Jim

Re: [PATCH 2/2] RISC-V: The object without priv spec attributes can be linked with any object.

[Jim Wilson]

On Sat, May 30, 2020 at 1:44 AM Nelson Chu <nelson.chu@sifive.com> wrote:
>         bfd/
>         * elfnn-riscv.c (riscv_merge_attributes): Add new boolean
>         priv_may_conflict, in_priv_zero and out_priv_zero to decide whether
>        ...

This looks good with a few minor comments.

> +       /* We check the priv version conflict when parsing the
> +          revision version.  */
> +       if (i != Tag_RISCV_priv_spec_revision)
> +         break;

It looks like there is an ordering assumption here.  That is, it looks
like you are assuming that priv_spec and priv_spec_minor always come
before priv_spec_revision, and hence all of the variables you need
will be set when handling priv_spec_revision.  Binutils does indeed do
this, but I'm not sure if the spec requires this order.  I think this
is OK for now, but we should watch for problems if someone emits them
in a different order.

> +       /* Allow to link the object wihtout the priv setting.  */

Typo wihtout -> without

> +             (_("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);

We might need to change the error to a warning if this causes problems
for people, but I think this is OK for now.

Jim

Re: [PATCH 0/2] RISC-V: Fix the conflicting priv spec problems

[Palmer Dabbelt]

On Sat, 30 May 2020 01:44:33 PDT (-0700), nelson.chu@sifive.com wrote:
>
> Hi binutils,
>
> The link compatibility problem is found by Jim in the following link,
> https://sourceware.org/pipermail/binutils/2020-May/111307.html
>
>
> There are two patches used to fix the compatibility problem,
> * RISC-V: Don't generate the ELF privilege attributes when no CSR are used.
> * RISC-V: The object without priv spec attributes can be linked with any object.

These fix the obvious problem, so consider them approved, but I think there's a
bigger issue at play.  Specifically, I don't think it's reasonable to expect
the linker to be able to determine whether or not objects that are built for
different version of the privelege spec are compatible with each other.  At a
minimum we'd need a way to force linking, but I think we should start by just
annotating

>
> For the first patch, the assembler will generate the ELF priv spec attributes
> only when,
>
> 1. We already set the ELF priv spec attributes in the assembly file.
> (.attribute priv_spec/priv_spec_minor/priv_spec_revision number)
>
> 2. The CSR is explicitly used by the CSR instruction.  Then assembler will
> generate the priv attributes by -mpriv-spec, --with-priv-spec or the default
> setting (the newest spec).

Presumably the other priv instructions should also be triggering the attribute?

>
> The behavior is basically the same as before (the version controling patches
> have not been applied), except the case 2 mentioned above.
>
> Therefore, we still need the second patch, which allows to link the object
> without any privilege set. That is, an object file without a priv spec
> version set should be allowed to link with any other object file.  And when
> the first linked object doesn't contain any priv set, then we should copy the
> verison according to the next linked object.
>
> I recover to the time before the version controling patches are not applied.
> And I get the following ld testsuite result by adding CC_FOR_TARGET to run
> the compiler needed cases.
>
>  === ld Summary ===
>
> # of expected passes            483
> # of unexpected failures        14
> # of expected failures          9
> # of unsupported tests          180
>
> Then I use "git pull" to test the upstream binutils (just build binutils and
> use the previous compiler), and also get the same conflicting priv spec errors.
>
> === ld Summary ===
>
> # of expected passes            350
> # of expected failures          7
> # of untested testcases         16
> # of unsupported tests          170
>
> After applying the first patch, the expected passes number is back.
> The extra passed one is "map to directory", which is added recently in the
> ld/testsuite/ld-scripts/map-address.exp.
>
> === ld Summary ===
>
> # of expected passes            484
> # of unexpected failures        14
> # of expected failures          9
> # of unsupported tests          180
>
> Ane then I get the same result when applying the second patches with the first one
> (extra 8 pass are the new testcases, used to check the second patch),
>
> === ld Summary ===
>
> # of expected passes            492
> # of unexpected failures        14
> # of expected failures          9
> # of unsupported tests          180
>
>
> We can only use the second patch to fix the compatibility problem, but it is
> hard to write testcases to test it (hard to generate an object without priv set).
> So I suggest to apply the first patch too, and the behavior should be more make sence.
>
> Thanks
> Nelson

Re: [PATCH 1/2] RISC-V: Don't generate the ELF privilege attributes when no CSR are used.

[Nelson Chu]

Committed, thanks.

Nelson

On Fri, Jun 5, 2020 at 12:51 AM Jim Wilson <jimw@sifive.com> wrote:
>
> On Sat, May 30, 2020 at 1:44 AM Nelson Chu <nelson.chu@sifive.com> wrote:
> >         gas/
> >         * config/tc-riscv.c (explicit_csr): New static boolean.
> >         Used to indicate CSR are explictly used.
> >         ...
>
> This looks good.
>
> Jim

Re: [PATCH 2/2] RISC-V: The object without priv spec attributes can be linked with any object.

[Nelson Chu]

Already fix the type.  And committed.

I shouldn't have the order assumption since we can use the backend
obj_attrs_order to change the order.  I will fix this and change the
link error to warning by another patch.

Thanks
Nelson

On Fri, Jun 5, 2020 at 1:23 AM Jim Wilson <jimw@sifive.com> wrote:
>
> On Sat, May 30, 2020 at 1:44 AM Nelson Chu <nelson.chu@sifive.com> wrote:
> >         bfd/
> >         * elfnn-riscv.c (riscv_merge_attributes): Add new boolean
> >         priv_may_conflict, in_priv_zero and out_priv_zero to decide whether
> >        ...
>
> This looks good with a few minor comments.
>
> > +       /* We check the priv version conflict when parsing the
> > +          revision version.  */
> > +       if (i != Tag_RISCV_priv_spec_revision)
> > +         break;
>
> It looks like there is an ordering assumption here.  That is, it looks
> like you are assuming that priv_spec and priv_spec_minor always come
> before priv_spec_revision, and hence all of the variables you need
> will be set when handling priv_spec_revision.  Binutils does indeed do
> this, but I'm not sure if the spec requires this order.  I think this
> is OK for now, but we should watch for problems if someone emits them
> in a different order.
>
> > +       /* Allow to link the object wihtout the priv setting.  */
>
> Typo wihtout -> without
>
> > +             (_("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);
>
> We might need to change the error to a warning if this causes problems
> for people, but I think this is OK for now.
>
> Jim

Re: [PATCH 0/2] RISC-V: Fix the conflicting priv spec problems

[Nelson Chu]

On Fri, Jun 5, 2020 at 6:23 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> These fix the obvious problem, so consider them approved,

Thank you very much :)

> Specifically, I don't think it's reasonable to expect
> the linker to be able to determine whether or not objects that are built for
> different version of the privelege spec are compatible with each other.  At any rate
> minimum we'd need a way to force linking, but I think we should start by just
> annotating

Kito had discussed with me that we might need a linker option to
choose the link compatibility levels.  And supporting three levels
might be enough,

-Wl,--link-attrs-mode=[strict? median? loose?]
* Strict: the linked objects must have the same privilege version.
* Median: Allow compatibility, but consider all conflicts  when we
link objects with different versions of specs.
* Loose: Force to link all objects without checking attrs.  Users must
be responsible when they use this option.
(The naming is just temporary.)

> Presumably the other priv instructions should also be triggering the attribute?

Thanks for the reminder.  This looks good.  I will add it in the future patches.

Nelson

Re: [PATCH 0/2] RISC-V: Fix the conflicting priv spec problems

[Kito Cheng]

> -Wl,--link-attrs-mode=[strict? median? loose?]

I would like apply this option on arch string merge too.
but I don't have strong opinion on using same option or separated
option for arch string

> * Strict: the linked objects must have the same privilege version.
> * Median: Allow compatibility, but consider all conflicts  when we
> link objects with different versions of specs.

Median is kind of confusing to me, maybe `compatible`?
and set this as default merge strategy.

> * Loose: Force to link all objects without checking attrs.  Users must
> be responsible when they use this option.

I would prefer using compatible strategy if possible, but discard the
attribute if any conflict for this mode.

Re: [PATCH 0/2] RISC-V: Fix the conflicting priv spec problems

[Palmer Dabbelt]

On Thu, 04 Jun 2020 22:41:19 PDT (-0700), nelson.chu@sifive.com wrote:
> On Fri, Jun 5, 2020 at 6:23 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> These fix the obvious problem, so consider them approved,
>
> Thank you very much :)
>
>> Specifically, I don't think it's reasonable to expect
>> the linker to be able to determine whether or not objects that are built for
>> different version of the privelege spec are compatible with each other.  At any rate
>> minimum we'd need a way to force linking, but I think we should start by just
>> annotating
>
> Kito had discussed with me that we might need a linker option to
> choose the link compatibility levels.  And supporting three levels
> might be enough,
>
> -Wl,--link-attrs-mode=[strict? median? loose?]
> * Strict: the linked objects must have the same privilege version.
> * Median: Allow compatibility, but consider all conflicts  when we
> link objects with different versions of specs.
> * Loose: Force to link all objects without checking attrs.  Users must
> be responsible when they use this option.
> (The naming is just temporary.)

I'm not sure the middle version actually has any value for the privilege spec.
For the user spec it could make sense, as old programs run on newer specs, but
that's not the case for the privileged spec.  I'd start with just "strict" and
"permissive", if we need a third we can add one.

It's also probably best to split out the privileged and user specs for this
sort of argument, as they really have different constraints.

>> Presumably the other priv instructions should also be triggering the attribute?
>
> Thanks for the reminder.  This looks good.  I will add it in the future patches.
>
> Nelson