[google] Disable getpagesize() for Android toolchain

[carrot, 2011-05-24 09:39:13]

Hi

This patch is for google/main.

In order to be compatible with current bionic and sysroot, we need to disable
getpagesize(). After getpagesize() in bionic is changed and ndk contains that
change, we can reenable it.

Jing can give more details about it.

This patch has been tested on arm qemu without regression.

thanks
Carrot

2011-05-24  Jing Yu  <jingyu@google.com>

	* ChangeLog.google-main: New file.
	* getpagesize.c(getpagesize): Disable it for bionic.


Index: ChangeLog.google-main
===================================================================
--- ChangeLog.google-main	(revision 0)
+++ ChangeLog.google-main	(revision 0)
@@ -0,0 +1,5 @@
+Copyright (C) 2011 Free Software Foundation, Inc.
+
+Copying and distribution of this file, with or without modification,
+are permitted in any medium without royalty provided the copyright
+notice and this notice are preserved.
Index: getpagesize.c
===================================================================
--- getpagesize.c	(revision 174099)
+++ getpagesize.c	(working copy)
@@ -60,11 +60,13 @@ BUGS
 # endif /* PAGESIZE */
 #endif /* GNU_OUR_PAGESIZE */
 
+#if DEFAULT_LIBC != LIBC_BIONIC
 int
 getpagesize (void)
 {
   return (GNU_OUR_PAGESIZE);
 }
+#endif
 
 #else /* VMS */
 

--
This patch is available for review at http://codereview.appspot.com/4515131

[joseph_codesourcery.com, 2011-05-24 11:19:31]

On Tue, 24 May 2011, Guozhi Wei wrote:

> Index: getpagesize.c
> ===================================================================
> --- getpagesize.c	(revision 174099)
> +++ getpagesize.c	(working copy)
> @@ -60,11 +60,13 @@ BUGS
>  # endif /* PAGESIZE */
>  #endif /* GNU_OUR_PAGESIZE */
>  
> +#if DEFAULT_LIBC != LIBC_BIONIC

This makes no sense to me.  getpagesize.c is in libiberty.  libiberty does 
not include any GCC-specific headers - and in particular, does not include 
tm.h, which is where the definitions of DEFAULT_LIBC and LIBC_BIONIC would 
come from (via tm_defines in config.gcc).

(In any case, I thought it was now accepted that libiberty should stop 
being built for the target, and obviously it doesn't make sense for this 
particular host-side functionality to depend on what the target is.)

-- 
Joseph S. Myers
joseph@codesourcery.com

[dougkwan, 2011-05-24 18:07:32]

Shouldn't we test

ifndef __ANDROID__

instead?

-Doug

On Tue, May 24, 2011 at 2:39 AM, Guozhi Wei <carrot@google.com> wrote:
> Hi
>
> This patch is for google/main.
>
> In order to be compatible with current bionic and sysroot, we need to disable
> getpagesize(). After getpagesize() in bionic is changed and ndk contains that
> change, we can reenable it.
>
> Jing can give more details about it.
>
> This patch has been tested on arm qemu without regression.
>
> thanks
> Carrot
>
> 2011-05-24  Jing Yu  <jingyu@google.com>
>
>        * ChangeLog.google-main: New file.
>        * getpagesize.c(getpagesize): Disable it for bionic.
>
>
> Index: ChangeLog.google-main
> ===================================================================
> --- ChangeLog.google-main       (revision 0)
> +++ ChangeLog.google-main       (revision 0)
> @@ -0,0 +1,5 @@
> +Copyright (C) 2011 Free Software Foundation, Inc.
> +
> +Copying and distribution of this file, with or without modification,
> +are permitted in any medium without royalty provided the copyright
> +notice and this notice are preserved.
> Index: getpagesize.c
> ===================================================================
> --- getpagesize.c       (revision 174099)
> +++ getpagesize.c       (working copy)
> @@ -60,11 +60,13 @@ BUGS
>  # endif /* PAGESIZE */
>  #endif /* GNU_OUR_PAGESIZE */
>
> +#if DEFAULT_LIBC != LIBC_BIONIC
>  int
>  getpagesize (void)
>  {
>   return (GNU_OUR_PAGESIZE);
>  }
> +#endif
>
>  #else /* VMS */
>
>
> --
> This patch is available for review at http://codereview.appspot.com/4515131
>

