[v15,1/3] eal: add diagnostics macros to make code portable

Message ID 1737237314-9844-2-git-send-email-andremue@linux.microsoft.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series add diagnostics macros to make code portable |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andre Muezerie Jan. 18, 2025, 9:55 p.m. UTC
It was a common pattern to have "GCC diagnostic ignored" pragmas
sprinkled over the code and only activate these pragmas for certain
compilers (gcc and clang). Clang supports GCC's pragma for
compatibility with existing source code, so #pragma GCC diagnostic
and #pragma clang diagnostic are synonyms for Clang
(https://clang.llvm.org/docs/UsersManual.html).

Now that effort is being made to make the code compatible with MSVC
these expressions would become more complex. It makes sense to hide
this complexity behind macros. This makes maintenance easier as these
macros are defined in a single place. As a plus the code becomes
more readable as well.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 lib/eal/include/rte_common.h | 46 ++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
  

Comments

Morten Brørup Jan. 21, 2025, 9:53 a.m. UTC | #1
> From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> Sent: Saturday, 18 January 2025 22.55
> 
> It was a common pattern to have "GCC diagnostic ignored" pragmas
> sprinkled over the code and only activate these pragmas for certain
> compilers (gcc and clang). Clang supports GCC's pragma for
> compatibility with existing source code, so #pragma GCC diagnostic
> and #pragma clang diagnostic are synonyms for Clang
> (https://clang.llvm.org/docs/UsersManual.html).
> 
> Now that effort is being made to make the code compatible with MSVC
> these expressions would become more complex. It makes sense to hide
> this complexity behind macros. This makes maintenance easier as these
> macros are defined in a single place. As a plus the code becomes
> more readable as well.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---
>  lib/eal/include/rte_common.h | 46 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/lib/eal/include/rte_common.h
> b/lib/eal/include/rte_common.h
> index 40592f71b1..4b87a0a352 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -156,6 +156,52 @@ typedef uint16_t unaligned_uint16_t;
>  #define RTE_DEPRECATED(x)
>  #endif
> 
> +/**
> + * Macros to cause the compiler to remember the state of the diagnostics as of
> + * each push, and restore to that point at each pop.
> + */
> +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> +#define __rte_diagnostic_push _Pragma("GCC diagnostic push")
> +#define __rte_diagnostic_pop  _Pragma("GCC diagnostic pop")
> +#else
> +#define __rte_diagnostic_push
> +#define __rte_diagnostic_pop
> +#endif
> +
> +/**
> + * Macro to disable compiler warnings about removing a type
> + * qualifier from the target type.
> + */
> +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> +#define __rte_diagnostic_ignored_wcast_qual \
> +		_Pragma("GCC diagnostic ignored \"-Wcast-qual\"")
> +#else
> +#define __rte_diagnostic_ignored_wcast_qual
> +#endif
> +
> +/**
> + * Workaround to discard qualifiers (such as const, volatile, restrict) from a pointer,
> + * without the compiler emitting a warning.
> + */
> +#define RTE_PTR_UNQUAL(X) ((void *)(uintptr_t)(X))

It seems the C23 typeof_unqual and the built-in pre-C23 __typeof_unqual__ couldn't be used.
Was it a generic issue, or only when operating on (the return value of) functions?

> +
> +/**
> + * Workaround to discard qualifiers (such as const, volatile, restrict) from a pointer
> + * and cast it to a specific type, without the compiler emitting a warning.

Propose new description with emphasis on casting rather than discarding qualifiers:

Workaround to cast a pointer to a specific type,
without the compiler emitting a warning about discarding qualifiers.

> + *
> + * @warning
> + * Although this macro can be abused for casting a pointer to point to a different type,
> + * alignment may be incorrect when casting to point to a larger type. E.g.:

This macro is now meant for abuse, so propose softening the warning:

When casting a pointer to point to a larger type,
the resulting pointer may be misaligned,
which causes undefined behavior.
E.g.:

> + *   struct s {
> + *       uint16_t a;
> + *       uint8_t  b;
> + *       uint8_t  c;
> + *       uint8_t  d;
> + *   } v;
> + *   uint16_t * p = RTE_CAST_PTR(uint16_t *, &v.c); // "p" is not 16 bit aligned!
> + */
> +#define RTE_CAST_PTR(type, ptr) ((type)(uintptr_t)(ptr))

I am somewhat concerned about these macros...

There's a good reason why MSVC doesn't allow casting to discard qualifiers or changing the type like this.

If in doubt, read this:
https://www.trust-in-soft.com/resources/blogs/2020-04-06-gcc-always-assumes-aligned-pointer-accesses

We need these workarounds because DPDK currently contains code with formally "undefined behavior".
And instead of fixing the root causes, we choose the pragmatic solution and introduce workarounds to allow it.

Would it be possible for the RTE_CAST_PTR macro to check if the casted-to pointer changes from a smaller type to a larger type, and warn/fail if it does?

Should the use of these workaround macros be disallowed in new code?
I.e. should checkpatches check for them?
  
Andre Muezerie Jan. 21, 2025, 2:28 p.m. UTC | #2
On Tue, Jan 21, 2025 at 10:53:14AM +0100, Morten Brørup wrote:
> > From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> > Sent: Saturday, 18 January 2025 22.55
> > 
> > It was a common pattern to have "GCC diagnostic ignored" pragmas
> > sprinkled over the code and only activate these pragmas for certain
> > compilers (gcc and clang). Clang supports GCC's pragma for
> > compatibility with existing source code, so #pragma GCC diagnostic
> > and #pragma clang diagnostic are synonyms for Clang
> > (https://clang.llvm.org/docs/UsersManual.html).
> > 
> > Now that effort is being made to make the code compatible with MSVC
> > these expressions would become more complex. It makes sense to hide
> > this complexity behind macros. This makes maintenance easier as these
> > macros are defined in a single place. As a plus the code becomes
> > more readable as well.
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > ---
> >  lib/eal/include/rte_common.h | 46 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> > 
> > diff --git a/lib/eal/include/rte_common.h
> > b/lib/eal/include/rte_common.h
> > index 40592f71b1..4b87a0a352 100644
> > --- a/lib/eal/include/rte_common.h
> > +++ b/lib/eal/include/rte_common.h
> > @@ -156,6 +156,52 @@ typedef uint16_t unaligned_uint16_t;
> >  #define RTE_DEPRECATED(x)
> >  #endif
> > 
> > +/**
> > + * Macros to cause the compiler to remember the state of the diagnostics as of
> > + * each push, and restore to that point at each pop.
> > + */
> > +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> > +#define __rte_diagnostic_push _Pragma("GCC diagnostic push")
> > +#define __rte_diagnostic_pop  _Pragma("GCC diagnostic pop")
> > +#else
> > +#define __rte_diagnostic_push
> > +#define __rte_diagnostic_pop
> > +#endif
> > +
> > +/**
> > + * Macro to disable compiler warnings about removing a type
> > + * qualifier from the target type.
> > + */
> > +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> > +#define __rte_diagnostic_ignored_wcast_qual \
> > +		_Pragma("GCC diagnostic ignored \"-Wcast-qual\"")
> > +#else
> > +#define __rte_diagnostic_ignored_wcast_qual
> > +#endif
> > +
> > +/**
> > + * Workaround to discard qualifiers (such as const, volatile, restrict) from a pointer,
> > + * without the compiler emitting a warning.
> > + */
> > +#define RTE_PTR_UNQUAL(X) ((void *)(uintptr_t)(X))
> 
> It seems the C23 typeof_unqual and the built-in pre-C23 __typeof_unqual__ couldn't be used.
> Was it a generic issue, or only when operating on (the return value of) functions?

I experimented with C23 typeof_unqual. It indeed works on gcc, clang and MSVC, but there are some details:

a) With gcc the project needs to be compiled with -std=c2x. Many other warnings show up, unrelated to the scope of this patchset. Some look suspicious and should be looked at. An error also showed up, for which I sent out a small patch.

b) When using typeof_unqual and passing "-Wcast-qual" to the compiler, a warning about the qualifier being dropped is emitted. The project currently uses "-Wcast-qual". Perhaps it shouldn't?

Due to (a) I decided to not use typeof_unqual for now, but it would be trivial to change the macro to do so in the future.

> 
> > +
> > +/**
> > + * Workaround to discard qualifiers (such as const, volatile, restrict) from a pointer
> > + * and cast it to a specific type, without the compiler emitting a warning.
> 
> Propose new description with emphasis on casting rather than discarding qualifiers:
> 
> Workaround to cast a pointer to a specific type,
> without the compiler emitting a warning about discarding qualifiers.
> 

I'll update this.

> > + *
> > + * @warning
> > + * Although this macro can be abused for casting a pointer to point to a different type,
> > + * alignment may be incorrect when casting to point to a larger type. E.g.:
> 
> This macro is now meant for abuse, so propose softening the warning:
> 
> When casting a pointer to point to a larger type,
> the resulting pointer may be misaligned,
> which causes undefined behavior.

I'll update this.

> E.g.:
> 
> > + *   struct s {
> > + *       uint16_t a;
> > + *       uint8_t  b;
> > + *       uint8_t  c;
> > + *       uint8_t  d;
> > + *   } v;
> > + *   uint16_t * p = RTE_CAST_PTR(uint16_t *, &v.c); // "p" is not 16 bit aligned!
> > + */
> > +#define RTE_CAST_PTR(type, ptr) ((type)(uintptr_t)(ptr))
> 
> I am somewhat concerned about these macros...
> 
> There's a good reason why MSVC doesn't allow casting to discard qualifiers or changing the type like this.
> 
> If in doubt, read this:
> https://www.trust-in-soft.com/resources/blogs/2020-04-06-gcc-always-assumes-aligned-pointer-accesses
> 
> We need these workarounds because DPDK currently contains code with formally "undefined behavior".
> And instead of fixing the root causes, we choose the pragmatic solution and introduce workarounds to allow it.
> 
> Would it be possible for the RTE_CAST_PTR macro to check if the casted-to pointer changes from a smaller type to a larger type, and warn/fail if it does?

I'll think about it.
> 
> Should the use of these workaround macros be disallowed in new code?
> I.e. should checkpatches check for them?

We can certainly add a check to checkpatches.
  
Morten Brørup Jan. 21, 2025, 2:41 p.m. UTC | #3
> From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> Sent: Tuesday, 21 January 2025 15.28
> 
> On Tue, Jan 21, 2025 at 10:53:14AM +0100, Morten Brørup wrote:
> > > From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> > > Sent: Saturday, 18 January 2025 22.55
> > >
> > > It was a common pattern to have "GCC diagnostic ignored" pragmas
> > > sprinkled over the code and only activate these pragmas for certain
> > > compilers (gcc and clang). Clang supports GCC's pragma for
> > > compatibility with existing source code, so #pragma GCC diagnostic
> > > and #pragma clang diagnostic are synonyms for Clang
> > > (https://clang.llvm.org/docs/UsersManual.html).
> > >
> > > Now that effort is being made to make the code compatible with MSVC
> > > these expressions would become more complex. It makes sense to hide
> > > this complexity behind macros. This makes maintenance easier as
> these
> > > macros are defined in a single place. As a plus the code becomes
> > > more readable as well.
> > >
> > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > ---
> > >  lib/eal/include/rte_common.h | 46
> ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 46 insertions(+)
> > >
> > > diff --git a/lib/eal/include/rte_common.h
> > > b/lib/eal/include/rte_common.h
> > > index 40592f71b1..4b87a0a352 100644
> > > --- a/lib/eal/include/rte_common.h
> > > +++ b/lib/eal/include/rte_common.h
> > > @@ -156,6 +156,52 @@ typedef uint16_t unaligned_uint16_t;
> > >  #define RTE_DEPRECATED(x)
> > >  #endif
> > >
> > > +/**
> > > + * Macros to cause the compiler to remember the state of the
> diagnostics as of
> > > + * each push, and restore to that point at each pop.
> > > + */
> > > +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> > > +#define __rte_diagnostic_push _Pragma("GCC diagnostic push")
> > > +#define __rte_diagnostic_pop  _Pragma("GCC diagnostic pop")
> > > +#else
> > > +#define __rte_diagnostic_push
> > > +#define __rte_diagnostic_pop
> > > +#endif
> > > +
> > > +/**
> > > + * Macro to disable compiler warnings about removing a type
> > > + * qualifier from the target type.
> > > + */
> > > +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> > > +#define __rte_diagnostic_ignored_wcast_qual \
> > > +		_Pragma("GCC diagnostic ignored \"-Wcast-qual\"")
> > > +#else
> > > +#define __rte_diagnostic_ignored_wcast_qual
> > > +#endif
> > > +
> > > +/**
> > > + * Workaround to discard qualifiers (such as const, volatile,
> restrict) from a pointer,
> > > + * without the compiler emitting a warning.
> > > + */
> > > +#define RTE_PTR_UNQUAL(X) ((void *)(uintptr_t)(X))
> >
> > It seems the C23 typeof_unqual and the built-in pre-C23
> __typeof_unqual__ couldn't be used.
> > Was it a generic issue, or only when operating on (the return value
> of) functions?
> 
> I experimented with C23 typeof_unqual. It indeed works on gcc, clang
> and MSVC, but there are some details:
> 
> a) With gcc the project needs to be compiled with -std=c2x. Many other
> warnings show up, unrelated to the scope of this patchset. Some look
> suspicious and should be looked at. An error also showed up, for which
> I sent out a small patch.
> 
> b) When using typeof_unqual and passing "-Wcast-qual" to the compiler,
> a warning about the qualifier being dropped is emitted. The project
> currently uses "-Wcast-qual". Perhaps it shouldn't?

