Patches to build libtcmalloc_minimal on Mac OS X about gperftools HOT 12 CLOSED

Thank you for contributing this patch!  The ppc code is particularly useful, 
since we
don't have a ppc macos box to test on.

I'd like to understand some of the changes you made, to see if they call for a 
more
general modification of the code -- that is, some implicit assumptions we had 
made,
that are easy to get rid of.  In particular:

1) In tcmalloc.cc, why did you have to #ifdef out the __libc_* functions?  What 
kinds
of errors did you get with this code in there?  Maybe __libc_* is only defined 
on
some systems?  Maybe we should try to define these things only on redhat 9 
systems,
since those are the ones we had a bug report about which prompted this code in 
the
first place.

2) I don't know anything about the ppc, but it seems to me the 
stacktrace_ppc-inl.h
file doesn't actually have any ppc-specific code in it.  Is that right?  I'm
wondering if it makes sense to put this code -- or something much like it -- in
stacktrace_generic-inl.h, and just run it on any system that supports
__builtin_frame_address.

3) Why did you get rid of the "if (!new_sp) break;" test in 
stacktrace_x86-inl.h (and
in stacktrace_ppc-inl.h)?

4) In atomicops.h, you have
   #if defined(__MACH__) && defined(__APPLE__) && defined(__ppc__)
Is the test for __APPLE__ necessary?  Or is just knowing it's Mach (OS X) on ppc
enough?  I just want to make sure I understand what's going on with the defines 
you use.

5) Why can't we define ATTRIBUTE_WEAK et al. on Mach (in basictypes.h)?

Thanks again for this contribution!  It's nice to see tcmalloc_minimal become 
usable
on so many different systems.

(Also, to be clear: this patch is against google-perftools 0.91.)

I posted the following to the group, I didn't know it wasn't reflected in the
tracker... Here it goes anyway:

On May 17, 8:56 am, [email protected] wrote:

> Issue 15: Patches to build libtcmalloc_minimal on Mac OS
Xhttp://code.google.com/p/google-perftools/issues/detail?id=15

> Comment #1 by csilvers:
> Thank you for contributing this patch!  The ppc code is particularly useful, 
since we
> don't have a ppc macos box to test on.

Nor do I. I just tested using the cross compiler installed on Mac OS X
boxes and ran it under Rosetta. The ppc code is heavily untested
though. I haven't verified the backtrace is correct.

> I'd like to understand some of the changes you made, to see if they call for 
a more
> general modification of the code -- that is, some implicit assumptions we had 
made,
> that are easy to get rid of.  In particular:

> 1) In tcmalloc.cc, why did you have to #ifdef out the __libc_* functions?  
What kinds
> of errors did you get with this code in there?  Maybe __libc_* is only 
defined on
> some systems?  Maybe we should try to define these things only on redhat 9 
systems,
> since those are the ones we had a bug report about which prompted this code 
in the
> first place.

Those are glibc-specific functions. __libc_* simply doesn't exist on
Darwin (or BSD systems for the matter).
Probably the Right Thing is enclosing that code in:

#if defined(__GLIBC__)

> 2) I don't know anything about the ppc, but it seems to me the 
stacktrace_ppc-inl.h
> file doesn't actually have any ppc-specific code in it.  Is that right?  I'm
> wondering if it makes sense to put this code -- or something much like it -- 
in
> stacktrace_generic-inl.h, and just run it on any system that supports
> __builtin_frame_address.

Could be an option, but first let me check if the code works
correctly :-) It was dumping something that looked like a backtrace,
but you never know...

> 3) Why did you get rid of the "if (!new_sp) break;" test in
stacktrace_x86-inl.h (and
> in stacktrace_ppc-inl.h)?

Because it was just noise? It made the whole thing look more
complicated than it really is.

> 4) In atomicops.h, you have
>    #if defined(__MACH__) && defined(__APPLE__) && 
defined(__ppc__)
> Is the test for __APPLE__ necessary?  Or is just knowing it's Mach (OS X) on 
ppc
> enough?  I just want to make sure I understand what's going on with the 
defines you
use.

It is necessary because OSAtomic.h is AFAIK only present in Darwin.
You could try to compile it on another mach-based OS and it would
fail.

> 5) Why can't we define ATTRIBUTE_WEAK et al. on Mach (in basictypes.h)?

Mach uses Mach-O as its object format, and it just won't link using
those constructs.

Greetings,
  Jorge Acereda 

I looked into this a bit more today.  It turns out Mach-O does support weak
references and named sections, but the syntax is a bit different than for ELF.  
I've
made the appropriate tweaks, and also incorporated some changes from your patch 
like
the #ifdef out of the __libc_foo calls, and have gotten tcmalloc_minimal to 
compile
and run on my local Mac OS X box as well.

These changes will make it into the next perftools release, hopefully within a 
few weeks.

I'd like to see more testing before adding stacktrace_ppc-inl.h into the 
mainline. 
If anyone happens to read this and decides to download this patch for their PPC 
OS X
boxes, please report your experience here!

> 2) I don't know anything about the ppc, but it seems to me the 
stacktrace_ppc-inl.h
> file doesn't actually have any ppc-specific code in it.  Is that right?  I'm
> wondering if it makes sense to put this code -- or something much like it -- 
in
> stacktrace_generic-inl.h, and just run it on any system that supports
> __builtin_frame_address.

I've coded a stacktrace_gcc-inl.h that should be more portable (based in
__builtin_get_return_address()).

I don't have my laptop here though, I'll send the patches on monday.

I actually played around quite a bit with __builtin_get_return_address (and
__builtin_get_frame_pointer) last night, and even went so far as to talk with 
the
libunwind maintainer.  It became clear pretty quickly that these approaches 
won't
work.  In particular, the code in question *does* have architecture-specific 
stuff:
it assumes that sp[0] points to the next frame, and sp[1] points to the return
address.  That's true on x86, but unlikely to be true on other architectures.  
In
particular, if it works on ppc it's a big coincidence!

So stacktrace_gcc-inl.h won't work.  I think our general approach is going to 
be to
have the "fallback" option be libunwind, which is designed to provide this
functionality for as many architectures as possible.

This is what I got. This one should be generic.

Yeah, I talked with the author of libunwind about doing something like this, 
but alas
it doesn't work either --  __builtin_return_address(x) is not guaranteed to 
give a
result for x > 0 on all architecctures.  I also talked with a gcc maintainer, 
who
said that __builtin_return_address(x>0) on some architectures may give an 
answer, but
the wrong one!

I think the only really practical thing to do is to use libunwind for 
architectures
we don't choose to support natively, and help libunwind support as many 
architectures
as possible.

libunwind seems to be too tied to ELF. If you want to rely on libunwind that's 
fine,
but keep in mind that it lacks support for lots of platforms.

I tried that code with the regression test and it seems to work. So an option 
would
be to run a configure-time test to determine if the hack works.

teste de comentario

google-perftools 0.92 has improved OS X support, including some preliminary (but
untested) PowerPC support.  I think it includes all the functionality in this 
patch,
so I'm closing this bug.