riscv: restore ABI compatibility (bug 24484)

Message ID mvmlfxefbqq.fsf@suse.de
State New
Headers show
Series
  • riscv: restore ABI compatibility (bug 24484)
Related show

Commit Message

Andreas Schwab July 4, 2019, 8:24 a.m.
The contents of the dynamic section are part of the ABI, thus
DL_RO_DYN_SECTION cannot be changed.

	[BZ #24484]
	* sysdeps/riscv/ldsodefs.h (DL_RO_DYN_SECTION): Define.
---
 sysdeps/riscv/ldsodefs.h | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.22.0


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Comments

Florian Weimer July 4, 2019, 8:28 a.m. | #1
* Andreas Schwab:

> The contents of the dynamic section are part of the ABI, thus

> DL_RO_DYN_SECTION cannot be changed.

>

> 	[BZ #24484]

> 	* sysdeps/riscv/ldsodefs.h (DL_RO_DYN_SECTION): Define.

> ---

>  sysdeps/riscv/ldsodefs.h | 5 +++++

>  1 file changed, 5 insertions(+)

>

> diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h

> index 5ec607e867..d7531b707a 100644

> --- a/sysdeps/riscv/ldsodefs.h

> +++ b/sysdeps/riscv/ldsodefs.h

> @@ -38,6 +38,11 @@ struct La_riscv_retval;

>  				       struct La_riscv_retval *,	\

>  				       const char *);

>  

> +/* Although the RISC-V ABI does not specify that the dynamic section has

> +   to be read-only, it needs to be kept for ABI compatibility.  */

> +

> +#define DL_RO_DYN_SECTION 1


Perhaps indicate somewhere that “a read-only dynamic section is not
relocated by the loader” or something like that, so that someone not
familiar with the MIPS mechanism understands the ABI difference?

I agree that restoring the #define is probably necessary at this
point. 8-(

Thanks,
Florian
Jim Wilson July 4, 2019, 8:32 a.m. | #2
On Thu, Jul 4, 2019 at 4:24 PM Andreas Schwab <schwab@suse.de> wrote:
> The contents of the dynamic section are part of the ABI, thus

> DL_RO_DYN_SECTION cannot be changed.

>

>         [BZ #24484]

>         * sysdeps/riscv/ldsodefs.h (DL_RO_DYN_SECTION): Define.


This is OK with me, though I'm not a glibc maintainer.

Jim
Andreas Schwab July 4, 2019, 8:37 a.m. | #3
On Jul 04 2019, Florian Weimer <fweimer@redhat.com> wrote:

> Perhaps indicate somewhere that “a read-only dynamic section is not

> relocated by the loader” or something like that, so that someone not

> familiar with the MIPS mechanism understands the ABI difference?


I think we should make DL_RO_DYN_SECTION the default for new ports, like
everyone else does.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Jim Wilson July 4, 2019, 8:41 a.m. | #4
On Thu, Jul 4, 2019 at 4:28 PM Florian Weimer <fweimer@redhat.com> wrote:
> Perhaps indicate somewhere that “a read-only dynamic section is not

> relocated by the loader” or something like that, so that someone not

> familiar with the MIPS mechanism understands the ABI difference?


You could point at the D_PTR macro definition in
sysdeps/generic/ldsodefs.h.  Or if you want a user code example there
is
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libphobos/libdruntime/gcc/sections/elf_shared.d;h=7f9036bf5052861960f8a1d4a20838437c5c519c;hb=HEAD#l758

Jim
Szabolcs Nagy July 4, 2019, 11:48 a.m. | #5
On 04/07/2019 09:37, Andreas Schwab wrote:
> On Jul 04 2019, Florian Weimer <fweimer@redhat.com> wrote:

> 

>> Perhaps indicate somewhere that “a read-only dynamic section is not

>> relocated by the loader” or something like that, so that someone not

>> familiar with the MIPS mechanism understands the ABI difference?

> 

> I think we should make DL_RO_DYN_SECTION the default for new ports, like

> everyone else does.


then DT_DEBUG does not work.

there is some mips hack to make it work.
i suspect the debug abi is broken on riscv now.
Andreas Schwab July 4, 2019, 12:31 p.m. | #6
On Jul 04 2019, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:

> On 04/07/2019 09:37, Andreas Schwab wrote:

>> On Jul 04 2019, Florian Weimer <fweimer@redhat.com> wrote:

>> 

>>> Perhaps indicate somewhere that “a read-only dynamic section is not

>>> relocated by the loader” or something like that, so that someone not

>>> familiar with the MIPS mechanism understands the ABI difference?

>> 

>> I think we should make DL_RO_DYN_SECTION the default for new ports, like

>> everyone else does.

>

> then DT_DEBUG does not work.


In which way?

> i suspect the debug abi is broken on riscv now.


I don't think so.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Szabolcs Nagy July 4, 2019, 12:42 p.m. | #7
On 04/07/2019 13:31, Andreas Schwab wrote:
> On Jul 04 2019, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:

> 

>> On 04/07/2019 09:37, Andreas Schwab wrote:

>>> On Jul 04 2019, Florian Weimer <fweimer@redhat.com> wrote:

>>>

>>>> Perhaps indicate somewhere that “a read-only dynamic section is not

>>>> relocated by the loader” or something like that, so that someone not

>>>> familiar with the MIPS mechanism understands the ABI difference?

>>>

>>> I think we should make DL_RO_DYN_SECTION the default for new ports, like

>>> everyone else does.

>>

>> then DT_DEBUG does not work.

> 

> In which way?


so is .dynamic actually mapped readonly?
or is this DL_RO_DYN_SECTION just about the 'relocated DT_ entries' abi?

if it's ro then ldso cannot set DT_DEBUG that the debugger looks at.
i think on mips the debugger looks at a different tag that has an indirection.


>> i suspect the debug abi is broken on riscv now.

> 

> I don't think so.

> 

> Andreas.

>
Andreas Schwab July 4, 2019, 12:46 p.m. | #8
On Jul 04 2019, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:

> On 04/07/2019 13:31, Andreas Schwab wrote:

>> On Jul 04 2019, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:

>> 

>>> On 04/07/2019 09:37, Andreas Schwab wrote:

>>>> On Jul 04 2019, Florian Weimer <fweimer@redhat.com> wrote:

>>>>

>>>>> Perhaps indicate somewhere that “a read-only dynamic section is not

>>>>> relocated by the loader” or something like that, so that someone not

>>>>> familiar with the MIPS mechanism understands the ABI difference?

>>>>

>>>> I think we should make DL_RO_DYN_SECTION the default for new ports, like

>>>> everyone else does.

>>>

>>> then DT_DEBUG does not work.

>> 

>> In which way?

>

> so is .dynamic actually mapped readonly?


No.  You would have to reorganize the segments to be able to do that.

> or is this DL_RO_DYN_SECTION just about the 'relocated DT_ entries' abi?


Yes.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Palmer Dabbelt July 4, 2019, 12:51 p.m. | #9
On Thu, 04 Jul 2019 01:24:45 PDT (-0700), schwab@suse.de wrote:
> The contents of the dynamic section are part of the ABI, thus

> DL_RO_DYN_SECTION cannot be changed.

>

> 	[BZ #24484]

> 	* sysdeps/riscv/ldsodefs.h (DL_RO_DYN_SECTION): Define.

> ---

>  sysdeps/riscv/ldsodefs.h | 5 +++++

>  1 file changed, 5 insertions(+)

>

> diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h

> index 5ec607e867..d7531b707a 100644

> --- a/sysdeps/riscv/ldsodefs.h

> +++ b/sysdeps/riscv/ldsodefs.h

> @@ -38,6 +38,11 @@ struct La_riscv_retval;

>  				       struct La_riscv_retval *,	\

>  				       const char *);

>

> +/* Although the RISC-V ABI does not specify that the dynamic section has

> +   to be read-only, it needs to be kept for ABI compatibility.  */

> +

> +#define DL_RO_DYN_SECTION 1

> +

>  #include_next <ldsodefs.h>

>

>  #endif


Sorry, I thought I'd committed this already.  You're welcome to commit, my
email is still a mess.

Thanks for picking this up!

Patch

diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h
index 5ec607e867..d7531b707a 100644
--- a/sysdeps/riscv/ldsodefs.h
+++ b/sysdeps/riscv/ldsodefs.h
@@ -38,6 +38,11 @@  struct La_riscv_retval;
 				       struct La_riscv_retval *,	\
 				       const char *);
 
+/* Although the RISC-V ABI does not specify that the dynamic section has
+   to be read-only, it needs to be kept for ABI compatibility.  */
+
+#define DL_RO_DYN_SECTION 1
+
 #include_next <ldsodefs.h>
 
 #endif