[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Wed, 3 Feb 2016 18:36:57 -0500
From: Rich Felker <dalias@...c.org>
To: Mahesh Bodapati <Mahesh.Bodapati@...tec.com>
Cc: Szabolcs Nagy <nsz@...t70.net>,
Jaydeep Patil <Jaydeep.Patil@...tec.com>,
Anand Takale <Anand.Takale@...tec.com>, musl@...ts.openwall.com
Subject: Re: mips n64 porting review
On Wed, Feb 03, 2016 at 03:41:13PM +0000, Mahesh Bodapati wrote:
> Hi Rich,
> I have attached the patch which has all the MIPS n64 porting work. I
> have created mips64port remote branch on GitHub and the repository
> is https://github.com/MaheshBodapati/musl/tree/mips64port which has
> the broken down patches and the base revision on which I have
> prepared patch is v1.1.12-41-g3abb094.
Some preliminary review:
The relocation support commit still needs a lot of work:
https://github.com/MaheshBodapati/musl/commit/55147891bc0caf757bb0e00a81d550aadc71750c
The good news is, I think it's mostly -'s that it needs. I was puzzled
at first by all the Elf64_Mips_Rel[a] stuff, and found it seems to be
have been copied verbatim from uClibc source (and possibly indirectly
from glibc) which would be a big problem, but I don't think it's
needed anyway. Here's the relevant comment:
https://git.uclibc.org/uClibc/tree/ldso/ldso/mips/dl-sysdep.h?id=966adfe8ed0364ae32cc80d375d736298972d031#n13
In short, somebody's mental model of how MIPS64 relocations work (and
this might actually have relevance for relocations used in .o files)
was that each relocation slot stores up to 3 actual relocations. In
reality, however, the only place where "more than one relocation"
appears in the same lot for dynamic relocations is R_MIPS_REL32
followed by R_MIPS_64, which you're actually treating as one
relocation:
#define REL_SYM_OR_REL 4611 /* (R_MIPS_64 << 8) | R_MIPS_REL32 */
and this latter treatment makes a whole lot more sense. In short, I
think you could remove all of the Elf64_Mips_Rel[a] stuff and the
associated #ifdef __mips64 and just use the standard ELF reloc
processing code paths, and everything should work. If not, it should
only be minor details keeping it from working and it should be easy to
fix them in a cleaner way.
Second thing I noticed was the invasive way you're dealing with the
broken kernel stat structure for mips. This should be done the same
way it's done on 32-bit mips, with the fixup function in
syscall_arch.h.
Third, I suspect socket.h needs to be checked. The kernel usually has
the wrong types for 64-bit archs, and some endian-specific padding is
required to work around it. See how x86_64 and aarch64 do it.
Fourth, I suspect ksigaction.h is mildly wrong ([4] should be [2]) and
this is why the #ifdefs to use _NSIG/8 instead of sizeof were needed.
Fixing that should eliminate the need to change non-arch-specific
files.
Fifth, I don't see why there's a mips64-specific definition for
RLIM_INFINITY; it's the same value as the generic definition.
There are a couple other things that you've got #ifdefs for that are
probably a matter of code that's changed upstream:
pthread_cancel.c should be using the MC_PC macro, which pthread_arch.h
should define, to access the member of mcontext_t containing the
program counter. The definition from 32-bit mips should work.
A recent commit also replaced the DYNAMIC_IS_RO macro with
DT_DEBUG_INDIRECT which was part of making gdb work right on mips. I
suspect the same solution works on mips64.
The bits deduplication patch removed the need for a lot of the bits
headers. The ones that are no longer needed should be left out when we
merge.
The .sub system for subarchs (soft float) was removed and replaced by
support for .S and .c files in arch dirs, so this also needs to be
updated when merging.
I haven't gotten a chance to review everything in full detail yet but
that should be the bulk of what needs to be changed. I'll follow up
with anything else I find.
Thanks for all your work on this!
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
