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

[Richard Smith]

On Thu, Nov 13, 2014 at 6:14 AM, pierre gousseau <pierregousseau14 at gmail.com
> wrote:

> 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 :-)
>

Thanks for the investigation and the patch!


> 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.
>

Yes, this is the right approach.


> 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());
>

You can use

  ID.AddInteger(S->getType()->castAs<BuiltinType>()->getKind());

here, because the integer literal is guaranteed to be of a builtin type.
(And no need to split the type manually; QualType's operator-> does the
right thing here.)


>  }
>
>  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());
>

Likewise here.

Do you have an SVN account, or do you need someone to commit this for you?


>  }
>
>  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);
> +}
> ---
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141113/ea1ea9c2/attachment.html>