Don't ship a full C++ demangler as part of libc++abi

Don't ship a full C++ demangler as part of libc++abi

The demangler is *huge* and only used in default_terminate_handler with
a fallback to printing mangled names instead. pexe size matters a lot,
so PNaCl only prints out mangled names when exceptions are uncaught.

R=jvoung@chromium.org, dschuff@chromium.org
BUG= https://code.google.com/p/nativeclient/issues/detail?id=3623

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-libcxxabi.git;a=commit;h=8ffb0bd

[JF, 2013-12-04 01:20:57]

Here are some gorry numbers on the life test from llvm-testsuite:

Size of
./pnacl/build/c++-stdlib-newlib-portable-libc++/pnacl-target/lib/libc++.a 
Before removing __cxa_demangle: 9259594
After:                          7039778

Compile life:
toolchain/pnacl_linux_x86/bin/pnacl-clang++ -O2
./pnacl/git/llvm-test-suite/MultiSource/Benchmarks/Prolangs-C++/life/life.cpp
--pnacl-driver-verbose --pnacl-allow-exceptions -o life.nonfinal.pexe && 
toolchain/pnacl_linux_x86/bin/pnacl-finalize --no-finalize life.nonfinal.pexe -o
life.final.pexe && toolchain/pnacl_linux_x86/bin/pnacl-translate  -arch x86-32
--pnacl-allow-exceptions --allow-llvm-bitcode-input life.final.pexe -o life.nexe

Before removing __cxa_demangle:
4205908 life.final.pexe
1709964 life.nexe*
4205908 life.nonfinal.pexe*

After:
 823184 life.final.pexe
 526468 life.nexe*
 823184 life.nonfinal.pexe*

[jvoung (off chromium), 2013-12-04 01:51:59]

Some minor comments below.

Too bad that this demangler is so big.  Besides internal cxx-abi uses, it sounds
like this is supposed to be supplied, but the pexe size increase is too much. If
we do end up wanting a demangler to fulfill the API requirements, maybe we could
do some frankenstein build and use the libiberty/libsupc++ one =/

https://codereview.chromium.org/102883003/diff/1/src/cxa_demangle.cpp
File src/cxa_demangle.cpp (right):

