diff mbox

Fix PCH on AArch64 (PR pch/60010)

Message ID 20140131195949.GE5857@redacted.bos.redhat.com
State New
Headers show

Commit Message

Kyle McMartin Jan. 31, 2014, 7:59 p.m. UTC
Hi,

Similar to other architectures, failing to set TRY_EMPTY_VM_SPACE
results in a Segmentation Fault and ICE in cc1plus when using
precompiled headers and randomize_va_space is set. This patch fixes the
issue, and now I can reliably build packages which use pch (wxGTK and
openjdk in particular would fail every time. wxGTK has survived 30 build
attempts without failure now.)

(The exact value is unimportant, as long as it's in an unused area. I
suspect that the fallback buffer_size code path hasn't been fixed up
since the exec-shield days and could use a re-think now that mmap
randomization is upstream. I've been trying to debug exactly why it
fails for all architctures, so we can remove this, but haven't had much
luck yet.)

This is similar to pch/45979, pch/14940, target/25343.

Bootstrapped and tested on aarch64-linux-gnu.

regards, Kyle

2014-01-31  Kyle McMartin <kyle@redhat.com>

	PR pch/60010
	* config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.

Comments

Andrew Pinski Jan. 31, 2014, 10:03 p.m. UTC | #1
On Fri, Jan 31, 2014 at 11:59 AM, Kyle McMartin <kmcmarti@redhat.com> wrote:
> Hi,
>
> Similar to other architectures, failing to set TRY_EMPTY_VM_SPACE
> results in a Segmentation Fault and ICE in cc1plus when using
> precompiled headers and randomize_va_space is set. This patch fixes the
> issue, and now I can reliably build packages which use pch (wxGTK and
> openjdk in particular would fail every time. wxGTK has survived 30 build
> attempts without failure now.)
>
> (The exact value is unimportant, as long as it's in an unused area. I
> suspect that the fallback buffer_size code path hasn't been fixed up
> since the exec-shield days and could use a re-think now that mmap
> randomization is upstream. I've been trying to debug exactly why it
> fails for all architctures, so we can remove this, but haven't had much
> luck yet.)
>
> This is similar to pch/45979, pch/14940, target/25343.
>
> Bootstrapped and tested on aarch64-linux-gnu.
>
> regards, Kyle
>
> 2014-01-31  Kyle McMartin <kyle@redhat.com>
>
>         PR pch/60010
>         * config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.


If this gets merged before my ILP32 changes, I will make sure I
include a fix for ILP32 also.

Thanks,
Andrew

>
> --- a/gcc/config/host-linux.c
> +++ b/gcc/config/host-linux.c
> @@ -86,6 +86,8 @@
>  # define TRY_EMPTY_VM_SPACE    0x60000000
>  #elif defined(__mc68000__)
>  # define TRY_EMPTY_VM_SPACE    0x40000000
> +#elif defined(__aarch64__)
> +# define TRY_EMPTY_VM_SPACE    0x1000000000
>  #elif defined(__ARM_EABI__)
>  # define TRY_EMPTY_VM_SPACE     0x60000000
>  #elif defined(__mips__) && defined(__LP64__)
Kyle McMartin Jan. 31, 2014, 10:09 p.m. UTC | #2
On Fri, Jan 31, 2014 at 02:03:25PM -0800, Andrew Pinski wrote:
> > 2014-01-31  Kyle McMartin <kyle@redhat.com>
> >
> >         PR pch/60010
> >         * config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.
> 
> 
> If this gets merged before my ILP32 changes, I will make sure I
> include a fix for ILP32 also.
> 

Ah, good thinking. I was wondering if it would require a test of
__LP64__ as well, but didn't think to look if AArch32 ABI was using
__aarch64__ && !LP64 or __aarch32__ or something.

Thanks Andrew!

regards, Kyle
Andrew Pinski Jan. 31, 2014, 10:13 p.m. UTC | #3
On Fri, Jan 31, 2014 at 2:09 PM, Kyle McMartin <kmcmarti@redhat.com> wrote:
> On Fri, Jan 31, 2014 at 02:03:25PM -0800, Andrew Pinski wrote:
>> > 2014-01-31  Kyle McMartin <kyle@redhat.com>
>> >
>> >         PR pch/60010
>> >         * config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.
>>
>>
>> If this gets merged before my ILP32 changes, I will make sure I
>> include a fix for ILP32 also.
>>
>
> Ah, good thinking. I was wondering if it would require a test of
> __LP64__ as well, but didn't think to look if AArch32 ABI was using
> __aarch64__ && !LP64 or __aarch32__ or something.


Well aarch32 uses __arm__ (the old arm toolchain in fact) but I am
talking about the ILP32 ABI for AARCH64.