The compiler is our friend; when more warnings enabled, the code quality requirements are higher.
Although this statement may not be universally true, I think it is for "-Wcast-qual".

> 
> Due to (a) I decided to not use typeof_unqual for now, but it would be
> trivial to change the macro to do so in the future.

How about __typeof_unqual__ (with double underscores prefix and postfix)?
It seems to be available in both GCC [1] and MSVC [2] without requiring C23.

[1]: https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
[2]: https://learn.microsoft.com/en-us/cpp/c-language/typeof-unqual-c?view=msvc-170

> 
> >
> > > +
> > > +/**
> > > + * Workaround to discard qualifiers (such as const, volatile,
> restrict) from a pointer
> > > + * and cast it to a specific type, without the compiler emitting a
> warning.
> >
> > Propose new description with emphasis on casting rather than
> discarding qualifiers:
> >
> > Workaround to cast a pointer to a specific type,
> > without the compiler emitting a warning about discarding qualifiers.
> >
> 
> I'll update this.
> 
> > > + *
> > > + * @warning
> > > + * Although this macro can be abused for casting a pointer to
> point to a different type,
> > > + * alignment may be incorrect when casting to point to a larger
> type. E.g.:
> >
> > This macro is now meant for abuse, so propose softening the warning:
> >
> > When casting a pointer to point to a larger type,
> > the resulting pointer may be misaligned,
> > which causes undefined behavior.
> 
> I'll update this.
> 
> > E.g.:
> >
> > > + *   struct s {
> > > + *       uint16_t a;
> > > + *       uint8_t  b;
> > > + *       uint8_t  c;
> > > + *       uint8_t  d;
> > > + *   } v;
> > > + *   uint16_t * p = RTE_CAST_PTR(uint16_t *, &v.c); // "p" is not
> 16 bit aligned!
> > > + */
> > > +#define RTE_CAST_PTR(type, ptr) ((type)(uintptr_t)(ptr))
> >
> > I am somewhat concerned about these macros...
> >
> > There's a good reason why MSVC doesn't allow casting to discard
> qualifiers or changing the type like this.
> >
> > If in doubt, read this:
> > https://www.trust-in-soft.com/resources/blogs/2020-04-06-gcc-always-
> assumes-aligned-pointer-accesses
> >
> > We need these workarounds because DPDK currently contains code with
> formally "undefined behavior".
> > And instead of fixing the root causes, we choose the pragmatic
> solution and introduce workarounds to allow it.
> >
> > Would it be possible for the RTE_CAST_PTR macro to check if the
> casted-to pointer changes from a smaller type to a larger type, and
> warn/fail if it does?
> 
> I'll think about it.
> >
> > Should the use of these workaround macros be disallowed in new code?
> > I.e. should checkpatches check for them?
> 
> We can certainly add a check to checkpatches.
  
