diff mbox

[google] Disable getpagesize() for Android toolchain (issue4515131)

Message ID 20110524093904.2498F2072D@guozhiwei.sha.corp.google.com
State New
Headers show

Commit Message

Carrot Wei May 24, 2011, 9:39 a.m. UTC
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.



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

Comments

Joseph Myers May 24, 2011, 11:19 a.m. UTC | #1
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.)
Doug Kwan (關振德) May 24, 2011, 6:07 p.m. UTC | #2
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
>
Jing Yu May 26, 2011, 12:17 a.m. UTC | #3
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
>>
>
Doug Kwan (關振德) May 26, 2011, 12:25 a.m. UTC | #4
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
>>>
>>
>
Jing Yu May 26, 2011, 4:27 a.m. UTC | #5
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
>>>>
>>>
>>
>
Joseph Myers May 26, 2011, noon UTC | #6
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.
diff mbox

Patch

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 */