bcopy.c: Use intptr_t as the copy type
ClosedPublic
Actions

Authored by arichardson on Apr 1 2021, 2:36 PM.

Details

Reviewers

kib
brooks
jrtc27

Group Reviewers

riscv

Commits

rG0e1e71400a0f: libc/string/bcopy.c: Use intptr_t as the copy type
rG0b4ad01d91a3: libc/string/bcopy.c: Use intptr_t as the copy type

Summary

While most 64-bit architectures have an assembly implementation of this
file RISC-V does not. As we now copy 8 bytes instead of 4 it should speed
up RISC-V. Using intptr_t instead of int also allows using this file for
CHERI pure-capability code since trying to copy pointers using integer
loads/stores will invalidate pointers.

Obtained from: CheriBSD (partially)

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 38679
Build 35568: arc lint + arc unit

Event Timeline

arichardson created this revision.Apr 1 2021, 2:36 PM

Herald added a subscriber: imp. · View Herald TranscriptApr 1 2021, 2:36 PM

arichardson requested review of this revision.Apr 1 2021, 2:36 PM

Harbormaster completed remote builds in B38252: Diff 86692.Apr 1 2021, 2:37 PM

intptr_t isn't sufficient for hybrid CHERI though, so we'd still need the diff to make it intcap_t downstream...

jrtc27 added inline comments.Apr 1 2021, 2:41 PM

lib/libc/string/bcopy.c
104–105	I don't see why the `void *` casts are needed? Is this just because of bogus char -> intptr_t alignment warnings?

In D29535#662078, @jrtc27 wrote:

intptr_t isn't sufficient for hybrid CHERI though, so we'd still need the diff to make it intcap_t downstream...

Yes, but that diff can't really be upstreamed.

lib/libc/string/bcopy.c
104–105	Yeah, I'm trying to include this outside of libc and that means -Wcast-align is enabled.

arichardson added a child revision: D28536: libthr: Avoid interposable calls to memcpy/memset/memmove.Apr 1 2021, 2:48 PM

ping?

arichardson edited the summary of this revision. (Show Details)Apr 15 2021, 10:36 AM

arichardson removed a child revision: D28536: libthr: Avoid interposable calls to memcpy/memset/memmove.

kib added inline comments.Apr 15 2021, 11:30 AM

lib/libc/string/bcopy.c
42–43	Add assert along this comment?
104–105	Is the line too long?
139	Why do you need these ?

arichardson added inline comments.Apr 19 2021, 9:48 AM

lib/libc/string/bcopy.c
42–43	I could add that but I think if sizeof(intptr_t) is not a power-of-two, we would have much bigger problems. Alternatively I could change the line below to `!__is_aligned((t \| (uintptr_t)dst), wsize)`, in which case Clang 10+ will give an error if it isn't.
104–105	Yes, will reformat.
139	Will drop those, I added that to allow including the file twice in the same source.