[jingyu, 2011-05-26 00:17:58]

Hi Joseph,

We notice that gcc starts to build libiberty for target as a multilib
since gcc-4.6, even if we give --disable-stdc++-v3.
Since then, we run into the incompatible getpagesize() error.

I am wondering how to disable build of libiberty for target? I
searched mailing list. One guy said --disable-libiberty controls the
host libiberty, not the target one, another guy said the option
controls both host and target builds.

In some environment we still need to build libstdc++ libraries, where
libiberty has to be built for target. Can we use #ifndef __ANDROID__
to wrap around the getpagesize() definition? It is working for us.

Thanks,
Jing


On Tue, May 24, 2011 at 11:07 AM, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> Shouldn't we test
>
> ifndef __ANDROID__
>
> instead?
>
> -Doug
>
> On Tue, May 24, 2011 at 2:39 AM, Guozhi Wei <carrot@google.com> wrote:
>> Hi
>>
>> This patch is for google/main.
>>
>> In order to be compatible with current bionic and sysroot, we need to disable
>> getpagesize(). After getpagesize() in bionic is changed and ndk contains that
>> change, we can reenable it.
>>
>> Jing can give more details about it.
>>
>> This patch has been tested on arm qemu without regression.
>>
>> thanks
>> Carrot
>>
>> 2011-05-24  Jing Yu  <jingyu@google.com>
>>
>>        * ChangeLog.google-main: New file.
>>        * getpagesize.c(getpagesize): Disable it for bionic.
>>
>>
>> Index: ChangeLog.google-main
>> ===================================================================
>> --- ChangeLog.google-main       (revision 0)
>> +++ ChangeLog.google-main       (revision 0)
>> @@ -0,0 +1,5 @@
>> +Copyright (C) 2011 Free Software Foundation, Inc.
>> +
>> +Copying and distribution of this file, with or without modification,
>> +are permitted in any medium without royalty provided the copyright
>> +notice and this notice are preserved.
>> Index: getpagesize.c
>> ===================================================================
>> --- getpagesize.c       (revision 174099)
>> +++ getpagesize.c       (working copy)
>> @@ -60,11 +60,13 @@ BUGS
>>  # endif /* PAGESIZE */
>>  #endif /* GNU_OUR_PAGESIZE */
>>
>> +#if DEFAULT_LIBC != LIBC_BIONIC
>>  int
>>  getpagesize (void)
>>  {
>>   return (GNU_OUR_PAGESIZE);
>>  }
>> +#endif
>>
>>  #else /* VMS */
>>
>>
>> --
>> This patch is available for review at http://codereview.appspot.com/4515131
>>
>

[dougkwan, 2011-05-26 00:25:21]

Jing

Can't we just skip libiberty in top-level configure.ac?  Look for the
comment "Disable target libiberty for some systems."

-Doug

