RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Petar Jovanovic]

Resending the previous message in a plain text format.

> -------- Original Message --------
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> Date: Thursday, April 16, 2015 10:38 PM CEST
> From: "Moore, Catherine" <Catherine_Moore@mentor.com>
> To: "Maciej W. Rozycki" <macro@linux-mips.org>, Petar Jovanovic<petar.jovanovic@rt-rk.com>
> CC: 'Matthew Fortune' <Matthew.Fortune@imgtec.com>, "gcc-patches@gcc.gnu.org"<gcc-patches@gcc.gnu.org>
> References: <003e01d04179$ccc38bc0$664aa340$@rt-rk.com><6D39441BF12EF246A7ABCE6654B0235320FCA3F1@LEMAIL01.le.imgtec.org><000201d04723$f69b1350$e3d139f0$@rt-rk.com><000501d07865$e26f2830$a74d7890$@rt-rk.com>
> Hi Petar,
> It looks like I answered a little too quickly the first time around. I'm sorry.
> In any case, Maciej is correct. I think a link-time test should do it.
> Thanks,
> Catherine
>
> > You'd need `objdump' for doing that and there is no ready-to-use solution
> > within the GCC test suite available, you'd have to cook something up
> > yourself, perhaps starting with `[find_binutils_prog objdump]'.
> >
> > Another solution might be using an auxiliary linker script (`-Wl,-T,...'
> > or maybe just an implicit linker script will do; see the LD manual for
> > details) to place the sections the jump is made between apart manually at
> > addresses appropriate for JAL to fail. They span does not have to be large --
> > when placed correctly, even `JAL .' can jump to the wrong place, which is
> > what LD is supposed to catch as a relocation error -- so a test executable
> > successfully linked with your correction in place won't be large, it doesn't
> > have to take more than 2 virtual pages.
> >
> > The downside of the latter solution are possible portability issues with the
> > linker script. You won't have to run your executable AFAICT from glibc BZ
> > #17601 as you'll get a link error if a jump target is out of range. So you could
> > make it a mere link test, no need to run it.
> >
> > Maciej
>
>

 Hi Maciej, Catherine,

 This issue will not trigger a linker error (I believe it treats the
 symbol referred by the relocation as a local symbol). This is a follow
 up to GLIBC BZ #17601, the problem is seen only at runtime. So, I think
 this brings back the need to run the test. I can look into separating
 sections with -Wl,-T.. (that may also require extending
 mips_option_groups in mips/mips.exp), if running executable is OK.

 Regards,
 Petar  

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Moore, Catherine]

> -----Original Message-----
> From: Petar Jovanovic [mailto:Petar.Jovanovic@rt-rk.com]
> Sent: Friday, April 17, 2015 2:23 PM
> To: Petar Jovanovic
> Cc: Maciej W. Rozycki; Matthew Fortune; gcc-patches@gcc.gnu.org; Moore,
> Catherine
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> Resending the previous message in a plain text format.
> 
> > -------- Original Message --------
> > Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> > Date: Thursday, April 16, 2015 10:38 PM CEST
> > From: "Moore, Catherine" <Catherine_Moore@mentor.com>
> > To: "Maciej W. Rozycki" <macro@linux-mips.org>, Petar
> > Jovanovic<petar.jovanovic@rt-rk.com>
> > CC: 'Matthew Fortune' <Matthew.Fortune@imgtec.com>,
> > "gcc-patches@gcc.gnu.org"<gcc-patches@gcc.gnu.org>
> > References:
> > <003e01d04179$ccc38bc0$664aa340$@rt-
> rk.com><6D39441BF12EF246A7ABCE6654
> >
> B0235320FCA3F1@LEMAIL01.le.imgtec.org><000201d04723$f69b1350$e3d13
> 9f0$
> > @rt-rk.com><000501d07865$e26f2830$a74d7890$@rt-rk.com>
> > Hi Petar,
> > It looks like I answered a little too quickly the first time around. I'm sorry.
> > In any case, Maciej is correct. I think a link-time test should do it.
> > Thanks,
> > Catherine
> >
> > > You'd need `objdump' for doing that and there is no ready-to-use
> > > solution within the GCC test suite available, you'd have to cook
> > > something up yourself, perhaps starting with `[find_binutils_prog
> objdump]'.
> > >
> > > Another solution might be using an auxiliary linker script (`-Wl,-T,...'
> > > or maybe just an implicit linker script will do; see the LD manual
> > > for
> > > details) to place the sections the jump is made between apart
> > > manually at addresses appropriate for JAL to fail. They span does
> > > not have to be large -- when placed correctly, even `JAL .' can jump
> > > to the wrong place, which is what LD is supposed to catch as a
> > > relocation error -- so a test executable successfully linked with
> > > your correction in place won't be large, it doesn't have to take more than
> 2 virtual pages.
> > >
> > > The downside of the latter solution are possible portability issues
> > > with the linker script. You won't have to run your executable AFAICT
> > > from glibc BZ
> > > #17601 as you'll get a link error if a jump target is out of range.
> > > So you could make it a mere link test, no need to run it.
> > >
> > > Maciej
> >
> >
> 
>  Hi Maciej, Catherine,
> 
>  This issue will not trigger a linker error (I believe it treats the  symbol
> referred by the relocation as a local symbol). This is a follow  up to GLIBC BZ
> #17601, the problem is seen only at runtime. So, I think  this brings back the
> need to run the test. I can look into separating  sections with -Wl,-T.. (that
> may also require extending  mips_option_groups in mips/mips.exp), if
> running executable is OK.
> 
Hi Petar,  
Running the executable is fine, but didn't you say that your test case takes hours to execute?
If so, that's not acceptable.  Is it possible to construct a test case that will run more quickly?
Thanks,
Catherine

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Petar Jovanovic]

-----Original Message-----
From: Moore, Catherine [mailto:Catherine_Moore@mentor.com] 
Sent: Friday, April 17, 2015 8:36 PM
To: Petar Jovanovic
Cc: Maciej W. Rozycki; Matthew Fortune; gcc-patches@gcc.gnu.org
Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

> 
> Hi Petar,
> Running the executable is fine, but didn't you say that your test case takes hours to execute?
> If so, that's not acceptable.  Is it possible to construct a test case that will run more quickly?
> Thanks,
> Catherine

Hi Catherine,

I was just raising concerns about the original example (that did run
long). Anyway, I have just uploaded a different example that triggers
the issue.

Regards,
Petar

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Maciej W. Rozycki]

On Fri, 17 Apr 2015, Petar Jovanovic wrote:

> This issue will not trigger a linker error (I believe it treats the
> symbol referred by the relocation as a local symbol). This is a follow
> up to GLIBC BZ #17601, the problem is seen only at runtime. So, I think
> this brings back the need to run the test. I can look into separating
> sections with -Wl,-T.. (that may also require extending
> mips_option_groups in mips/mips.exp), if running executable is OK.

 If the assembler or the linker knowingly (or under conditions where it 
can be determined) creates a broken executable, then it is a separate bug 
that needs to be filed against binutils.  Due to the unusual constraints 
of absolute MIPS jump instructions any associated symbol references have 
to result in external relocations, to be resolved in the final static link 
only.  If this is not the case or such a relocation resolves successfully 
despite a range overflow, then this has to be fixed in binutils.  Can you 
double-check if this is the case?

 I see this in a dump from crtbegin.o:

Disassembly of section .init:

00000000 <.init>:
   0:	04110001 	bal	8 <.init+0x8>
   4:	00000000 	nop
   8:	0c00004b 	jal	12c <frame_dummy>
			8: R_MIPS_26	.text
   c:	00000000 	nop

-- if `.text+0x12c' is out of range for JAL (R_MIPS_26) from `.init+0xc', 
that is the 4 most significant bits of both final addresses are not the 
same (the range calculation for this instruction/relocation is relative to 
the delay slot), then the static link is supposed to fail.  I think this 
can be easily verified and if needed, converted to an LD test case.

 As to a GCC test case I agree with Catherine that a run-time test case 
