This is an archive of the discontinued LLVM Phabricator instance.

[ArgPromotion] Fix a truncated variable
ClosedPublic

Authored by mstorsjo on May 4 2017, 2:12 AM.

Details

Summary

This fixes a regression since SVN rev 273808 (which was supposed to not change functionality).

The regression caused miscompilations (noted in the wild when targeting AArch64) on platforms with 32 bit long.

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.May 4 2017, 2:12 AM

Wouldn't it be better to set the type manually to something like uint32_t or uint64_t? To make sure they're the same across all arches?

This fixes PR32917.

Wouldn't it be better to set the type manually to something like uint32_t or uint64_t? To make sure they're the same across all arches?

That'd work as well. Although in this case, ArgIndex.second is a std::vector<uint64_t>, so auto here always means uint64_t, regardless of platform.

rengolin accepted this revision.May 4 2017, 3:15 AM

Right, makes sense. This is an obvious fix, so I feel comfortable approving this one. LGTM. Thanks!

This revision is now accepted and ready to land.May 4 2017, 3:15 AM
This revision was automatically updated to reflect the committed changes.
sanjoy added a subscriber: sanjoy.May 7 2017, 1:54 PM

If I understand what happened, this is scary -- if clang-tidy introduces bugs like this perhaps we should think twice before checking in large mechanical changes like 273808?

Have you filed a bug for clang-tidy specifically?