libcasper: fix warnings when _ALIGN preserves types

[brooks]

Summary

Without the void * casts, the compiler complains about an allignment
requirement increase.

Obtained from: CheriBSD
Effort: CHERI upstreaming
Sponsored by: DARPA, AFRL

[kib]

lib/libcasper/services/cap_grp/cap_grp.c

105

I do not understand neither the old code, nor the new one.
After *bufferp is evaluated, we get the char * value, which is then aligned. Why is it cast to char ** at all?

[brooks]

lib/libcasper/services/cap_grp/cap_grp.c

105

Without the change we get:

/home/bed22/cheri/ca-cheri/lib/libcasper/services/cap_grp/cap_grp.c -o cap_grp.pico
/home/bed22/cheri/ca-cheri/lib/libcasper/services/cap_grp/cap_grp.c:105:13: error: cast from 'char *' to 'char **' increases required alignment from 1 to 8 [-Werror,-Wcast-align]
  105 |                 outstrs = (char **)_ALIGN(*bufferp);
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/bed22/cheri/ca-cheri/lib/libcasper/services/cap_grp/cap_grp.c:127:12: error: cast from 'char *' to 'char **' increases required alignment from 1 to 8 [-Werror,-Wcast-align]
  127 |         outstrs = (char **)_ALIGN(*bufferp);
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.

The problem is that *bufferp is of type char * which points to data of an alignment of 1. The void * cast suppresses that diagnostic.

[kib]

lib/libcasper/services/cap_grp/cap_grp.c

105

This is not what I am asking about.
I do not understand the code with or without the warning fixed: outstrs should have the char * type, not char **.

[brooks]

lib/libcasper/services/cap_grp/cap_grp.c

105

outstrs is definitely an array of pointers to strings (with the strings tacked on the end) and thus I think is correctly char **. It's more obvious in the loop below.

This code is a mess and needs to be replaced with something that doesn't send pointers over IPC.

[kib]

lib/libcasper/services/cap_grp/cap_grp.c

105

Yes, outstrs is char **, but the value that is aligned is only char *. So the compiler warning is correct.

I suspect that the deref of bufferp in the expression is erronous.

[brooks]

lib/libcasper/services/cap_grp/cap_grp.c

105

I've spent some time looking over the whole code. I believe it's correct that it's dereferencing bufferp. bufferp holds a pointer to a pointer to the next byte in a buffer used to store values in a struct grp. This allow buffer in the caller to be updated at the end of group_unpack_members.[0] It's dereferenced before passing to _ALIGN because *bufferp is the buffer point. I think the warning is actually wrong because the compiler should be able to observe that the pointer is now properly aligned as a result of the align_up, but it's diagnosed so we need the cast.

[0] This update is ultimately pointless as there are no further consumers of buffer, but it matches group_unpack_string.