will be fine, and actually inevitable if the linker fails to fail.

  Maciej

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Matthew Fortune]

Hi Petar,

I've put your patch inline below and switched to plain text. I suspect
your post was bounced by gcc-patches.

I'm OK with this change but I'd like Catherine to comment before committing.
It seems a shame to duplicate the block of code but it is probably just as
ugly to define a macro for the la/dla instruction.

For future reference, it is best not to include changelog content in a patch
but instead just paste into the email. Also the ChangeLog you need for this
change is gcc/ChangeLog (as confusing as that may be at first).

Thanks,
Matthew

> From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] 
> Sent: 05 February 2015 19:28
> To: gcc-patches@gcc.gnu.org; 'Maciej W. Rozycki'; Matthew Fortune
> Subject: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
>
> v2:
> - add ChangeLog entry
> - use DLA instead of LA for n64
>
> PTAL. Thanks.
>
> Regards,
> Petar

---
 ChangeLog              |  5 +++++
 gcc/config/mips/mips.h | 23 +++++++++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5c61c66..3a15f4a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2015-02-05  Petar Jovanovic  <petar.jovanovic@rt-rk.com>
+
+	* config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Fix the macro to use
+	la/jalr instead of jal.
+
 2015-02-02  Janis Johnson  <janis.marie.johnson@gmail.com>
 
 	* MAINTAINERS (Various Maintainers: testsuite): Remove myself.
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index ec69ed5..4bd83f5 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -3034,11 +3034,11 @@ while (0)
 	nop\n\
 1:	.cpload $31\n\
 	.set reorder\n\
