[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

[Dustin L. Howett via Phabricator via cfe-commits]

DHowett-MSFT created this revision.
DHowett-MSFT added reviewers: rnk, majnemer.
DHowett-MSFT added a project: clang.
Herald added subscribers: cfe-commits, eraman.

The MSVC compiler rejects `__forceinline` on variadic functions with the following warning (at /https://reviews.llvm.org/W4):

  C4714: function 'void msvc_variadic(int,...)' marked as __forceinline not inlined

This fixes a bug in LLVM where a variadic was getting inlined into a function of calling convention x86_thiscallcc. The LLVM lowering passes cannot consume an `@llvm.va_start` intrinsic call in a thiscall function without emitting an assertion.

Inlining variadics should almost certainly be possible; however, this is a fix to bring Clang in line with MSVC.


Repository:
  rC Clang

https://reviews.llvm.org/D44646

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/ms-forceinline-on-variadic.cpp


Index: test/Sema/ms-forceinline-on-variadic.cpp
===================================================================
--- /dev/null
+++ test/Sema/ms-forceinline-on-variadic.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -emit-llvm -o - -triple i686-windows -verify -fms-extensions %s \
+// RUN:            | FileCheck %s
+//
+// (instruction scheduling crash test)
+// RUN: %clang_cc1 -S -o - -triple i686-windows -fms-extensions %s
+
+inline __attribute__((always_inline)) void attribute_variadic(int f, ...) { } // expected-warning {{inlining attribute 'always_inline' ignored on variadic function}}
+
+__forceinline inline void msvc_variadic(int f, ...) { // expected-warning {{inlining attribute '__forceinline' ignored on variadic function}}
+// CHECK-DAG: define {{.*}} void [[MSVC_VARIADIC:@".*msvc_variadic.*"]](
+    __builtin_va_list ap;
+    __builtin_va_start(ap, f);
+    __builtin_va_end(ap);
+}
+
+struct a {
+    // members are, by default, thiscall; enforce it for the purposes of the test
+    void __thiscall dispatcher();
+};
+
+void __thiscall a::dispatcher() {
+    msvc_variadic(1, 2, 3);
+// CHECK-DAG: define dso_local x86_thiscallcc void @"{{.*dispatcher.*}}"(
+// CHECK-DAG: call void {{.*}} [[MSVC_VARIADIC]]
+}
+
+void t() {
+    a{}.dispatcher();
+}
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -3886,6 +3886,12 @@
     return nullptr;
   }
 
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft()
+     && hasFunctionProto(D) && isFunctionOrMethodVariadic(D)) {
+    Diag(Range.getBegin(), diag::warn_always_inline_on_variadic) << Ident;
+    return nullptr;
+  }
+
   if (D->hasAttr<AlwaysInlineAttr>())
     return nullptr;
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2694,6 +2694,9 @@
 def warn_attribute_after_definition_ignored : Warning<
   "attribute %0 after definition is ignored">,
    InGroup<IgnoredAttributes>;
+def warn_always_inline_on_variadic : Warning<
+  "inlining attribute %0 ignored on variadic function">,
+   InGroup<IgnoredAttributes>;
 def warn_unknown_attribute_ignored : Warning<
   "unknown attribute %0 ignored">, InGroup<UnknownAttributes>;
 def warn_cxx11_gnu_attribute_on_type : Warning<


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44646.138990.patch
Type: text/x-patch
Size: 2439 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180319/d2ac0b6b/attachment.bin>