Thanks,
Andrew

>
> Thanks Andrew!
>
> regards, Kyle
Richard Earnshaw Feb. 14, 2014, 1:43 p.m. UTC | #4
On 31/01/14 19:59, Kyle McMartin wrote:
> Hi,
> 
> Similar to other architectures, failing to set TRY_EMPTY_VM_SPACE
> results in a Segmentation Fault and ICE in cc1plus when using
> precompiled headers and randomize_va_space is set. This patch fixes the
> issue, and now I can reliably build packages which use pch (wxGTK and
> openjdk in particular would fail every time. wxGTK has survived 30 build
> attempts without failure now.)
> 
> (The exact value is unimportant, as long as it's in an unused area. I
> suspect that the fallback buffer_size code path hasn't been fixed up
> since the exec-shield days and could use a re-think now that mmap
> randomization is upstream. I've been trying to debug exactly why it
> fails for all architctures, so we can remove this, but haven't had much
> luck yet.)
> 
> This is similar to pch/45979, pch/14940, target/25343.
> 
> Bootstrapped and tested on aarch64-linux-gnu.
> 
> regards, Kyle
> 
> 2014-01-31  Kyle McMartin <kyle@redhat.com>
> 
> 	PR pch/60010
> 	* config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.
> 

This is OK, subject to RM approval.

R.

> --- a/gcc/config/host-linux.c
> +++ b/gcc/config/host-linux.c
> @@ -86,6 +86,8 @@
>  # define TRY_EMPTY_VM_SPACE	0x60000000
>  #elif defined(__mc68000__)
>  # define TRY_EMPTY_VM_SPACE	0x40000000
> +#elif defined(__aarch64__)
> +# define TRY_EMPTY_VM_SPACE	0x1000000000
>  #elif defined(__ARM_EABI__)
>  # define TRY_EMPTY_VM_SPACE     0x60000000
>  #elif defined(__mips__) && defined(__LP64__)
>
Richard Biener Feb. 14, 2014, 1:46 p.m. UTC | #5
On Fri, 14 Feb 2014, Richard Earnshaw wrote:

> On 31/01/14 19:59, Kyle McMartin wrote:
> > Hi,
> > 
> > Similar to other architectures, failing to set TRY_EMPTY_VM_SPACE
> > results in a Segmentation Fault and ICE in cc1plus when using
> > precompiled headers and randomize_va_space is set. This patch fixes the
> > issue, and now I can reliably build packages which use pch (wxGTK and
> > openjdk in particular would fail every time. wxGTK has survived 30 build
> > attempts without failure now.)
> > 
> > (The exact value is unimportant, as long as it's in an unused area. I
> > suspect that the fallback buffer_size code path hasn't been fixed up
> > since the exec-shield days and could use a re-think now that mmap
> > randomization is upstream. I've been trying to debug exactly why it
> > fails for all architctures, so we can remove this, but haven't had much
> > luck yet.)
> > 
> > This is similar to pch/45979, pch/14940, target/25343.
> > 
> > Bootstrapped and tested on aarch64-linux-gnu.
> > 
> > regards, Kyle
> > 
> > 2014-01-31  Kyle McMartin <kyle@redhat.com>
> > 
> > 	PR pch/60010
> > 	* config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.
> > 
> 
> This is OK, subject to RM approval.

Works for me.

Richard.

> R.
> 
> > --- a/gcc/config/host-linux.c
> > +++ b/gcc/config/host-linux.c
> > @@ -86,6 +86,8 @@
> >  # define TRY_EMPTY_VM_SPACE	0x60000000
> >  #elif defined(__mc68000__)
> >  # define TRY_EMPTY_VM_SPACE	0x40000000
> > +#elif defined(__aarch64__)
> > +# define TRY_EMPTY_VM_SPACE	0x1000000000
> >  #elif defined(__ARM_EABI__)
> >  # define TRY_EMPTY_VM_SPACE     0x60000000
> >  #elif defined(__mips__) && defined(__LP64__)
> > 
> 
> 
>
Jakub Jelinek Feb. 14, 2014, 1:47 p.m. UTC | #6
On Fri, Feb 14, 2014 at 01:43:26PM +0000, Richard Earnshaw wrote:
> On 31/01/14 19:59, Kyle McMartin wrote:
> > 2014-01-31  Kyle McMartin <kyle@redhat.com>
> > 
> > 	PR pch/60010
> > 	* config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.
> > 
> 
> This is OK, subject to RM approval.

Ok.  Kyle, do you have commit access, or Richard, are you going to check it
in for Kyle?