-	jal " USER_LABEL_PREFIX #FUNC "\n\
+	la $25, " USER_LABEL_PREFIX #FUNC "\n\
+	jalr $25\n\
 	.set pop\n\
 	" TEXT_SECTION_ASM_OP);
-#elif ((defined _ABIN32 && _MIPS_SIM == _ABIN32) \
-       || (defined _ABI64 && _MIPS_SIM == _ABI64))
+#elif (defined _ABIN32 && _MIPS_SIM == _ABIN32)
 #define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
    asm (SECTION_OP "\n\
 	.set push\n\
@@ -3048,7 +3048,22 @@ while (0)
 	nop\n\
 1:	.set reorder\n\
 	.cpsetup $31, $2, 1b\n\
-	jal " USER_LABEL_PREFIX #FUNC "\n\
+	la $25, " USER_LABEL_PREFIX #FUNC "\n\
+	jalr $25\n\
+	.set pop\n\
+	" TEXT_SECTION_ASM_OP);
+#elif (defined _ABI64 && _MIPS_SIM == _ABI64)
+#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
+   asm (SECTION_OP "\n\
+	.set push\n\
+	.set nomips16\n\
+	.set noreorder\n\
+	bal 1f\n\
+	nop\n\
+1:	.set reorder\n\
+	.cpsetup $31, $2, 1b\n\
+	dla $25, " USER_LABEL_PREFIX #FUNC "\n\
+	jalr $25\n\
 	.set pop\n\
 	" TEXT_SECTION_ASM_OP);
 #endif
-- 
1.8.2.1

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Maciej W. Rozycki]

On Thu, 5 Feb 2015, Matthew Fortune wrote:

> I'm OK with this change but I'd like Catherine to comment before committing.
> It seems a shame to duplicate the block of code but it is probably just as
> ugly to define a macro for the la/dla instruction.

 Native systems have <sys/asm.h> for such ABI dependencies, including 
stuff to set up $gp.  Perhaps we could reuse these bits, the licence I 
think allows us to.

  Maciej

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Matthew Fortune]

Maciej W. Rozycki <macro@linux-mips.org> writes:
> On Thu, 5 Feb 2015, Matthew Fortune wrote:
> 
> > I'm OK with this change but I'd like Catherine to comment before
> committing.
> > It seems a shame to duplicate the block of code but it is probably just
> as
> > ugly to define a macro for the la/dla instruction.
> 
>  Native systems have <sys/asm.h> for such ABI dependencies, including
> stuff to set up $gp.  Perhaps we could reuse these bits, the licence I
> think allows us to.

That's a good idea. Perhaps I should take that on as part of some cleanup
of the MIPS backend in the next stage1. I'm looking to rework how the
ISA_HAS logic works so perhaps there would be value in doing this mostly in
a header that can also be used for assembly programmers. That would naturally
mean we get all the other nice assembly macros available in the backend of
GCC too.

Thanks,
Matthew

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Maciej W. Rozycki]

On Fri, 6 Feb 2015, Matthew Fortune wrote:

