On 4/3/19 8:48 AM, Miroslav Benes wrote:
>
>> and it looks like the combined KLP_MODULE_RELOC still contains the two
>> unique symbol position values (2 and 3):
>>
>> % objcopy -O binary --only-section=.klp.module_relocs.vmlinux lib/livepatch/test_klp_convert.klp.o >(hexdump -C)
>> 00000000 00 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00 |................|
>> 00000010 00 00 00 00 00 00 00 00 03 00 00 00 |............|
>> 0000001c
>
> Nice :/
>
>> Maybe we can work around this by modifying the annotation macros and/or
>> klp-convert, or live with this for now.
>
> The question is (and I'll check later. I cannot wrap my head around it
> now) if it at least works if there are two references of the same symbol
> in two different .o. It would be same state_show in this case and not two
> different ones. If it works then I think we can live with it for a while,
> because after all duplicate symbols are quite rare in the kernel.
Possibly, but in testing that scenario I found another issue. Check out what
happens to the combined .klp.module_relocs.vmlinux section for:
test_klp_convert_a.c
KLP_MODULE_RELOC(vmlinux) vmlinux_relocs_a[] = {
KLP_SYMPOS(state_show, 2)
KLP_SYMPOS(joe, 10)
KLP_SYMPOS(joe2, 11)
};
test_klp_convert_b.c
KLP_MODULE_RELOC(vmlinux) vmlinux_relocs_b[] = {
KLP_SYMPOS(state_show, 2)
};
The second file's klp_module_reloc are not aligned with the first,
so I think there is additional padding to push the second set to a
word boundary:
% objcopy -O binary --only-section=.klp.module_relocs.vmlinux lib/livepatch/test_klp_convert.klp.o >(hexdump -C)
00000000 00 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00
|-*sym----------------| |--sympos-| |-*sym-----
00000010 00 00 00 00 0a 00 00 00 00 00 00 00 00 00 00 00
----------| |--sympos-| |-*sym----------------|
00000020 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|-sympos--| |-*sym----------------|
^^^^^^^^^^^
padding
00000030 02 00 00 00
|-sympos--|
in this case, klp-convert thought the last symbol's sympos was
incorrectly 0 and not 2.
If the packed attribute is merely a space optimization, can we
simply pull that (or can we specify slightly looser alignment to
account for the padding)?
I'll continue working on putting together v3 and add this new item
to the TODO list.
Thanks,
-- Joe