Stephen Hemminger Jan. 21, 2025, 3:01 p.m. UTC | #4
On Tue, 21 Jan 2025 06:28:16 -0800
Andre Muezerie <andremue@linux.microsoft.com> wrote:

> On Tue, Jan 21, 2025 at 10:53:14AM +0100, Morten Brørup wrote:
> > > From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> > > Sent: Saturday, 18 January 2025 22.55
> > > 
> > > It was a common pattern to have "GCC diagnostic ignored" pragmas
> > > sprinkled over the code and only activate these pragmas for certain
> > > compilers (gcc and clang). Clang supports GCC's pragma for
> > > compatibility with existing source code, so #pragma GCC diagnostic
> > > and #pragma clang diagnostic are synonyms for Clang
> > > (https://clang.llvm.org/docs/UsersManual.html).
> > > 
> > > Now that effort is being made to make the code compatible with MSVC
> > > these expressions would become more complex. It makes sense to hide
> > > this complexity behind macros. This makes maintenance easier as these
> > > macros are defined in a single place. As a plus the code becomes
> > > more readable as well.
> > > 
> > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > ---
> > >  lib/eal/include/rte_common.h | 46 ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 46 insertions(+)
> > > 
> > > diff --git a/lib/eal/include/rte_common.h
> > > b/lib/eal/include/rte_common.h
> > > index 40592f71b1..4b87a0a352 100644
> > > --- a/lib/eal/include/rte_common.h
> > > +++ b/lib/eal/include/rte_common.h
> > > @@ -156,6 +156,52 @@ typedef uint16_t unaligned_uint16_t;
> > >  #define RTE_DEPRECATED(x)
> > >  #endif
> > > 
> > > +/**
> > > + * Macros to cause the compiler to remember the state of the diagnostics as of
> > > + * each push, and restore to that point at each pop.
> > > + */
> > > +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> > > +#define __rte_diagnostic_push _Pragma("GCC diagnostic push")
> > > +#define __rte_diagnostic_pop  _Pragma("GCC diagnostic pop")
> > > +#else
> > > +#define __rte_diagnostic_push
> > > +#define __rte_diagnostic_pop
> > > +#endif
> > > +
> > > +/**
> > > + * Macro to disable compiler warnings about removing a type
> > > + * qualifier from the target type.
> > > + */
> > > +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> > > +#define __rte_diagnostic_ignored_wcast_qual \
> > > +		_Pragma("GCC diagnostic ignored \"-Wcast-qual\"")
> > > +#else
> > > +#define __rte_diagnostic_ignored_wcast_qual
> > > +#endif
> > > +
> > > +/**
> > > + * Workaround to discard qualifiers (such as const, volatile, restrict) from a pointer,
> > > + * without the compiler emitting a warning.
> > > + */
> > > +#define RTE_PTR_UNQUAL(X) ((void *)(uintptr_t)(X))  
> > 
> > It seems the C23 typeof_unqual and the built-in pre-C23 __typeof_unqual__ couldn't be used.
> > Was it a generic issue, or only when operating on (the return value of) functions?  
> 
> I experimented with C23 typeof_unqual. It indeed works on gcc, clang and MSVC, but there are some details:
> 
> a) With gcc the project needs to be compiled with -std=c2x. Many other warnings show up, unrelated to the scope of this patchset. Some look suspicious and should be looked at. An error also showed up, for which I sent out a small patch.
> 
> b) When using typeof_unqual and passing "-Wcast-qual" to the compiler, a warning about the qualifier being dropped is emitted. The project currently uses "-Wcast-qual". Perhaps it shouldn't?
> 
> Due to (a) I decided to not use typeof_unqual for now, but it would be trivial to change the macro to do so in the future.


