Re: [lttng-dev] [PATCH] Avoid calling caa_container_of on NULL pointer in cds_lfhash macros

On 6/22/23 06:45, Ondřej Surý via lttng-dev wrote:
> (Sorry, I missed closing brackets in both macros, so resending fixed patch...)
> 
> The cds_lfht_for_each_entry and cds_lfht_for_each_entry_duplicate macros
> would call caa_container_of() macro on NULL pointer.  This is not a
> problem under normal circumstances as the check in the for loop fails
> and the loop-statement is not called with invalid (pos) value.
> 
> However AddressSanitizer doesn't like that and complains about this:
> 
>      runtime error: applying non-zero offset 18446744073709551056 to null pointer
> 
> Move the cds_lfht_iter_get_node(iter) != NULL from the cond-expression
> of the for loop into both init-clause and iteration-expression as
> conditional operator and check for (pos) value in the cond-expression
> instead.

I've taken the liberty to reimplement this with a new helper "cds_lfht_entry".

Can you review and try the following commits please ?

https://review.lttng.org/c/userspace-rcu/+/10445 compiler.h: Introduce caa_unqual_scalar_typeof
https://review.lttng.org/c/userspace-rcu/+/10446 Avoid calling caa_container_of on NULL pointer in cds_lfht macros

Thanks!

Mathieu

> 
> Signed-off-by: Ondřej Surý <[email protected]>
> ---
>   include/urcu/rculfhash.h | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/include/urcu/rculfhash.h b/include/urcu/rculfhash.h
> index fbd33cc..64cc18f 100644
> --- a/include/urcu/rculfhash.h
> +++ b/include/urcu/rculfhash.h
> @@ -546,22 +546,22 @@ void cds_lfht_resize(struct cds_lfht *ht, unsigned long new_size);
>   
>   #define cds_lfht_for_each_entry(ht, iter, pos, member)			\
>   	for (cds_lfht_first(ht, iter),					\
> -			pos = caa_container_of(cds_lfht_iter_get_node(iter), \
> -					__typeof__(*(pos)), member);	\
> -		cds_lfht_iter_get_node(iter) != NULL;			\
> +			pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
> +					__typeof__(*(pos)), member) : NULL);	\
> +		pos != NULL;			\
>   		cds_lfht_next(ht, iter),				\
> -			pos = caa_container_of(cds_lfht_iter_get_node(iter), \
> -					__typeof__(*(pos)), member))
> +			pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
> +					__typeof__(*(pos)), member) : NULL))
>   
>   #define cds_lfht_for_each_entry_duplicate(ht, hash, match, key,		\
>   				iter, pos, member)			\
>   	for (cds_lfht_lookup(ht, hash, match, key, iter),		\
> -			pos = caa_container_of(cds_lfht_iter_get_node(iter), \
> -					__typeof__(*(pos)), member);	\
> -		cds_lfht_iter_get_node(iter) != NULL;			\
> +			pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
> +					__typeof__(*(pos)), member) : NULL);	\
> +		pos != NULL;			\
>   		cds_lfht_next_duplicate(ht, match, key, iter),		\
> -			pos = caa_container_of(cds_lfht_iter_get_node(iter), \
> -					__typeof__(*(pos)), member))
> +			pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
> +					__typeof__(*(pos)), member) : NULL))
>   
>   #ifdef __cplusplus
>   }

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
[email protected]
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev