of: Build warn for missing fn() in _OF_DECLARE

[PATCH] of: Build warn for missing fn() in _OF_DECLARE

Posted by Liya Huang 8 months ago

The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
function pointers are ignored.

This patch introduces a check to handle cases where fn() is NULL. If fn()
is found to be NULL, a warning is issued during compilation to notify
developers about the missing function pointer.

---
The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
function pointers are ignored.

This patch introduces a check to handle cases where fn() is NULL. If fn()
is found to be NULL, a warning is issued during compilation to notify 
developers about the missing function pointer.

Link: https://lore.kernel.org/all/CAL_JsqK733Q9bbxC0Wz5uxyZ9m7bs+bci5kUJF9GJMv73-dO4w@mail.gmail.com/

Signed-off-by: Liya Huang <1425075683@qq.com>
---
 include/linux/of.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index a62154aeda1b6a600c2b155ac486c0e0b56e0bf2..99d1d553e65b7970a3ecb5158774ca5185f297a0 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1523,6 +1523,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
 }
 
 #define _OF_DECLARE_STUB(table, name, compat, fn, fn_type)		\
+	static_assert((fn) != NULL);	\
 	static const struct of_device_id __of_table_##name		\
 		__attribute__((unused))					\
 		 = { .compatible = compat,				\
@@ -1530,6 +1531,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
 
 #if defined(CONFIG_OF) && !defined(MODULE)
 #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
+	static_assert((fn) != NULL);	\
 	static const struct of_device_id __of_table_##name		\
 		__used __section("__" #table "_of_table")		\
 		__aligned(__alignof__(struct of_device_id))		\

---
base-commit: 8ffd015db85fea3e15a77027fda6c02ced4d2444
change-id: 20250417-_of_declare-ac80b59d952d

Best regards,
-- 
Liya Huang <1425075683@qq.com>

Re: [PATCH] of: Build warn for missing fn() in _OF_DECLARE

Posted by Robin Murphy 7 months, 3 weeks ago

On 2025-04-17 2:23 pm, Liya Huang wrote:
> The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
> in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
> function pointers are ignored.
> 
> This patch introduces a check to handle cases where fn() is NULL. If fn()
> is found to be NULL, a warning is issued during compilation to notify
> developers about the missing function pointer.
> 
> ---
> The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
> in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
> function pointers are ignored.
> 
> This patch introduces a check to handle cases where fn() is NULL. If fn()
> is found to be NULL, a warning is issued during compilation to notify
> developers about the missing function pointer.

This patch in -next appears to be responsible for syzbot complaining 
about build errors for some configs:

"
kernel/dma/coherent.c:410:1: error: static assertion expression is not 
an integral constant expression
kernel/dma/contiguous.c:497:1: error: static assertion expression is not 
an integral constant expression
"

https://lore.kernel.org/linux-iommu/6808d00a.050a0220.7184a.0010.GAE@google.com/

Also on closer inspection, just outside the diff context we still seem 
to be explicitly anticipating fn being NULL with:

	.data = (fn == (fn_type)NULL) ? fn : fn

so something doesn't seem quite right...

Thanks,
Robin.

> Link: https://lore.kernel.org/all/CAL_JsqK733Q9bbxC0Wz5uxyZ9m7bs+bci5kUJF9GJMv73-dO4w@mail.gmail.com/
> 
> Signed-off-by: Liya Huang <1425075683@qq.com>
> ---
>   include/linux/of.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index a62154aeda1b6a600c2b155ac486c0e0b56e0bf2..99d1d553e65b7970a3ecb5158774ca5185f297a0 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1523,6 +1523,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
>   }
>   
>   #define _OF_DECLARE_STUB(table, name, compat, fn, fn_type)		\
> +	static_assert((fn) != NULL);	\
>   	static const struct of_device_id __of_table_##name		\
>   		__attribute__((unused))					\
>   		 = { .compatible = compat,				\
> @@ -1530,6 +1531,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
>   
>   #if defined(CONFIG_OF) && !defined(MODULE)
>   #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
> +	static_assert((fn) != NULL);	\
>   	static const struct of_device_id __of_table_##name		\
>   		__used __section("__" #table "_of_table")		\
>   		__aligned(__alignof__(struct of_device_id))		\
> 
> ---
> base-commit: 8ffd015db85fea3e15a77027fda6c02ced4d2444
> change-id: 20250417-_of_declare-ac80b59d952d
> 
> Best regards,

Re: [PATCH] of: Build warn for missing fn() in _OF_DECLARE

Posted by Rob Herring 7 months, 3 weeks ago

On Wed, Apr 23, 2025 at 7:18 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2025-04-17 2:23 pm, Liya Huang wrote:
> > The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
> > in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
> > function pointers are ignored.
> >
> > This patch introduces a check to handle cases where fn() is NULL. If fn()
> > is found to be NULL, a warning is issued during compilation to notify
> > developers about the missing function pointer.
> >
> > ---
> > The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
> > in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
> > function pointers are ignored.
> >
> > This patch introduces a check to handle cases where fn() is NULL. If fn()
> > is found to be NULL, a warning is issued during compilation to notify
> > developers about the missing function pointer.
>
> This patch in -next appears to be responsible for syzbot complaining
> about build errors for some configs:
>
> "
> kernel/dma/coherent.c:410:1: error: static assertion expression is not
> an integral constant expression
> kernel/dma/contiguous.c:497:1: error: static assertion expression is not
> an integral constant expression
> "
>
> https://lore.kernel.org/linux-iommu/6808d00a.050a0220.7184a.0010.GAE@google.com/

Humm, doesn't seem to repro for me with clang-19.

>
> Also on closer inspection, just outside the diff context we still seem
> to be explicitly anticipating fn being NULL with:
>
>         .data = (fn == (fn_type)NULL) ? fn : fn

No, that is checking that the function parameters match the defined
type. If fn's type doesn't match fn_type, then you get a compiler
error.

Anyway, dropping it for now.

Rob

Re: [PATCH] of: Build warn for missing fn() in _OF_DECLARE

Posted by 1425075683@qq.com 7 months, 3 weeks ago

>On 2025-04-17 2:23 pm, Liya Huang wrote:
>> The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
>> in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
>> function pointers are ignored.
>> 
>> This patch introduces a check to handle cases where fn() is NULL. If fn()
>> is found to be NULL, a warning is issued during compilation to notify
>> developers about the missing function pointer.
>> 
>> ---
>> The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
>> in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
>> function pointers are ignored.
>> 
>> This patch introduces a check to handle cases where fn() is NULL. If fn()
>> is found to be NULL, a warning is issued during compilation to notify
>> developers about the missing function pointer.
>
>This patch in -next appears to be responsible for syzbot complaining 
>about build errors for some configs:
>
>"
>kernel/dma/coherent.c:410:1: error: static assertion expression is not 
>an integral constant expression
>kernel/dma/contiguous.c:497:1: error: static assertion expression is not 
>an integral constant expression
>"
>
>https://lore.kernel.org/linux-iommu/6808d00a.050a0220.7184a.0010.GAE@google.com/
>
>Also on closer inspection, just outside the diff context we still seem 
>to be explicitly anticipating fn being NULL with:
>
>	.data = (fn == (fn_type)NULL) ? fn : fn
>
>so something doesn't seem quite right...
>
>Thanks,
>Robin.

I tested this patch, and it compiled successfully with GCC but failed with
Clang.I couldn't find a better way to consistently check for null function
pointers during compilation across these two compilers.
I even tried using the method of preventing negative array indexing, but 
it failed to compile with GCC instead.
Perhaps it would be better to abandon this patch. :(

--
Thanks, 
Liya Huang <1425075683@qq.com>

Re: [PATCH] of: Build warn for missing fn() in _OF_DECLARE

Posted by Rob Herring (Arm) 7 months, 3 weeks ago

On Thu, 17 Apr 2025 21:23:38 +0800, Liya Huang wrote:
> The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
> in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
> function pointers are ignored.
> 
> This patch introduces a check to handle cases where fn() is NULL. If fn()
> is found to be NULL, a warning is issued during compilation to notify
> developers about the missing function pointer.
> 
> ---
> The function pointer fn() in _OF_DECLARE macro might be NULL. For example,
> in __reserved_mem_init_node(), only non-NULL cases are handled, and NULL
> function pointers are ignored.
> 
> This patch introduces a check to handle cases where fn() is NULL. If fn()
> is found to be NULL, a warning is issued during compilation to notify
> developers about the missing function pointer.
> 
> Link: https://lore.kernel.org/all/CAL_JsqK733Q9bbxC0Wz5uxyZ9m7bs+bci5kUJF9GJMv73-dO4w@mail.gmail.com/
> 
> Signed-off-by: Liya Huang <1425075683@qq.com>
> ---
>  include/linux/of.h | 2 ++
>  1 file changed, 2 insertions(+)
> 

Applied, thanks!