[PATCH] D31976: Avoid assert when a non-static member function is qualified with __unaligned

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300686: Avoid assert when a non-static member function is 
qualified with __unaligned (authored by rogfer01).

Changed prior to commit:
  https://reviews.llvm.org/D31976?vs=94962=95723#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31976

Files:
  cfe/trunk/lib/AST/ItaniumMangle.cpp
  cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp


Index: cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
===
--- cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
+++ cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only 
%s -verify
+
+struct A
+{
+int x;
+void foo() __unaligned;
+void foo();
+};
+
+void A::foo() __unaligned
+{
+this->x++;
+}
+
+void A::foo() // expected-error {{definition with same mangled name as another 
definition}}
+  // expected-note@-6 {{previous definition is here}}
+{
+this->x++;
+}
+
Index: cfe/trunk/lib/AST/ItaniumMangle.cpp
===
--- cfe/trunk/lib/AST/ItaniumMangle.cpp
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp
@@ -1455,10 +1455,12 @@
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast(ND)) {
 Qualifiers MethodQuals =
-Qualifiers::fromCVRMask(Method->getTypeQualifiers());
+Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
 // We do not consider restrict a distinguishing attribute for overloading
 // purposes so we must not mangle it.
 MethodQuals.removeRestrict();
+// __unaligned is not currently mangled in any way, so remove it.
+MethodQuals.removeUnaligned();
 mangleQualifiers(MethodQuals);
 mangleRefQualifier(Method->getRefQualifier());
   }


Index: cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
===
--- cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
+++ cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only %s -verify
+
+struct A
+{
+int x;
+void foo() __unaligned;
+void foo();
+};
+
+void A::foo() __unaligned
+{
+this->x++;
+}
+
+void A::foo() // expected-error {{definition with same mangled name as another definition}}
+  // expected-note@-6 {{previous definition is here}}
+{
+this->x++;
+}
+
Index: cfe/trunk/lib/AST/ItaniumMangle.cpp
===
--- cfe/trunk/lib/AST/ItaniumMangle.cpp
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp
@@ -1455,10 +1455,12 @@
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast(ND)) {
 Qualifiers MethodQuals =
-Qualifiers::fromCVRMask(Method->getTypeQualifiers());
+Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
 // We do not consider restrict a distinguishing attribute for overloading
 // purposes so we must not mangle it.
 MethodQuals.removeRestrict();
+// __unaligned is not currently mangled in any way, so remove it.
+MethodQuals.removeUnaligned();
 mangleQualifiers(MethodQuals);
 mangleRefQualifier(Method->getRefQualifier());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31976: Avoid assert when a non-static member function is qualified with __unaligned

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I see, thank you for the explanation.


https://reviews.llvm.org/D31976



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31976: Avoid assert when a non-static member function is qualified with __unaligned

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

`clang::Sema::IsOverload` explicitly forbids the `__restrict` case for the 
qualifier of non-static member functions. I assume `__unaligned` is not 
forbidden because the MSVC ABI does encode this qualifier while the Itanium ABI 
currently does not.

This patch just makes the attached test case fail like the one below:

  // Compile with"-target x86_64-unknown-linux-gnu -x c++ 
-fms-extensions" 
  // and compare it with "-target x86_64-pc-windows-msvc -x c++ 
-fms-extensions" 
  void foo(__unaligned int* a) { }
  void foo(int *a) { }

Regards.


https://reviews.llvm.org/D31976



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31976: Avoid assert when a non-static member function is qualified with __unaligned

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp:15
+
+void A::foo() // expected-error {{definition with same mangled name as another 
definition}}
+  // expected-note@-6 {{previous definition is here}}

Do you know why clang doesn't error out until it reaches IRGen when compiling 
this test? I found it interesting that Sema detects the redeclaration and 
errors out when the function is marked "restrict", but doesn't do so when it's 
marked "unaligned".


https://reviews.llvm.org/D31976



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31976: Avoid assert when a non-static member function is qualified with __unaligned

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

This seems reasonable to me, but you should wait for confirmation before 
committing (I'm not as familiar with the mangler as others are).




Comment at: test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp:16
+void A::foo() // expected-error {{definition with same mangled name as another 
definition}}
+  // expected-note@-6 {{previous definition is here}}
+{

Might as well put this note with the declaration rather than using an offset 
for the message.


https://reviews.llvm.org/D31976



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31976: Avoid assert when a non-static member function is qualified with __unaligned

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.

Under `-fms-extensions` `__unaligned` is a type-qualifier that can be applied 
to a non-static member function declaration.

This causes an assertion when mangling the name under Itanium, where that 
qualifier is not mangled.

This patch justs makes the minimal change to avoid the crash and avoid mangling 
`__unaligned`.

The test just makes sure the clash is actually detected.


https://reviews.llvm.org/D31976

Files:
  lib/AST/ItaniumMangle.cpp
  test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp


Index: test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
===
--- /dev/null
+++ test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only 
%s -verify
+
+struct A
+{
+int x;
+void foo() __unaligned;
+void foo();
+};
+
+void A::foo() __unaligned
+{
+this->x++;
+}
+
+void A::foo() // expected-error {{definition with same mangled name as another 
definition}}
+  // expected-note@-6 {{previous definition is here}}
+{
+this->x++;
+}
+
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -1455,10 +1455,12 @@
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast(ND)) {
 Qualifiers MethodQuals =
-Qualifiers::fromCVRMask(Method->getTypeQualifiers());
+Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
 // We do not consider restrict a distinguishing attribute for overloading
 // purposes so we must not mangle it.
 MethodQuals.removeRestrict();
+// __unaligned is not currently mangled in any way, so remove it.
+MethodQuals.removeUnaligned();
 mangleQualifiers(MethodQuals);
 mangleRefQualifier(Method->getRefQualifier());
   }


Index: test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
===
--- /dev/null
+++ test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only %s -verify
+
+struct A
+{
+int x;
+void foo() __unaligned;
+void foo();
+};
+
+void A::foo() __unaligned
+{
+this->x++;
+}
+
+void A::foo() // expected-error {{definition with same mangled name as another definition}}
+  // expected-note@-6 {{previous definition is here}}
+{
+this->x++;
+}
+
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -1455,10 +1455,12 @@
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast(ND)) {
 Qualifiers MethodQuals =
-Qualifiers::fromCVRMask(Method->getTypeQualifiers());
+Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
 // We do not consider restrict a distinguishing attribute for overloading
 // purposes so we must not mangle it.
 MethodQuals.removeRestrict();
+// __unaligned is not currently mangled in any way, so remove it.
+MethodQuals.removeUnaligned();
 mangleQualifiers(MethodQuals);
 mangleRefQualifier(Method->getRefQualifier());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits