This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 2/2] [RFC] Add IFUNC support for MIPS (v4)
- From: "Maciej W. Rozycki" <macro at imgtec dot com>
- To: Faraz Shahbazker <faraz dot shahbazker at imgtec dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, <rdsandiford at googlemail dot com>
- Date: Sun, 6 Mar 2016 07:18:18 +0000
- Subject: Re: [PATCH 2/2] [RFC] Add IFUNC support for MIPS (v4)
- Authentication-results: sourceware.org; auth=none
- References: <5583540C dot 7070800 at imgtec dot com> <87381jtr31 dot fsf at googlemail dot com> <55899D52 dot 1050000 at imgtec dot com> <87vbeegucz dot fsf at googlemail dot com> <5589AFCD dot 10905 at imgtec dot com> <DCB1C42372B1674B8F912A294CCB775A71680718 at BADAG02 dot ba dot imgtec dot org> <87615awnv8 dot fsf at googlemail dot com> <5600517C dot 1030608 at imgtec dot com> <874mig14xs dot fsf at googlemail dot com> <561D2820 dot 10107 at imgtec dot com> <871tc0acam dot fsf at googlemail dot com> <5678829D dot 4080108 at imgtec dot com> <877fk6jewx dot fsf at googlemail dot com> <568EFE8A dot 5060404 at imgtec dot com>
Hi Faraz,
> Test cases
>
> ld/testsuite/ChangeLog:
>
> * ld-ifunc/ifunc.exp: Enable IFUNC tests for MIPS targets
> * ld-mips-elf/mips-ifunc.exp: IFUNC test script
Please use full stops at the end of sentences.
> * ifunc-3-n32.r, ifunc-3-n32.sym, ifunc-3-n32.t, ifunc-3-n64.r,
... and so on, and so on -- you're mixing test case sources and dumps.
Please separate them and group functionally. Please include subdirectory
names and keep one file per line. Using a shell script to generate such a
listing is probably the easiest way. See the existing examples.
> diff --git a/ld/testsuite/ld-mips-elf/ifunc-10-o32.sec b/ld/testsuite/ld-mips-elf/ifunc-10-o32.sec
> new file mode 100644
> index 0000000..8f68890
> --- /dev/null
> +++ b/ld/testsuite/ld-mips-elf/ifunc-10-o32.sec
> @@ -0,0 +1,5 @@
> +#...
> +.* \.iplt .*
> +.*
> +.* \.igot .*
> +#pass
> \ No newline at end of file
Missing new line here, please fix this up. Lots of cases like this
throughout, I won't be listing them all.
If your editor does not automatically add a new-line character at the end
by default, then I suggest that you review your patches manually before
submitting -- it may be easier for you to strip these lines with `sed' on
an otherwise ready patch file actually.
> diff --git a/ld/testsuite/ld-mips-elf/ifunc-11-o32.g b/ld/testsuite/ld-mips-elf/ifunc-11-o32.g
> new file mode 100644
> index 0000000..2d42af5
> --- /dev/null
> +++ b/ld/testsuite/ld-mips-elf/ifunc-11-o32.g
> @@ -0,0 +1,7 @@
> +#...
> +004101a0 <_GLOBAL_OFFSET_TABLE_>:
> + 4101a0: 00000000 nop
> + 4101a4: 80000000 lb zero,0\(zero\)
> + 4101a8: 00400000 0x400000
> + 4101ac: 00400170 0x400170
> +#pass
This dump does not make sense to me, you are disassembling data. If you
want to verify GOT contents, then use `readelf -A'; it'll add extra
information like symbol names to verify too. Again, lots of other cases
like this.
Please resubmit with these global corrections; I haven't gone through the
rest of your proposal as especially the last change will make your patch
substantially different.
Thanks,
Maciej