[Crash-utility] Re: [PATCH v2] symbols: expand all kernel module symtable if not all expanded previously
Lianbo,
My apologies, I mixed up your ack for the other one with for this one...
Please check this later just in case.
https://github.com/crash-utility/crash/commit/f2ee6fa6c841ddc37ba665909da...
Thanks,
Kazu
On 2023/11/20 15:30, Tao Liu wrote:
> Hi Kazu,
>
> On Mon, Nov 20, 2023 at 2:16 PM HAGIO KAZUHITO(萩尾 一仁)
> <k-hagio-ab(a)nec.com> wrote:
>>
>> On 2023/11/17 16:52, Tao Liu wrote:
>>
>>> For the stacktrace of "dis -rl", it calls dw2_expand_all_symtabs()
to expand
>>> all symtable of the objfile, or "*.ko.debug" in our case. However
for
>>> the stacktrace of "bt", it doesn't expand all, but only a
subset of symtable
>>> which is enough to find a symbol by dw2_lookup_symbol(). As a result, the
>>> objfile->compunit_symtabs, which is the head of a single linked list of
>>> struct compunit_symtab, is not NULL but didn't contain all symtables.
It
>>> will not be reinitialized in gdb_get_line_number() by "dis -rl"
because
>>> !objfile_has_full_symbols(objfile) check will fail, so it cannot display
>>> the proper code line number data.
>>>
>>> Since objfile_has_full_symbols(objfile) check cannot ensure all symbols
>>> been expanded, this patch add a new member as a flag for struct objfile
>>> to record if all symbols have been expanded. The flag will be set only
ofter
>>> expand_all_symtabs been called.
>>
>> Thank you for the v2.
>> Great, I can't think of any better way than this for now.
>>
>> Acked-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
>>
>> (maybe I will reduce the commit log a bit when merging.)
>>
> Thanks a lot for the patch review. Please go ahead if the commit log
> is too long.
>
> Thanks,
> Tao Liu
>
>> Thanks,
>> Kazu
>>
>>>
>>> Signed-off-by: Tao Liu <ltao(a)redhat.com>
>>> ---
>>> v1 -> v2: new method for kernel module symtabs expansion.
>>> v1: expand all kernel modules symtabs when loading by mod -s/-S
>>> v2: record if a specific kernel module's symtab been all
expanded,
>>> and skip re-expansion if true.
>>> ---
>>> gdb-10.2.patch | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 50 insertions(+)
>>>
>>> diff --git a/gdb-10.2.patch b/gdb-10.2.patch
>>> index d81030d..2f7d585 100644
>>> --- a/gdb-10.2.patch
>>> +++ b/gdb-10.2.patch
>>> @@ -3187,3 +3187,53 @@ exit 0
>>> result = stringtab + symbol_entry->_n._n_n._n_offset;
>>> }
>>> else
>>> +--- gdb-10.2/gdb/objfiles.h.orig
>>> ++++ gdb-10.2/gdb/objfiles.h
>>> +@@ -712,6 +712,8 @@ struct objfile
>>> + next time. If an objfile does not have the symbols, it will
>>> + never have them. */
>>> + bool skip_jit_symbol_lookup = false;
>>> ++
>>> ++ bool all_symtabs_expanded = false;
>>> + };
>>> +
>>> + /* A deleter for objfile. */
>>> +--- gdb-10.2/gdb/symfile.c.orig
>>> ++++ gdb-10.2/gdb/symfile.c
>>> +@@ -1133,8 +1133,10 @@ symbol_file_add_with_addrs (bfd *abfd, const char
*name,
>>> + printf_filtered (_("Expanding full symbols from %ps...\n"),
>>> + styled_string (file_name_style.style (), name));
>>> +
>>> +- if (objfile->sf)
>>> ++ if (objfile->sf) {
>>> + objfile->sf->qf->expand_all_symtabs (objfile);
>>> ++ objfile->all_symtabs_expanded = true;
>>> ++ }
>>> + }
>>> +
>>> + /* Note that we only print a message if we have no symbols and have
>>> +--- gdb-10.2/gdb/symtab.c.orig
>>> ++++ gdb-10.2/gdb/symtab.c
>>> +@@ -7097,8 +7097,9 @@ gdb_get_line_number(struct gnu_request *req)
>>> + */
>>> + if (req->lm) {
>>> + objfile = req->lm->loaded_objfile;
>>> +- if (!objfile_has_full_symbols(objfile) &&
objfile->sf) {
>>> ++ if (!objfile->all_symtabs_expanded &&
objfile->sf) {
>>> +
objfile->sf->qf->expand_all_symtabs(objfile);
>>> ++ objfile->all_symtabs_expanded = true;
>>> + sal = find_pc_line(pc, 0);
>>> + }
>>> + }
>>> +@@ -7761,8 +7765,10 @@ iterate_datatypes (struct gnu_request *req)
>>> + {
>>> + for (objfile *objfile : current_program_space->objfiles ())
>>> + {
>>> +- if (objfile->sf)
>>> ++ if (objfile->sf) {
>>> + objfile->sf->qf->expand_all_symtabs(objfile);
>>> ++ objfile->all_symtabs_expanded = true;
>>> ++ }
>>> +
>>> + for (compunit_symtab *cust : objfile->compunits ())
>>> + {