Be careful, C23 changes some things like default initialization of padding bits.
Might break other stuff.
  
Andre Muezerie Jan. 21, 2025, 8:17 p.m. UTC | #5
On Tue, Jan 21, 2025 at 03:41:09PM +0100, Morten Brørup wrote:
> > From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> > Sent: Tuesday, 21 January 2025 15.28
> > 
> > On Tue, Jan 21, 2025 at 10:53:14AM +0100, Morten Brørup wrote:
> > > > From: Andre Muezerie [mailto:andremue@linux.microsoft.com]
> > > > Sent: Saturday, 18 January 2025 22.55
> > > >
> > > > It was a common pattern to have "GCC diagnostic ignored" pragmas
> > > > sprinkled over the code and only activate these pragmas for certain
> > > > compilers (gcc and clang). Clang supports GCC's pragma for
> > > > compatibility with existing source code, so #pragma GCC diagnostic
> > > > and #pragma clang diagnostic are synonyms for Clang
> > > > (https://clang.llvm.org/docs/UsersManual.html).
> > > >
> > > > Now that effort is being made to make the code compatible with MSVC
> > > > these expressions would become more complex. It makes sense to hide
> > > > this complexity behind macros. This makes maintenance easier as
> > these
> > > > macros are defined in a single place. As a plus the code becomes
> > > > more readable as well.
> > > >
> > > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > > > ---
> > > >  lib/eal/include/rte_common.h | 46
> > ++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 46 insertions(+)
> > > >
> > > > diff --git a/lib/eal/include/rte_common.h
> > > > b/lib/eal/include/rte_common.h
> > > > index 40592f71b1..4b87a0a352 100644
> > > > --- a/lib/eal/include/rte_common.h
> > > > +++ b/lib/eal/include/rte_common.h
> > > > @@ -156,6 +156,52 @@ typedef uint16_t unaligned_uint16_t;
> > > >  #define RTE_DEPRECATED(x)
> > > >  #endif
> > > >
> > > > +/**
> > > > + * Macros to cause the compiler to remember the state of the
> > diagnostics as of
> > > > + * each push, and restore to that point at each pop.
> > > > + */
> > > > +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> > > > +#define __rte_diagnostic_push _Pragma("GCC diagnostic push")
> > > > +#define __rte_diagnostic_pop  _Pragma("GCC diagnostic pop")
> > > > +#else
> > > > +#define __rte_diagnostic_push
> > > > +#define __rte_diagnostic_pop
> > > > +#endif
> > > > +
> > > > +/**
> > > > + * Macro to disable compiler warnings about removing a type
> > > > + * qualifier from the target type.
> > > > + */
> > > > +#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
> > > > +#define __rte_diagnostic_ignored_wcast_qual \
> > > > +		_Pragma("GCC diagnostic ignored \"-Wcast-qual\"")
> > > > +#else
> > > > +#define __rte_diagnostic_ignored_wcast_qual
> > > > +#endif
> > > > +
> > > > +/**
> > > > + * Workaround to discard qualifiers (such as const, volatile,
> > restrict) from a pointer,
> > > > + * without the compiler emitting a warning.
> > > > + */
> > > > +#define RTE_PTR_UNQUAL(X) ((void *)(uintptr_t)(X))
> > >
> > > It seems the C23 typeof_unqual and the built-in pre-C23
> > __typeof_unqual__ couldn't be used.
> > > Was it a generic issue, or only when operating on (the return value
> > of) functions?
> > 
> > I experimented with C23 typeof_unqual. It indeed works on gcc, clang
> > and MSVC, but there are some details:
> > 
> > a) With gcc the project needs to be compiled with -std=c2x. Many other
> > warnings show up, unrelated to the scope of this patchset. Some look
> > suspicious and should be looked at. An error also showed up, for which
> > I sent out a small patch.
> > 
> > b) When using typeof_unqual and passing "-Wcast-qual" to the compiler,
> > a warning about the qualifier being dropped is emitted. The project
> > currently uses "-Wcast-qual". Perhaps it shouldn't?
> 
> The compiler is our friend; when more warnings enabled, the code quality requirements are higher.

