Re: [[RESEND & PING]PATCH V3 1/1] Add backend support for C-SKY

[Mark Wielaard <mark@klomp.org>]

On Mon, 2019-07-15 at 16:55 +0800, Mao Han wrote:
> I can't find the origin mail thread in my mailbox, as my mutt can
> only keep the newest 5000 mails. So I just reply in a new thread.

Sorry about taking so long to go over these patches.
Thanks for answering the questions. I do have the older emails and I do
see I did ask some of these earlier and you already had answered them.

Most of my confusion seems to come back to this RCE/MCORE vs CSKY
thing.

> The ELF_ARCH was defined as 39 inside linux kernel(before 4.20), so
> the the core file generated was EM_RCE.
> I'v regenerated the core file with ELF_ARCH defined as 252:
> https://github.com/c-sky/test-result/blob/master/elfutils/zeroptr
> https://github.com/c-sky/test-result/raw/master/elfutils/core.zeroptr
> We have extended our core dump reg set last year, linux kernel have
> upstreamed this change, but the gdb does not. I'v only got an old
> released toolchain(EM_RCE) and the upstream toolchain(out of date
> reg set) in my enviroment, so I can't generate/verify EM_CSKY core
> file with any toolchain I have.

So the zeroptr binary looks correct now.
But the core.zeroptr is probably generated by an older linux kernel and
still is EM_RCE. This confusion is a bit confusing since I makes it
really hard to check the details (register numbering, etc.) are really
correct. I can tweak the code a bit to accept EM_RCE as EM_CSKY but
then the PRSTATUS core note doesn't look correct. I am not sure that is
because the register set as dumped by the kernel is bogus or because
the prstatus_regs definition in csky_corenote.c is incorrect.

> > > 5. run-reverse-sections-self.sh
> > > elfcopy --reverse-offs dose not work properly on C-SKY object
> > > file,
> > > [...]
> This case can pass with the newest elfutils. I believe it is fixed
> with commit d08c68.

Cool.

> > We can list the v1 variants also, their numbers don't overlap.
> > Just checking it is done deliberately. If they can never occur in
> > v2
> > ELF binaries I would leave them out.
> 
> The content of the ABI manual is quite old, binutils souce code seems
> much reliable to refer to. binutils/bfd/elf32-csky.c says,
> only three relocations are abiv1 exclusive:
> R_CKCORE_PCREL_JSR_IMM11BY2
> R_CKCORE_PCREL_IMM8BY4
> R_CKCORE_PCREL_IMM11BY2
> 
> The following relocations can occur in v2 ELF binaries.
> R_CKCORE_PCREL32
> R_CKCORE_GOTOFF
> R_CKCORE_GOTPC
> R_CKCORE_GOT32
> R_CKCORE_PLT32
> R_CKCORE_ADDRGOT
> R_CKCORE_ADDRPLT
> 
> R_CKCORE_GOTOFF_IMM18 seems unused at present.

OK. It is fine as is then. Just wanted to make sure it was as intended.

> > On Thu, 2019-07-11 at 11:19 +0800, Mao Han wrote:
> > > On Thu, Jul 11, 2019 at 12:11:17AM +0200, Mark Wielaard wrote:
> > > > As far as I can see this is like a SHT_GNU_ATTRIBUTES or
> > > > SHT_ARM_ATTRIBUTES section. If so, you will also want something
> > > > like:
> > > > 
> > > > diff --git a/src/readelf.c b/src/readelf.c
> > > > index cda7c93..8017dc4 100644
> > > > --- a/src/readelf.c
> > > > +++ b/src/readelf.c
> > > > @@ -3571,7 +3571,9 @@ print_attributes (Ebl *ebl, const
> > > > GElf_Ehdr
> > > > *ehdr)
> > > > 
> > > >        if (shdr == NULL || (shdr->sh_type != SHT_GNU_ATTRIBUTES
> > > >                            && (shdr->sh_type !=
> > > > SHT_ARM_ATTRIBUTES
> > > > -                              || ehdr->e_machine != EM_ARM)))
> > > > +                              || ehdr->e_machine != EM_ARM)
> > > > +                          && (shdr->sh_type !=
> > > > SHT_CSKY_ATTRIBUTES
> > > > +                              || ehdr->e_machine != EM_CSKY)))
> > > >         continue;
> > > > 
> > > >        printf (gettext ("\
> > > > 
> > > > And then hook check_object_attribute to display any CSKY
> > > > specific
> > > > attribute names/values.
> > > 
> > > Ok, I'll add check for C-SKY here.
> > 
> > BTW. I couldn't find what generates the SHT_CSKY_ATTRIBUTES or what
> > the
> > defined attribute constants are. Are they documented somewhere?
> 
> These attributes including cpu name and some other ISA related
> descriptions.
> Some thing like:
> CSKY_ARCH_NAME: "ck810"
> CSKY_CPU_NAME:  "ck810f"
> CSKY_ISA_FLAG:  0x12345678
> CSKY_ISA_EXT_FLAG:  5
> They are not documented yet.
> I'v ask the person who is responsible for these to update the ABI
> documents, but I think it will take a quite long time for them to
> do that. They are quite busy at present.

OK. If you can add that tweak to src/readelf.c and add an
check_object_attribute hook that handles the above attributes that
would be good.

Ideally you also add a testcase for tests/readelf-A.sh
Some of those tests cheat and create the attributes by hand.
But it would be nice if you could generate a small .o file with the
latest toolchain to be used as testcase in some other tests.

Look for example at tests/run-addrcfi.sh. Which has this somewhat
"standard" test object file:

# = bar.c =
#
# static int b1 = 1;
# int b2 = 1;
#
# static int
# foo (int a)
# {
#   return a + b2;
# }
#
# int bar (int b)
# {
#   return b - foo (b - b1);
# }
#
# = foo.c =
#
# extern int bar (int b);
# extern int b2;
#
# int
# main (int argc, char ** argv)
# {
#   return bar (argc + b2);
# }
#
# gcc -g -O2 -m32 -c foo.c
# gcc -g -O2 -m32 -c bar.c
# gcc -g -O2 m32 -o testfileppc32 foo.o bar.o

If you could generate a 'testfilecsky' that could then be used
as test for tests/run-addrcfi.sh to check register mapping and CFI
rules.

If you are able to generate a core file with a current kernel that does
properly set the EM value and register PR_STATUS then it would also be
nice to have a testfile-csky-core for use in tests/run-readelf-mixed-
corenote.sh and tests/run-allregs.sh.

And to test to debug relocation resolving you might want to generate a
small kernel module for tests/run-strip-reloc.sh (there are
instructions in the test how to generate such a module).

If it is possible to generate such testcase binaries then it should
also be easier to test the csky backend on other arches.

Thanks,

Mark