[PATCH] Wrong mangling of template argument in presence of another template function.

[pierre gousseau]

Dear All,

I would like to propose a fix to an ABI issue found with the new ABI test
suite.
This is my first patch for clang so please bear with me :-)

The test case is:

clang version 3.6.0 (221593)
clang -cc1 -triple i386-pc-linux -S -emit-llvm
----
#ifdef BAD
template <class T> void g3(char (&buffer)[sizeof(T() + 5)]) {}
void call_g3() {
char buffer[sizeof(int)];
g3<int>(buffer);
}
#endif
template <class T> void g4(char (&buffer)[sizeof(T() + 5L)]) {}
void call_g4() {
char buffer[sizeof(long int)];
g4<long int>(buffer);
}
----

In the test case above the mangled name for g4 is different depending on
the presence of g3.

When g3 is not defined, g4 mangled name is : _Z2g4IlEvRAszplcvT__ELl5E_c
When g3 is defined, g4 mangled name is      : _Z2g4IlEvRAszplcvT__ELi5E_c

The issue seem to be caused by the AST mechanism that decides when existing
nodes can be reused to deduce the type of X in the expression "char
(&)[X]".
Where "X" is a type dependent expression (eg T() + 5L).
The mechanism seems to only take the size of the integer literal "5" to
discriminate between "5" and "5L".
So the patch adds the "builtin kind" to the list of attributes to consider,
when discriminating between AST nodes.

While the anomaly was initially spotted between "int" and "long int" on
X86, the investigation led to see that the problem also existed for targets
where "double" and "long double" are the same size so the triple
"mips-none-none" was chosen for the regression test.

Please let me know if this is an acceptable change.

Regards,

Pierre Gousseau
SN Systems - Sony Computer Entertainment Group

---
Index: svn/upstream/tools/clang/lib/AST/StmtProfile.cpp
===================================================================
--- svn/upstream/tools/clang/lib/AST/StmtProfile.cpp (revision 221593)
+++ svn/upstream/tools/clang/lib/AST/StmtProfile.cpp (working copy)
@@ -501,6 +501,10 @@
 void StmtProfiler::VisitIntegerLiteral(const IntegerLiteral *S) {
   VisitExpr(S);
   S->getValue().Profile(ID);
+  SplitQualType splitType = S->getType().split();
+  const BuiltinType * builtinType = splitType.Ty->getAs<BuiltinType>();
+  if (builtinType)
+    ID.AddInteger(builtinType->getKind());
 }

 void StmtProfiler::VisitCharacterLiteral(const CharacterLiteral *S) {
@@ -513,6 +517,10 @@
   VisitExpr(S);
   S->getValue().Profile(ID);
   ID.AddBoolean(S->isExact());
+  SplitQualType splitType = S->getType().split();
+  const BuiltinType * builtinType = splitType.Ty->getAs<BuiltinType>();
+  if (builtinType)
+    ID.AddInteger(builtinType->getKind());
 }

 void StmtProfiler::VisitImaginaryLiteral(const ImaginaryLiteral *S) {
Index:
svn/upstream/tools/clang/test/CodeGen/mips-wrong-template-mangling.cpp
===================================================================
--- svn/upstream/tools/clang/test/CodeGen/mips-wrong-template-mangling.cpp
(revision
0)
+++ svn/upstream/tools/clang/test/CodeGen/mips-wrong-template-mangling.cpp
(working
copy)
@@ -0,0 +1,38 @@
+// This checks that the mangling of g4 and g6 is not dependent on the
presence
+// of g3 and g5, the bug was due to the AST nodes for the array size
expressions
+// being wrongly shared between g3/g4 and g5/g6 on targets where the single
+// "long" keyword has no effects
+// RUN: %clang_cc1 -triple mips-none-none -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple mips-none-none -DBAD -emit-llvm -o - %s |
FileCheck -check-prefix=CHECK-BAD %s
+
+#ifdef BAD
+template <class T> void g3(char (&buffer)[sizeof(T() + 5.0)]) {}
+void call_g3() {
+  char buffer[sizeof(double)];
+  // CHECK-BAD: _Z2g3IdEvRAszplcvT__ELd4014000000000000E_c
+  g3<double>(buffer);
+}
+#endif
+template <class T> void g4(char (&buffer)[sizeof(T() + 5.0L)]) {}
+void call_g4() {
+  char buffer[sizeof(long double)];
+  // CHECK: _Z2g4IeEvRAszplcvT__ELe4014000000000000E_c
+  // CHECK-BAD: _Z2g4IeEvRAszplcvT__ELe4014000000000000E_c
+  g4<long double>(buffer);
+}
+
+#ifdef BAD
+template <class T> void g5(char (&buffer)[sizeof(T() + 5)]) {}
+void call_g5() {
+  char buffer[sizeof(int)];
+  // CHECK-BAD: _Z2g5IiEvRAszplcvT__ELi5E_c
+  g5<int>(buffer);
+}
+#endif
+template <class T> void g6(char (&buffer)[sizeof(T() + 5L)]) {}
+void call_g6() {
+  char buffer[sizeof(long int)];
+  // CHECK: _Z2g6IlEvRAszplcvT__ELl5E_c
+  // CHECK-BAD: _Z2g6IlEvRAszplcvT__ELl5E_c
+  g6<long int>(buffer);
+}
---
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141113/6d1dc344/attachment.html>