public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
From: Mark Wielaard <mark@klomp.org>
To: Ying Huang <ying.huang@oss.cipunited.com>
Cc: elfutils-devel@sourceware.org, yunqiang.su@oss.cipunited.com
Subject: Re: [PATCH 1/5] strip: Adapt src/strip -o -f on mips
Date: Sun, 21 May 2023 23:13:02 +0200	[thread overview]
Message-ID: <20230521211302.GH3420@gnu.wildebeest.org> (raw)
In-Reply-To: <9035496c-3414-ac66-bd86-281592aa2a77@oss.cipunited.com>

Hi Ying,

On Tue, May 16, 2023 at 02:38:45PM +0800, Ying Huang wrote:
> 在 2023/5/9 23:15, Mark Wielaard 写道:
> > On Tue, 2023-04-11 at 16:12 +0800, Ying Huang wrote:
> >> In mips64 little-endian, r_info consists of four byte fields(contains
> >> three reloc types) and a 32-bit symbol index. In order to adapt
> >> GELF_R_SYM and GELF_R_TYPE, need convert raw data to get correct symbol
> >> index and type.
> > Is there a spec that describes this?
>    
> 
>  references:
> 
>     https://www.linux-mips.org/pub/linux/mips/doc/ABI/elf64-2.4.pdf
> 
>     Page40 && Page41

Thanks. Interesting. If possible please include this URL in a comment.

> > I see you adjusted elf.h to include:
> >
> > +#define ELF64_MIPS_R_TYPE(i)		((i) & 0xff)
> > +#define ELF64_MIPS_R_TYPE2(i)           (((i) >> 8) & 0xff)
> > +#define ELF64_MIPS_R_TYPE3(i)           (((i) >> 16) & 0xff)
> >
> > And various new relocation types for MIPS variants.
> > Our elf.h comes from the glibc project. Have you also submitted these
> > changes to libc-alpha@sourceware.org ?
>     Has submitted.
> 
>     https://sourceware.org/pipermail/libc-alpha/2023-May/148026.html
> 
>     https://sourceware.org/pipermail/libc-alpha/2023-May/148112.html

Thanks. We'll sync them as soon as v3 lands.

>     Has tested on big/little endian machine with big/little endian
> ELF file, all were OK.

Could you post those ELF files somewhere?
Maybe we can add some as test files?

> > It feels like this is in the wrong place and doesn't take into account
> > whether the program itself is running on a big or little endian
> > machine.
> >
> > Maybe I am misunderstanding why this conversion is needed always. But I
> > would have expected a specific conversion function for MIPS for
> > ELF_T_REL and/or ELF_T_RELA (which are currently generic).
>
>     1.Considering that some people directly use the original data instead of the interface function gelf_getrela;
> 
>     2.Because there is only one parameter, so can not modify the macro ELF64_R_TYPE;
> 
>     3.If the mips interface function is added, other packages using libelf will not be compatible with mips, and the place where gelf_getrela is used need be modified.
> 
>     Where do you think is the most suitable place to do mips conversion?

So for the convert_data case in elf_getdata.c I would expect a special
conversion function being defined for ELF_T_REL and ELF_T_RELA. This
normally comes from __elf_xfctstom. I realize now that cannot easily
take an Elf of machine type. But I would expect to have the conversion
be done by a new function that can be assigned to fp in convert_data:

      /* Get the conversion function.  */
      fp = __elf_xfctstom[eclass - 1][type];

I still don't fully understand the need of the code in elf_update. I
assume that is needed in case the file was used with ELF_C_RDWR_MMAP?

Cheers,

Mark

  reply	other threads:[~2023-05-21 21:13 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-11  8:12 [PATCH 0/5] Add support for MIPS Ying Huang
2023-04-11  8:12 ` [PATCH 1/5] strip: Adapt src/strip -o -f on mips Ying Huang
2023-05-09 15:15   ` Mark Wielaard
2023-05-16  6:38     ` Ying Huang
2023-05-21 21:13       ` Mark Wielaard [this message]
2023-05-24  6:21         ` Ying Huang
2023-05-16  6:46     ` Ying Huang
2023-05-16  7:34       ` Ying Huang
2023-05-21 21:14         ` Mark Wielaard
2023-05-26  2:48           ` Ying Huang
2023-05-16  8:05     ` [EXTERNAL] " Luke Diamand
2023-04-11  8:12 ` [PATCH 2/5] readelf: Adapt src/readelf -h/-S/-r/-w/-l/-d/-a " Ying Huang
2023-05-11 14:31   ` Mark Wielaard
2023-05-16  8:01     ` Ying Huang
2023-07-24  8:35     ` Ying Huang
2023-07-25  8:15       ` Ying Huang
2023-07-27  6:08         ` Ying Huang
2023-08-01 21:43           ` Mark Wielaard
2023-08-01 13:14         ` Mark Wielaard
2023-08-01  9:25       ` Mark Wielaard
2023-04-11  8:12 ` [PATCH 3/5] elflint: Fix invalid type of relocation info and other issues " Ying Huang
2023-05-11 15:59   ` Mark Wielaard
2023-05-17  9:14     ` Ying Huang
2023-04-11  8:12 ` [PATCH 4/5] stack: Fix stack unwind failure " Ying Huang
2023-05-11 16:07   ` Mark Wielaard
2023-05-18  6:14     ` Ying Huang
2023-04-11  8:12 ` [PATCH 5/5] backends: Fix run-native-test.sh and run-funcretval++11.sh run fail " Ying Huang
2023-05-11 16:38   ` Mark Wielaard
2023-05-18  9:06     ` Ying Huang
2023-05-04  2:24 ` [PATCH 0/5] Add support for MIPS 黄莺

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230521211302.GH3420@gnu.wildebeest.org \
    --to=mark@klomp.org \
    --cc=elfutils-devel@sourceware.org \
    --cc=ying.huang@oss.cipunited.com \
    --cc=yunqiang.su@oss.cipunited.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).