This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] [RFC] Add IFUNC support for MIPS (v4)

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.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]