void* is used to align to 32-bits or 64-bits on 32 and 64 bits archs. Purecap breaks this assumption. As struct __kernel_sockaddr_storage is used for both Aarch64 and purecap user structs, we cannot reliably use void* for alignment.
Change it to an unsigned long. Musl uses the same type for its definition of struct sockaddr_storage.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com --- include/uapi/linux/socket.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h index 51d6bb2f6765..28ff6a4aebca 100644 --- a/include/uapi/linux/socket.h +++ b/include/uapi/linux/socket.h @@ -22,7 +22,7 @@ struct __kernel_sockaddr_storage { /* space to achieve desired size, */ /* _SS_MAXSIZE value minus size of ss_family */ }; - void *__align; /* implementation specific desired alignment */ + unsigned long __align; /* implementation specific desired alignment */ }; };
On 13/12/2022 18:06, Teo Couprie Diaz wrote:
void* is used to align to 32-bits or 64-bits on 32 and 64 bits archs.
... respectively.
Purecap breaks this assumption. As struct __kernel_sockaddr_storage is used for both Aarch64 and purecap
We shouldn't talk about AArch64 for generic code, besides it's clear this struct is used regardless of the ABI. I think pointing out that void * has an alignment of 16 in PCuABI would be more helpful.
Kevin
user structs, we cannot reliably use void* for alignment.
Change it to an unsigned long. Musl uses the same type for its definition of struct sockaddr_storage.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
include/uapi/linux/socket.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h index 51d6bb2f6765..28ff6a4aebca 100644 --- a/include/uapi/linux/socket.h +++ b/include/uapi/linux/socket.h @@ -22,7 +22,7 @@ struct __kernel_sockaddr_storage { /* space to achieve desired size, */ /* _SS_MAXSIZE value minus size of ss_family */ };
void *__align; /* implementation specific desired alignment */
unsigned long __align; /* implementation specific desired alignment */};
On 14/12/2022 14:43, Kevin Brodsky wrote:
On 13/12/2022 18:06, Teo Couprie Diaz wrote:
void* is used to align to 32-bits or 64-bits on 32 and 64 bits archs.
... respectively.
That does sound better, thanks.
Purecap breaks this assumption. As struct __kernel_sockaddr_storage is used for both Aarch64 and purecap
We shouldn't talk about AArch64 for generic code, besides it's clear this struct is used regardless of the ABI. I think pointing out that void * has an alignment of 16 in PCuABI would be more helpful.
That makes sense, I understand. Would something along the lines of the following make sense ?
Purecap breaks this assumptions as void * has an alignment of 16, rather than 8 on a 64-bits arch.
Kevin
Thanks, Téo
user structs, we cannot reliably use void* for alignment.
Change it to an unsigned long. Musl uses the same type for its definition of struct sockaddr_storage.
Signed-off-by: Teo Couprie Diaz teo.coupriediaz@arm.com
include/uapi/linux/socket.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h index 51d6bb2f6765..28ff6a4aebca 100644 --- a/include/uapi/linux/socket.h +++ b/include/uapi/linux/socket.h @@ -22,7 +22,7 @@ struct __kernel_sockaddr_storage { /* space to achieve desired size, */ /* _SS_MAXSIZE value minus size of ss_family */ };
void *__align; /* implementation specific desired alignment */
unsigned long __align; /* implementation specific desired alignment */
On 14/12/2022 16:44, Teo Couprie Diaz wrote:
Purecap breaks this assumption. As struct __kernel_sockaddr_storage is used for both Aarch64 and purecap
We shouldn't talk about AArch64 for generic code, besides it's clear this struct is used regardless of the ABI. I think pointing out that void * has an alignment of 16 in PCuABI would be more helpful.
That makes sense, I understand. Would something along the lines of the following make sense ?
Purecap breaks this assumptions as void * has an alignment of 16, rather than 8 on a 64-bits arch.
Getting there :) I'd use the longer "The pure-capability ABI" just in case, and clarify a bit arch vs ABI: on 64-bit archs, we expect an alignment of 8; without this patch, this holds for standard ABIs, but not purecap. I'm insisting on this angle because it is quite confusing out of context: the alignment we want is linked to the "word size" (32/64-bit) and it remains the same whether your ABI is purecap or not, what changes is the size of pointers.
Kevin
On 15/12/2022 08:54, Kevin Brodsky wrote:
On 14/12/2022 16:44, Teo Couprie Diaz wrote:
Purecap breaks this assumption. As struct __kernel_sockaddr_storage is used for both Aarch64 and purecap
We shouldn't talk about AArch64 for generic code, besides it's clear this struct is used regardless of the ABI. I think pointing out that void * has an alignment of 16 in PCuABI would be more helpful.
That makes sense, I understand. Would something along the lines of the following make sense ?
Purecap breaks this assumptions as void * has an alignment of 16, rather than 8 on a 64-bits arch.
Getting there :) I'd use the longer "The pure-capability ABI" just in case, and clarify a bit arch vs ABI: on 64-bit archs, we expect an alignment of 8; without this patch, this holds for standard ABIs, but not purecap. I'm insisting on this angle because it is quite confusing out of context: the alignment we want is linked to the "word size" (32/64-bit) and it remains the same whether your ABI is purecap or not, what changes is the size of pointers.
I understand where you're coming from. How about :
The pure-capability ABI breaks this assumption as the pointer size is different than the word size, which is where the alignment requirement comes from. Indeed, the pointer size, and so void *, is now 16 bytes whereas the expected alignment is still 8 bytes, as it is a different ABI but for a 64-bit arch.
Maybe a bit verbose ? Hopefully I'm getting closer to the meaning you want to convey !
Kevin
Regards, Téo
On 15/12/2022 11:00, Teo Couprie Diaz wrote:
On 15/12/2022 08:54, Kevin Brodsky wrote:
On 14/12/2022 16:44, Teo Couprie Diaz wrote:
Purecap breaks this assumption. As struct __kernel_sockaddr_storage is used for both Aarch64 and purecap
We shouldn't talk about AArch64 for generic code, besides it's clear this struct is used regardless of the ABI. I think pointing out that void * has an alignment of 16 in PCuABI would be more helpful.
That makes sense, I understand. Would something along the lines of the following make sense ?
Purecap breaks this assumptions as void * has an alignment of 16, rather than 8 on a 64-bits arch.
Getting there :) I'd use the longer "The pure-capability ABI" just in case, and clarify a bit arch vs ABI: on 64-bit archs, we expect an alignment of 8; without this patch, this holds for standard ABIs, but not purecap. I'm insisting on this angle because it is quite confusing out of context: the alignment we want is linked to the "word size" (32/64-bit) and it remains the same whether your ABI is purecap or not, what changes is the size of pointers.
I understand where you're coming from. How about :
The pure-capability ABI breaks this assumption as the pointer size is different than the word size, which is where the alignment requirement comes from. Indeed, the pointer size, and so void *, is now 16 bytes whereas the expected alignment is still 8 bytes, as it is a different ABI but for a 64-bit arch.
Maybe a bit verbose ? Hopefully I'm getting closer to the meaning you want to convey !
On the contrary I think this is just the right level of verbosity :) Better use more words when things are subtle! I'd just suggest this for the second sentence (a couple small changes):
Indeed, the pointer size, and so void *, is 16 bytes in purecap while the expected alignment is still 8 bytes, as it is a different ABI but still for a 64-bit arch.
Feel free to send v3 with that wording, and thanks for bearing with me!
Kevin
On 15/12/2022 16:57, Kevin Brodsky wrote:
On 15/12/2022 11:00, Teo Couprie Diaz wrote:
On 15/12/2022 08:54, Kevin Brodsky wrote:
On 14/12/2022 16:44, Teo Couprie Diaz wrote:
Purecap breaks this assumption. As struct __kernel_sockaddr_storage is used for both Aarch64 and purecap
We shouldn't talk about AArch64 for generic code, besides it's clear this struct is used regardless of the ABI. I think pointing out that void * has an alignment of 16 in PCuABI would be more helpful.
That makes sense, I understand. Would something along the lines of the following make sense ?
Purecap breaks this assumptions as void * has an alignment of 16, rather than 8 on a 64-bits arch.
Getting there :) I'd use the longer "The pure-capability ABI" just in case, and clarify a bit arch vs ABI: on 64-bit archs, we expect an alignment of 8; without this patch, this holds for standard ABIs, but not purecap. I'm insisting on this angle because it is quite confusing out of context: the alignment we want is linked to the "word size" (32/64-bit) and it remains the same whether your ABI is purecap or not, what changes is the size of pointers.
I understand where you're coming from. How about :
The pure-capability ABI breaks this assumption as the pointer size is different than the word size, which is where the alignment requirement comes from. Indeed, the pointer size, and so void *, is now 16 bytes whereas the expected alignment is still 8 bytes, as it is a different ABI but for a 64-bit arch.
Maybe a bit verbose ? Hopefully I'm getting closer to the meaning you want to convey !
On the contrary I think this is just the right level of verbosity :) Better use more words when things are subtle! I'd just suggest this for the second sentence (a couple small changes):
Indeed, the pointer size, and so void *, is 16 bytes in purecap while the expected alignment is still 8 bytes, as it is a different ABI but still for a 64-bit arch.
That sounds good, thanks !
Feel free to send v3 with that wording, and thanks for bearing with me!
No worries, I agree that being clear and precise is quite important ! It always needs a bit of work :) v3 sent with the updated wordings.
Kevin
Thanks for the review, Téo
linux-morello@op-lists.linaro.org
-
Kevin Brodsky -
Teo Couprie Diaz