https://codereview.chromium.org/102883003/diff/1/src/cxa_demangle.cpp#newcode...
src/cxa_demangle.cpp:4607: *status = 1;
The API says that status may be a null pointer (in which case the caller
wouldn't care about the status), so would be good to check first.

Also, it seems like the documented status values are 0, -1, -2, -3:
http://mentorembedded.github.io/cxx-abi/abi.html#demangler

Maybe do -1 and pretend that there is no memory for it?

[JF, 2013-12-04 02:10:25]

https://codereview.chromium.org/102883003/diff/1/src/cxa_demangle.cpp
File src/cxa_demangle.cpp (right):

https://codereview.chromium.org/102883003/diff/1/src/cxa_demangle.cpp#newcode...
src/cxa_demangle.cpp:4607: *status = 1;
On 2013/12/04 01:52:00, jvoung (cr) wrote:
> The API says that status may be a null pointer (in which case the caller
> wouldn't care about the status), so would be good to check first.
> 
> Also, it seems like the documented status values are 0, -1, -2, -3:
> http://mentorembedded.github.io/cxx-abi/abi.html#demangler
> 
> Maybe do -1 and pretend that there is no memory for it?

I fixed this and also moved the code down so it still does the checks below.

[jvoung (off chromium), 2013-12-04 02:32:38]

lgtm

[JF, 2013-12-04 03:07:16]

Committed patchset #3 manually as r8ffb0bd (presubmit successful).

[eliben_google.com, 2013-12-04 17:20:37]

On Tue, Dec 3, 2013 at 5:51 PM, <jvoung@chromium.org> wrote:

> Some minor comments below.
>
> Too bad that this demangler is so big.  Besides internal cxx-abi uses, it
> sounds
> like this is supposed to be supplied, but the pexe size increase is too
> much. If
> we do end up wanting a demangler to fulfill the API requirements, maybe we
> could
> do some frankenstein build and use the libiberty/libsupc++ one =/
>

Can we provide it on the translator side as a native library?


>
>
>
>
> https://codereview.chromium.org/102883003/diff/1/src/cxa_demangle.cpp
> File src/cxa_demangle.cpp (right):
>
> https://codereview.chromium.org/102883003/diff/1/src/cxa_
> demangle.cpp#newcode4607
> src/cxa_demangle.cpp:4607: *status = 1;
> The API says that status may be a null pointer (in which case the caller
> wouldn't care about the status), so would be good to check first.
>
> Also, it seems like the documented status values are 0, -1, -2, -3:
> http://mentorembedded.github.io/cxx-abi/abi.html#demangler
>
> Maybe do -1 and pretend that there is no memory for it?
>
>
> https://codereview.chromium.org/102883003/
>
> --
> You received this message because you are subscribed to the Google Groups
> "Native-Client-Reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to native-client-reviews+unsubscribe@googlegroups.com.
> To post to this group, send email to native-client-reviews@
> googlegroups.com.
> Visit this group at http://groups.google.com/group/native-client-reviews.
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/groups/opt_out.

[JF, 2013-12-04 17:40:22]

> Can we provide it on the translator side as a native library?

It's in:
  tools/llvm-symbolizer/LLVMSymbolize.cpp

Though it calls the function directly so we'd need some magic to stub out the
behavior for the translator but not for the native tool (I assume you mean as a
dev tool?).

[jvoung - send to chromium..., 2013-12-04 17:59:41]

On 2013/12/04 17:20:37, eliben_google.com wrote:
> On Tue, Dec 3, 2013 at 5:51 PM, <mailto:jvoung@chromium.org> wrote:
> 
> > Some minor comments below.
> >
> > Too bad that this demangler is so big.  Besides internal cxx-abi uses, it
> > sounds
> > like this is supposed to be supplied, but the pexe size increase is too
> > much. If
> > we do end up wanting a demangler to fulfill the API requirements, maybe we
> > could
> > do some frankenstein build and use the libiberty/libsupc++ one =/
> >
> 
> Can we provide it on the translator side as a native library?
> 


Is this so that it can still be exposed to pexes, but be shared between
different pexes without bloating download size and increasing translation time?

For the pexe to call into a native library we would need to make an intrinsic
for this (at least that's how we've done it before).  It would also add to the
translator's size instead.

I'm not sure of the other use cases for __cxa_demangle that a developer would
want for their pexe (so is this worth it?). llvm-symbolizer is one example that
JF listed, and printing error messages for unhandled exceptions is another. If
people want to make a c++filt webapp, or a linker webapp with pretty error
messages, they might want it too. Will have to ask some of the more experienced
folks for other use cases =)

On the other hand, the app developer could grab a demangler from libiberty,
libsupc++, or libcxxabi, but name it differently (__my_cxa_demangle) so that it
doesn't clash with this stubbed out __cxa_demangle, and have the app call the
__my_cxa_demangle instead.

[Mark Seaborn, 2013-12-04 18:10:51]

So, both libstdc++ and libc++ have/had a working __cxa_demangle():

$ pnacl-clang++ cxa_demangle.cc -o cxa_demangle.pexe -O2 -stdlib=libstdc++
$ ls -l cxa_demangle.pexe
-rwxr-x--- 1 mseaborn eng 898356 Dec  4 10:07 cxa_demangle.pexe

$ pnacl-clang++ cxa_demangle.cc -o cxa_demangle.pexe -O2 -stdlib=libc++
$ ls -l cxa_demangle.pexe
-rwxr-x--- 1 mseaborn eng 3370820 Dec  4 10:07 cxa_demangle.pexe

How come the former is so much smaller?  Does it handle fewer cases?  Is it
better written?  Or does it pull in a smaller set of dependencies?

[jvoung (off chromium), 2013-12-04 18:21:30]

On 2013/12/04 18:10:51, Mark Seaborn wrote:
> So, both libstdc++ and libc++ have/had a working __cxa_demangle():
> 
> $ pnacl-clang++ cxa_demangle.cc -o cxa_demangle.pexe -O2 -stdlib=libstdc++
> $ ls -l cxa_demangle.pexe
> -rwxr-x--- 1 mseaborn eng 898356 Dec  4 10:07 cxa_demangle.pexe
> 
> $ pnacl-clang++ cxa_demangle.cc -o cxa_demangle.pexe -O2 -stdlib=libc++
> $ ls -l cxa_demangle.pexe
> -rwxr-x--- 1 mseaborn eng 3370820 Dec  4 10:07 cxa_demangle.pexe
> 
> How come the former is so much smaller?  Does it handle fewer cases?  Is it
> better written?  Or does it pull in a smaller set of dependencies?

There is some related discussion here:
http://clang-developers.42468.n3.nabble.com/libc-abi-Why-is-cxa-demangle-h-a-...

It sounds like it is doing more and has more dependencies:
- "create a syntax tree and expose that syntax tree for use by the compiler
and/or debugger."
- maybe the libc++ one handles more c++11-isms, while the libstdc++ version is
older
- the libc++ ones uses a bunch of C++ vectors and other C++ library facilities,
while the libstdc++/libiberty one is written in C.

[nfullagar, 2013-12-04 19:37:06]

XRay (profiler) uses cxa_demangle.  It can be built without cxa_demangle,
but the output, when sent directly to about:tracing, is less readable.
 Another option for projects built with xray is to run c++filt on the
mapfile as part of the build process.



On Wed, Dec 4, 2013 at 10:21 AM, <jvoung@chromium.org> wrote:

> On 2013/12/04 18:10:51, Mark Seaborn wrote:
>
>> So, both libstdc++ and libc++ have/had a working __cxa_demangle():
>>
>
>  $ pnacl-clang++ cxa_demangle.cc -o cxa_demangle.pexe -O2 -stdlib=libstdc++
>> $ ls -l cxa_demangle.pexe
>> -rwxr-x--- 1 mseaborn eng 898356 Dec  4 10:07 cxa_demangle.pexe
>>
>
>  $ pnacl-clang++ cxa_demangle.cc -o cxa_demangle.pexe -O2 -stdlib=libc++
>> $ ls -l cxa_demangle.pexe
>> -rwxr-x--- 1 mseaborn eng 3370820 Dec  4 10:07 cxa_demangle.pexe
>>
>
>  How come the former is so much smaller?  Does it handle fewer cases?  Is
>> it
>> better written?  Or does it pull in a smaller set of dependencies?
>>
>
> There is some related discussion here:
> http://clang-developers.42468.n3.nabble.com/libc-abi-Why-is-
> cxa-demangle-h-a-public-header-td4032531.html
>
> It sounds like it is doing more and has more dependencies:
> - "create a syntax tree and expose that syntax tree for use by the compiler
> and/or debugger."
> - maybe the libc++ one handles more c++11-isms, while the libstdc++
> version is
> older
> - the libc++ ones uses a bunch of C++ vectors and other C++ library
> facilities,
> while the libstdc++/libiberty one is written in C.
>
>
>
>
>
>
> https://codereview.chromium.org/102883003/
>
> --
> You received this message because you are subscribed to the Google Groups
> "Native-Client-Reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to native-client-reviews+unsubscribe@googlegroups.com.
> To post to this group, send email to native-client-reviews@
> googlegroups.com.
> Visit this group at http://groups.google.com/group/native-client-reviews.
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/groups/opt_out.

[Derek Schuff, 2013-12-04 19:39:57]

Maybe instead of killing cxa_demangle entirely, we could instead remove its use
in default_terminate_handler, which would keep it from getting linked into
everything, but still have it around for users that want to call it themselves?