On 03/02/18 01:20, Geert Uytterhoeven wrote:
> Hi Frank,
>
> On Fri, Mar 2, 2018 at 2:51 AM, <frowand.list@xxxxxxxxx> wrote:
>> There are still some functions in unittest.c that should be tagged
>> __init due to changes in this patch, but modpost is not warning of
>> them and they are not a risk because they are only called from
>> __init functions. A sweep of unittest.c for functions that
>> should be tagged __init is on the todo list.
>
> If modpost doesn't warn, that merely means your compiler decided to
> inline all functions with wrong annotations, hiding the problem.
> Other (versions of) compilers may behave differently, so we do want
> to get this right.
>
> With my trusty gcc-4.1.2:
>
> WARNING: vmlinux.o(.text+0x342dd4): Section mismatch in reference
> from the function of_unittest_apply_revert_overlay_check() to the
> function .init.text:of_unittest_apply_overlay()
> The function of_unittest_apply_revert_overlay_check() references
> the function __init of_unittest_apply_overlay().
> This is often because of_unittest_apply_revert_overlay_check lacks a __init
> annotation or the annotation of of_unittest_apply_overlay is wrong.
>
> To fix the above:
>
> -static int of_unittest_apply_revert_overlay_check(int overlay_nr,
> +static int __init of_unittest_apply_revert_overlay_check(int overlay_nr,
> -static void of_unittest_overlay_5(void)
> +static void __init of_unittest_overlay_5(void)
> -static void of_unittest_overlay_11(void)
> +static void __init of_unittest_overlay_11(void)
Yes, that is exactly the extra set of functions I was talking about. Even
though I would prefer to annotate them, in practice they will not be a
problem because they only get called from __init functions (either directly
or indirectly). But if Rob will take a patch with them annotated, I will
spin the series.
>> --- a/drivers/of/unittest.c
>> +++ b/drivers/of/unittest.c
>
>> @@ -2290,18 +2275,29 @@ static __init void of_unittest_overlay_high_level(void)
>> __of_attach_node_sysfs(np);
>>
>> if (of_symbols) {
>> + struct property *new_prop;
>> for_each_property_of_node(overlay_base_symbols, prop) {
>
> drivers/of/unittest.c: In function ‘of_unittest_overlay_high_level’:
> drivers/of/unittest.c:2193: warning: ‘overlay_base_symbols’ may be
> used uninitialized in this function
>
> This isn't a new warning, so I guess I never reported it before because I
> thought it was a false positive (misguided by the "if (of_symbols)" test?).
>
> However, now I believe it is not, and an uninitialized pointer will be
> dereferenced if of_root has a __symbols__ node, but overlay_base_root hasn't.
Yes, thanks for reporting it. My gcc isn't this smart.
Fortunately overlay_base_root does have a __symbols__ node. But I will fix it
in a patch outside this series.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
