[OpenRISC] [PATCH v3 04/13] or1k: startup and dynamic linking code

Adhemerval Zanella
2021-12-20T19:45:21Z

On 17/12/2021 20:03, Stafford Horne wrote:
> On Thu, Dec 16, 2021 at 07:42:46AM -0300, Adhemerval Zanella wrote:
>>
>>
>> On 10/12/2021 20:34, Stafford Horne via Libc-alpha wrote:
>>> Code for C runtime startup and dynamic loading including PLT layout.
>>
>> Patch looks ok, although I comment much on ABI and assembly code. Some
>> comments below.
>>
>>> ---
>>> sysdeps/or1k/bits/link.h | 51 ++++++
>>> sysdeps/or1k/dl-machine.h | 323 +++++++++++++++++++++++++++++++++++++
>>> sysdeps/or1k/dl-start.S | 98 +++++++++++
>>> sysdeps/or1k/ldsodefs.h | 40 +++++
>>> sysdeps/or1k/sotruss-lib.c | 51 ++++++
>>> sysdeps/or1k/start.S | 99 ++++++++++++
>>> sysdeps/or1k/tst-audit.h | 24 +++
>>> 7 files changed, 686 insertions(+)
>>> create mode 100644 sysdeps/or1k/bits/link.h
>>> create mode 100644 sysdeps/or1k/dl-machine.h
>>> create mode 100644 sysdeps/or1k/dl-start.S
>>> create mode 100644 sysdeps/or1k/ldsodefs.h
>>> create mode 100644 sysdeps/or1k/sotruss-lib.c
>>> create mode 100644 sysdeps/or1k/start.S
>>> create mode 100644 sysdeps/or1k/tst-audit.h
>>>
>>> diff --git a/sysdeps/or1k/bits/link.h b/sysdeps/or1k/bits/link.h
>>> new file mode 100644
>>> index 0000000000..ad183c9625
>>> --- /dev/null
>>> +++ b/sysdeps/or1k/bits/link.h
>>> @@ -0,0 +1,51 @@
>>> +/* Declarations for dynamic linker interface. OpenRISC version.
>>> + Copyright (C) 2021 Free Software Foundation, Inc.
>>> + This file is part of the GNU C Library.
>>> +
>>> + The GNU C Library is free software; you can redistribute it and/or
>>> + modify it under the terms of the GNU Lesser General Public
>>> + License as published by the Free Software Foundation; either
>>> + version 2.1 of the License, or (at your option) any later version.
>>> +
>>> + The GNU C Library is distributed in the hope that it will be useful,
>>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + Lesser General Public License for more details.
>>> +
>>> + You should have received a copy of the GNU Lesser General Public
>>> + License along with the GNU C Library. If not, see
>>> + <https://www.gnu.org/licenses/>. */
>>> +
>>> +#ifndef _LINK_H
>>> +# error "Never include <bits/link.h> directly; use <link.h> instead."
>>> +#endif
>>> +
>>> +/* Registers for entry into PLT. */
>>> +typedef struct La_or1k_regs
>>> +{
>>> + uint32_t lr_reg[31];
>>> +} La_or1k_regs;
>>> +
>>
>> So is the intent is to provide caller all the register, no only the caller-saved
>> registers?
>
> As I see it...
>
> Shouldn't these usually be the argument registers used in la_pltenter to print
> our the function call? i.e.
>
> print_enter (refcook, defcook, symname,
> regs->lr_reg[0], regs->lr_reg[1], regs->lr_reg[2],
> *flags);
>
> These registers get setup in sysdeps/{arch}/dl-trampoline.S by
> _dl_runtime_profile, which then pass them to _dl_profile_fixup.
>
> The problem is openrisc doesn't implement _dl_runtime_profile so we haven't
> defined what the registers are.
>
> I noticed the same for arc, csky, riscv.
>
> These are needed to be defined for sotruss, but I dont seem them being used.
>
> Is that correct?

The register are really dependend of the function calling ABI of the architecture,
if you check other architecture like x86 and powerpc you will see that La_i86_regs
and La_ppc64_regs only saves the caller-saved ones. It is mainly because for
PLT tracking, as done by audit modules, other register are not really useful
(since they are just internal ones to the caller function).

