This is an archive of the discontinued LLVM Phabricator instance.

[builtins] Fix integer/pointer confusion in gcc_personality_v0.c
ClosedPublic

Authored by jrtc27 on Jan 27 2021, 8:54 AM.

Details

Summary

This fixes the implementation for architectures like CHERI with strong
pointer provenance (pointers, and thus uintptr_t, are represented as
hardware capabilities). On all currently-supported architectures this
should be an NFC, as size_t and uintptr_t end up being the same
underlying plain integer type.

Diff Detail

Event Timeline

jrtc27 requested review of this revision.Jan 27 2021, 8:54 AM
jrtc27 created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2021, 8:54 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
arichardson added inline comments.Jan 27 2021, 9:39 AM
compiler-rt/lib/builtins/gcc_personality_v0.c
215

Maybe rename this to landingPadOffset to indicate that it's not a pointer?

Which statement has the type confusion?

Which statement has the type confusion?

Comments added to hopefully explain it.

compiler-rt/lib/builtins/gcc_personality_v0.c
46

This isn't strictly necessary but is more accurate, since this function just creates a machine word sized integer, not a pointer.

215

We could, it's just slightly more churn.

219

This start + length adds two uintptr_ts together, which does not make sense unless at least one is an integer rather than a pointer.

227

This funcStart + landingPad also adds two uintptr_ts together.

MaskRay accepted this revision.Jan 27 2021, 10:39 AM

Thanks!

compiler-rt/lib/builtins/gcc_personality_v0.c
217

If I wrote it in the first place, I'd place landingPad == 0 below the range check, but it has no difference in practice, because __attribute__((cleanup)) has a landing pad.

This revision is now accepted and ready to land.Jan 27 2021, 10:39 AM
This revision was landed with ongoing or failed builds.Jan 27 2021, 11:34 AM
This revision was automatically updated to reflect the committed changes.