diff mbox

riscv: restore ABI compatibility (bug 24484)

Message ID mvmlfxefbqq.fsf@suse.de
State Committed
Commit 484b7af3ccb782ccb3491b541a68de9c5d6f063b
Headers

Commit Message

Andreas Schwab July 4, 2019, 8:24 a.m. UTC 
  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(+)

Comments

Florian Weimer July 4, 2019, 8:28 a.m. UTC | #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. UTC | #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. UTC | #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.
Jim Wilson July 4, 2019, 8:41 a.m. UTC | #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. UTC | #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. UTC | #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.
Szabolcs Nagy July 4, 2019, 12:42 p.m. UTC | #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. UTC | #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.
Palmer Dabbelt July 4, 2019, 12:51 p.m. UTC | #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!
diff mbox

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