On Wed, May 25, 2011 at 5:17 PM, Jing Yu <jingyu@google.com> wrote:
> Hi Joseph,
>
> We notice that gcc starts to build libiberty for target as a multilib
> since gcc-4.6, even if we give --disable-stdc++-v3.
> Since then, we run into the incompatible getpagesize() error.
>
> I am wondering how to disable build of libiberty for target? I
> searched mailing list. One guy said --disable-libiberty controls the
> host libiberty, not the target one, another guy said the option
> controls both host and target builds.
>
> In some environment we still need to build libstdc++ libraries, where
> libiberty has to be built for target. Can we use #ifndef __ANDROID__
> to wrap around the getpagesize() definition? It is working for us.
>
> Thanks,
> Jing
>
>
> On Tue, May 24, 2011 at 11:07 AM, Doug Kwan (關振德) <dougkwan@google.com> wrote:
>> Shouldn't we test
>>
>> ifndef __ANDROID__
>>
>> instead?
>>
>> -Doug
>>
>> On Tue, May 24, 2011 at 2:39 AM, Guozhi Wei <carrot@google.com> wrote:
>>> Hi
>>>
>>> This patch is for google/main.
>>>
>>> In order to be compatible with current bionic and sysroot, we need to
disable
>>> getpagesize(). After getpagesize() in bionic is changed and ndk contains
that
>>> change, we can reenable it.
>>>
>>> Jing can give more details about it.
>>>
>>> This patch has been tested on arm qemu without regression.
>>>
>>> thanks
>>> Carrot
>>>
>>> 2011-05-24  Jing Yu  <jingyu@google.com>
>>>
>>>        * ChangeLog.google-main: New file.
>>>        * getpagesize.c(getpagesize): Disable it for bionic.
>>>
>>>
>>> Index: ChangeLog.google-main
>>> ===================================================================
>>> --- ChangeLog.google-main       (revision 0)
>>> +++ ChangeLog.google-main       (revision 0)
>>> @@ -0,0 +1,5 @@
>>> +Copyright (C) 2011 Free Software Foundation, Inc.
>>> +
>>> +Copying and distribution of this file, with or without modification,
>>> +are permitted in any medium without royalty provided the copyright
>>> +notice and this notice are preserved.
>>> Index: getpagesize.c
>>> ===================================================================
>>> --- getpagesize.c       (revision 174099)
>>> +++ getpagesize.c       (working copy)
>>> @@ -60,11 +60,13 @@ BUGS
>>>  # endif /* PAGESIZE */
>>>  #endif /* GNU_OUR_PAGESIZE */
>>>
>>> +#if DEFAULT_LIBC != LIBC_BIONIC
>>>  int
>>>  getpagesize (void)
>>>  {
>>>   return (GNU_OUR_PAGESIZE);
>>>  }
>>> +#endif
>>>
>>>  #else /* VMS */
>>>
>>>
>>> --
>>> This patch is available for review at http://codereview.appspot.com/4515131
>>>
>>
>

[jingyu, 2011-05-26 04:27:55]

Actually I don't know why in gcc-4.6 libiberty is built for target
when --disable-stdc++-v3 is given. Is libiberty used by other target
libraries now? If not, we can skip libiberty for general Android
toolchain.
But, I still think it would be nice to make libiberty work, in case it
is needed in some cases.

Jing

On Wed, May 25, 2011 at 5:25 PM, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> Jing
>
> Can't we just skip libiberty in top-level configure.ac?  Look for the
> comment "Disable target libiberty for some systems."
>
> -Doug
>
> On Wed, May 25, 2011 at 5:17 PM, Jing Yu <jingyu@google.com> wrote:
>> Hi Joseph,
>>
>> We notice that gcc starts to build libiberty for target as a multilib
>> since gcc-4.6, even if we give --disable-stdc++-v3.
>> Since then, we run into the incompatible getpagesize() error.
>>
>> I am wondering how to disable build of libiberty for target? I
>> searched mailing list. One guy said --disable-libiberty controls the
>> host libiberty, not the target one, another guy said the option
>> controls both host and target builds.
>>
>> In some environment we still need to build libstdc++ libraries, where
>> libiberty has to be built for target. Can we use #ifndef __ANDROID__
>> to wrap around the getpagesize() definition? It is working for us.
>>
>> Thanks,
>> Jing
>>
>>
>> On Tue, May 24, 2011 at 11:07 AM, Doug Kwan (關振德) <dougkwan@google.com>
wrote:
>>> Shouldn't we test
>>>
>>> ifndef __ANDROID__
>>>
>>> instead?
>>>
>>> -Doug
>>>
>>> On Tue, May 24, 2011 at 2:39 AM, Guozhi Wei <carrot@google.com> wrote:
>>>> Hi
>>>>
>>>> This patch is for google/main.
>>>>
>>>> In order to be compatible with current bionic and sysroot, we need to
disable
>>>> getpagesize(). After getpagesize() in bionic is changed and ndk contains
that
>>>> change, we can reenable it.
>>>>
>>>> Jing can give more details about it.
>>>>
>>>> This patch has been tested on arm qemu without regression.
>>>>
>>>> thanks
>>>> Carrot
>>>>
>>>> 2011-05-24  Jing Yu  <jingyu@google.com>
>>>>
>>>>        * ChangeLog.google-main: New file.
>>>>        * getpagesize.c(getpagesize): Disable it for bionic.
>>>>
>>>>
>>>> Index: ChangeLog.google-main
>>>> ===================================================================
>>>> --- ChangeLog.google-main       (revision 0)
>>>> +++ ChangeLog.google-main       (revision 0)
>>>> @@ -0,0 +1,5 @@
>>>> +Copyright (C) 2011 Free Software Foundation, Inc.
>>>> +
>>>> +Copying and distribution of this file, with or without modification,
>>>> +are permitted in any medium without royalty provided the copyright
>>>> +notice and this notice are preserved.
>>>> Index: getpagesize.c
>>>> ===================================================================
>>>> --- getpagesize.c       (revision 174099)
>>>> +++ getpagesize.c       (working copy)
>>>> @@ -60,11 +60,13 @@ BUGS
>>>>  # endif /* PAGESIZE */
>>>>  #endif /* GNU_OUR_PAGESIZE */
>>>>
>>>> +#if DEFAULT_LIBC != LIBC_BIONIC
>>>>  int
>>>>  getpagesize (void)
>>>>  {
>>>>   return (GNU_OUR_PAGESIZE);
>>>>  }
>>>> +#endif
>>>>
>>>>  #else /* VMS */
>>>>
>>>>
>>>> --
>>>> This patch is available for review at http://codereview.appspot.com/4515131
>>>>
>>>
>>
>