> > --- a/gcc/config/host-linux.c
> > +++ b/gcc/config/host-linux.c
> > @@ -86,6 +86,8 @@
> >  # define TRY_EMPTY_VM_SPACE	0x60000000
> >  #elif defined(__mc68000__)
> >  # define TRY_EMPTY_VM_SPACE	0x40000000
> > +#elif defined(__aarch64__)
> > +# define TRY_EMPTY_VM_SPACE	0x1000000000
> >  #elif defined(__ARM_EABI__)
> >  # define TRY_EMPTY_VM_SPACE     0x60000000
> >  #elif defined(__mips__) && defined(__LP64__)

	Jakub
Richard Earnshaw Feb. 14, 2014, 2:14 p.m. UTC | #7
On 14/02/14 13:47, Jakub Jelinek wrote:
> On Fri, Feb 14, 2014 at 01:43:26PM +0000, Richard Earnshaw wrote:
>> On 31/01/14 19:59, Kyle McMartin wrote:
>>> 2014-01-31  Kyle McMartin <kyle@redhat.com>
>>>
>>> 	PR pch/60010
>>> 	* config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.
>>>
>>
>> This is OK, subject to RM approval.
> 
> Ok.  Kyle, do you have commit access, or Richard, are you going to check it
> in for Kyle?
> 
>>> --- a/gcc/config/host-linux.c
>>> +++ b/gcc/config/host-linux.c
>>> @@ -86,6 +86,8 @@
>>>  # define TRY_EMPTY_VM_SPACE	0x60000000
>>>  #elif defined(__mc68000__)
>>>  # define TRY_EMPTY_VM_SPACE	0x40000000
>>> +#elif defined(__aarch64__)
>>> +# define TRY_EMPTY_VM_SPACE	0x1000000000
>>>  #elif defined(__ARM_EABI__)
>>>  # define TRY_EMPTY_VM_SPACE     0x60000000
>>>  #elif defined(__mips__) && defined(__LP64__)
> 
> 	Jakub
> 

I've put it in.

R.
Richard Earnshaw Feb. 14, 2014, 2:15 p.m. UTC | #8
On 14/02/14 14:14, Richard Earnshaw wrote:
> On 14/02/14 13:47, Jakub Jelinek wrote:
>> On Fri, Feb 14, 2014 at 01:43:26PM +0000, Richard Earnshaw wrote:
>>> On 31/01/14 19:59, Kyle McMartin wrote:
>>>> 2014-01-31  Kyle McMartin <kyle@redhat.com>
>>>>
>>>> 	PR pch/60010
>>>> 	* config/host-linux.c (TRY_EMPTY_VM_SPACE): Define for AArch64.
>>>>
>>>
>>> This is OK, subject to RM approval.
>>
>> Ok.  Kyle, do you have commit access, or Richard, are you going to check it
>> in for Kyle?
>>
>>>> --- a/gcc/config/host-linux.c
>>>> +++ b/gcc/config/host-linux.c
>>>> @@ -86,6 +86,8 @@
>>>>  # define TRY_EMPTY_VM_SPACE	0x60000000
>>>>  #elif defined(__mc68000__)
>>>>  # define TRY_EMPTY_VM_SPACE	0x40000000
>>>> +#elif defined(__aarch64__)
>>>> +# define TRY_EMPTY_VM_SPACE	0x1000000000
>>>>  #elif defined(__ARM_EABI__)
>>>>  # define TRY_EMPTY_VM_SPACE     0x60000000
>>>>  #elif defined(__mips__) && defined(__LP64__)
>>
>> 	Jakub
>>
> 
> I've put it in.
> 
> R.
> 

Kyle, the PR is against 4.8.  Have you tested a back-port?

R.
Kyle McMartin Feb. 17, 2014, 2:38 p.m. UTC | #9
On Fri, Feb 14, 2014 at 02:15:52PM +0000, Richard Earnshaw wrote:
> > 
> > I've put it in.
> > 
> > R.
> > 
> 
> Kyle, the PR is against 4.8.  Have you tested a back-port?
> 

Yeah, I've built it against both 4.9 and 4.8... I suspect it'll work
fine for 4.7 too if anyone is still using it.

regards, Kyle
diff mbox

Patch

--- a/gcc/config/host-linux.c
+++ b/gcc/config/host-linux.c
@@ -86,6 +86,8 @@ 
 # define TRY_EMPTY_VM_SPACE	0x60000000
 #elif defined(__mc68000__)
 # define TRY_EMPTY_VM_SPACE	0x40000000
+#elif defined(__aarch64__)
+# define TRY_EMPTY_VM_SPACE	0x1000000000
 #elif defined(__ARM_EABI__)
 # define TRY_EMPTY_VM_SPACE     0x60000000
 #elif defined(__mips__) && defined(__LP64__)