> >  Native systems have <sys/asm.h> for such ABI dependencies, including
> > stuff to set up $gp.  Perhaps we could reuse these bits, the licence I
> > think allows us to.
> 
> That's a good idea. Perhaps I should take that on as part of some cleanup
> of the MIPS backend in the next stage1. I'm looking to rework how the
> ISA_HAS logic works so perhaps there would be value in doing this mostly in
> a header that can also be used for assembly programmers. That would naturally
> mean we get all the other nice assembly macros available in the backend of
> GCC too.

 This consideration made me realise I've had a patch outstanding for some 
10 years to convert all the `BAL x' instructions there to `BLTZAL $0, x'.  
This has always been a good idea in case implementations recognised the 
special case and avoided involving branch prediction, and I believe it has 
become even more apparent with r6 calling it NAL.

 I'll see if I can submit it to glibc soon -- as you may have been aware 
10 years ago wasn't exactly the most friendly period in glibc maintenance 
and hence I wasn't very prompt with patch submissions.

  Maciej

Re: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Mike Stump]

On Feb 6, 2015, at 4:23 AM, Maciej W. Rozycki <macro@linux-mips.org> wrote:
> This consideration made me realise I've had a patch outstanding for some 
> 10 years to convert all the `BAL x' instructions there to `BLTZAL $0, x'.  
> This has always been a good idea in case implementations recognised the 
> special case and avoided involving branch prediction, and I believe it has 
> become even more apparent with r6 calling it NAL.

Ick, no.

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Matthew Fortune]

Mike Stump <mikestump@comcast.net> writes:
> On Feb 6, 2015, at 4:23 AM, Maciej W. Rozycki <macro@linux-mips.org>
> wrote:
> > This consideration made me realise I've had a patch outstanding for
> > some
> > 10 years to convert all the `BAL x' instructions there to `BLTZAL $0,
> x'.
> > This has always been a good idea in case implementations recognised
> > the special case and avoided involving branch prediction, and I
> > believe it has become even more apparent with r6 calling it NAL.
> 
> Ick, no.

What part of this are you referring to?

NAL (bizarre name or not) is the least intrusive way to obtain the PC on
MIPS <= R5. The use of BAL for this, albeit common, has a high risk of
affecting hardware optimisations like return predictors by introducing
a call that will never return.

This is a change that I am also planning to propagate to as many projects
as possible. If you can see a problem with using BLTZAL for this purpose
please could you explain as it may have been overlooked?

Thanks,
Matthew

Re: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Mike Stump]

On Feb 6, 2015, at 9:41 AM, Matthew Fortune <Matthew.Fortune@imgtec.com> wrote:
> Mike Stump <mikestump@comcast.net> writes:
>> On Feb 6, 2015, at 4:23 AM, Maciej W. Rozycki <macro@linux-mips.org>
>> wrote:
>>> This consideration made me realise I've had a patch outstanding for
>>> some
>>> 10 years to convert all the `BAL x' instructions there to `BLTZAL $0,
>> x'.
>>> This has always been a good idea in case implementations recognised
>>> the special case and avoided involving branch prediction, and I
>>> believe it has become even more apparent with r6 calling it NAL.
>> 
>> Ick, no.
> 
> What part of this are you referring to?

The first part.  Ah, yes, I had mentally flipped the two cases.  I mistakingly thought you wanted to change all the BALs to BLTZAL, which you don’t want to do.  You only want to flip the non-calls to that form, which is perfectly reasonable.

Personally, the call form of bal in my book should be called call, and the non-call form of it should be called bal, but, I realize it is likely to late to do much about now.  If one went down this path, then even changing it away from bal is wrong.  That’s the basis of why I fired up the first email.  Maybe more a lament at this point.

Re: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Maciej W. Rozycki]

On Fri, 6 Feb 2015, Mike Stump wrote:

> Personally, the call form of bal in my book should be called call, and 
> the non-call form of it should be called bal, but, I realize it is 
> likely to late to do much about now.  If one went down this path, then 
> even changing it away from bal is wrong.  ThatÂ’s the basis of why I 
> fired up the first email.  Maybe more a lament at this point.

 I'm confused, these all are conditional short-distance calls.  And I 
suspect that they were included in the original MIPS architecture for the 
very purpose of letting relocatable code find its own position in memory 
at the run time -- and the rest was added to maximise the benefit of the 
opcode(s) that had to be there anyway.

 And it's just, owing to how the architecture has been structured, that 