[carrot, 2011-05-26 08:58:12]

Hi

I've tested the

#ifndef __ANDROID__

on arm qemu without regression. And also built Android toolchain
without this error.

thanks
Carrot


2011-05-26  Jing Yu  <jingyu@google.com>

        * ChangeLog.google-main: New file.
        * getpagesize.c(getpagesize): Disable it for bionic.


Index: ChangeLog.google-main
===================================================================
--- ChangeLog.google-main	(revision 0)
+++ ChangeLog.google-main	(revision 0)
@@ -0,0 +1,5 @@
+Copyright (C) 2011 Free Software Foundation, Inc.
+
+Copying and distribution of this file, with or without modification,
+are permitted in any medium without royalty provided the copyright
+notice and this notice are preserved.
Index: getpagesize.c
===================================================================
--- getpagesize.c	(revision 174099)
+++ getpagesize.c	(working copy)
@@ -60,11 +60,13 @@ BUGS
 # endif /* PAGESIZE */
 #endif /* GNU_OUR_PAGESIZE */

+#ifndef __ANDROID__
 int
 getpagesize (void)
 {
   return (GNU_OUR_PAGESIZE);
 }
+#endif

 #else /* VMS */


On Wed, May 25, 2011 at 2:07 AM, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> Shouldn't we test
>
> ifndef __ANDROID__
>
> instead?
>
> -Doug
>
> On Tue, May 24, 2011 at 2:39 AM, Guozhi Wei <carrot@google.com> wrote:
>> Hi
>>
>> This patch is for google/main.
>>
>> In order to be compatible with current bionic and sysroot, we need to disable
>> getpagesize(). After getpagesize() in bionic is changed and ndk contains that
>> change, we can reenable it.
>>
>> Jing can give more details about it.
>>
>> This patch has been tested on arm qemu without regression.
>>
>> thanks
>> Carrot
>>
>> 2011-05-24  Jing Yu  <jingyu@google.com>
>>
>>        * ChangeLog.google-main: New file.
>>        * getpagesize.c(getpagesize): Disable it for bionic.
>>
>>
>> Index: ChangeLog.google-main
>> ===================================================================
>> --- ChangeLog.google-main       (revision 0)
>> +++ ChangeLog.google-main       (revision 0)
>> @@ -0,0 +1,5 @@
>> +Copyright (C) 2011 Free Software Foundation, Inc.
>> +
>> +Copying and distribution of this file, with or without modification,
>> +are permitted in any medium without royalty provided the copyright
>> +notice and this notice are preserved.
>> Index: getpagesize.c
>> ===================================================================
>> --- getpagesize.c       (revision 174099)
>> +++ getpagesize.c       (working copy)
>> @@ -60,11 +60,13 @@ BUGS
>>  # endif /* PAGESIZE */
>>  #endif /* GNU_OUR_PAGESIZE */
>>
>> +#if DEFAULT_LIBC != LIBC_BIONIC
>>  int
>>  getpagesize (void)
>>  {
>>   return (GNU_OUR_PAGESIZE);
>>  }
>> +#endif
>>
>>  #else /* VMS */
>>
>>
>> --
>> This patch is available for review at http://codereview.appspot.com/4515131
>>
>

