Hi, Tao
Thanks for the comments.
On Wed, Apr 3, 2024 at 11:45 AM Tao Liu <l...@redhat.com> wrote:
> Hi Lianbo,
>
> On Wed, Mar 6, 2024 at 2:32 PM Lianbo Jiang <liji...@redhat.com> wrote:
> >
> > Some objects format may potentially support copy relocations, but
> > currently the maybe_copied is always initialized to 0 in the symbol().
> > And the type is 'mst_file_bss', not always the 'mst_bss' or 'mst_data'
> > in the lookup_minimal_symbol_linkage(). For example:
> >
> > (gdb) p *msymbol
> > $42 = {<general_symbol_info> = {m_name = 0x349812f "test_no_static",
> value = {ivalue = 8, block = 0x8,
> > bytes = 0x8 <error: Cannot access memory at address 0x8>, address
> = 8, common_block = 0x8, chain = 0x8}, language_specific = {
> > obstack = 0x0, demangled_name = 0x0}, m_language = language_auto,
> ada_mangled = 0, section = 20}, size = 4,
> > filename = 0x6db3440 "test_sanity.c", type = mst_file_bss,
> created_by_gdb = 0, target_flag_1 = 0, target_flag_2 = 0, has_size = 1,
> > maybe_copied = 0, name_set = 1, hash_next = 0x0, demangled_hash_next =
> 0x0}
> >
> The whole bug is around test_no_static, which is a static variable in
> the ko. However the added code to lookup_minimal_symbol_linkage() is
> mst_file_bss/mst_file_data. So I guess test_no_static's symbol type
>
Just to be safe I added the type 'mst_file_data' check, because there might
be a similar situation, but I can not prove it for the time being. Do you
have any specific concerns about it?
should be one of the 2 right? If that is the case, I suggest printing
> "test_no_static" instead of "test_sanity.c" for "p *msymbol", so it
> can be clearer for the adding of mst_file_bss/mst_file_data.
> Personally I think the symbol "test_sanity.c" doesn't make sense here.
>
>
The "test_sanity.c" is a module name, and the "test_no_static" is a static
variable in this module.
I just printed the debug info to help understand the current issue. In
fact, the current issue happened in the productive environment, and the
test module "test_sanity.c" is only used to help reproduce this issue.
Hope this helps.
Thanks
Lianbo
Thanks,
> Tao Liu
>
> > This causes a problem that the 'p' command can not work well as
> > expected, and always gets an error:
> >
> > crash> mod -s test_sanity /home/test_sanity.ko
> > MODULE NAME BASE SIZE
> OBJECT FILE
> > ffffffffc1084040 test_sanity ffffffffc1082000 16384
> /home/test_sanity.ko
> > crash> p test_no_static
> > p: gdb request failed: p test_no_static
> > crash>
> >
> > With the patch:
> > crash> mod -s test_sanity /home/test_sanity.ko
> > MODULE NAME BASE SIZE
> OBJECT FILE
> > ffffffffc1084040 test_sanity ffffffffc1082000 16384
> /home/test_sanity.ko
> > crash> p test_no_static
> > test_no_static = $1 = 5
> > crash>
> >
> > Signed-off-by: Lianbo Jiang <liji...@redhat.com>
> > ---
> > Here is the test case:
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> >
> > int test_no = 0;
> > static int test_no_static = 0;
> >
> > static int test_init(void)
> > {
> > test_no += 5;
> > test_no_static += 5;
> > printk(KERN_INFO "%d static=%d\n", test_no, test_no_static);
> > return 0;
> > }
> > static void test_exit(void)
> >
> > {
> > test_no += 7;
> > test_no_static += 7;
> > printk(KERN_INFO "%d static=%d\n", test_no, test_no_static);
> > }
> > module_init(test_init);
> > module_exit(test_exit);
> > MODULE_LICENSE("GPL");
> >
> >
> > gdb-10.2.patch | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/gdb-10.2.patch b/gdb-10.2.patch
> > index a7018a249118..35388ba03e25 100644
> > --- a/gdb-10.2.patch
> > +++ b/gdb-10.2.patch
> > @@ -16057,3 +16057,27 @@ exit 0
> > m10200-dis.c
> > m10200-opc.c
> > m10300-dis.c
> > +--- gdb-10.2/gdb/minsyms.c.orig
> > ++++ gdb-10.2/gdb/minsyms.c
> > +@@ -535,7 +535,9 @@ lookup_minimal_symbol_linkage (const char *name,
> struct objfile *objf)
> > + {
> > + if (strcmp (msymbol->linkage_name (), name) == 0
> > + && (MSYMBOL_TYPE (msymbol) == mst_data
> > +- || MSYMBOL_TYPE (msymbol) == mst_bss))
> > ++ || MSYMBOL_TYPE (msymbol) == mst_bss
> > ++ || MSYMBOL_TYPE (msymbol) == mst_file_bss
> > ++ || MSYMBOL_TYPE (msymbol) == mst_file_data))
> > + return {msymbol, objfile};
> > + }
> > + }
> > +--- gdb-10.2/gdb/symtab.h.orig
> > ++++ gdb-10.2/gdb/symtab.h
> > +@@ -1110,7 +1110,7 @@ struct symbol : public general_symbol_info,
> public allocate_on_obstack
> > + is_objfile_owned (1),
> > + is_argument (0),
> > + is_inlined (0),
> > +- maybe_copied (0),
> > ++ maybe_copied (1),/* The objfile potentially supports copy
> relocations. */
> > + subclass (SYMBOL_NONE)
> > + {
> > + /* We can't use an initializer list for members of a base class,
> and
> > --
> > 2.41.0
> > --
> > Crash-utility mailing list -- devel@lists.crash-utility.osci.io
> > To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
> > https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
> > Contribution Guidelines: https://github.com/crash-utility/crash/wiki
>
>
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
