[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX created this revision.
ArcsinX added a reviewer: rsmith.
ArcsinX added a project: clang.
Herald added a subscriber: cfe-commits.
ArcsinX edited the summary of this revision.

`__builtin_va_*()` and `__builtin_ms_va_*()` declared as functions with reference parameters.

This patch fix clang crashes at using these functions in C and passing incompatible structures as parameters in case of `__builtin_va_list`/`__builtin_ms_va_list` are structures.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82805

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/init-ref-c.c


Index: clang/test/Sema/init-ref-c.c
===================================================================
--- /dev/null
+++ clang/test/Sema/init-ref-c.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple arm-unknown-gnu -fsyntax-only -verify %s
+
+void f() {
+  struct EmptyStruct {};
+  struct EmptyStruct S;
+  __builtin_va_end(S); // no-crash, expected-error {{non-const lvalue reference to type '__builtin_va_list' cannot bind to a value of unrelated type 'struct EmptyStruct'}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4535,39 +4535,41 @@
       S.isCompleteType(Kind.getLocation(), T2)) {
     // The type we're converting from is a class type, enumerate its conversion
     // functions.
-    CXXRecordDecl *T2RecordDecl = cast<CXXRecordDecl>(T2RecordType->getDecl());
-
-    const auto &Conversions = T2RecordDecl->getVisibleConversionFunctions();
-    for (auto I = Conversions.begin(), E = Conversions.end(); I != E; ++I) {
-      NamedDecl *D = *I;
-      CXXRecordDecl *ActingDC = cast<CXXRecordDecl>(D->getDeclContext());
-      if (isa<UsingShadowDecl>(D))
-        D = cast<UsingShadowDecl>(D)->getTargetDecl();
-
-      FunctionTemplateDecl *ConvTemplate = dyn_cast<FunctionTemplateDecl>(D);
-      CXXConversionDecl *Conv;
-      if (ConvTemplate)
-        Conv = cast<CXXConversionDecl>(ConvTemplate->getTemplatedDecl());
-      else
-        Conv = cast<CXXConversionDecl>(D);
-
-      // If the conversion function doesn't return a reference type,
-      // it can't be considered for this conversion unless we're allowed to
-      // consider rvalues.
-      // FIXME: Do we need to make sure that we only consider conversion
-      // candidates with reference-compatible results? That might be needed to
-      // break recursion.
-      if ((AllowRValues ||
-           Conv->getConversionType()->isLValueReferenceType())) {
+    if (CXXRecordDecl *T2RecordDecl =
+        dyn_cast<CXXRecordDecl>(T2RecordType->getDecl())) {
+      const auto &Conversions = T2RecordDecl->getVisibleConversionFunctions();
+      for (auto I = Conversions.begin(), E = Conversions.end(); I != E; ++I) {
+        NamedDecl *D = *I;
+        CXXRecordDecl *ActingDC = cast<CXXRecordDecl>(D->getDeclContext());
+        if (isa<UsingShadowDecl>(D))
+          D = cast<UsingShadowDecl>(D)->getTargetDecl();
+
+        FunctionTemplateDecl *ConvTemplate = dyn_cast<FunctionTemplateDecl>(D);
+        CXXConversionDecl *Conv;
         if (ConvTemplate)
-          S.AddTemplateConversionCandidate(
-              ConvTemplate, I.getPair(), ActingDC, Initializer, DestType,
-              CandidateSet,
-              /*AllowObjCConversionOnExplicit=*/false, AllowExplicitConvs);
+          Conv = cast<CXXConversionDecl>(ConvTemplate->getTemplatedDecl());
         else
-          S.AddConversionCandidate(
-              Conv, I.getPair(), ActingDC, Initializer, DestType, CandidateSet,
-              /*AllowObjCConversionOnExplicit=*/false, AllowExplicitConvs);
+          Conv = cast<CXXConversionDecl>(D);
+
+        // If the conversion function doesn't return a reference type,
+        // it can't be considered for this conversion unless we're allowed to
+        // consider rvalues.
+        // FIXME: Do we need to make sure that we only consider conversion
+        // candidates with reference-compatible results? That might be needed
+        // to break recursion.
+        if ((AllowRValues ||
+             Conv->getConversionType()->isLValueReferenceType())) {
+          if (ConvTemplate)
+            S.AddTemplateConversionCandidate(
+                ConvTemplate, I.getPair(), ActingDC, Initializer, DestType,
+                CandidateSet,
+                /*AllowObjCConversionOnExplicit=*/false, AllowExplicitConvs);
+          else
+            S.AddConversionCandidate(
+                Conv, I.getPair(), ActingDC, Initializer, DestType,
+                CandidateSet, /*AllowObjCConversionOnExplicit=*/false,
+                AllowExplicitConvs);
+        }
       }
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82805.274227.patch
Type: text/x-patch
Size: 4162 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200629/9b2b51a8/attachment.bin>