I agree 100% with this.

> Although this statement may not be universally true, I think it is for "-Wcast-qual".
> 
> > 
> > Due to (a) I decided to not use typeof_unqual for now, but it would be
> > trivial to change the macro to do so in the future.
> 
> How about __typeof_unqual__ (with double underscores prefix and postfix)?
> It seems to be available in both GCC [1] and MSVC [2] without requiring C23.
> 
> [1]: https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
> [2]: https://learn.microsoft.com/en-us/cpp/c-language/typeof-unqual-c?view=msvc-170

__typeof_unqual__ (with double underscores prefix and postfix) requires gcc-14. On gcc-13 this keyword is not defined.
It works exactly like C23 typeof_unqual: When -Wcast-qual is passed to the compiler a warning is emitted when discarding a qualifier from the pointer target type, which is treated as error in the DPDK build. Without -Wcast-qual it compiles cleanly.

> 
> > 
> > >
> > > > +
> > > > +/**
> > > > + * Workaround to discard qualifiers (such as const, volatile,
> > restrict) from a pointer
> > > > + * and cast it to a specific type, without the compiler emitting a
> > warning.
> > >
> > > Propose new description with emphasis on casting rather than
> > discarding qualifiers:
> > >
> > > Workaround to cast a pointer to a specific type,
> > > without the compiler emitting a warning about discarding qualifiers.
> > >
> > 
> > I'll update this.
> > 
> > > > + *
> > > > + * @warning
> > > > + * Although this macro can be abused for casting a pointer to
> > point to a different type,
> > > > + * alignment may be incorrect when casting to point to a larger
> > type. E.g.:
> > >
> > > This macro is now meant for abuse, so propose softening the warning:
> > >
> > > When casting a pointer to point to a larger type,
> > > the resulting pointer may be misaligned,
> > > which causes undefined behavior.
> > 
> > I'll update this.
> > 
> > > E.g.:
> > >
> > > > + *   struct s {
> > > > + *       uint16_t a;
> > > > + *       uint8_t  b;
> > > > + *       uint8_t  c;
> > > > + *       uint8_t  d;
> > > > + *   } v;
> > > > + *   uint16_t * p = RTE_CAST_PTR(uint16_t *, &v.c); // "p" is not
> > 16 bit aligned!
> > > > + */
> > > > +#define RTE_CAST_PTR(type, ptr) ((type)(uintptr_t)(ptr))
> > >
> > > I am somewhat concerned about these macros...
> > >
> > > There's a good reason why MSVC doesn't allow casting to discard
> > qualifiers or changing the type like this.
> > >
> > > If in doubt, read this:
> > > https://www.trust-in-soft.com/resources/blogs/2020-04-06-gcc-always-
> > assumes-aligned-pointer-accesses
> > >
> > > We need these workarounds because DPDK currently contains code with
> > formally "undefined behavior".
> > > And instead of fixing the root causes, we choose the pragmatic
> > solution and introduce workarounds to allow it.
> > >
> > > Would it be possible for the RTE_CAST_PTR macro to check if the
> > casted-to pointer changes from a smaller type to a larger type, and
> > warn/fail if it does?
> > 
> > I'll think about it.
> > >
> > > Should the use of these workaround macros be disallowed in new code?
> > > I.e. should checkpatches check for them?
> > 
> > We can certainly add a check to checkpatches.
  

