This is an archive of the discontinued LLVM Phabricator instance.

Win64 ABI shouldn't extend integer type arguments.
ClosedPublic

Authored by jlerouge on Jul 3 2014, 3:15 PM.

Details

Summary

MSVC doesn't extend integer types smaller than 64bit, so to preserve
binary compatibility, clang shouldn't either.

For example, the following C code built with MSVC:

unsigned test(unsigned v);
unsigned foobar(unsigned short);
int main() { return test(0xffffffff) + foobar(28); }

Produces the following:

0000000000000004: B9 FF FF FF FF     mov         ecx,0FFFFFFFFh
0000000000000009: E8 00 00 00 00     call        test
000000000000000E: 89 44 24 20        mov         dword ptr [rsp+20h],eax
0000000000000012: 66 B9 1C 00        mov         cx,1Ch
0000000000000016: E8 00 00 00 00     call        foobar

And as you can see, when setting up the call to foobar, only cx is overwritten.

If foobar is compiled with clang, then the zero extension added by clang means
the rest of the register, which contains garbage, could be used.

For example if foobar is:

unsigned foobar(unsigned short v) {

return v;

}

Compiled with clang -fomit-frame-pointer -O3 gives the following assembly:

foobar:

0000000000000000: 89 C8              mov         eax,ecx
0000000000000002: C3                 ret

And that function would return garbage because the 16 most significant bits of
ecx still contain garbage from the first call.

With this change, the code for that function is now:

foobar:

0000000000000000: 0F B7 C1           movzx       eax,cx
0000000000000003: C3                 ret

Diff Detail

Event Timeline

jlerouge updated this revision to Diff 11067.Jul 3 2014, 3:15 PM
jlerouge retitled this revision from to Win64 ABI shouldn't extend integer type arguments..
jlerouge updated this object.
jlerouge edited the test plan for this revision. (Show Details)
jlerouge added reviewers: rnk, chapuni.
jlerouge added a subscriber: Unknown Object (MLST).

I think this deserves more testing than just adding another target to an XFAIL line.

jlerouge updated this revision to Diff 11071.Jul 3 2014, 5:58 PM

Add an explicit test.

Just check that clang doesn't add zext/sext attributes on small integer types.

rnk accepted this revision.Jul 7 2014, 1:46 PM
rnk edited edge metadata.

lgtm

For the record, mingw64 GCC does the same thing:

_foobar:
      .seh_endprologue
      movzwl  %cx, %eax
      ret
      .seh_endproc
test/CodeGen/2007-06-18-SextAttrAggregate.c
2

Let's just give this an explicit x86 triple.

This revision is now accepted and ready to land.Jul 7 2014, 1:46 PM
jlerouge closed this revision.Aug 26 2014, 3:01 PM

One more update to unbreak CodeGenCXX/microsoft-abi-member-pointers.cpp in r216507. We can preserve the zeroext on bool types, that still gives us code compatible with MSVC, since it's only extending to 8bits.