>
>>> +/* Return values for calls from PLT. */
>>> +typedef struct La_or1k_retval
>>> +{
>>> + uint32_t lrv_r3;
>>> +} La_or1k_retval;
>>> +
>>> +__BEGIN_DECLS
>>> +
>>> +extern ElfW(Addr) la_or1k_gnu_pltenter (ElfW(Sym) *__sym, unsigned int __ndx,
>>> + uintptr_t *__refcook,
>>> + uintptr_t *__defcook,
>>> + La_or1k_regs *__regs,
>>> + unsigned int *__flags,
>>> + const char *__symname,
>>> + long int *__framesizep);
>>> +extern unsigned int la_or1k_gnu_pltexit (ElfW(Sym) *__sym, unsigned int __ndx,
>>> + uintptr_t *__refcook,
>>> + uintptr_t *__defcook,
>>> + const La_or1k_regs *__inregs,
>>> + La_or1k_retval *__outregs,
>>> + const char *__symname);
>>> +
>>> +__END_DECLS
>>> diff --git a/sysdeps/or1k/dl-machine.h b/sysdeps/or1k/dl-machine.h
>>> new file mode 100644
>>> index 0000000000..d41554769e
>>> --- /dev/null
>>> +++ b/sysdeps/or1k/dl-machine.h
>>> @@ -0,0 +1,323 @@
>>> +/* Machine-dependent ELF dynamic relocation inline functions. OpenRISC version.
>>> + Copyright (C) 2021 Free Software Foundation, Inc.
>>> + This file is part of the GNU C Library.
>>> +
>>> + The GNU C Library is free software; you can redistribute it and/or
>>> + modify it under the terms of the GNU Lesser General Public
>>> + License as published by the Free Software Foundation; either
>>> + version 2.1 of the License, or (at your option) any later version.
>>> +
>>> + The GNU C Library is distributed in the hope that it will be useful,
>>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + Lesser General Public License for more details.
>>> +
>>> + You should have received a copy of the GNU Lesser General Public
>>> + License along with the GNU C Library; if not, see
>>> + <https://www.gnu.org/licenses/>. */
>>> +
>>> +#ifndef dl_machine_h
>>> +#define dl_machine_h
>>> +
>>> +#define ELF_MACHINE_NAME "or1k"
>>> +
>>> +#include <sys/cdefs.h>
>>> +#include <sys/param.h>
>>> +#include <tls.h>
>>> +#include <dl-irel.h>
>>> +#include <dl-static-tls.h>
>>> +#include <dl-machine-rel.h>
>>> +
>>> +/* Return nonzero iff ELF header is compatible with the running host. */
>>> +static inline int __attribute__ ((unused))
>>> +elf_machine_matches_host (const Elf32_Ehdr *ehdr)
>>> +{
>>> + return ehdr->e_machine == EM_OPENRISC;
>>> +}
>>> +
>>> +static inline Elf32_Addr *
>>> +or1k_get_got (void)
>>> +{
>>> + Elf32_Addr *got;
>>> + register long int linkreg asm ("r9");
>>
>> Is the 'r9' a required register by the ABI to use on this assembly snippet?
>> I am asking because local name 'registers' are only a hint to gcc, as we
>> found recently [1], if you really need 'r9' you will probably need to use
>> a global name resgister.
>
> The r9 is to avoid the snippet from clobbering r9. The result of l.jal will
> update r9 (link register) we want to make sure that is what we used for %1.
>
> I think there were issues with other ways I tried.

Should you add it the clobbered list instead?

>
> Let me look into it more.
>
>>> + asm ("l.jal 0x8\n"
>>> + " l.movhi %0, gotpchi(_GLOBAL_OFFSET_TABLE_-4)\n"
>>> + "l.ori %0, %0, gotpclo(_GLOBAL_OFFSET_TABLE_+0)\n"
>>> + "l.add %0, %0, %1\n"
>>> + : "=r" (got), "=r" (linkreg));

With something like (unstested):

static inline Elf32_Addr *
or1k_get_got (void)
{
Elf32_Addr *got;
asm ("l.jal 0x8\n"
" l.movhi %0, gotpchi(_GLOBAL_OFFSET_TABLE_-4)\n"
"l.ori %0, %0, gotpclo(_GLOBAL_OFFSET_TABLE_+0)\n"
"l.add %0, %0, %1\n"
: "=r" (got)
:
: "r9");
}