Page MenuHomePhabricator

[ExecutionEngine] Modify static_casts to not be tautological in some COFF i386 relocations
ClosedPublic

Authored by xiaobai on Oct 12 2017, 6:39 PM.

Details

Summary

I'm pretty sure an int32_t will always be between INT32_MAX and
INT32_MIN, inclusive.

Event Timeline

xiaobai created this revision.Oct 12 2017, 6:39 PM
smeenai edited edge metadata.Oct 12 2017, 7:08 PM

I'd wait for @compnerd to take a look before acting on my comments, since I don't have much context on this code.

lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFI386.h
146–147

A VA is unsigned, so "relocation underflow" doesn't make much sense to me. I'd just assert Result <= UINT32_MAX instead.

160–161

Same here.

198–199

Same here (though you'll need a cast to uint64_t).

compnerd accepted this revision.Oct 13 2017, 11:08 AM
compnerd added inline comments.
lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFI386.h
146–147

Yeah, given that it is a VA, we can remove the underflow check as it cannot happen.

This revision is now accepted and ready to land.Oct 13 2017, 11:08 AM
xiaobai updated this revision to Diff 118981.Oct 13 2017, 3:45 PM

Addressed comments and fixed incorrect test.

compnerd closed this revision.Oct 19 2017, 9:58 AM

SVN r316169