scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
diff mbox series

Message ID da22e0ac-8da8-8a16-e8dd-b7065752cb4d@mir.dev
State New
Headers show
Series
  • scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
Related show

Commit Message

Mikhail Petrov March 10, 2020, 8:34 p.m. UTC
There is the code in the read_symbol function in 'scripts/kallsyms.c':

	if (is_ignored_symbol(name, type))
		return NULL;

	/* Ignore most absolute/undefined (?) symbols. */
	if (strcmp(name, "_text") == 0)
		_text = addr;

But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.

It makes the wrong kallsyms_relative_base symbol as a result of the code:

	if (base_relative) {
		output_label("kallsyms_relative_base");
		output_address(relative_base);
		printf("\n");
	}

Because the output_address function uses the _text variable.

So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:

	Call Trace:
	[aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
	[aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
	[aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
	[aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
	[aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010

The right stack trace:

	Call Trace:
	[aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
	[aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
	[aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
	[aa095f28] [80002ed0] kernel_init+0x14/0x124
	[aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c

Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>

---

Comments

Masahiro Yamada March 11, 2020, 6:06 a.m. UTC | #1
Hi Mikhail,

On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>
> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>
>         if (is_ignored_symbol(name, type))
>                 return NULL;
>
>         /* Ignore most absolute/undefined (?) symbols. */
>         if (strcmp(name, "_text") == 0)
>                 _text = addr;
>
> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>
> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>
>         if (base_relative) {
>                 output_label("kallsyms_relative_base");
>                 output_address(relative_base);
>                 printf("\n");
>         }
>
> Because the output_address function uses the _text variable.
>
> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>
>         Call Trace:
>         [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>         [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>         [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>         [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>         [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>
> The right stack trace:
>
>         Call Trace:
>         [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>         [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>         [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>         [aa095f28] [80002ed0] kernel_init+0x14/0x124
>         [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>
> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
>
> ---


Thanks for the patch.

Just for curiosity, on which architecrure
did you see  name="_text" and type='a' case ?


Could you wrap the commit log to avoid
this checkpatch warning?
WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)


Also, could you shorten the patch subject
to make it fit in this limit?

Thanks.
Mikhail Petrov March 11, 2020, 6:18 p.m. UTC | #2
Hi Masahiro,

On 11.03.2020 9:06, Masahiro Yamada wrote:
> Hi Mikhail,
> 
> On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>
>> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>>
>>         if (is_ignored_symbol(name, type))
>>                 return NULL;
>>
>>         /* Ignore most absolute/undefined (?) symbols. */
>>         if (strcmp(name, "_text") == 0)
>>                 _text = addr;
>>
>> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>>
>> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>>
>>         if (base_relative) {
>>                 output_label("kallsyms_relative_base");
>>                 output_address(relative_base);
>>                 printf("\n");
>>         }
>>
>> Because the output_address function uses the _text variable.
>>
>> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>>
>>         Call Trace:
>>         [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>>         [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>>         [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>>         [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>>         [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>>
>> The right stack trace:
>>
>>         Call Trace:
>>         [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>>         [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>>         [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>>         [aa095f28] [80002ed0] kernel_init+0x14/0x124
>>         [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>>
>> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
>>
>> ---
> 
> 
> Thanks for the patch.
> 
> Just for curiosity, on which architecrure
> did you see  name="_text" and type='a' case ?

Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.

nm -n .tmp_vmlinux1 looks like:

...
         w kallsyms_token_table
         w mach_powermac
00000007 a LG_CACHELINE_BYTES
00000007 a LG_CACHELINE_BYTES
00000007 a LG_CACHELINE_BYTES
00000020 a reg
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000400 a dcr
80000000 T _start
80000000 A _stext
80000000 A _text
80000088 t interrupt_base
800000a0 t CriticalInput
80000180 t MachineCheck
80000260 t MachineCheckA
80000360 t DataStorage
80000420 t InstructionStorage
80000500 t ExternalInput
800005c0 t Alignment
80000680 t Program
80000740 t FloatingPointUnavailable
80000820 t SystemCall
80000900 t AuxillaryProcessorUnavailable
...

 
> Could you wrap the commit log to avoid
> this checkpatch warning?
> WARNING: Possible unwrapped commit description (prefer a maximum 75
> chars per line)
> 
> Also, could you shorten the patch subject
> to make it fit in this limit?

Sorry for that. Now I know about scripts/checkpatch.pl. I will improve and resubmit the patch soon.

Thanks.
Masahiro Yamada March 11, 2020, 8:56 p.m. UTC | #3
On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>
> Hi Masahiro,
>
> On 11.03.2020 9:06, Masahiro Yamada wrote:
> > Hi Mikhail,
> >
> > On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
> >>
> >> There is the code in the read_symbol function in 'scripts/kallsyms.c':
> >>
> >>         if (is_ignored_symbol(name, type))
> >>                 return NULL;
> >>
> >>         /* Ignore most absolute/undefined (?) symbols. */
> >>         if (strcmp(name, "_text") == 0)
> >>                 _text = addr;
> >>
> >> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
> >>
> >> It makes the wrong kallsyms_relative_base symbol as a result of the code:
> >>
> >>         if (base_relative) {
> >>                 output_label("kallsyms_relative_base");
> >>                 output_address(relative_base);
> >>                 printf("\n");
> >>         }
> >>
> >> Because the output_address function uses the _text variable.
> >>
> >> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
> >>
> >>         Call Trace:
> >>         [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
> >>         [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
> >>         [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
> >>         [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
> >>         [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
> >>
> >> The right stack trace:
> >>
> >>         Call Trace:
> >>         [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
> >>         [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
> >>         [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
> >>         [aa095f28] [80002ed0] kernel_init+0x14/0x124
> >>         [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
> >>
> >> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
> >>
> >> ---
> >
> >
> > Thanks for the patch.
> >
> > Just for curiosity, on which architecrure
> > did you see  name="_text" and type='a' case ?
>
> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
>
> nm -n .tmp_vmlinux1 looks like:
>
> ...
>          w kallsyms_token_table
>          w mach_powermac
> 00000007 a LG_CACHELINE_BYTES
> 00000007 a LG_CACHELINE_BYTES
> 00000007 a LG_CACHELINE_BYTES
> 00000020 a reg
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> 80000000 T _start
> 80000000 A _stext
> 80000000 A _text


Hmm, I am still not able to reproduce this.

I compiled ARCH=powerpc, but
'powerpc-linux-nm -n .tmp_vmlinux1' got this.


0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000400 a dcr
c0000000 T _start
c0000000 T _stext
c0000000 T _text
c00000b8 t interrupt_base
c00000c0 t CriticalInput
c00001a0 t MachineCheck
c0000280 t MachineCheckA




Which defconfig did you use?


(I also CCed the ppc maintainer,
I am just curious what makes _text absolute.)







> 80000088 t interrupt_base
> 800000a0 t CriticalInput
> 80000180 t MachineCheck
> 80000260 t MachineCheckA
> 80000360 t DataStorage
> 80000420 t InstructionStorage
> 80000500 t ExternalInput
> 800005c0 t Alignment
> 80000680 t Program
> 80000740 t FloatingPointUnavailable
> 80000820 t SystemCall
> 80000900 t AuxillaryProcessorUnavailable
> ...
>
>
> > Could you wrap the commit log to avoid
> > this checkpatch warning?
> > WARNING: Possible unwrapped commit description (prefer a maximum 75
> > chars per line)
> >
> > Also, could you shorten the patch subject
> > to make it fit in this limit?
>
> Sorry for that. Now I know about scripts/checkpatch.pl. I will improve and resubmit the patch soon.
>
> Thanks.


--
Best Regards
Masahiro Yamada
Michael Ellerman March 12, 2020, 5:12 a.m. UTC | #4
Masahiro Yamada <masahiroy@kernel.org> writes:
> On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>> On 11.03.2020 9:06, Masahiro Yamada wrote:
>> > On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>> >>
>> >> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>> >>
>> >>         if (is_ignored_symbol(name, type))
>> >>                 return NULL;
>> >>
>> >>         /* Ignore most absolute/undefined (?) symbols. */
>> >>         if (strcmp(name, "_text") == 0)
>> >>                 _text = addr;
>> >>
>> >> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>> >>
>> >> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>> >>
>> >>         if (base_relative) {
>> >>                 output_label("kallsyms_relative_base");
>> >>                 output_address(relative_base);
>> >>                 printf("\n");
>> >>         }
>> >>
>> >> Because the output_address function uses the _text variable.
>> >>
>> >> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>> >>
>> >>         Call Trace:
>> >>         [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>> >>         [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>> >>         [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>> >>         [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>> >>         [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>> >>
>> >> The right stack trace:
>> >>
>> >>         Call Trace:
>> >>         [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>> >>         [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>> >>         [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>> >>         [aa095f28] [80002ed0] kernel_init+0x14/0x124
>> >>         [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>> >
>> > Thanks for the patch.
>> >
>> > Just for curiosity, on which architecrure
>> > did you see  name="_text" and type='a' case ?
>>
>> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
>>
>> nm -n .tmp_vmlinux1 looks like:
>>
>> ...
>>          w kallsyms_token_table
>>          w mach_powermac
>> 00000007 a LG_CACHELINE_BYTES
>> 00000007 a LG_CACHELINE_BYTES
>> 00000007 a LG_CACHELINE_BYTES
>> 00000020 a reg
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000400 a dcr
>> 80000000 T _start
>> 80000000 A _stext
>> 80000000 A _text
>
>
> Hmm, I am still not able to reproduce this.
>
> I compiled ARCH=powerpc, but
> 'powerpc-linux-nm -n .tmp_vmlinux1' got this.
>
>
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> c0000000 T _start
> c0000000 T _stext
> c0000000 T _text
> c00000b8 t interrupt_base
> c00000c0 t CriticalInput
> c00001a0 t MachineCheck
> c0000280 t MachineCheckA
>
>
> Which defconfig did you use?
>
>
> (I also CCed the ppc maintainer,
> I am just curious what makes _text absolute.)

I have no idea sorry.

arch/powerpc has about 20 sub-platforms that do weird and wonderful
things. Presumably this happens on one of those.

I played around with some of the defconfigs but couldn't reproduce this.

The only config we have that puts the kernel at 0x80000000 is:

  $ git grep 80000000  arch/powerpc/configs/
  arch/powerpc/configs/85xx/xes_mpc85xx_defconfig:CONFIG_PAGE_OFFSET=0x80000000

But that's not a PPC476 platform.

And _text is not 'A':

  $ nm .tmp_vmlinux1 | grep -w _text
  80000000 T _text


cheers
Mikhail Petrov March 12, 2020, 7:36 p.m. UTC | #5
On 11.03.2020 23:56, Masahiro Yamada wrote:
> On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>
>> Hi Masahiro,
>>
>> On 11.03.2020 9:06, Masahiro Yamada wrote:
>>> Hi Mikhail,
>>>
>>> On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>>>
>>>> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>>>>
>>>>         if (is_ignored_symbol(name, type))
>>>>                 return NULL;
>>>>
>>>>         /* Ignore most absolute/undefined (?) symbols. */
>>>>         if (strcmp(name, "_text") == 0)
>>>>                 _text = addr;
>>>>
>>>> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>>>>
>>>> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>>>>
>>>>         if (base_relative) {
>>>>                 output_label("kallsyms_relative_base");
>>>>                 output_address(relative_base);
>>>>                 printf("\n");
>>>>         }
>>>>
>>>> Because the output_address function uses the _text variable.
>>>>
>>>> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>>>>
>>>>         Call Trace:
>>>>         [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>>>>         [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>>>>         [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>>>>         [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>>>>         [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>>>>
>>>> The right stack trace:
>>>>
>>>>         Call Trace:
>>>>         [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>>>>         [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>>>>         [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>>>>         [aa095f28] [80002ed0] kernel_init+0x14/0x124
>>>>         [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>>>>
>>>> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
>>>>
>>>> ---
>>>
>>>
>>> Thanks for the patch.
>>>
>>> Just for curiosity, on which architecrure
>>> did you see  name="_text" and type='a' case ?
>>
>> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
>>
>> nm -n .tmp_vmlinux1 looks like:
>>
>> ...
>>          w kallsyms_token_table
>>          w mach_powermac
>> 00000007 a LG_CACHELINE_BYTES
>> 00000007 a LG_CACHELINE_BYTES
>> 00000007 a LG_CACHELINE_BYTES
>> 00000020 a reg
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000400 a dcr
>> 80000000 T _start
>> 80000000 A _stext
>> 80000000 A _text
> 
> 
> Hmm, I am still not able to reproduce this.
> 
> I compiled ARCH=powerpc, but
> 'powerpc-linux-nm -n .tmp_vmlinux1' got this.
> 
> 
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> c0000000 T _start
> c0000000 T _stext
> c0000000 T _text
> c00000b8 t interrupt_base
> c00000c0 t CriticalInput
> c00001a0 t MachineCheck
> c0000280 t MachineCheckA
> 
> 
> 
> 
> Which defconfig did you use?

I use a custom config file for a custom SoC with two PPC476FS cores. The config is not in the upstream repository. The same effect can be reached with '44x/akebono_defconfig'.

I did some investigation with the GCC version.

GCC version  4.8.1 (akebono_defconfig):

00000007 a LG_CACHELINE_BYTES
00000020 a reg
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000400 a dcr
00000400 a spr
c0000000 T _start
c0000000 A _stext
c0000000 A _text
c0000088 t interrupt_base
c00000a0 t CriticalInput
c0000180 t MachineCheck

GCC version 7.5.0 (akebono_defconfig):

00000007 a LG_CACHELINE_BYTES
00000020 a reg
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000400 a dcr
00000400 a spr
c0000000 T _start
c0000000 T _stext
c0000000 T _text
c0000088 t interrupt_base
c00000a0 t CriticalInput
c0000180 t MachineCheck

So, I used an old version of GCC. Changing the GCC version solved the problem. Maybe the patch is not necessary.


> 
> 
> (I also CCed the ppc maintainer,
> I am just curious what makes _text absolute.)
> 
> 
> 
> 
> 
> 
> 
>> 80000088 t interrupt_base
>> 800000a0 t CriticalInput
>> 80000180 t MachineCheck
>> 80000260 t MachineCheckA
>> 80000360 t DataStorage
>> 80000420 t InstructionStorage
>> 80000500 t ExternalInput
>> 800005c0 t Alignment
>> 80000680 t Program
>> 80000740 t FloatingPointUnavailable
>> 80000820 t SystemCall
>> 80000900 t AuxillaryProcessorUnavailable
>> ...
>>
>>
>>> Could you wrap the commit log to avoid
>>> this checkpatch warning?
>>> WARNING: Possible unwrapped commit description (prefer a maximum 75
>>> chars per line)
>>>
>>> Also, could you shorten the patch subject
>>> to make it fit in this limit?
>>
>> Sorry for that. Now I know about scripts/checkpatch.pl. I will improve and resubmit the patch soon.
>>
>> Thanks.
> 
> 
> --
> Best Regards
> Masahiro Yamada
> 

--
Best Regards,
Mikhail Petrov
Mikhail Petrov March 12, 2020, 7:51 p.m. UTC | #6
Hi Michael,

On 12.03.2020 8:12, Michael Ellerman wrote:
> Masahiro Yamada <masahiroy@kernel.org> writes:
>> On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>> On 11.03.2020 9:06, Masahiro Yamada wrote:
>>>> On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>>>>
>>>>> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>>>>>
>>>>>         if (is_ignored_symbol(name, type))
>>>>>                 return NULL;
>>>>>
>>>>>         /* Ignore most absolute/undefined (?) symbols. */
>>>>>         if (strcmp(name, "_text") == 0)
>>>>>                 _text = addr;
>>>>>
>>>>> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>>>>>
>>>>> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>>>>>
>>>>>         if (base_relative) {
>>>>>                 output_label("kallsyms_relative_base");
>>>>>                 output_address(relative_base);
>>>>>                 printf("\n");
>>>>>         }
>>>>>
>>>>> Because the output_address function uses the _text variable.
>>>>>
>>>>> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>>>>>
>>>>>         Call Trace:
>>>>>         [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>>>>>         [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>>>>>         [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>>>>>         [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>>>>>         [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>>>>>
>>>>> The right stack trace:
>>>>>
>>>>>         Call Trace:
>>>>>         [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>>>>>         [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>>>>>         [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>>>>>         [aa095f28] [80002ed0] kernel_init+0x14/0x124
>>>>>         [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>>>>
>>>> Thanks for the patch.
>>>>
>>>> Just for curiosity, on which architecrure
>>>> did you see  name="_text" and type='a' case ?
>>>
>>> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
>>>
>>> nm -n .tmp_vmlinux1 looks like:
>>>
>>> ...
>>>          w kallsyms_token_table
>>>          w mach_powermac
>>> 00000007 a LG_CACHELINE_BYTES
>>> 00000007 a LG_CACHELINE_BYTES
>>> 00000007 a LG_CACHELINE_BYTES
>>> 00000020 a reg
>>> 0000007f a CACHELINE_MASK
>>> 0000007f a CACHELINE_MASK
>>> 0000007f a CACHELINE_MASK
>>> 00000080 a CACHELINE_BYTES
>>> 00000080 a CACHELINE_BYTES
>>> 00000080 a CACHELINE_BYTES
>>> 00000400 a dcr
>>> 80000000 T _start
>>> 80000000 A _stext
>>> 80000000 A _text
>>
>>
>> Hmm, I am still not able to reproduce this.
>>
>> I compiled ARCH=powerpc, but
>> 'powerpc-linux-nm -n .tmp_vmlinux1' got this.
>>
>>
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000400 a dcr
>> c0000000 T _start
>> c0000000 T _stext
>> c0000000 T _text
>> c00000b8 t interrupt_base
>> c00000c0 t CriticalInput
>> c00001a0 t MachineCheck
>> c0000280 t MachineCheckA
>>
>>
>> Which defconfig did you use?
>>
>>
>> (I also CCed the ppc maintainer,
>> I am just curious what makes _text absolute.)
> 
> I have no idea sorry.
> 
> arch/powerpc has about 20 sub-platforms that do weird and wonderful
> things. Presumably this happens on one of those.
> 
> I played around with some of the defconfigs but couldn't reproduce this.
> 
> The only config we have that puts the kernel at 0x80000000 is:
> 
>   $ git grep 80000000  arch/powerpc/configs/
>   arch/powerpc/configs/85xx/xes_mpc85xx_defconfig:CONFIG_PAGE_OFFSET=0x80000000
> 
> But that's not a PPC476 platform.

It is a custom config for a custom SoC. The config is not in the upstream repository. PAGE_OFFSET has been changed for increasing the address space for PCI windows and some devices.


> 
> And _text is not 'A':
> 
>   $ nm .tmp_vmlinux1 | grep -w _text
>   80000000 T _text

I used the old GCC version - 4.8.1. In modern versions of GCC, the record has type 'T'.

> 
> 
> cheers
> 

--
Best Regards
Mikhail Petrov
Masahiro Yamada March 19, 2020, 8:02 a.m. UTC | #7
Hi Mikhail,

On Fri, Mar 13, 2020 at 4:36 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>
>
>
> On 11.03.2020 23:56, Masahiro Yamada wrote:
> > On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
> >>
> >> Hi Masahiro,
> >>
> >> On 11.03.2020 9:06, Masahiro Yamada wrote:
> >>> Hi Mikhail,
> >>>
> >>> On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
> >>>>
> >>>> There is the code in the read_symbol function in 'scripts/kallsyms.c':
> >>>>
> >>>>         if (is_ignored_symbol(name, type))
> >>>>                 return NULL;
> >>>>
> >>>>         /* Ignore most absolute/undefined (?) symbols. */
> >>>>         if (strcmp(name, "_text") == 0)
> >>>>                 _text = addr;
> >>>>
> >>>> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
> >>>>
> >>>> It makes the wrong kallsyms_relative_base symbol as a result of the code:
> >>>>
> >>>>         if (base_relative) {
> >>>>                 output_label("kallsyms_relative_base");
> >>>>                 output_address(relative_base);
> >>>>                 printf("\n");
> >>>>         }
> >>>>
> >>>> Because the output_address function uses the _text variable.
> >>>>
> >>>> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
> >>>>
> >>>>         Call Trace:
> >>>>         [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
> >>>>         [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
> >>>>         [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
> >>>>         [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
> >>>>         [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
> >>>>
> >>>> The right stack trace:
> >>>>
> >>>>         Call Trace:
> >>>>         [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
> >>>>         [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
> >>>>         [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
> >>>>         [aa095f28] [80002ed0] kernel_init+0x14/0x124
> >>>>         [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
> >>>>
> >>>> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
> >>>>
> >>>> ---
> >>>
> >>>
> >>> Thanks for the patch.
> >>>
> >>> Just for curiosity, on which architecrure
> >>> did you see  name="_text" and type='a' case ?
> >>
> >> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
> >>
> >> nm -n .tmp_vmlinux1 looks like:
> >>
> >> ...
> >>          w kallsyms_token_table
> >>          w mach_powermac
> >> 00000007 a LG_CACHELINE_BYTES
> >> 00000007 a LG_CACHELINE_BYTES
> >> 00000007 a LG_CACHELINE_BYTES
> >> 00000020 a reg
> >> 0000007f a CACHELINE_MASK
> >> 0000007f a CACHELINE_MASK
> >> 0000007f a CACHELINE_MASK
> >> 00000080 a CACHELINE_BYTES
> >> 00000080 a CACHELINE_BYTES
> >> 00000080 a CACHELINE_BYTES
> >> 00000400 a dcr
> >> 80000000 T _start
> >> 80000000 A _stext
> >> 80000000 A _text
> >
> >
> > Hmm, I am still not able to reproduce this.
> >
> > I compiled ARCH=powerpc, but
> > 'powerpc-linux-nm -n .tmp_vmlinux1' got this.
> >
> >
> > 0000007f a CACHELINE_MASK
> > 0000007f a CACHELINE_MASK
> > 0000007f a CACHELINE_MASK
> > 00000080 a CACHELINE_BYTES
> > 00000080 a CACHELINE_BYTES
> > 00000080 a CACHELINE_BYTES
> > 00000400 a dcr
> > c0000000 T _start
> > c0000000 T _stext
> > c0000000 T _text
> > c00000b8 t interrupt_base
> > c00000c0 t CriticalInput
> > c00001a0 t MachineCheck
> > c0000280 t MachineCheckA
> >
> >
> >
> >
> > Which defconfig did you use?
>
> I use a custom config file for a custom SoC with two PPC476FS cores. The config is not in the upstream repository. The same effect can be reached with '44x/akebono_defconfig'.
>
> I did some investigation with the GCC version.
>
> GCC version  4.8.1 (akebono_defconfig):
>
> 00000007 a LG_CACHELINE_BYTES
> 00000020 a reg
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> 00000400 a spr
> c0000000 T _start
> c0000000 A _stext
> c0000000 A _text
> c0000088 t interrupt_base
> c00000a0 t CriticalInput
> c0000180 t MachineCheck
>
> GCC version 7.5.0 (akebono_defconfig):
>
> 00000007 a LG_CACHELINE_BYTES
> 00000020 a reg
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> 00000400 a spr
> c0000000 T _start
> c0000000 T _stext
> c0000000 T _text
> c0000088 t interrupt_base
> c00000a0 t CriticalInput
> c0000180 t MachineCheck
>
> So, I used an old version of GCC. Changing the GCC version solved the problem. Maybe the patch is not necessary.



Confirmed.

I was able to reproduce it with the following toolchain.

https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/x86_64-gcc-4.6.3-nolibc_powerpc64-linux.tar.xz



Actually, this is not GCC, but linker (binutils) issue.

This happens on any architecture
if you use GNU binutils <= 2.22


The current minimal supported version is
2.21, so I will pick up v2.





>
> >
> >
> > (I also CCed the ppc maintainer,
> > I am just curious what makes _text absolute.)
> >
> >
> >
> >
> >
> >
> >
> >> 80000088 t interrupt_base
> >> 800000a0 t CriticalInput
> >> 80000180 t MachineCheck
> >> 80000260 t MachineCheckA
> >> 80000360 t DataStorage
> >> 80000420 t InstructionStorage
> >> 80000500 t ExternalInput
> >> 800005c0 t Alignment
> >> 80000680 t Program
> >> 80000740 t FloatingPointUnavailable
> >> 80000820 t SystemCall
> >> 80000900 t AuxillaryProcessorUnavailable
> >> ...
> >>
> >>
> >>> Could you wrap the commit log to avoid
> >>> this checkpatch warning?
> >>> WARNING: Possible unwrapped commit description (prefer a maximum 75
> >>> chars per line)
> >>>
> >>> Also, could you shorten the patch subject
> >>> to make it fit in this limit?
> >>
> >> Sorry for that. Now I know about scripts/checkpatch.pl. I will improve and resubmit the patch soon.
> >>
> >> Thanks.
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
>
> --
> Best Regards,
> Mikhail Petrov
>


--
Best Regards
Masahiro Yamada

Patch
diff mbox series

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 0133dfaaf352..3e8dea6e0a95 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -195,13 +195,13 @@  static struct sym_entry *read_symbol(FILE *in)
 		return NULL;
 	}
 
-	if (is_ignored_symbol(name, type))
-		return NULL;
-
-	/* Ignore most absolute/undefined (?) symbols. */
 	if (strcmp(name, "_text") == 0)
 		_text = addr;
 
+	/* Ignore most absolute/undefined (?) symbols. */
+	if (is_ignored_symbol(name, type))
+		return NULL;
+
 	check_symbol_range(name, addr, text_ranges, ARRAY_SIZE(text_ranges));
 	check_symbol_range(name, addr, &percpu_range, 1);