[Bug runtime/30415] New: conflicting types for ‘kallsyms_on_each_symbol’

[Bug runtime/30415] New: conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

            Bug ID: 30415
           Summary: conflicting types for ‘kallsyms_on_each_symbol’
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: runtime
          Assignee: systemtap at sourceware dot org
          Reporter: mcermak at redhat dot com
  Target Milestone: ---

Following is one of the problems CI gating shows for systemtap-4.9-1.fc39:

[root@rawh ~]# uname -r; rpm -q systemtap
6.4.0-0.rc0.20230428git33afd4b76393.7.fc39.x86_64
systemtap-4.9-1.fc39.x86_64
[root@rawh ~]# 
[root@rawh ~]# 
[root@rawh ~]# stap -vp4 -e 'probe nfsd.close? { printf("%s\n", filename) }
probe never { exit() }'
Pass 1: parsed user script and 484 library scripts using
124832virt/101128res/11136shr/89524data kb, in 180usr/40sys/219real ms.
Pass 2: analyzed script: 1 probe, 1 function, 0 embeds, 0 globals using
213068virt/193116res/14828shr/177760data kb, in 2040usr/430sys/2487real ms.
Pass 3: translated to C into
"/tmp/stapcVlKLM/stap_9691453a981c3fed6300ada89e578986_1302_src.c" using
213068virt/193244res/14956shr/177760data kb, in 0usr/0sys/0real ms.
In file included from /usr/local/share/systemtap/runtime/linux/runtime.h:288,
                 from /usr/local/share/systemtap/runtime/runtime.h:26,
                 from
/tmp/stapcVlKLM/stap_9691453a981c3fed6300ada89e578986_1302_src.c:21:
/usr/local/share/systemtap/runtime/sym.c:1159:5: error: conflicting types for
‘kallsyms_on_each_symbol’; have ‘int(int (*)(void *, const char *, struct
module *, long unsigned int), void *)’
 1159 | int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct
module *,
      |     ^~~~~~~~~~~~~~~~~~~~~~~
In file included from ./include/linux/ftrace.h:13,
                 from ./include/linux/kprobes.h:28,
                 from /usr/local/share/systemtap/runtime/linux/runtime.h:21:
