Don't trap when passing non-POD arguments to variadic functions in MS-compatibility mode

[hans]

Summary

Clang warns (treated as error by default, but still ignored in system headers) when passing non-POD arguments to variadic functions, and generates a trap instruction to crash the program if that code is ever run.

Unfortunately, MSVC happily generates code for such calls without a warning, and there is code in system headers that use it.

This patch makes Clang not insert the trap instruction when in -fms-compatibility mode, while still generating the warning/error message.

[rnk]

Works for me if Richard is OK with it.

It's also worth noting that we already do exactly what MSVC does, which is a bitwise copy of the argument. The callee does *not* destroy the copy, so the program behaves "correctly" despite the UB. We don't need to go further out of our way to make it work.

test/CodeGenCXX/vararg-non-pod-ms-compat.cpp

1

Can you add a win64 test case? IIRC we coerce to i64 there, which is compatible with what MSVC does.

[hans]

Adding 64-bit test case as suggested by Reid.

[rsmith]

Looks terrible, go ahead. =)

lib/Sema/SemaExpr.cpp

878–880

Maybe this should be a separate VAK_ value, VAK_ValidInMSVC or similar?

[hans]

lib/Sema/SemaExpr.cpp

878–880

I used VAK_MSVCUndefined.

[hans]

Closed by commit rL218640 (authored by @hans).

lib/Sema/SemaExpr.cpp

ExprResult Sema::DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,
   
   ExprResult ExprRes = DefaultArgumentPromotion(E);
   if (ExprRes.isInvalid())
     return ExprError();
   E = ExprRes.get();
 
   // Diagnostics regarding non-POD argument types are
   // emitted along with format string checking in Sema::CheckFunctionCall().
-  if (isValidVarArgType(E->getType()) == VAK_Undefined) {
+  if (isValidVarArgType(E->getType()) == VAK_Undefined &&
+      !LangOpts.MSVCCompat) {
     // Turn this into a trap.
     CXXScopeSpec SS;
     SourceLocation TemplateKWLoc;
     UnqualifiedId Name;
     Name.setIdentifier(PP.getIdentifierInfo("__builtin_trap"),
                        E->getLocStart());
     ExprResult TrapFn = ActOnIdExpression(TUScope, SS, TemplateKWLoc,
                                           Name, true, false);
     if (TrapFn.isInvalid())

test/CodeGenCXX/vararg-non-pod-ms-compat.cpp

+// RUN: %clang_cc1 -Wno-error=non-pod-varargs -triple i686-pc-win32 -fms-compatibility -emit-llvm -o - %s | FileCheck %s
+
+struct X {
+  X();
+  ~X();
+  int data;
+};
+
+void vararg(...);
+
+// CHECK-LABEL: define void @"\01?test@@YAXUX@@@Z"
+void test(X x) {
+  // CHECK: %[[argmem:[^ ]*]] = alloca inalloca <{ %struct.X }>
+  // CHECK: call void (<{ %struct.X }>*, ...)* bitcast (void (...)* @"\01?vararg@@YAXZZ" to void (<{ %struct.X }>*, ...)*)(<{ %struct.X }>* inalloca %[[argmem]])
+  // CHECK-NOT: llvm.trap
+  vararg(x);
+  // CHECK: ret void
+}
+

test/SemaCXX/vararg-non-pod.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -fblocks %s -Wno-error=non-pod-varargs
 
+// Check that the warning is still there under -fms-compatibility.
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks %s -Wno-error=non-pod-varargs -fms-compatibility
+
 extern char version[];
 
 class C {
 public:
   C(int);
   void g(int a, ...);
   static void h(int a, ...);
 };