Tcl Source Code

The attached patch series contains a number of code cleanups needed
(with the exception of patches 1 and 8 which aren't needed but I
believe are a good idea to have and discovered during this porting) to
support CHERI, and thus Arm's Morello prototype[1]. With these changes
there are no regressions when running make check on Morello CheriBSD
compared with plain AArch64 CheriBSD (for some reason both fail to run
the chanio tests with "kevent: Operation not supported by device"; I
don't know if that's a Tcl bug, a FreeBSD bug, a CheriBSD bug or an
issue with my environment, but the result is the same for both and
pre-existing on AArch64 so it's not a bug in porting Tcl to CHERI).

[1] https://www.arm.com/architecture/cpu/morello

For the issue related to the chan-io test failing, see https://sourceforge.net/p/tcl/mailman/message/57263727/

Not sure how I missed these comments from last August and only saw today's follow-up, but here's my response:

It's been long enough that I don't recall whether patch 2 is needed for correctness. However, we do emit a warning on pointer<->integer casts that don't go through (u)intptr_t because there's a high chance the code is wrong (and it's quite hard to tell statically for certain), so even if none of the users need to be able to do pointer->integer->pointer losslessly it's going to warn as-is, which is a warning we often like to make an error so we can catch all such potentially-problematic cases. If what you say is true then the way we'd express it would be `((void *)(intptr_t)(ptrdiff_t)(p))` and `((ptrdiff_t)(intptr_t)(p))`, which are semantically equivalent to the original code, but use a redundant intermediary cast to express to the compiler that your intent is indeed to be using just a plain integer. Would you accept such a patch instead? Either way I'll need to test that there is indeed nothing trying to use these for actual pointers.

Re custom/wrapping allocators, yes, those are a nuisance to deal with and these days are often a bad idea for performance anyway (though maybe still with some allocators). You can either make your allocator use the necessary intrinsics to apply bounds (and be able to cope with the reduced bounds on free) or turn it off; which is the right option depends on the use case. Our porting of most software of late has been focused on just getting it working without extensively auditing it for things like custom memory allocators where adaptation is needed to avoid slight holes in the security story.

Re __LP64__ I don't think I spotted such usage, but it may well be there are cases not exercised by the test suite where stupid things end up happening because it thinks it's 32-bit. There only seem to be 16 results between the tcl and tk repos (at least according to GitHub search, which can be a bit dodgy) though so perhaps not, and it wouldn't take long to audit them.

Usage of __LP64__ in Tcl/Tk (e.g. for detecting 64-bit long) likely also requires auditing, as CHERI does not define this macro. See slide 6 of https://soft-dev.org/events/cheritech22/slides/Richardson.pdf which suggests checking __SIZEOF_LONG__ == 8 instead.

All provide patches, except 0002, now found their way into core-8-branch and trunk. Assuming CHERI now works, this ticket can be closed. Please confirm.

If any other issues with CHERI arise, feel free to provide a new ticket or re-open this one.

Thank you for those patches! The 008_* patch has already been applied to core-8-6-branch and up. I will have a look at the other ones as well.

One remark: the PTR2INT/INT2PTR macro's only guarantee round-trip from size_t -> pointer -> size_t (in Tcl 8.6 it was int -> pointer -> int), not pointer -> size_t -> pointer, since size(size_t) might be smaller than sizeof(void *). So, whenever size_t is insufficient to store what the code wants, use a pointer. I don't think the PTR2INT/INT2PTR should be modified. In stead the code should be modified not to use size_t/ptrdiff_t in the places where it is problematic.

Anyway, Thanks! I will have a further look.