Patch

diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 40592f71b1..4b87a0a352 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -156,6 +156,52 @@  typedef uint16_t unaligned_uint16_t;
 #define RTE_DEPRECATED(x)
 #endif
 
+/**
+ * Macros to cause the compiler to remember the state of the diagnostics as of
+ * each push, and restore to that point at each pop.
+ */
+#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
+#define __rte_diagnostic_push _Pragma("GCC diagnostic push")
+#define __rte_diagnostic_pop  _Pragma("GCC diagnostic pop")
+#else
+#define __rte_diagnostic_push
+#define __rte_diagnostic_pop
+#endif
+
+/**
+ * Macro to disable compiler warnings about removing a type
+ * qualifier from the target type.
+ */
+#if !defined(__INTEL_COMPILER) && !defined(RTE_TOOLCHAIN_MSVC)
+#define __rte_diagnostic_ignored_wcast_qual \
+		_Pragma("GCC diagnostic ignored \"-Wcast-qual\"")
+#else
+#define __rte_diagnostic_ignored_wcast_qual
+#endif
+
+/**
+ * Workaround to discard qualifiers (such as const, volatile, restrict) from a pointer,
+ * without the compiler emitting a warning.
+ */
+#define RTE_PTR_UNQUAL(X) ((void *)(uintptr_t)(X))
+
+/**
+ * Workaround to discard qualifiers (such as const, volatile, restrict) from a pointer
+ * and cast it to a specific type, without the compiler emitting a warning.
+ *
+ * @warning
+ * Although this macro can be abused for casting a pointer to point to a different type,
+ * alignment may be incorrect when casting to point to a larger type. E.g.:
+ *   struct s {
+ *       uint16_t a;
+ *       uint8_t  b;
+ *       uint8_t  c;
+ *       uint8_t  d;
+ *   } v;
+ *   uint16_t * p = RTE_CAST_PTR(uint16_t *, &v.c); // "p" is not 16 bit aligned!
+ */
+#define RTE_CAST_PTR(type, ptr) ((type)(uintptr_t)(ptr))
+
 /**
  * Mark a function or variable to a weak reference.
  */