Re: [patch, testsuite] Applying non_bionic effective target to particular tests
can you file bugs against bionic for stuff like this? use b.android.com (and feel free to mail me to ensure that they get noticed). one thing we'd like to do is get to a point where we're building gcc/gdb et cetera without any local hacks, and when we've got to that point, we're going to have to go through anything that made it upstream to check that that's sane. (the weird "-shared implies -Bsymbolic" GCC hack springs to mind.) On Fri, Aug 15, 2014 at 10:25 AM, Mike Stump wrote: > On Aug 15, 2014, at 6:49 AM, Alexander Ivchenko wrote: >> * lib/target-supports.exp (error_h): New check. >> (libc_has_complex_functions): Ditto. >> (tgmath_h): Ditto. >> * gcc.dg/builtins-59.c: Add libc_has_complex_functions check. >> * gcc.dg/builtins-61.c: Likewise. >> * gcc.dg/builtins-67.c: Disable test for Bionic. >> * gcc.dg/strlenopt-14g.c: Likewise. >> * gcc.dg/strlenopt-14gf.c: Likewise. >> * gcc.dg/c99-tgmath-1.c: Add tgmath_h check. >> * gcc.dg/c99-tgmath-2.c: Likewise. >> * gcc.dg/c99-tgmath-3.c: Likewise. >> * gcc.dg/c99-tgmath-4.c: Likewise. >> * gcc.dg/dfp/convert-dfp-round-thread.c: Add error_h check. > > Ok, thanks.
Re: [patch, testsuite] Applying non_bionic effective target to particular tests
On Fri, Aug 15, 2014 at 10:56 AM, Mike Stump wrote: > On Aug 15, 2014, at 10:32 AM, enh wrote: >> can you file bugs against bionic for stuff like this? > > I suspect you meant to ask Alexander this question… but just in case you did > intend to ask me, no. I don’t have any visibility into android local patches > or procedures. yeah, i meant people proposing Android-related changes. in this case, Alexander. --elliott
Re: [patch, testsuite] Applying non_bionic effective target to particular tests
On Fri, Aug 15, 2014 at 1:05 PM, Alexander Ivchenko wrote: > 2014-08-15 21:32 GMT+04:00 enh : >> can you file bugs against bionic for stuff like this? use >> b.android.com (and feel free to mail me to ensure that they get >> noticed). > > Sure, I will do that. > >> one thing we'd like to do is get to a point where we're building >> gcc/gdb et cetera without any local hacks, and when we've got to that >> point, we're going to have to go through anything that made it >> upstream to check that that's sane. (the weird "-shared implies >> -Bsymbolic" GCC hack springs to mind.) > > There are more of those hacks for sure (actually "shared implies > -Bsymbolic" is in gcc trunk, so it is not a local hack (Although, it > doesn't neceseraly mean that it doesn't have to be changed. yeah, this is the kind of thing that worries me most: where bad ideas have been upstreamed, so now not only do we need to remove them from Android's copy, we need to get them out of upstream GCC too. > But there > are certanly other things that are local and have to be upstreamed). > From our side we are trying to upstream things first and then, if > neccessary, to port them to ndk. > > --Alexander
Re: [Android] Enable ifuncs on Android
On Thu, Nov 13, 2014 at 9:36 AM, Jeff Law wrote:
> On 11/12/14 03:02, Alexander Ivchenko wrote:
>>
>> Hi,
>>
>> Bionic - Android libc - supports indirect functions right now, but
>> they are disabled in gcc;
>>
>> We cannot do the configure-time check for that, because there is only
>> one version of each compiler shipped in ndk for all Android platforms.
>> On the other hand, having different runtime targets like -mandroid-19,
>> -mandroid-20 would be a nightmare. But, keeping in mind that the last
>> version of Android is a priority, I think that enabling ifuncs
>> unconditionally would be the right thing.
>>
>> Is the patch ok? Bootstrapped/regtested on x86_64-unknown-linux-gnu +
>> checked that the behavior of i686-linux-android is correct.
>>
>>
>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index eac19cf..9932323 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,8 @@
>> +2014-11-11 Alexander Ivchenko
>> +
>> + * config/linux.c (linux_has_ifunc_p): Remove.
>> + * config/linux.h (TARGET_HAS_IFUNC_P): Use the default version.
>
> This feels like a bad idea to me simply because a new compiler with an old
> runtime will generate code that fails, right?
yes, but that's already true of PIE or gnu-style hash or...
> If you can't do a configure-time test, then the way to go is either a
> compile-time option, or to use a different target. If there's some minimum
> version of android that has this capability, then this isn't terribly hard.
> You may not even need a config file for this since you could define
> LIBC_BIONIC_USE_IFUNCS or something like that when configured for a suitably
> new android version.
this won't make any difference to developers, though. they get their
prebuilt compilers from us, and we'll just turn all the latest options
on anyway. we don't ship a compilers for each Android version. we
already have 6 architectures * {clang,gcc} * {current,previous}version
to ship.
it doesn't seem worth having configuration in GCC that no one will ever use.
Re: [Android] Enable ifuncs on Android
On Thu, Nov 13, 2014 at 5:12 PM, Jeff Law wrote:
> On 11/13/14 10:46, enh wrote:
>>
>> This feels like a bad idea to me simply because a new compiler with an
>> old runtime will generate code that fails, right?
>>
>>
>> yes, but that's already true of PIE or gnu-style hash or...
>
> That doesn't make it the right thing to do. I would argue that's a bug that
> really needs to be fixed.
it's not a bug. no one wants 22 different compilers or configuration
options, with a new one every six months or so.
>> If you can't do a configure-time test, then the way to go is either a
>> compile-time option, or to use a different target. If there's some
>> minimum version of android that has this capability, then this isn't
>> terribly hard. You may not even need a config file for this since
>> you
>> could define LIBC_BIONIC_USE_IFUNCS or something like that when
>> configured for a suitably new android version.
>>
>>
>> this won't make any difference to the developers, though. they get their
>> prebuilt compilers from us, and we'll just turn all the latest options
>> on. we don't ship a compilers for each Android version. we already have
>> 6 architectures * {clang,gcc} * {current,previous}version to ship.
>
> But that's no reason to have a compiler which produces bogus binaries.
>
> I really think this patch is a bad idea.
so we should just change linux_has_ifunc_p to return
HAVE_GNU_INDIRECT_FUNCTION; instead?
Re: Remove unnecessary and harmful fixincludes for Android
On Tue, Aug 5, 2014 at 7:10 AM, Bruce Korb wrote:
> Hi,
>
> On Tue, Aug 5, 2014 at 4:35 AM, Alexander Ivchenko wrote:
>>> Testing for *android* is less than ideal, because of the possibility of
>>> configuring a *-linux* toolchain to have multilibs using various different
>>> C libraries (with -mandroid being used to select the Android multilib).
>>> So, specifying a bypass based on some relevant text that appears in the
>>> header would be better.
>>>
>>> --
>>> Joseph S. Myers
>>> jos...@codesourcery.com
>>
>> I've added check for "BIONIC" keyword. Hopefully it won't disappear.
>
> "based on some relevant text"
> I think that's important, too (that it be relevant).
> "BIONIC" is just some improbable text you found in the header.
> My guess is that testing for '*android*' would be more selective,
> and certainly less obscure. Who would ever guess that
> "BIONIC" implies "android"?
>
>> Updated patch:
>>
>> diff --git a/fixincludes/ChangeLog b/fixincludes/ChangeLog
>> index f7effee..e05412e 100644
>> --- a/fixincludes/ChangeLog
>> +++ b/fixincludes/ChangeLog
>> @@ -1,3 +1,10 @@
>> +2014-08-04 Alexander Ivchenko
>> +
>> + * inclhack.def (stdio_va_list): Bypass fix for Bionic.
>> + (complier_h_tradcpp): Remove.
>> + * fixincl.x: Regenerate.
>> + * tests/base/linux/compiler.h: Remove.
>> +
>> 2014-04-22 Rainer Orth
>>
>> * inclhack.def (math_exception): Bypass on *-*-solaris2.1[0-9]*.
>> diff --git a/fixincludes/fixincl.x b/fixincludes/fixincl.x
>
> [[generated text is not needed for approval]]
>
>> diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
>> index 6a1136c..bf452c6 100644
>> --- a/fixincludes/inclhack.def
>> +++ b/fixincludes/inclhack.def
>> @@ -1140,20 +1140,6 @@ fix = {
>> };
>>
>> /*
>> - * Old Linux kernel's header breaks Traditional CPP
>> - */
>> -fix = {
>> -hackname = complier_h_tradcpp;
>
> [[ OK ]]
>
>> @@ -3722,8 +3708,9 @@ fix = {
>> fix = {
>> hackname = stdio_va_list;
>> files= stdio.h;
>> -bypass = '__gnuc_va_list|_BSD_VA_LIST_|__DJ_va_list|_G_va_list';
>> +bypass =
>> '__gnuc_va_list|_BSD_VA_LIST_|__DJ_va_list|_G_va_list|BIONIC';
>> /*
>> + * In Bionic va_list is always appropriately typedefed to
>> __gnuc_va_list.
>
> And that typedef does not live in stdio.h either.
> If there is no better way to identify this file, then
>
>> Is it ok?
>
> It is "okay". (You may be left with little choice -- I can't see the header
> :).
you can see the current version of bionic's stdio.h here:
https://android.googlesource.com/platform/bionic/+/master/libc/include/stdio.h
i'm happy to add any string to the header file that makes things
easier. if you want 'x-gcc-no-fixincludes' or whatever in there, just
say :-)
(you're correct that the string 'BIONIC' is currently there only as a
side-effect; our FORTIFY_SOURCE implementation uses a
__BIONIC_FORTIFY_INLINE macro.)
Re: Remove unnecessary and harmful fixincludes for Android
does https://android-review.googlesource.com/103445 look okay? On Tue, Aug 5, 2014 at 12:01 PM, Bruce Korb wrote: > Hi, > > On Tue, Aug 5, 2014 at 10:36 AM, enh wrote: >> you can see the current version of bionic's stdio.h here: >> >> https://android.googlesource.com/platform/bionic/+/master/libc/include/stdio.h >> >> i'm happy to add any string to the header file that makes things >> easier. if you want 'x-gcc-no-fixincludes' or whatever in there, just >> say :-) > > That would be great, but you could also add: > >/* this file depends on __gnuc_va_list being used for va_list */ > > and not bother changing fixincludes at all. :) But either of those two > comments added to the header would be preferable to looking for "BIONIC". > Thank you! > > With one of the two changes, the patch is approved. Thanks!
Re: Remove unnecessary and harmful fixincludes for Android
On Tue, Aug 5, 2014 at 5:26 PM, Bruce Korb wrote: > Hi, > > Lines 42 & 43 are not needed for fixincludes, but it is your choice. > With that change, you should not need to add that test to fixincludes > because __gnuc_va_list will be found within the comment and satisfy > the "bypass" expression. okay, i'll reword to explicitly say that it's the reference to __gnuc_va_list that gets us the fixincludes behavior we want (which should also ensure that no one "cleans up" the reference to, say, __builtin_va_list). > That was the long way of saying: > Looks good to me. > > On Tue, Aug 5, 2014 at 5:09 PM, enh wrote: >> does https://android-review.googlesource.com/103445 look okay? >> >> On Tue, Aug 5, 2014 at 12:01 PM, Bruce Korb wrote: >>> Hi, >>> >>> On Tue, Aug 5, 2014 at 10:36 AM, enh wrote: >>>> you can see the current version of bionic's stdio.h here: >>>> >>>> https://android.googlesource.com/platform/bionic/+/master/libc/include/stdio.h >>>> >>>> i'm happy to add any string to the header file that makes things >>>> easier. if you want 'x-gcc-no-fixincludes' or whatever in there, just >>>> say :-) >>> >>> That would be great, but you could also add: >>> >>>/* this file depends on __gnuc_va_list being used for va_list */ >>> >>> and not bother changing fixincludes at all. :) But either of those two >>> comments added to the header would be preferable to looking for "BIONIC". >>> Thank you! >>> >>> With one of the two changes, the patch is approved. Thanks!