./include/linux/kallsyms.h:70:5: note: previous declaration of
‘kallsyms_on_each_symbol’ with type ‘int(int (*)(void *, const char *, long
unsigned int), void *)’
   70 | int kallsyms_on_each_symbol(int (*fn)(void *, const char *, unsigned
long),
      |     ^~~~~~~~~~~~~~~~~~~~~~~
/usr/local/share/systemtap/runtime/sym.c: In function
‘kallsyms_on_each_symbol’:
/usr/local/share/systemtap/runtime/sym.c:1166:85: error: passing argument 1 of
‘(int (*)(int (*)(void *, const char *, long unsigned int), void
*))_stp_kallsyms_on_each_symbol’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
 1166 |                 return (*
(kallsyms_on_each_symbol_fn)_stp_kallsyms_on_each_symbol)(fn, data);
      |                                                                        
            ^~
      |                                                                        
            |
      |                                                                        
            int (*)(void *, const char *, struct module *, long unsigned int)
/usr/local/share/systemtap/runtime/sym.c:1166:85: note: expected ‘int (*)(void
*, const char *, long unsigned int)’ but argument is of type ‘int (*)(void *,
const char *, struct module *, long unsigned int)’
In file included from ./include/linux/kernel.h:30,
                 from ./arch/x86/include/asm/percpu.h:27,
                 from ./arch/x86/include/asm/preempt.h:6,
                 from ./include/linux/preempt.h:78,
                 from ./include/linux/spinlock.h:56,
                 from ./include/linux/mmzone.h:8,
                 from ./include/linux/gfp.h:7,
                 from
/usr/local/share/systemtap/runtime/linux/runtime_defines.h:20,
                 from /usr/local/share/systemtap/runtime/runtime_defines.h:8,
                 from
/tmp/stapcVlKLM/stap_9691453a981c3fed6300ada89e578986_1302_src.c:12:
/usr/local/share/systemtap/runtime/linux/print.c: In function
‘_stp_print_kernel_info’:
/usr/local/share/systemtap/runtime/linux/print.c:365:43: error: ‘struct module’
has no member named ‘module_core’
  365 |                (unsigned long) THIS_MODULE->module_core,
      |                                           ^~
./include/linux/printk.h:427:33: note: in definition of macro
‘printk_index_wrap’
  427 |                 _p_func(_fmt, ##__VA_ARGS__);                          
\
      |                                 ^~~~~~~~~~~
/usr/local/share/systemtap/runtime/linux/print.c:348:9: note: in expansion of
macro ‘printk’
  348 |         printk(KERN_DEBUG
      |         ^~~~~~
/usr/local/share/systemtap/runtime/linux/print.c:366:44: error: ‘struct module’
has no member named ‘core_size’
  366 |                (unsigned long) (THIS_MODULE->core_size -
THIS_MODULE->core_text_size)/1024,
      |                                            ^~
./include/linux/printk.h:427:33: note: in definition of macro
‘printk_index_wrap’
  427 |                 _p_func(_fmt, ##__VA_ARGS__);                          
\
      |                                 ^~~~~~~~~~~
/usr/local/share/systemtap/runtime/linux/print.c:348:9: note: in expansion of
macro ‘printk’
  348 |         printk(KERN_DEBUG
      |         ^~~~~~
/usr/local/share/systemtap/runtime/linux/print.c:366:71: error: ‘struct module’
has no member named ‘core_text_size’; did you mean ‘kprobes_text_size’?
  366 |                (unsigned long) (THIS_MODULE->core_size -
THIS_MODULE->core_text_size)/1024,
      |                                                                      
^~~~~~~~~~~~~~
./include/linux/printk.h:427:33: note: in definition of macro
‘printk_index_wrap’
  427 |                 _p_func(_fmt, ##__VA_ARGS__);                          
\
      |                                 ^~~~~~~~~~~
/usr/local/share/systemtap/runtime/linux/print.c:348:9: note: in expansion of
macro ‘printk’
  348 |         printk(KERN_DEBUG
      |         ^~~~~~
/usr/local/share/systemtap/runtime/linux/print.c:367:46: error: ‘struct module’
has no member named ‘core_text_size’; did you mean ‘kprobes_text_size’?
  367 |                (unsigned long) (THIS_MODULE->core_text_size)/1024,
      |                                              ^~~~~~~~~~~~~~
./include/linux/printk.h:427:33: note: in definition of macro
‘printk_index_wrap’
  427 |                 _p_func(_fmt, ##__VA_ARGS__);                          
\
      |                                 ^~~~~~~~~~~
/usr/local/share/systemtap/runtime/linux/print.c:348:9: note: in expansion of
macro ‘printk’
  348 |         printk(KERN_DEBUG
      |         ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [scripts/Makefile.build:252:
/tmp/stapcVlKLM/stap_9691453a981c3fed6300ada89e578986_1302_src.o] Error 1
make: *** [Makefile:2043: /tmp/stapcVlKLM] Error 2
WARNING: kbuild exited with status: 2
Pass 4: compiled C into "stap_9691453a981c3fed6300ada89e578986_1302.ko" in
20970usr/4470sys/13543real ms.
Pass 4: compilation failed.  [man error::pass4]
[root@rawh ~]#

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #1 from Martin Cermak <mcermak at redhat dot com> ---
The reproducer is even simpler:

[root@rawh ~]# uname -r; rpm -q systemtap
6.4.0-0.rc0.20230428git33afd4b76393.7.fc39.x86_64
systemtap-4.9-1.fc39.x86_64
[root@rawh ~]# 
[root@rawh ~]# stap -vp4 -e 'probe begin { exit() }'
Pass 1: parsed user script and 484 library scripts using
124832virt/101128res/11136shr/89524data kb, in 190usr/40sys/223real ms.
Pass 2: analyzed script: 1 probe, 1 function, 0 embeds, 0 globals using
126284virt/103048res/11520shr/90976data kb, in 0usr/0sys/8real ms.
Pass 3: translated to C into
"/tmp/stapLy8yoU/stap_6881d4c66122cefc1b5d5c230a431144_1182_src.c" using
126284virt/103432res/11904shr/90976data kb, in 0usr/0sys/0real ms.
In file included from /usr/local/share/systemtap/runtime/linux/runtime.h:288,
                 from /usr/local/share/systemtap/runtime/runtime.h:26,
                 from
/tmp/stapLy8yoU/stap_6881d4c66122cefc1b5d5c230a431144_1182_src.c:21:
/usr/local/share/systemtap/runtime/sym.c:1159:5: error: conflicting types for
‘kallsyms_on_each_symbol’;

[ ... stuff deleted ... ]

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

Martin Cermak <mcermak at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |serhei at serhei dot io

--- Comment #2 from Martin Cermak <mcermak at redhat dot com> ---
Seems related to systemtap commit 1eaeb16dfa0c59e67eda8e84b72c7cee6efdeac2 /
PR26074 .

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

William Cohen <wcohen at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wcohen at redhat dot com

--- Comment #3 from William Cohen <wcohen at redhat dot com> ---
This issue looks to be related to the upstream change in the kernel for
kallsyms_on_each_symbol:

commit 3703bd54cd37e7875f51ece8df8c85c184e40bba

Author: Zhen Lei <thunder.leizhen@huawei.com>  2023-03-08 02:38:46
Committer: Luis Chamberlain <mcgrof@kernel.org>  2023-03-19 16:27:19
Parent: 7ce93729091dff24e6a1c3578b58b36f4b1cf10a (dyndbg: cleanup dynamic usage
in ib_srp.c)
Child:  3c17655ab13704582fe25e8ea3200a9b2f8bf20a (module/decompress: Never use
kunmap() for local un-mappings)
Branches: master, remotes/origin/master, remotes/origin/x86-uaccess-cleanup
Follows: v6.3-rc1
Precedes: v6.4-rc1

    kallsyms: Delete an unused parameter related to
{module_}kallsyms_on_each_symbol()

    The parameter 'struct module *' in the hook function associated with
    {module_}kallsyms_on_each_symbol() is no longer used. Delete it.

    Suggested-by: Petr Mladek <pmladek@suse.com>
    Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
    Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
    Acked-by: Jiri Olsa <jolsa@kernel.org>
    Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
    Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>


new kernels have in include/linux/kallsyms.h:

int kallsyms_on_each_symbol(int (*fn)(void *, const char *, unsigned long),
                            void *data);


old kernels (<6.4) in include/linux/kallsyms.h:

int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
                                      unsigned long),
                            void *data);

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #4 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 14899
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14899&action=edit
possible patch

Thank-you for the analysis!

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #5 from William Cohen <wcohen at redhat dot com> ---
Hi, Martin.

I originally tried something similar to that.  However, the
stapkp_symbol_callback function in the runtime actually uses the struct module
argument when the kallsyms_on_each_symbol call back function is invoked.  Just
removing the struct module arg from stapkp_symbol_callback_function doesn't
look to be an option.  Changing to systemtap runtime replacement
kallsyms_on_each_function ends up with pretty much all the tests failing to
compile when "make installcheck" is run with errors like the following in the
systemtap.log:

TEST
PWD=/home/wcohen/systemtap_write/systemtap/testsuite/systemtap.examples/general
meta taglines 'test_check: stap -p4 alias_suffixes.stp' tag 'test_check' value
'stap -p4 alias_suffixes.stp'
attempting command stap -p4 alias_suffixes.stp
OUT In file included from
/tmp/stapeFLono/stap_ca5fcb74ddbf75aa56734f2ec18887bc_85967_src.c:6192:
/home/wcohen/systemtap_write/install/share/systemtap/runtime/linux/kprobes.c:
In function 'stapkp_init':
/home/wcohen/systemtap_write/install/share/systemtap/runtime/linux/kprobes.c:766:32:
error: passing argument 1 of 'kallsyms_on_each_symbol' from incompatible
pointer type [-Werror=incompatible-pointer-types]
  766 |        kallsyms_on_each_symbol(stapkp_symbol_callback, &sd);
      |                                ^~~~~~~~~~~~~~~~~~~~~~
      |                                |
      |                                int (*)(void *, const char *, struct
module *, long unsigned int)
In file included from
/home/wcohen/systemtap_write/install/share/systemtap/runtime/linux/runtime.h:293,
                 from
/home/wcohen/systemtap_write/install/share/systemtap/runtime/runtime.h:26,
                 from
/tmp/stapeFLono/stap_ca5fcb74ddbf75aa56734f2ec18887bc_85967_src.c:21:
/home/wcohen/systemtap_write/install/share/systemtap/runtime/sym.c:1159:35:
note: expected 'int (*)(void *, const char *, long unsigned int)' but argument
is of type 'int (*)(void *, const char *, struct module *, long unsigned int)'
 1159 | int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
      |                             ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
 1160 |                                       unsigned long),
      |                                       ~~~~~~~~~~~~~~
/home/wcohen/systemtap_write/install/share/systemtap/runtime/linux/kprobes.c:
In function 'stapkp_refresh':
/home/wcohen/systemtap_write/install/share/systemtap/runtime/linux/kprobes.c:836:34:
error: passing argument 1 of 'kallsyms_on_each_symbol' from incompatible
pointer type [-Werror=incompatible-pointer-types]
  836 |          kallsyms_on_each_symbol(stapkp_symbol_callback, &sd);
      |                                  ^~~~~~~~~~~~~~~~~~~~~~
      |                                  |
      |                                  int (*)(void *, const char *, struct
module *, long unsigned int)
/home/wcohen/systemtap_write/install/share/systemtap/runtime/sym.c:1159:35:
note: expected 'int (*)(void *, const char *, long unsigned int)' but argument
is of type 'int (*)(void *, const char *, struct module *, long unsigned int)'
 1159 | int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
      |                             ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
 1160 |                                       unsigned long),
      |                                       ~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:252:
/tmp/stapeFLono/stap_ca5fcb74ddbf75aa56734f2ec18887bc_85967_src.o] Error 1
make[3]: *** [Makefile:2044: /tmp/stapeFLono] Error 2
WARNING: kbuild exited with status: 2
Pass 4: compilation failed.  [man error::pass4]
child process exited abnormally
RC 1
FAIL: systemtap.examples/general/alias_suffixes build

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #6 from William Cohen <wcohen at redhat dot com> ---
Did some kernel archeology.  There are two kallsyms_on_each_symbol functions:
the kallsyms_on_each_symbol and another function
module_kallsyms_on_each_symbol.  The initial kernel commit
75a66614db21007bcc8c37f9c5d5b922981387b9  has the kallsyms_on_each_symbol
iterate through the kernel symbols if there was no non-zero return from he
callback the module_kallsyms_on_each_symbol was run.  The
module_kallsyms_on_each_symbol would invoke the callback for each symbol in
each module.  There have been changes on the module_kallsyms_on_each_module as
now it takes an argument on which module's symbols to apply the callback to
(NULL could be passed in to get it to apply to all the modules).  The module
argument was added by kernel git commit
07cc2c931e8e1083a31f4c51d2244fe264af63bf in Jan 2023.


The question is how to get stapkp_symbol_callback working without the struct
module argument.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #7 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 14905
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14905&action=edit
possible patch

I'm wondering if something as simple as the attached patch would work.  My
impression is that with this, either stapkp_symbol_callback() runs as a
callback for a module symbol, and then module is known.  Or it runs as a
callback for a kernel symbol, the module isn't known, but even in this case the
if-else branching in the function should proceed meaningfully (?).

However, when testing this I'm getting following warnings when trying to
compile alias_suffixes.stp, and I'm not sure if it's related and what to do
about it (if anything at all):

39 x86_64 # stap --poison-cache -p4
/usr/local/share/systemtap/examples/general/alias_suffixes.stp
/tmp/stapBnU41h/stap_12c1257a7321a3bb17fbe980edd13be4_84218.o: warning:
objtool: _stp_vsprint_memory+0x1e9: call to __get_user_nocheck_1() with UACCESS
enabled
/tmp/stapBnU41h/stap_12c1257a7321a3bb17fbe980edd13be4_84218.o: warning:
objtool: .altinstr_replacement+0x1d: recursive UACCESS enable
/tmp/stapBnU41h/stap_12c1257a7321a3bb17fbe980edd13be4_84218.o: warning:
objtool: probe_6413+0x3a7: call to __get_user_nocheck_1() with UACCESS enabled
/root/.systemtap/cache/12/stap_12c1257a7321a3bb17fbe980edd13be4_84218.ko
39 x86_64 #

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

Martin Cermak <mcermak at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #14905|0                           |1
        is obsolete|                            |

--- Comment #8 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 14906
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14906&action=edit
possible patch

Fix a typo.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #9 from William Cohen <wcohen at redhat dot com> ---
Hi Martin,

The kallsyms_on_each_symbol in runtime/sym.c is acting a wrapper for the
non-export kernel kallsyms_on_each_symbol. 

In older linux 5.11 and older versions of the kernel's kallsyms_on_each_symbol
the code iterates through the kernel's symbols.  If none of the callback
functions return a non zero value, then then module_kallsyms_on_each_symbol is
called: https://elixir.bootlin.com/linux/v4.20.17/source/kernel/kallsyms.c#L178
.  However, in linux 5.12 kernels and newer
https://elixir.bootlin.com/linux/v5.12/source/kernel/kallsyms.c#L185 the
module_kallsyms_on_each_symbol at the end of function has been removed.  The
was changed by kernel git commit 013c1667cf78c1d847152f7116436d82dcab3db4.  

The module_kallsyms_on_each_symbol callback includes the struct module
argument,
https://elixir.bootlin.com/linux/v5.11.22/source/kernel/module.c#L4499 . 
Looking at how these functions are used in live patching.  The live patching
makes a distinction between the kernel and modules.

It looks like need to have two callback functions: one for kernels and one for
modules, but might be able to make the kernel just a wrapper to the module
callback, the existing stapkp_symbol_callback.  Then would need code to find
the module_kallsyms_on_each_function and have two passes: one for the kernel
and one for the modules. 

One other concern about the relocation mechanism is how to make sure that
getting the correct symbol as names may not be unique.  There can be multiple
symbols with the same name in kallsym output. This can be observed with the
following :

sudo cat /proc/kallsyms | awk '{print $3}' |sort |uniq -c |grep -v " 1 " |sort
-nr | more

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #10 from Martin Cermak <mcermak at redhat dot com> ---
Gotcha (probably ;) so since module_kallsyms_on_each_symbol() isn't exported
just like kallsyms_on_each_symbol() isn't, and since starting from kernel
commit 013c1667cf78c1d847152f7116436d82dcab3db4
module_kallsyms_on_each_symbol() isn't called from kallsyms_on_each_symbol(),
we basically need to duplicate most of the kallsyms_on_each_symbol() related
logic (mostly in runtime/ and staprun/) in order to tap
module_kallsyms_on_each_symbol() just like kallsyms_on_each_symbol() is tapped
(via send_a_relocation() in staprun)..

Not sure yet what to do about the non-uniqueness of the symbols, but I assume
this is not a new problem, so something in the existing code needs to already
address this to some extent, and the new code can follow that model..

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #11 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 14909
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14909&action=edit
possible patch

This patch *seems* to work across my rawhide/el9/el8 test systems.  Any
thoughts?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #12 from William Cohen <wcohen at redhat dot com> ---
Hi, I have been trying out the patch.  It definitely improves the situation on
rawhide.  However, I noticed when running the various examples testcases on
RHEL8 x86_64 there were additional failures:

 sudo make installcheck RUNTESTFLAGS="--debug systemtap.examples/check.exp"

Without the patch:


                === systemtap Summary ===

# of expected passes            408
# of unexpected failures        8
# of untested testcases         12

With the latest proposed patch:


                === systemtap Summary ===

# of expected passes            317
# of unexpected failures        99
# of untested testcases         12


Looking through the systemtap.log of the patched build see that not finding the
module_kallsyms_on_each_symbol:

PASS: systemtap.examples/general/also_ran build
meta taglines 'test_installcheck: stap also_ran.stp -T 1' tag
'test_installcheck' value 'stap also_ran.stp -T 1'
attempting command stap also_ran.stp -T 1
OUT WARNING: "module_kallsyms_on_each_symbol"
[/tmp/stapXYrv8r/stap_cdac50d140cbc59757fc1b2df270d7f7_71121.ko] undefined!
ERROR: Couldn't insert module
'/tmp/stapXYrv8r/stap_cdac50d140cbc59757fc1b2df270d7f7_71121.ko': Unknown
symbol in module
WARNING: /home/wcohen/systemtap_write/install/bin/staprun exited with status: 1
Pass 5: run failed.  [man error::pass5]
child process exited abnormally
RC 1
FAIL: systemtap.examples/general/also_ran run

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #13 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 14912
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14912&action=edit
possible patch

Thanks a ton for looking into this, Will.

I've forgotten to take into account Comment#9, which says that starting from
kernel commit 013c1667cf78 on, the module_kallsyms_on_each_symbol() isn't
called from kallsyms_on_each_symbol(). So, looking at the kernel git tags:

$ git log -1000000  --oneline --decorate |& fgrep -e '(tag:' -e 013c1667cf |
fgrep -C3 013c1667cf
a38fd8748464 (tag: v5.12-rc2) Linux 5.12-rc2
fe07bfda2fb9 (tag: v5.12-rc1-dontuse) Linux 5.12-rc1
f40ddce88593 (tag: v5.11) Linux 5.11
013c1667cf78 kallsyms: refactor {,module_}kallsyms_on_each_symbol
92bf22614b21 (tag: v5.11-rc7) Linux 5.11-rc7
1048ba83fb1c (tag: v5.11-rc6) Linux 5.11-rc6
6ee1d745b7c9 (tag: v5.11-rc5) Linux 5.11-rc5
$

This seems to mean that some of the 5.11 kernel versions did have this commit
and some didn't. So I think it is safe to expect that kernel versions 5.12 and
higher have that commit. I've added KERNEL_VERSION() gates to reflect this, and
it seems to solve the problem.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #14 from William Cohen <wcohen at redhat dot com> ---
Hi Martin,
The patch is looking pretty good.  There are just a few minor comments I have
on it.

One concern have is if RHEL backport some of the live patching support
to RHEL kernel the kernel version checks might not catch that.
However, probably better to just use the kernel version as currently
done and fix the problem with autoconf check if/when RHEL backports
patches.


Is there a need for both stapkp_kernel_symbol_callback and
stapkp_module_symbol_callback?  in kprobes.c?  They look the same with
the exception of the function name.  It would simplfy the code if
those were combined into a single function.


In runtime/linux/kprobe.c have:

extern void *_stp_module_kallsyms_on_each_symbol;

However, its static in runtime/linux/runtime.h:

static void *_stp_module_kallsyms_on_each_symbol;

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #15 from Martin Cermak <mcermak at redhat dot com> ---
Created attachment 14913
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14913&action=edit
possible patch

(In reply to William Cohen from comment #14)
> Is there a need for both stapkp_kernel_symbol_callback and
> stapkp_module_symbol_callback?  in kprobes.c?  They look the same with
> the exception of the function name.  It would simplfy the code if
> those were combined into a single function.

Absolutely, I've combined them into one single stapkp_symbol_callback()

> In runtime/linux/kprobe.c have:
> 
> extern void *_stp_module_kallsyms_on_each_symbol;
> 
> However, its static in runtime/linux/runtime.h:
> 
> static void *_stp_module_kallsyms_on_each_symbol;

I've dropped the 'static' access control constraint in the header.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[wcohen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

--- Comment #16 from William Cohen <wcohen at redhat dot com> ---
I took a look at the patch attached 2023-06-02 and tried it out on rawhide,
rhel8, and rhel9.  It looks good to me.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug runtime/30415] conflicting types for ‘kallsyms_on_each_symbol’

[mcermak at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30415

Martin Cermak <mcermak at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #17 from Martin Cermak <mcermak at redhat dot com> ---
Thank-you for your reviews and feedback!

Fixed in commit 33fae2d01 .

-- 
You are receiving this mail because:
You are the assignee for the bug.