This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Correctly handle va_arg of zero-sized structures
ClosedPublic

Authored by quantum on Aug 14 2019, 2:04 PM.

Details

Summary

D66168 passes size 0 structs indirectly, while the wasm backend expects it to
be passed directly. This causes subsequent variadic arguments to be read
incorrectly.

This diff changes it so that size 0 structs are passed directly.

Diff Detail

Repository
rL LLVM

Event Timeline

quantum created this revision.Aug 14 2019, 2:04 PM
dschuff added inline comments.Aug 14 2019, 2:24 PM
clang/test/CodeGen/wasm-varargs.c
104 ↗(On Diff #215242)

This should maybe be called "test_empty_struct" since its size is actually 1 and not zero.

115 ↗(On Diff #215242)

could use CHECK_LABEL here and above.

125 ↗(On Diff #215242)

It looks like most of the output (up to here) is boilerplate for va_arg setup and duplicates the CHECKs above. It might be more clear if we just omitted the duplicated stuff and only CHECK for the things that are different from the above test.

quantum updated this revision to Diff 215272.Aug 14 2019, 3:39 PM
quantum marked 6 inline comments as done.

Address review feedback.

clang/test/CodeGen/wasm-varargs.c
104 ↗(On Diff #215242)

Done.

115 ↗(On Diff #215242)

Can't because the line has match patterns.

125 ↗(On Diff #215242)

We can't remove the duplicate checks for the second argument because the broken code resulted from the second argument being incorrectly read when the first argument is a blank structure.

Instead, I am changing the whole test to use CHECK-NEXT so we can be sure the exact generated code is correct.

dschuff accepted this revision.Aug 15 2019, 12:21 PM
This revision is now accepted and ready to land.Aug 15 2019, 12:21 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 15 2019, 12:33 PM