[ovs-dev,v2] compiler: Fix errors in Clang 17 ubsan checks.

This patch attempts to fix a large number of ubsan error messages that
take the following form:

lib/netlink-notifier.c:237:13: runtime error: call to function
    route_table_change through pointer to incorrect function type
    'void (*)(const void *, void *)'

In Clang 17 the undefined behaviour sanatizer check for function
pointers was enabled by default, whereas it was previously disabled
while compiling C code. These warnings are a false positive in the case
of OVS, as our macros already check to make sure the function parameter
is the correct size.

So that check is disabled in the single function that is causing all of
the errors.

Signed-off-by: Mike Pattrick <mkp@redhat.com>
---
v2: Changed macro name
---
 include/openvswitch/compiler.h | 11 +++++++++++
 lib/ovs-rcu.c                  |  1 +
 2 files changed, 12 insertions(+)

Recheck-request: github-robot

On Thu, May 16, 2024 at 9:58 AM Mike Pattrick <mkp@redhat.com> wrote:
>
> This patch attempts to fix a large number of ubsan error messages that
> take the following form:
>
> lib/netlink-notifier.c:237:13: runtime error: call to function
>     route_table_change through pointer to incorrect function type
>     'void (*)(const void *, void *)'
>
> In Clang 17 the undefined behaviour sanatizer check for function
> pointers was enabled by default, whereas it was previously disabled
> while compiling C code. These warnings are a false positive in the case
> of OVS, as our macros already check to make sure the function parameter
> is the correct size.
>
> So that check is disabled in the single function that is causing all of
> the errors.
>
> Signed-off-by: Mike Pattrick <mkp@redhat.com>
> ---
> v2: Changed macro name
> ---
>  include/openvswitch/compiler.h | 11 +++++++++++
>  lib/ovs-rcu.c                  |  1 +
>  2 files changed, 12 insertions(+)
>
> diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h
> index 878c5c6a7..f49b23683 100644
> --- a/include/openvswitch/compiler.h
> +++ b/include/openvswitch/compiler.h
> @@ -69,6 +69,17 @@
>  #define OVS_UNLIKELY(CONDITION) (!!(CONDITION))
>  #endif
>
> +/* Clang 17's implementation of ubsan enables checking that function pointers
> + * match the type of the called function. This currently breaks ovs-rcu, which
> + * calls multiple different types of callbacks via a generic void *(void*)
> + * function pointer type. This macro enables disabling that check for specific
> + * functions. */
> +#if __clang__ && __has_feature(undefined_behavior_sanitizer)
> +#define OVS_NO_SANITIZE_FUNCTION __attribute__((no_sanitize("function")))
> +#else
> +#define OVS_NO_SANITIZE_FUNCTION
> +#endif
> +
>  #if __has_feature(c_thread_safety_attributes)
>  /* "clang" annotations for thread safety check.
>   *
> diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c
> index 9e07d9bab..597fe6826 100644
> --- a/lib/ovs-rcu.c
> +++ b/lib/ovs-rcu.c
> @@ -327,6 +327,7 @@ ovsrcu_postpone__(void (*function)(void *aux), void *aux)
>  }
>
>  static bool
> +OVS_NO_SANITIZE_FUNCTION
>  ovsrcu_call_postponed(void)
>  {
>      struct ovsrcu_cbset *cbset;
> --
> 2.39.3
>

On 16.05.24 15:57, Mike Pattrick wrote:
> This patch attempts to fix a large number of ubsan error messages that
> take the following form:
>
> lib/netlink-notifier.c:237:13: runtime error: call to function
>     route_table_change through pointer to incorrect function type
>     'void (*)(const void *, void *)'
>
> In Clang 17 the undefined behaviour sanatizer check for function
> pointers was enabled by default, whereas it was previously disabled
> while compiling C code. These warnings are a false positive in the case
> of OVS, as our macros already check to make sure the function parameter
> is the correct size.
>
> So that check is disabled in the single function that is causing all of
> the errors.
>
> Signed-off-by: Mike Pattrick <mkp@redhat.com>
> ---
> v2: Changed macro name
> ---
>  include/openvswitch/compiler.h | 11 +++++++++++
>  lib/ovs-rcu.c                  |  1 +
>  2 files changed, 12 insertions(+)

Thank you, Mike! This solves most issues I had when running OVS tests on Fedora Rawhide with Clang 18 and "-fsanitize=undefined".

Acked-by: Jakob Meng <jmeng@redhat.com>

On 16 May 2024, at 15:57, Mike Pattrick wrote:

> This patch attempts to fix a large number of ubsan error messages that
> take the following form:
>
> lib/netlink-notifier.c:237:13: runtime error: call to function
>     route_table_change through pointer to incorrect function type
>     'void (*)(const void *, void *)'
>
> In Clang 17 the undefined behaviour sanatizer check for function
> pointers was enabled by default, whereas it was previously disabled
> while compiling C code. These warnings are a false positive in the case
> of OVS, as our macros already check to make sure the function parameter
> is the correct size.
>
> So that check is disabled in the single function that is causing all of
> the errors.
>
> Signed-off-by: Mike Pattrick <mkp@redhat.com>

Thanks for fixing the naming.

Acked-by: Eelco Chaudron <echaudro@redhat.com>

On 5/17/24 14:59, Eelco Chaudron wrote:
> 
> 
> On 16 May 2024, at 15:57, Mike Pattrick wrote:
> 
>> This patch attempts to fix a large number of ubsan error messages that
>> take the following form:
>>
>> lib/netlink-notifier.c:237:13: runtime error: call to function
>>     route_table_change through pointer to incorrect function type
>>     'void (*)(const void *, void *)'
>>
>> In Clang 17 the undefined behaviour sanatizer check for function
>> pointers was enabled by default, whereas it was previously disabled
>> while compiling C code. These warnings are a false positive in the case
>> of OVS, as our macros already check to make sure the function parameter
>> is the correct size.
>>
>> So that check is disabled in the single function that is causing all of
>> the errors.
>>
>> Signed-off-by: Mike Pattrick <mkp@redhat.com>
> 
> Thanks for fixing the naming.
> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>


Thanks, Mike, Jakob and Eelco!

I fixed the comment spacing and moved the attribute to the same line
with a return type as we normally do.  With that, applied to all the
branches down to 2.17.

Best regards, Ilya Maximets.