[cfe-dev] A problem with names that can not be demangled.

[Srivastava, Sunil]

Hi,

We are running into a problem created by renaming of static symbols by llvm-link.  It first
showed up using LTO, but we can illustrate this by using llvm-link as well.

Say we have two files with the same named static symbol Bye

--------------- t1.cpp ---------
static void Bye(int* ba1) { ba1[0] /= ba1[2] - 2; }
void main_a( int* inB) { void (*func)(int*) = Bye; func(inB); }
--------------- t2.cpp ---------
static void Bye(int* ba1) { ba1[0] *= ba1[2] + 2; }
void main_b( int* inB) { void (*func)(int*) = Bye; func(inB+1); }

--------- cmd sequence -------
$ clang++ -c -emit-llvm t1.cpp -o t1.bc
$ clang++ -c -emit-llvm t1.cpp -o t2.bc
$ llvm-link t1.bc t2.bc -o t23.bc
$ clang -c t23.bc
$ nm t23.o

t1.o and t2.o have the same named function "_ZL3ByePi". In order to distinguish them,
one gets a '1' appended to it, making it  "_ZL3ByePi1".

While the code is all correct, the problem is that this modified name cannot be demangled.

That is what I am trying to fix.

In similar situations gcc appends a '.' before appending the discriminating number, making "_ZL3ByePi.1"

The following change in lib/IR/ValueSymbolTable.cpp seems to fix this problem.

------------ start diff -------------------
@@ -54,5 +54,5 @@ void ValueSymbolTable::reinsertValue(Value* V) {
     // Trim any suffix off and append the next number.
     UniqueName.resize(BaseSize);
-    raw_svector_ostream(UniqueName) << ++LastUnique;
+    raw_svector_ostream(UniqueName) <<  "."  << ++LastUnique;

     // Try insert the vmap entry with this suffix.
-------------- end diff ---------------------

However it causes 60 test failures. These are tests where some names that are expecting
to get a plain numeric suffix now have a '.' before it. These are all local symbols, so I think
the generated code will always be correct, but the tests as written do not pass. For
example, take test/CodeGen/ARM/global-merge-addrspace.ll

; RUN: llc < %s -mtriple=thumb-apple-darwin -O3 | FileCheck %s
; Test the GlobalMerge pass. Check that the pass does not crash when using
; multiple address spaces.
; CHECK: _MergedGlobals:
@g1 = internal addrspace(1) global i32 1
@g2 = internal addrspace(1) global i32 2
; CHECK: _MergedGlobals1:
@g3 = internal addrspace(2) global i32 3
@g4 = internal addrspace(2) global i32 4

With my change, the symbol is named MergedGlobals.1, hence it fails this test.

I could change these 60 tests to match the new behavior. That will fix these 60 failures.
However, I do have a concern that there may be other places in llvm that expect the
names to be pure identifiers. Adding a '.' may cause them to fail. No such failure has been
seen in running the whole clang test, but the concern is still there.

I should note that even local symbols are treated similarly, so for example, a parameter
named 'str' becomes 'str.1' with my change, instead of 'str1' currently (an actual
example from a test).

Alternatively, I could try to limit my change to just mangled names.

Any suggestion about how this should be fixed ?

There is another similar change about 40 lines below in ValueSymbolTable::createValueName().
That is not needed to fix this particular problem, but looks similar, so perhaps should be treated
similarly for consistency. It causes 66 more failures of the same nature though.

Thanks

Sunil Srivastava
Sony Computer Entertainment

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150414/05b4e956/attachment.html>