riscv: remove DL_RO_DYN_SECTION

  While working on enabling D front-end (GDC) in GCC we noticed that druntime
was segfaulting if it is linked dynamically. This was tracked to
DL_RO_DYN_SECTION.

DL_RO_DYN_SECTION lines seem to be copied from MIPS file (which is the only
user of it), but the comment doesn't apply to RISC-V. There is no such
requirement in RISC-V ABI.

	* sysdeps/riscv/ldsodefs.h: Remove DL_RO_DYN_SECTION as it is not
	required by RISC-V ABI.

Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com>
Cc: jimw@sifive.com
Cc: palmer@sifive.com
Cc: dj@delorie.com
---
 sysdeps/riscv/ldsodefs.h | 4 ----
 1 file changed, 4 deletions(-)
  

* David Abdurachmanov:

> While working on enabling D front-end (GDC) in GCC we noticed that druntime
> was segfaulting if it is linked dynamically. This was tracked to
> DL_RO_DYN_SECTION.

Is it possible to extract a test case from that which we could put into
glibc?

Since this appears to be a user-visible bug, it needs a bug in
Bugzilla.  Would you please file one?

Thanks,
Florian
  

On Tue, Apr 9, 2019 at 4:42 AM Florian Weimer <fweimer@redhat.com> wrote:
> * David Abdurachmanov:

Do you or your employer have copyright assignments?  For a trivial
patch like this, a copyright assignment shouldn't be necessary, but
would be necessary if you want to contribute more patches.

> > While working on enabling D front-end (GDC) in GCC we noticed that druntime
> > was segfaulting if it is linked dynamically. This was tracked to
> > DL_RO_DYN_SECTION.

My analysis of this when it first came up...

I believe this was blindly copied from MIPS, and that no one on the
RISC-V side will know why it is there, because it was blindly copied
from MIPS.

Looking at the MIPS SVR4 ABI, I see
.dynamic This is the same as the generic ABI section of the same type, but
the MIPS-specific version does not include the SHF_WRITE at-
tribute.

I don't see any other reason in the spec for this to be read-only, but
making it read-only probably improves program security.  I don't see
an equivalent statement in the RISC-V ABI.

Looking at bfd/elfxx-mips.c _bfd_mips_elf_create_dynamic_sections(), we see code
  flags = (SEC_ALLOC | SEC_LOAD | SEC_HAS_CONTENTS | SEC_IN_MEMORY
           | SEC_LINKER_CREATED | SEC_READONLY);

  /* The psABI requires a read-only .dynamic section, but the VxWorks
     EABI doesn't.  */
  if (!htab->is_vxworks)
    {
      s = bfd_get_linker_section (abfd, ".dynamic");
      if (s != NULL)
        {
          if (! bfd_set_section_flags (abfd, s, flags))
            return FALSE;
        }
    }
that forces .dynamic to be read-only everywhere except vxworks.  I
don't see equivalent code in the RISC-V bfd port.

Looking at ld, in emulparams/, there are multiple mips scripts that
set TEXT_DYNAMIC, which forces .dynamic into the text segment instead
of the data segment.  This is then unset in the vxworks scripts.
Interestingly, hppa also sets TEXT_DYNAMIC, but the hppa support in
glibc is probably not well maintained.  Anyways, there is no
equivalent code for RISC-V.

So it appears unnecessary for the RISC-V glibc port.

If MIPS dynamic linking support is ever added to the druntime library,
then it will need the same change that you made for RISC-V, so in that
case it would not be a RISC-V specific change.  However, there aren't
a lot of people doing MIPS development work anymore, so this may never
come up.

Jim
PS Since I wrote my analysis, MIPS has been resurrected, so maybe
someone will care about that again.
  

On Tue, 09 Apr 2019 04:25:29 PDT (-0700), david.abdurachmanov@gmail.com wrote:
> While working on enabling D front-end (GDC) in GCC we noticed that druntime
> was segfaulting if it is linked dynamically. This was tracked to
> DL_RO_DYN_SECTION.
>
> DL_RO_DYN_SECTION lines seem to be copied from MIPS file (which is the only
> user of it), but the comment doesn't apply to RISC-V. There is no such
> requirement in RISC-V ABI.
>
> 	* sysdeps/riscv/ldsodefs.h: Remove DL_RO_DYN_SECTION as it is not
> 	required by RISC-V ABI.
>
> Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com>
> Cc: jimw@sifive.com
> Cc: palmer@sifive.com
> Cc: dj@delorie.com
> ---
>  sysdeps/riscv/ldsodefs.h | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h
> index f46cf4158e..5ec607e867 100644
> --- a/sysdeps/riscv/ldsodefs.h
> +++ b/sysdeps/riscv/ldsodefs.h
> @@ -38,10 +38,6 @@ struct La_riscv_retval;
>  				       struct La_riscv_retval *,	\
>  				       const char *);
>
> -/* The RISC-V ABI specifies that the dynamic section has to be read-only.  */
> -
> -#define DL_RO_DYN_SECTION 1
> -
>  #include_next <ldsodefs.h>
>
>  #endif

Thanks.  I went ahead and pushed it without a test case so we don't lose the
fix.  I also opened a Bugzilla:

https://sourceware.org/bugzilla/show_bug.cgi?id=24484
  

On Tue, 9 Apr 2019, Jim Wilson wrote:

> I believe this was blindly copied from MIPS, and that no one on the
> RISC-V side will know why it is there, because it was blindly copied
> from MIPS.
> 
> Looking at the MIPS SVR4 ABI, I see
> .dynamic This is the same as the generic ABI section of the same type, but
> the MIPS-specific version does not include the SHF_WRITE at-
> tribute.
> 
> I don't see any other reason in the spec for this to be read-only, but
> making it read-only probably improves program security.  I don't see
> an equivalent statement in the RISC-V ABI.

 For the record the MIPS port can get away with a read-only dynamic 
segment (which is indeed somewhat advantageous security-wise), because it 
does not use the DT_DEBUG entry which requires an update at runtime and 
relies on DT_MIPS_RLD_MAP and (more recently for PIE) DT_MIPS_RLD_MAP_REL 
instead.  For DSOs DT_DEBUG is not used and therefore the dynamic segment 
can be read-only regardless I believe.

 Maybe we can arrange for a read-only dynamic segment in the RISC-V ABI 
after all?  At the time DT_MIPS_RLD_MAP_REL was being crafted I advocated 
for a generic tag[1], but the idea was rejected.  We can make our own 
processor-specific tag, such as DT_RISCV_RLD_MAP_REL, or reconsider my 
idea of DT_GNU_RLD_MAP (or DT_GNU_RLD_MAP_REL).

References:

[1] "Support shared library debug with MIPS PIE", 
    <https://sourceware.org/ml/binutils/2015-06/msg00227.html>

 HTH,

  Maciej