Page MenuHomeFreeBSD

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

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

Details

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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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

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?

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.

lib/libc/string/bcopy.c
42–43

Add assert along this comment?

104–105

Is the line too long?

138

Why do you need these ?

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.

138

Will drop those, I added that to allow including the file twice in the same source.

lib/libc/string/bcopy.c
42–43

Or drop the comment. It stating more or less trivial property IN CAPS definitely not matching our current approach to style.

arichardson marked 5 inline comments as done.

address review feedback

This revision is now accepted and ready to land.Mon, Apr 19, 10:50 AM