[PATCH] sym.c: fix module_kallsyms_on_each_symbol not exported

[PATCH] sym.c: fix module_kallsyms_on_each_symbol not exported

[tonyj]

module_kallsyms_on_each_symbol has never been exported (as noted by
33fae2d0107f ("This one seems simply like a non-export").  

If kallsyms_on_each_symbol is exported (a revert of upstream in our 
SLES case) then '!defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)' 
is false and the local definition of module_kallsyms_on_each_symbol is 
skipped.

This results in:
ERROR: modpost: "module_kallsyms_on_each_symbol" undefined!

Fixes: 33fae2d0107fb6166b4eac3fdffd277829849ab0
Signed-off-by: Tony Jones <tonyj@suse.de>

diff --git a/runtime/sym.c b/runtime/sym.c
index 23dd3be30..102257965 100644
--- a/runtime/sym.c
+++ b/runtime/sym.c
@@ -1180,7 +1180,8 @@ unsigned long kallsyms_lookup_name (const char *name)
 }
 #endif
 
-#if defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL) && !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
+#if defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL)
+#if !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
 #ifndef KALLSYMS_H_INCLUDED
 #include <linux/kallsyms.h>
 #endif
@@ -1209,11 +1210,12 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
         _stp_error("BUG: attempting to use unavailable kallsyms_on_each_symbol!!\n");
         return 0;
 }
+#endif // !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
 
+// XXX module_kallsyms_on_each_symbol has never been exported
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(5,12,0)
 typedef typeof(&module_kallsyms_on_each_symbol) module_kallsyms_on_each_symbol_fn;
 
-// XXX Will be linked in place of the kernel's module_kallsyms_on_each_symbol:
 #if defined(STAPCONF_KALLSYMS_6_4)
 int module_kallsyms_on_each_symbol(const char *modname,
                                    int (*fn)(void *, const char *,

Re: [PATCH] sym.c: fix module_kallsyms_on_each_symbol not exported

[William Cohen]

Hi,
I plan on taking a look at this patch Thursday.  Do you have a pointer to the environment (distribution, specific kernel) where systemtap failed WITHOUT this patch? Thanks,

-Will
On 6/18/24 12:52, tonyj@suse.de wrote:
> module_kallsyms_on_each_symbol has never been exported (as noted by
> 33fae2d0107f ("This one seems simply like a non-export").  
> 
> If kallsyms_on_each_symbol is exported (a revert of upstream in our 
> SLES case) then '!defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)' 
> is false and the local definition of module_kallsyms_on_each_symbol is 
> skipped.
> 
> This results in:
> ERROR: modpost: "module_kallsyms_on_each_symbol" undefined!
> 
> Fixes: 33fae2d0107fb6166b4eac3fdffd277829849ab0
> Signed-off-by: Tony Jones <tonyj@suse.de>
> 
> diff --git a/runtime/sym.c b/runtime/sym.c
> index 23dd3be30..102257965 100644
> --- a/runtime/sym.c
> +++ b/runtime/sym.c
> @@ -1180,7 +1180,8 @@ unsigned long kallsyms_lookup_name (const char *name)
>  }
>  #endif
>  
> -#if defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL) && !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
> +#if defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL)
> +#if !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
>  #ifndef KALLSYMS_H_INCLUDED
>  #include <linux/kallsyms.h>
>  #endif
> @@ -1209,11 +1210,12 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
>          _stp_error("BUG: attempting to use unavailable kallsyms_on_each_symbol!!\n");
>          return 0;
>  }
> +#endif // !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
>  
> +// XXX module_kallsyms_on_each_symbol has never been exported
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(5,12,0)
>  typedef typeof(&module_kallsyms_on_each_symbol) module_kallsyms_on_each_symbol_fn;
>  
> -// XXX Will be linked in place of the kernel's module_kallsyms_on_each_symbol:
>  #if defined(STAPCONF_KALLSYMS_6_4)
>  int module_kallsyms_on_each_symbol(const char *modname,
>                                     int (*fn)(void *, const char *,
> 

Re: [PATCH] sym.c: fix module_kallsyms_on_each_symbol not exported

[tonyj]

On Wed, Jun 19, 2024 at 09:37:58PM -0400, William Cohen wrote:
> Hi,
> I plan on taking a look at this patch Thursday.  Do you have a pointer to the environment (distribution, specific kernel) where systemtap failed WITHOUT this patch? Thanks,

Hi. 

In SLE15SP5/SP6 we historically have reverted upstream 0bd476e6c671
(see: https://github.com/SUSE/kernel-source/commit/d62679f1948

It only started showing up in SP6 because we updated to stap 5.0 and 
so picked up stap commit 33fae2d0107f (tags/release-5.0a~94).

I suspect the issue would show on any upstream kernel prior to 
0bd476e6c671 (tags/v5.7-rc1~58^2~15)

The issue is fairly simple.  module_kallsyms_on_each_symbol has never
been exported so it can't be guarded by STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED

Thanks

Tony


> 
> -Will
> On 6/18/24 12:52, tonyj@suse.de wrote:
> > module_kallsyms_on_each_symbol has never been exported (as noted by
> > 33fae2d0107f ("This one seems simply like a non-export").  
> > 
> > If kallsyms_on_each_symbol is exported (a revert of upstream in our 
> > SLES case) then '!defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)' 
> > is false and the local definition of module_kallsyms_on_each_symbol is 
> > skipped.
> > 
> > This results in:
> > ERROR: modpost: "module_kallsyms_on_each_symbol" undefined!
> > 
> > Fixes: 33fae2d0107fb6166b4eac3fdffd277829849ab0
> > Signed-off-by: Tony Jones <tonyj@suse.de>
> > 
> > diff --git a/runtime/sym.c b/runtime/sym.c
> > index 23dd3be30..102257965 100644
> > --- a/runtime/sym.c
> > +++ b/runtime/sym.c
> > @@ -1180,7 +1180,8 @@ unsigned long kallsyms_lookup_name (const char *name)
> >  }
> >  #endif
> >  
> > -#if defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL) && !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
> > +#if defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL)
> > +#if !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
> >  #ifndef KALLSYMS_H_INCLUDED
> >  #include <linux/kallsyms.h>
> >  #endif
> > @@ -1209,11 +1210,12 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
> >          _stp_error("BUG: attempting to use unavailable kallsyms_on_each_symbol!!\n");
> >          return 0;
> >  }
> > +#endif // !defined(STAPCONF_KALLSYMS_ON_EACH_SYMBOL_EXPORTED)
> >  
> > +// XXX module_kallsyms_on_each_symbol has never been exported
> >  #if LINUX_VERSION_CODE >= KERNEL_VERSION(5,12,0)
> >  typedef typeof(&module_kallsyms_on_each_symbol) module_kallsyms_on_each_symbol_fn;
> >  
> > -// XXX Will be linked in place of the kernel's module_kallsyms_on_each_symbol:
> >  #if defined(STAPCONF_KALLSYMS_6_4)
> >  int module_kallsyms_on_each_symbol(const char *modname,
> >                                     int (*fn)(void *, const char *,
> > 
> 

-- 
Tony Jones
SUSE Kernel Performance Team