add types: (u)int64ptr_t
AcceptedPublic
Actions

Authored by brooks on Wed, Nov 19, 11:19 AM.

Details

Reviewers

imp
kib
markj

Group Reviewers

cheri

Summary

This type represents an integer value of at least 64 bits which is
capable of being cast to and from pointer types. It is intended to
replace various spellings of (u)int64_t there the value is expected to
hold a pointer. This is common in Linux code to allow 32-bit and 64-bit
structures to be the same and used other places including OpenZFS. With
the introduction of CHERI this no longer works, but we need to preserve
the ABI for integer pointer targets. Rather than adding ifdefs in every
case, we introduce a new type.

Effort: CHERI upstreaming
Sponsored by: Innovate UK

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 68731
Build 65614: arc lint + arc unit

Event Timeline

brooks requested review of this revision.Wed, Nov 19, 11:19 AM

brooks created this revision.

Harbormaster completed remote builds in B68731: Diff 166756.Wed, Nov 19, 11:19 AM

brooks added a parent revision: D53822: sys/_types.h: add __intptr_t to __mbstate_t.Wed, Nov 19, 11:19 AM

brooks added inline comments.Wed, Nov 19, 11:22 AM

sys/sys/_stdint.h
81	We could include sys/_visible.h instead of checking for the definition.
83	A reasonable argument could be made that we should stick to the `__` prefixed variants, but in CheriBSD we have intruded on the standard namespace.

markj accepted this revision.Wed, Nov 19, 3:45 PM

markj added a subscriber: markj.

markj added inline comments.

sys/sys/_stdint.h
83	I think this type is useful enough that it makes sense to have the unprefixed variant. I used it in quite a few places in the cheri port of dtrace for instance.

This revision is now accepted and ready to land.Wed, Nov 19, 3:45 PM

There should be a description of the intended type use somewhere, even if only as a comment in the header. It is non-standard, and its proper usage should be documented instead of being a knowledge.

sys/sys/_stdint.h
83	The argument is not in the usefulness, but in possible conflict with some future standard that reuses the name (or event just namespace). IMO keeping it prefixed in the impl-private namespace is much safer bet.

Describe the types

This revision now requires review to proceed.Thu, Nov 20, 10:41 AM

Harbormaster completed remote builds in B68760: Diff 166829.Thu, Nov 20, 10:41 AM

Spelling

Harbormaster completed remote builds in B68761: Diff 166830.Thu, Nov 20, 10:42 AM

kib added inline comments.Thu, Nov 20, 11:57 AM

sys/sys/_stdint.h

BTW is the defined() guard needed? Aren't undefined symbols automatically replaced by 0:

After all replacements due to macro expansion and
the defined unary operator have been performed, all remaining identiﬁers (including those lexically
identical to keywords) are replaced with the pp-number 0, and then each preprocessing token is
converted into a token.

jrtc27 added a subscriber: jrtc27.Thu, Nov 20, 12:27 PM

jrtc27 added inline comments.