This is an archive of the discontinued LLVM Phabricator instance.

[ConstantInt] Check active bits before calling getZExtValue.
ClosedPublic

Authored by fhahn on Dec 10 2018, 9:39 PM.

Details

Summary

Without this check, we hit an assertion in getZExtValue, if the constant
value does not fit into an uint64_t.

As getZExtValue returns an uint64_t, should we update
getAggregateElement to take an uin64_t as well?

This fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=6109.

Diff Detail

Repository
rL LLVM

Event Timeline

fhahn created this revision.Dec 10 2018, 9:39 PM
efriedma accepted this revision.Dec 11 2018, 11:39 AM

LGTM; arrays/struct types are currently limited to 2^64 elements anyway.

should we update getAggregateElement to take an uin64_t as well?

Yes, all the ConstantAggregate-related code should use uint64_t to match the type system. See also https://reviews.llvm.org/D55169, where this just came up.

This revision is now accepted and ready to land.Dec 11 2018, 11:39 AM
This revision was automatically updated to reflect the committed changes.
fhahn added a comment.Dec 11 2018, 6:33 PM

LGTM; arrays/struct types are currently limited to 2^64 elements anyway.

should we update getAggregateElement to take an uin64_t as well?

Yes, all the ConstantAggregate-related code should use uint64_t to match the type system. See also https://reviews.llvm.org/D55169, where this just came up.

Thanks Eli, I'll see if I can put together a follow-up patch in the next few days.