there are exactly two cases (of the 64 encodings available) where the 
condition is fixed to either `true' or `false', respectively.  And if 
anyhow, I'd call any assembly-language idiom of `BLTZAL $0, foo' just 
`MOVE $31, $pc' or maybe to avoid confusion `[D]ADDIU $31, $pc, 8'.

 Note that `foo' is never used in that operation (and that the MIPS16 and 
microMIPS, and now also r6 instruction sets actually have such direct 
operations that have a wider range of immediates available; although care 
has to be taken about them as they may have unusual alignment or other 
restrictions).

  Maciej

Re: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Maciej W. Rozycki]

On Fri, 6 Feb 2015, Mike Stump wrote:

> > This consideration made me realise I've had a patch outstanding for some 
> > 10 years to convert all the `BAL x' instructions there to `BLTZAL $0, x'.  
> > This has always been a good idea in case implementations recognised the 
> > special case and avoided involving branch prediction, and I believe it has 
> > become even more apparent with r6 calling it NAL.
> 
> Ick, no.

 Hmm, have you hit `send' too quickly by any chance?  There's surely a 
further part missing from your post where you'd explain what you mean.

  Maciej

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Moore, Catherine]

> -----Original Message-----
> From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com]
> Sent: Thursday, February 05, 2015 3:52 PM
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> > 
> I've put your patch inline below and switched to plain text. I suspect your
> post was bounced by gcc-patches.
> 
> I'm OK with this change but I'd like Catherine to comment before committing.
> It seems a shame to duplicate the block of code but it is probably just as ugly
> to define a macro for the la/dla instruction.

The patch itself is OK, although I agree with other's comments about the unfortunate duplication of code.
Petar,  would you please add your testcase to the gcc testsuite and repost the patch.
Thanks,
Catherine

> 
> For future reference, it is best not to include changelog content in a patch
> but instead just paste into the email. Also the ChangeLog you need for this
> change is gcc/ChangeLog (as confusing as that may be at first).
> 
> Thanks,
> Matthew
> 
> > From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com]
> > Sent: 05 February 2015 19:28
> > To: gcc-patches@gcc.gnu.org; 'Maciej W. Rozycki'; Matthew Fortune
> > Subject: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> >
> > v2:
> > - add ChangeLog entry
> > - use DLA instead of LA for n64
> >
> > PTAL. Thanks.
> >
> > Regards,
> > Petar
> 
> ---
>  ChangeLog              |  5 +++++
>  gcc/config/mips/mips.h | 23 +++++++++++++++++++----
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 5c61c66..3a15f4a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-02-05  Petar Jovanovic  <petar.jovanovic@rt-rk.com>
> +
> +	* config/mips/mips.h (CRT_CALL_STATIC_FUNCTION): Fix the macro
> to use
> +	la/jalr instead of jal.
> +
>  2015-02-02  Janis Johnson  <janis.marie.johnson@gmail.com>
> 
>  	* MAINTAINERS (Various Maintainers: testsuite): Remove myself.
> diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> ec69ed5..4bd83f5 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -3034,11 +3034,11 @@ while (0)
>  	nop\n\
>  1:	.cpload $31\n\
>  	.set reorder\n\
> -	jal " USER_LABEL_PREFIX #FUNC "\n\
> +	la $25, " USER_LABEL_PREFIX #FUNC "\n\
> +	jalr $25\n\
>  	.set pop\n\
>  	" TEXT_SECTION_ASM_OP);
> -#elif ((defined _ABIN32 && _MIPS_SIM == _ABIN32) \
> -       || (defined _ABI64 && _MIPS_SIM == _ABI64))
> +#elif (defined _ABIN32 && _MIPS_SIM == _ABIN32)
>  #define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
>     asm (SECTION_OP "\n\
>  	.set push\n\
> @@ -3048,7 +3048,22 @@ while (0)
>  	nop\n\
>  1:	.set reorder\n\
>  	.cpsetup $31, $2, 1b\n\
> -	jal " USER_LABEL_PREFIX #FUNC "\n\
> +	la $25, " USER_LABEL_PREFIX #FUNC "\n\
> +	jalr $25\n\
> +	.set pop\n\
> +	" TEXT_SECTION_ASM_OP);
> +#elif (defined _ABI64 && _MIPS_SIM == _ABI64)
> +#define CRT_CALL_STATIC_FUNCTION(SECTION_OP, FUNC)	\
> +   asm (SECTION_OP "\n\
> +	.set push\n\
> +	.set nomips16\n\
> +	.set noreorder\n\
> +	bal 1f\n\
> +	nop\n\
> +1:	.set reorder\n\
> +	.cpsetup $31, $2, 1b\n\
> +	dla $25, " USER_LABEL_PREFIX #FUNC "\n\
> +	jalr $25\n\
>  	.set pop\n\
>  	" TEXT_SECTION_ASM_OP);
>  #endif
> --
> 1.8.2.1

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Petar Jovanovic]

-----Original Message-----
From: Moore, Catherine [mailto:Catherine_Moore@mentor.com] 
Sent: Friday, February 6, 2015 4:13 PM
To: Matthew Fortune; Petar Jovanovic; gcc-patches@gcc.gnu.org; 'Maciej W.
Rozycki'
Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro


> Petar,  would you please add your testcase to the gcc testsuite and repost
the patch.

Hi Catherine,

Sure, I can repost the patch and add the test case I used.
My concern - and I would appreciate feedback from you and others on it - is
that the test I originally used to report [1] the problem creates a large
executable (380 MB).

Here is a variant of it:

/* { dg-do run } */

#include <stdio.h>

#if defined(__GLIBC__) && (__GLIBC__ < 2 || (__GLIBC__ == 2 &&
__GLIBC_MINOR__ < 21))
#define BROKEN
#endif

int
main (void)
{
#ifndef BROKEN
  asm(".fill 100000000, 4, 0x00000000\n");
#endif
  return 0;
}

The test also takes time to execute. Obviously, I can make it slightly more
complex and make execute time short. Another path is to introduce and use
options such as "Wl,-Ttext-segment" to make its size smaller. Any other
ideas?

Regards,
Petar

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=17601

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Moore, Catherine]

> -----Original Message-----
> From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com]
> Sent: Thursday, February 12, 2015 7:28 PM
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> -----Original Message-----
> From: Moore, Catherine [mailto:Catherine_Moore@mentor.com]
> Sent: Friday, February 6, 2015 4:13 PM
> To: Matthew Fortune; Petar Jovanovic; gcc-patches@gcc.gnu.org; 'Maciej W.
> Rozycki'
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> 
> > Petar,  would you please add your testcase to the gcc testsuite and repost
> the patch.
> 
> Hi Catherine,
> 
> Sure, I can repost the patch and add the test case I used.
> My concern - and I would appreciate feedback from you and others on it - is
> that the test I originally used to report [1] the problem creates a large
> executable (380 MB).
> 
> 
> The test also takes time to execute. Obviously, I can make it slightly more
> complex and make execute time short. Another path is to introduce and use
> options such as "Wl,-Ttext-segment" to make its size smaller. Any other
> ideas?
> 
Hi Petar,

There isn't any need to execute a large testcase.  Instead, try adding a short version of your test to the directory gcc/testsuite/gcc.target/mips.
There are examples of other tests there they check for specific assembler sequences by using scan-assembler.   See umips-swp-1.c (and others) for a specific example of how to do this.
Let me know if you need additional help.
Thanks,
Catherine

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Petar Jovanovic]

-----Original Message-----
From: Moore, Catherine [mailto:Catherine_Moore@mentor.com] 
Sent: Friday, February 13, 2015 2:37 AM
To: Petar Jovanovic; 'Matthew Fortune'; gcc-patches@gcc.gnu.org; 'Maciej W.
Rozycki'
Cc: Moore, Catherine
Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

> Hi Petar,
> 
> There isn't any need to execute a large testcase.  Instead, try adding a
short version of your test to the directory gcc/testsuite/gcc.target/mips.
> There are examples of other tests there they check for specific assembler
sequences by using scan-assembler.   See umips-swp-1.c (and others) for a
specific example of how to do this.
> Let me know if you need additional help.
> Thanks,
> Catherine

Hi Catherine,

Sorry for late response, I have missed to follow up on this.
IIUC, scan-assembler will scan assembly file generated from a test file.
This particular change will - on the other hand - modify crtend.o and thus
init section. Is there a 'scan-assembler' functionality over a final linked
executable, as that would actually test the change?

Let me know. Thanks.

Regards,
Petar

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Maciej W. Rozycki]

On Thu, 16 Apr 2015, Petar Jovanovic wrote:

> > There isn't any need to execute a large testcase.  Instead, try adding a
> short version of your test to the directory gcc/testsuite/gcc.target/mips.
> > There are examples of other tests there they check for specific assembler
> sequences by using scan-assembler.   See umips-swp-1.c (and others) for a
> specific example of how to do this.
> > Let me know if you need additional help.
> > Thanks,
> > Catherine
> 
> Hi Catherine,
> 
> Sorry for late response, I have missed to follow up on this.
> IIUC, scan-assembler will scan assembly file generated from a test file.
> This particular change will - on the other hand - modify crtend.o and thus
> init section. Is there a 'scan-assembler' functionality over a final linked
> executable, as that would actually test the change?

 You'd need `objdump' for doing that and there is no ready-to-use solution 
