[PATCH] D31976: Avoid assert when a non-static member function is qualified with __unaligned
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&id=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
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
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
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 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
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