[PATCH] drm/ioctl: turn on -Woverride-init for IOCTL table

[PATCH] drm/ioctl: turn on -Woverride-init for IOCTL table

[Simon Ser]

Recently two patches [1] [2] have hit a case of mistakenly picking
an IOCTL number which was already in-use. This is hard to debug
because the last definition wins and there is no indication that
there is a conflict.

Fix this by enabling -Werror=override-init for the IOCTL table.
When there is a duplicate entry, the compiler now errors out:

      CC [M]  drivers/gpu/drm/drm_ioctl.o
    drivers/gpu/drm/drm_ioctl.c:555:33: error: initialized field overwritten [-Werror=override-init]
      555 |         [DRM_IOCTL_NR(ioctl)] = {               \
          |                                 ^
    drivers/gpu/drm/drm_ioctl.c:708:9: note: in expansion of macro ‘DRM_IOCTL_DEF’
      708 |         DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_EVENTFD, drm_syncobj_reset_ioctl,
          |         ^~~~~~~~~~~~~
    drivers/gpu/drm/drm_ioctl.c:555:33: note: (near initialization for ‘drm_ioctls[207]’)
      555 |         [DRM_IOCTL_NR(ioctl)] = {               \
          |                                 ^
    drivers/gpu/drm/drm_ioctl.c:708:9: note: in expansion of macro ‘DRM_IOCTL_DEF’
      708 |         DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_EVENTFD, drm_syncobj_reset_ioctl,
          |         ^~~~~~~~~~~~~
    cc1: some warnings being treated as errors

[1]: https://lore.kernel.org/dri-devel/20230714111257.11940-1-contact@emersion.fr/
[2]: https://lore.kernel.org/dri-devel/vVFDBgHpdcB0vOwnl02QPOFmAZPEbIV56E_wQ8B012K2GZ-fAGyG0JMbSrMu3-IcKYVf0JpJyrf71e6KFHfeMoSPJlYRACxlxy91eW9c6Fc=@emersion.fr/

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Erik Kurzinger <ekurzinger@nvidia.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/drm_ioctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index f03ffbacfe9b..cd485eb54d2a 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -566,6 +566,8 @@ static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 #endif
 
 /* Ioctl table */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic error "-Woverride-init"
 static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_VERSION, drm_version, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0),
@@ -718,6 +720,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER),
 };
+#pragma GCC diagnostic pop
 
 #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE(drm_ioctls)
 
-- 
2.41.0

Re: [PATCH] drm/ioctl: turn on -Woverride-init for IOCTL table

[Jani Nikula]

On Thu, 20 Jul 2023, Simon Ser <contact@emersion.fr> wrote:
> Recently two patches [1] [2] have hit a case of mistakenly picking
> an IOCTL number which was already in-use. This is hard to debug
> because the last definition wins and there is no indication that
> there is a conflict.
>
> Fix this by enabling -Werror=override-init for the IOCTL table.
> When there is a duplicate entry, the compiler now errors out:
>
>       CC [M]  drivers/gpu/drm/drm_ioctl.o
>     drivers/gpu/drm/drm_ioctl.c:555:33: error: initialized field overwritten [-Werror=override-init]
>       555 |         [DRM_IOCTL_NR(ioctl)] = {               \
>           |                                 ^
>     drivers/gpu/drm/drm_ioctl.c:708:9: note: in expansion of macro ‘DRM_IOCTL_DEF’
>       708 |         DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_EVENTFD, drm_syncobj_reset_ioctl,
>           |         ^~~~~~~~~~~~~
>     drivers/gpu/drm/drm_ioctl.c:555:33: note: (near initialization for ‘drm_ioctls[207]’)
>       555 |         [DRM_IOCTL_NR(ioctl)] = {               \
>           |                                 ^
>     drivers/gpu/drm/drm_ioctl.c:708:9: note: in expansion of macro ‘DRM_IOCTL_DEF’
>       708 |         DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_EVENTFD, drm_syncobj_reset_ioctl,
>           |         ^~~~~~~~~~~~~
>     cc1: some warnings being treated as errors
>
> [1]: https://lore.kernel.org/dri-devel/20230714111257.11940-1-contact@emersion.fr/
> [2]: https://lore.kernel.org/dri-devel/vVFDBgHpdcB0vOwnl02QPOFmAZPEbIV56E_wQ8B012K2GZ-fAGyG0JMbSrMu3-IcKYVf0JpJyrf71e6KFHfeMoSPJlYRACxlxy91eW9c6Fc=@emersion.fr/
>
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Erik Kurzinger <ekurzinger@nvidia.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index f03ffbacfe9b..cd485eb54d2a 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -566,6 +566,8 @@ static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
>  #endif
>  
>  /* Ioctl table */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic error "-Woverride-init"

You should probably use __diag_push(), __diag_warn(), and __diag_pop()
compiler agnostic wrappers instead of using #pragmas directly.

BR,
Jani.

>  static const struct drm_ioctl_desc drm_ioctls[] = {
>  	DRM_IOCTL_DEF(DRM_IOCTL_VERSION, drm_version, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0),
> @@ -718,6 +720,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER),
>  };
> +#pragma GCC diagnostic pop
>  
>  #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE(drm_ioctls)

-- 
Jani Nikula, Intel Open Source Graphics Center