within the GCC test suite available, you'd have to cook something up 
yourself, perhaps starting with `[find_binutils_prog objdump]'.

 Another solution might be using an auxiliary linker script (`-Wl,-T,...' 
or maybe just an implicit linker script will do; see the LD manual for 
details) to place the sections the jump is made between apart manually at 
addresses appropriate for JAL to fail.  They span does not have to be 
large -- when placed correctly, even `JAL .' can jump to the wrong place, 
which is what LD is supposed to catch as a relocation error -- so a test 
executable successfully linked with your correction in place won't be 
large, it doesn't have to take more than 2 virtual pages.

 The downside of the latter solution are possible portability issues with 
the linker script.  You won't have to run your executable AFAICT from 
glibc BZ #17601 as you'll get a link error if a jump target is out of 
range.  So you could make it a mere link test, no need to run it.

  Maciej

RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro

[Moore, Catherine]

> -----Original Message-----
> From: Maciej W. Rozycki [mailto:macro@linux-mips.org]
> Sent: Thursday, April 16, 2015 2:23 PM
> To: Petar Jovanovic
> Cc: Moore, Catherine; 'Matthew Fortune'; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH v2][MIPS] fix CRT_CALL_STATIC_FUNCTION macro
> 
> On Thu, 16 Apr 2015, Petar Jovanovic wrote:
> 
> > > There isn't any need to execute a large testcase.  Instead, try
> > > adding a
> > short version of your test to the directory gcc/testsuite/gcc.target/mips.
> > > There are examples of other tests there they check for specific
> > > assembler
> > sequences by using scan-assembler.   See umips-swp-1.c (and others) for a
> > specific example of how to do this.
> > > Let me know if you need additional help.
> > > Thanks,
> > > Catherine
> >
> > Hi Catherine,
> >
> > Sorry for late response, I have missed to follow up on this.
> > IIUC, scan-assembler will scan assembly file generated from a test file.
> > This particular change will - on the other hand - modify crtend.o and
> > thus init section. Is there a 'scan-assembler' functionality over a
> > final linked executable, as that would actually test the change?
> 
Hi Petar,
It looks like I answered a little too quickly the first time around.  I'm sorry.
In any case, Maciej is correct.  I think a link-time test should do it.
Thanks,
Catherine

>  You'd need `objdump' for doing that and there is no ready-to-use solution
> within the GCC test suite available, you'd have to cook something up
> yourself, perhaps starting with `[find_binutils_prog objdump]'.
> 
>  Another solution might be using an auxiliary linker script (`-Wl,-T,...'
> or maybe just an implicit linker script will do; see the LD manual for
> details) to place the sections the jump is made between apart manually at
> addresses appropriate for JAL to fail.  They span does not have to be large --
> when placed correctly, even `JAL .' can jump to the wrong place, which is
> what LD is supposed to catch as a relocation error -- so a test executable
> successfully linked with your correction in place won't be large, it doesn't
> have to take more than 2 virtual pages.
> 
>  The downside of the latter solution are possible portability issues with the
> linker script.  You won't have to run your executable AFAICT from glibc BZ
> #17601 as you'll get a link error if a jump target is out of range.  So you could
> make it a mere link test, no need to run it.
> 
>   Maciej