[joseph_codesourcery.com, 2011-05-26 12:00:23]

On Wed, 25 May 2011, Jing Yu wrote:

> I am wondering how to disable build of libiberty for target? I

Tear out all the target-libiberty code unconditionally?  See 
<http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01308.html> and references 
therein; building target libiberty at all is a bug in my view.

> In some environment we still need to build libstdc++ libraries, where
> libiberty has to be built for target. Can we use #ifndef __ANDROID__
> to wrap around the getpagesize() definition? It is working for us.

See what I said in 
<http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01311.html>.  libstdc++-v3 
does not use target libiberty; it uses one source file from the libiberty 
directory.

-- 
Joseph S. Myers
joseph@codesourcery.com

[jingyu, 2011-05-26 23:47:46]

I have tested the following patch to skip building target libiberty
for arm*-*-linux-androideabi. It works as intended when building
Android arm-linux-androideabi toolchain.

Doug, do we want to skip building libiberty for non arm android
toolchain, say *-linux-android*?

Joseph, do you think if similar patch is possible for trunk? I will
send a separate email for review if this approach is the right way to
go.

Thanks,
Jing

Index: configure.ac
===================================================================
--- configure.ac	(revision 174299)
+++ configure.ac	(working copy)
@@ -515,7 +515,7 @@
   sh*-*-pe|mips*-*-pe|*arm-wince-pe)
     noconfigdirs="$noconfigdirs target-libiberty"
     ;;
-  arm*-*-symbianelf*)
+  arm*-*-symbianelf*|arm*-*-linux-androideabi)
     noconfigdirs="$noconfigdirs target-libiberty"
     ;;
   avr-*-*)
Index: configure
===================================================================
--- configure	(revision 174299)
+++ configure	(working copy)
@@ -3069,7 +3069,7 @@
   sh*-*-pe|mips*-*-pe|*arm-wince-pe)
     noconfigdirs="$noconfigdirs target-libiberty"
     ;;
-  arm*-*-symbianelf*)
+  arm*-*-symbianelf*|arm*-*-linux-androideabi)
     noconfigdirs="$noconfigdirs target-libiberty"
     ;;
   avr-*-*)




On Thu, May 26, 2011 at 5:00 AM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Wed, 25 May 2011, Jing Yu wrote:
>
>> I am wondering how to disable build of libiberty for target? I
>
> Tear out all the target-libiberty code unconditionally?  See
> <http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01308.html> and references
> therein; building target libiberty at all is a bug in my view.
>
>> In some environment we still need to build libstdc++ libraries, where
>> libiberty has to be built for target. Can we use #ifndef __ANDROID__
>> to wrap around the getpagesize() definition? It is working for us.
>
> See what I said in
> <http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01311.html>.  libstdc++-v3
> does not use target libiberty; it uses one source file from the libiberty
> directory.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>

[pinskia_gmail.com, 2011-05-26 23:49:24]

On Thu, May 26, 2011 at 4:47 PM, Jing Yu <jingyu@google.com> wrote:
> I have tested the following patch to skip building target libiberty
> for arm*-*-linux-androideabi. It works as intended when building
> Android arm-linux-androideabi toolchain.
>
> Doug, do we want to skip building libiberty for non arm android
> toolchain, say *-linux-android*?
>
> Joseph, do you think if similar patch is possible for trunk? I will
> send a separate email for review if this approach is the right way to
> go.

What Joseph recommended was ripping out all support for building
libiberty for the target side as it is not needed.  Your patch just
skips it for those targets.

-- Pinski