On Wed, Feb 12, 2025 at 07:31:26PM +1030, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
>
> So, in order to avoid ending up with flexible-array members in the
> middle of other structs, we use the `struct_group_tagged()` helper
> to separate the flexible arrays from the rest of the members in the
> flexible structures. We then use the newly created tagged `struct
> nvif_ioctl_v0_hdr` and `struct nvif_ioctl_mthd_v0_hdr` to replace the
> type of the objects causing trouble in multiple structures.
>
> We also want to ensure that when new members need to be added to the
> flexible structures, they are always included within the newly created
> tagged structs. For this, we use `static_assert()`. This ensures that the
> memory layout for both the flexible structure and the new tagged struct
> is the same after any changes.
>
> So, with these changes, fix the following warnings:
> drivers/gpu/drm/nouveau/nvif/object.c:60:38: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/nouveau/nvif/object.c:233:38: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/nouveau/nvif/object.c:214:38: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/nouveau/nvif/object.c:152:38: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/nouveau/nvif/object.c:138:38: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/nouveau/nvif/object.c:104:38: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/nouveau/nouveau_svm.c:83:35: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/gpu/drm/nouveau/nouveau_svm.c:82:30: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
>
> Signed-off-by: Gustavo A. R. Silva <gustavo...@kernel.org>
@Faith, Karol: Can I get an ACK from mesa for this one?
> ---
> drivers/gpu/drm/nouveau/include/nvif/ioctl.h | 32 +++++++++++++-------
> drivers/gpu/drm/nouveau/nouveau_svm.c | 4 +--
> drivers/gpu/drm/nouveau/nvif/object.c | 12 ++++----
> 3 files changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> b/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> index e825c8a1d9ca..00015412cb3e 100644
> --- a/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> +++ b/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> @@ -3,25 +3,30 @@
> #define __NVIF_IOCTL_H__
>
> struct nvif_ioctl_v0 {
> - __u8 version;
> + /* New members MUST be added within the struct_group() macro below. */
> + struct_group_tagged(nvif_ioctl_v0_hdr, __hdr,
> + __u8 version;
> #define NVIF_IOCTL_V0_SCLASS
> 0x01
> #define NVIF_IOCTL_V0_NEW
> 0x02
> #define NVIF_IOCTL_V0_DEL
> 0x03
> #define NVIF_IOCTL_V0_MTHD
> 0x04
> #define NVIF_IOCTL_V0_MAP
> 0x07
> #define NVIF_IOCTL_V0_UNMAP
> 0x08
> - __u8 type;
> - __u8 pad02[4];
> + __u8 type;
> + __u8 pad02[4];
> #define NVIF_IOCTL_V0_OWNER_NVIF
> 0x00
> #define NVIF_IOCTL_V0_OWNER_ANY
> 0xff
> - __u8 owner;
> + __u8 owner;
> #define NVIF_IOCTL_V0_ROUTE_NVIF
> 0x00
> #define NVIF_IOCTL_V0_ROUTE_HIDDEN
> 0xff
> - __u8 route;
> - __u64 token;
> - __u64 object;
> + __u8 route;
> + __u64 token;
> + __u64 object;
> + );
> __u8 data[]; /* ioctl data (below) */
> };
> +static_assert(offsetof(struct nvif_ioctl_v0, data) == sizeof(struct
> nvif_ioctl_v0_hdr),
> + "struct member likely outside of struct_group()");
>
> struct nvif_ioctl_sclass_v0 {
> /* nvif_ioctl ... */
> @@ -51,12 +56,17 @@ struct nvif_ioctl_del {
> };
>
> struct nvif_ioctl_mthd_v0 {
> - /* nvif_ioctl ... */
> - __u8 version;
> - __u8 method;
> - __u8 pad02[6];
> + /* New members MUST be added within the struct_group() macro below. */
> + struct_group_tagged(nvif_ioctl_mthd_v0_hdr, __hdr,
> + /* nvif_ioctl ... */
> + __u8 version;
> + __u8 method;
> + __u8 pad02[6];
> + );
> __u8 data[]; /* method data (class.h) */
> };
> +static_assert(offsetof(struct nvif_ioctl_mthd_v0, data) == sizeof(struct
> nvif_ioctl_mthd_v0_hdr),
> + "struct member likely outside of struct_group()");
>
> struct nvif_ioctl_map_v0 {
> /* nvif_ioctl ... */
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index b4da82ddbb6b..fc64c3d3169e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -79,8 +79,8 @@ struct nouveau_svm {
> #define SVM_ERR(s,f,a...) NV_WARN((s)->drm, "svm: "f"\n", ##a)
>
> struct nouveau_pfnmap_args {
> - struct nvif_ioctl_v0 i;
> - struct nvif_ioctl_mthd_v0 m;
> + struct nvif_ioctl_v0_hdr i;
> + struct nvif_ioctl_mthd_v0_hdr m;
> struct nvif_vmm_pfnmap_v0 p;
> };
>
> diff --git a/drivers/gpu/drm/nouveau/nvif/object.c
> b/drivers/gpu/drm/nouveau/nvif/object.c
> index 0b87278ac0f8..70af63d70976 100644
> --- a/drivers/gpu/drm/nouveau/nvif/object.c
> +++ b/drivers/gpu/drm/nouveau/nvif/object.c
> @@ -57,7 +57,7 @@ int
> nvif_object_sclass_get(struct nvif_object *object, struct nvif_sclass
> **psclass)
> {
> struct {
> - struct nvif_ioctl_v0 ioctl;
> + struct nvif_ioctl_v0_hdr ioctl;
> struct nvif_ioctl_sclass_v0 sclass;
> } *args = NULL;
> int ret, cnt = 0, i;
> @@ -101,7 +101,7 @@ int
> nvif_object_mthd(struct nvif_object *object, u32 mthd, void *data, u32 size)
> {
> struct {
> - struct nvif_ioctl_v0 ioctl;
> + struct nvif_ioctl_v0_hdr ioctl;
> struct nvif_ioctl_mthd_v0 mthd;
> } *args;
> u32 args_size;
> @@ -135,7 +135,7 @@ void
> nvif_object_unmap_handle(struct nvif_object *object)
> {
> struct {
> - struct nvif_ioctl_v0 ioctl;
> + struct nvif_ioctl_v0_hdr ioctl;
> struct nvif_ioctl_unmap unmap;
> } args = {
> .ioctl.type = NVIF_IOCTL_V0_UNMAP,
> @@ -149,7 +149,7 @@ nvif_object_map_handle(struct nvif_object *object, void
> *argv, u32 argc,
> u64 *handle, u64 *length)
> {
> struct {
> - struct nvif_ioctl_v0 ioctl;
> + struct nvif_ioctl_v0_hdr ioctl;
> struct nvif_ioctl_map_v0 map;
> } *args;
> u32 argn = sizeof(*args) + argc;
> @@ -211,7 +211,7 @@ void
> nvif_object_dtor(struct nvif_object *object)
> {
> struct {
> - struct nvif_ioctl_v0 ioctl;
> + struct nvif_ioctl_v0_hdr ioctl;
> struct nvif_ioctl_del del;
> } args = {
> .ioctl.type = NVIF_IOCTL_V0_DEL,
> @@ -230,7 +230,7 @@ nvif_object_ctor(struct nvif_object *parent, const char
> *name, u32 handle,
> s32 oclass, void *data, u32 size, struct nvif_object *object)
> {
> struct {
> - struct nvif_ioctl_v0 ioctl;
> + struct nvif_ioctl_v0_hdr ioctl;
> struct nvif_ioctl_new_v0 new;
> } *args;
> int ret = 0;
> --
> 2.43.0
>
