This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by hans on Sep 25 2014, 9:58 AM.

Details

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.

Diff Detail

Event Timeline

hans updated this revision to Diff 14077.Sep 25 2014, 9:58 AM
hans retitled this revision from to Don't trap when passing non-POD arguments to variadic functions in MS-compatibility mode.
hans updated this object.
hans edited the test plan for this revision. (Show Details)
hans added reviewers: rnk, rsmith.
hans added subscribers: Unknown Object (MLST), hansw.
rnk edited edge metadata.Sep 25 2014, 11:19 AM

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 updated this revision to Diff 14082.Sep 25 2014, 11:42 AM
hans edited edge metadata.

Adding 64-bit test case as suggested by Reid.

rsmith edited edge metadata.Sep 29 2014, 12:26 PM

Looks terrible, go ahead. =)

lib/Sema/SemaExpr.cpp
878–880

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

hans added inline comments.Sep 29 2014, 4:16 PM
lib/Sema/SemaExpr.cpp
878–880

I used VAK_MSVCUndefined.

hans closed this revision.Sep 29 2014, 4:17 PM
hans updated this revision to Diff 14195.

Closed by commit rL218640 (authored by @hans).