Symbolization of globals is broken on Android

[GoogleCodeExporter]

There is no __cxa_demangle.
On the other hand, llvm-symbolizer works perfectly well with both CODE and DATA 
requests.
We should teach it to understand DEMANGLE requests and use them instead of 
__cxa_demangle. Or at least as a fallback.

Original issue reported on code.google.com by euge...@google.com on 14 Feb 2014 at 3:54

[GoogleCodeExporter]

Okay, this is weird.
By default, Android apps are built with static libstdc++ (or some other c++ 
library), and shared ASan runtime. As a result, __cxa_demangle is not exported 
from the binary (it is normally not even included in the binary, unless it is 
used in the application).

Original comment by euge...@google.com on 17 Feb 2014 at 8:42

[GoogleCodeExporter]

Fixed in r201545 by always linking __cxa_demangle into ASan runtime.

Original comment by euge...@google.com on 18 Feb 2014 at 7:41

• Changed state: Fixed

[GoogleCodeExporter]

Unfixed: stlport NDK toolchain does not have __cxa_demangle. At all.
Need compile-time detection.

Original comment by euge...@google.com on 12 Mar 2014 at 8:15

• Changed state: Accepted

[GoogleCodeExporter]

This is also an issue for MSan when running with instrumented libc++abi.
I think we should delegate demangling to the symbolizer, after all.

Original comment by euge...@google.com on 30 Jun 2014 at 11:37

[GoogleCodeExporter]

Just to make sure I understand the problem: llvm-symbolizer also just issues a 
call to __cxa_demangle. If __cxa_demangle is unavailable on Android, where 
would llvm-symbolizer on Android find it?

We can also consider getting rid of global name demangling completely - e.g. do 
smth. similar as LLVM r212188 and pass the unmangled qualified global names 
from frontend to the instrumentation pass, and then emit them to binary.

Original comment by samso...@google.com on 2 Jul 2014 at 9:39

[GoogleCodeExporter]

On Android c++ stdlib is normally linked statically. Some of them have 
__cxa_demangle, some don't. When building llvm-symbolizer we can pick the right 
one.

Emitting unmangled names sounds like a better solution. Would you have time to 
look at it?

Original comment by euge...@google.com on 3 Jul 2014 at 8:02

[GoogleCodeExporter]

Yep, I'll be able to look at it next week.

Original comment by samso...@google.com on 3 Jul 2014 at 3:33

[GoogleCodeExporter]

Yes, I think collecting the unmangled names in the frontend would work. 
Currently ASan creates a string literal with global name during the 
instrumentation. Instead, we may create this literal in the frontend and pass 
it to instrumentation pass in llvm.asan.globals metadata.

I have a small patch for this, and it works fine except for the one minor case:
it's tempting to calculate the unmangled global name in the frontend by simply 
calling "NamedDecl::printQualifiedName", the same method which is used in Clang 
diagnostics. However, it adds qualifiers only for namespaces and classes, i.e. 
global
  namespace N { int x; }
will have qualified name "N::x", while global
  void foo() { static int x; }
will have qualified name "x". I asked around and it seems to be a conscious 
decision - diagnostics should only print valid C++ names, and one can't write 
"foo::x" to refer to function static variable.

I suggest to live with it and just print "global variable 'x'" in ASan error 
reports in the latter case. We would also print the exact source location 
(file/line/column) of global declaration, so users won't get confused. If 
you're OK with it, I will proceed with the change.

Original comment by samso...@google.com on 10 Jul 2014 at 10:14

[GoogleCodeExporter]

I think it's perfectly fine to omit foo():: in this case.

Original comment by euge...@google.com on 11 Jul 2014 at 7:52

[GoogleCodeExporter]

By the way, while you are at it, is it possible to do something about the empty 
names for temporaries?

Original comment by euge...@google.com on 11 Jul 2014 at 6:41

[GoogleCodeExporter]

This issue should be fixed by LLVM r212872. I'd appreciate if you could verify 
that we have unmangled global names on Android now, and re-enable corresponding 
test cases.

Regarding temporaries, do you mean the names of local variables (i.e. the 
contents of the stack frame)? This is definitely on my radar, but I haven't yet 
started working on this.

Original comment by samso...@google.com on 12 Jul 2014 at 12:53

[GoogleCodeExporter]

Yes, local variables. We get really bad names like "agg.tmp" or "" in cases 
like this:
class A{...};

{
...
f(A());
...
}

Original comment by euge...@google.com on 14 Jul 2014 at 8:50

[GoogleCodeExporter]

Tests now XPASS. You broke the bot :) Thanks!

Sounds like the code in MaybeDemangleGlobalName can be removed? I'll leave it 
to you.

I just realized that the issue in MSan is with function names, not global names.
And they are duplicated in the report anyway, so we could just disable 
demangling in the "created by an allocation" line - it would also make reports 
more compact.

Original comment by euge...@google.com on 14 Jul 2014 at 9:13

[GoogleCodeExporter]

Ok, I'm marking this issue as "Fixed" now, as it is only about the globals.

Original comment by samso...@google.com on 14 Jul 2014 at 10:12

• Changed state: Fixed