add a macro that really identifies glibc

add a macro that really identifies glibc

uClibc pretends to be glibc, so just checking for __GLIBC__ doesn't always
work.  Rather than check for defined(__GLIBC__) && !defined(__UCLIBC__) in
multiple places, do it once and define LIBC_GLIBC if we're certain that we're
really using glibc.

NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201761

[Mostyn Bramley-Moore, 2013-05-18 06:40:15]

This patch makes it simpler to check for glibc before using glibc specific
features, and includes one sample usage.

[agl, 2013-05-20 15:18:22]

https://codereview.chromium.org/15405003/diff/1/base/process_util_linux.cc
File base/process_util_linux.cc (right):

https://codereview.chromium.org/15405003/diff/1/base/process_util_linux.cc#ne...
base/process_util_linux.cc:748: #if defined(GLIBC) && !defined(USE_TCMALLOC) &&
\
It's not clear that we don't actually want this malloc behaviour on Android. It
may have been an expedient choice during porting that hasn't been fixed.

It's doubly not clear that we don't want this malloc behaviour for uClibc in
general. We might need to do a dlsym dance to get the original pointers, but
that's very different than just disabling it.

[Mostyn Bramley-Moore, 2013-05-20 20:41:41]

https://codereview.chromium.org/15405003/diff/1/base/process_util_linux.cc
File base/process_util_linux.cc (right):

https://codereview.chromium.org/15405003/diff/1/base/process_util_linux.cc#ne...
base/process_util_linux.cc:748: #if defined(GLIBC) && !defined(USE_TCMALLOC) &&
\
On 2013/05/20 15:18:22, agl wrote:
> It's not clear that we don't actually want this malloc behaviour on Android.
It
> may have been an expedient choice during porting that hasn't been fixed.

I downloaded android-ndk-r8e and inspected
platforms/android-14/arch-arm/usr/lib/libc.so - it does not have any of the
__libc_* symbols used below.  Similarly for the x86 version, and also with
uClibc.  So it seems safe to say that this code will only work with glibc,
though I don't have definitive documentation of this.  

> It's doubly not clear that we don't want this malloc behaviour for uClibc in
> general. We might need to do a dlsym dance to get the original pointers, but
> that's very different than just disabling it. 

You're probably right, but I think this might be better implemented in a
followup task.  Would you settle for a TODO note inside some rearranged ifdefs?
ie something like:

#if !defined(ADDRESS_SANITIZER) && !defined(MEMORY_SANITIZER) &&
!defined(THREAD_SANITIZER)

#if defined(GLIBC) && !defined(USE_TCMALLOC)
... make use of __libc_* ...
#else
// TODO: dylsym dance
#endif

#endif

[agl, 2013-05-21 14:40:20]

On 2013/05/20 20:41:41, Mostyn Bramley-Moore wrote:
> You're probably right, but I think this might be better implemented in a
> followup task.  Would you settle for a TODO note inside some rearranged
ifdefs?
> ie something like:
> 
> #if !defined(ADDRESS_SANITIZER) && !defined(MEMORY_SANITIZER) &&
> !defined(THREAD_SANITIZER)
> 
> #if defined(GLIBC) && !defined(USE_TCMALLOC)
> ... make use of __libc_* ...
> #else
> // TODO: dylsym dance
> #endif
> 
> #endif

It seems clear that this isn't making the situation any worse at least, so I
think that would be fine.

(Although I'm not OWNERS in base/, so someone else would have to approve.)

[Mostyn Bramley-Moore, 2013-05-21 14:59:48]

On 2013/05/21 14:40:20, agl wrote:
> It seems clear that this isn't making the situation any worse at least, so I
> think that would be fine.

Done.  I think this is a slight improvement because the glibc dependency has
been made obvious.  

> (Although I'm not OWNERS in base/, so someone else would have to approve.)

Added another base OWNER (brettw)...

[Mostyn Bramley-Moore, 2013-05-22 20:29:18]

Renamed GLIBC to LIBC_GLIBC on Paweł's advice, to avoid collisions.

[jar (doing other things), 2013-05-22 21:27:10]

Seems reasonable... and based on agl and pawel commentary.. LGTM

[commit-bot: I haz the power, 2013-05-22 21:43:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mostynb@opera.com/15405003/10001

[tfarina, 2013-05-22 23:26:08]

https://codereview.chromium.org/15405003/diff/10001/base/process_util_linux.cc
File base/process_util_linux.cc (right):

https://codereview.chromium.org/15405003/diff/10001/base/process_util_linux.c...
base/process_util_linux.cc:833: // TODO: dlsym dance
nit: TODO(username):

please.

[Mostyn Bramley-Moore, 2013-05-22 23:32:16]

https://codereview.chromium.org/15405003/diff/10001/base/process_util_linux.cc
File base/process_util_linux.cc (right):

https://codereview.chromium.org/15405003/diff/10001/base/process_util_linux.c...
base/process_util_linux.cc:833: // TODO: dlsym dance
On 2013/05/22 23:26:09, tfarina wrote:
> nit: TODO(username):
> 
> please.

Done.

[commit-bot: I haz the power, 2013-05-22 23:33:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mostynb@opera.com/15405003/33001

[Mostyn Bramley-Moore, 2013-05-23 11:51:25]

Passed all the related try bots, only failing on win7_aura with an unrelated
error (this patch only affects linux).

Resubmitting with NOTRY=true based on advice from nkostylev.

[commit-bot: I haz the power, 2013-05-23 11:51:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mostynb@opera.com/15405003/33001

[commit-bot: I haz the power, 2013-05-23 12:00:11]

Change committed as 201761