Re: [PATCH v2 2/6] livepatch: Add klp-convert tool

[Lukas Hruska]

Hello Joe,
> > +#define KLP_RELOC_SYMBOL_POS(LP_OBJ_NAME, SYM_OBJ_NAME, SYM_NAME, SYM_POS)	\
> > +	asm("\".klp.sym.rela." #LP_OBJ_NAME "." #SYM_OBJ_NAME "." #SYM_NAME "," #SYM_POS "\"")
>                                                                             ^^^
> I think I found a potential bug, or at least compatiblity problem with
> including a comma "," character in this symbol format and older versions
> of the GNU assembler.  The good news is that other delimiter characters
> like "." or "#" seem to work out fine.

Thank you for spotting this. I was using binutils 2.38, so I did not
even notice this problem. Unfortunately, I was not able to make it work
with "#" as a delimiter; only "." worked. Additionally, any type of
parenthesis apparently has some special purpose even in labels, so they
are also not an option.

> If you want to reproduce, you'll need a version of `as` like binutils
> 2.36.1 and try building the samples/livepatch/livepatch-extern-symbol.ko
> and you should get an error like:
> 
>   Assembler messages:
>   Warning: missing closing '"'
>   Warning: missing closing '"'
>   Error: too many memory references for `movq'
> 
> 
> If you want to retrace my adventure, here are my steps:
> 
>   1) Clone klp-convert-tree repo branch containing this patchset +
>   Petr's review comments + a few helpful things for klp-convert
>   development:
>   
>     $ git clone \
>         --single-branch --branch=klp-convert-minimal-v1-review --depth=9 \
>         https://github.com/joe-lawrence/klp-convert-tree.git
>     [ ... snip ... ]
>     $ cd klp-convert-tree
>   
>   2) Override .cross-dev defaults:
>   
>     $ export BUILD_ARCHES=x86_64
>     $ export COMPILER=gcc-11.1.0
>     $ export URL=https://cdn.kernel.org/pub/tools/crosstool/files/bin/x86_64/
>     $ export OUTDIR_PREFIX=$(pwd)/build
>     $ export COMPILER_INSTALL_PATH=$(pwd)/cross-compiler
>   
>   3) Setup x86_64 default .config (this will download and install the
>   gcc-11.1.0 compiler from cdn.kernel.org):
>   
>     $ ./cross-dev make defconfig
>     
>     x86_64 : make defconfig ...
>     Compiler will be installed in /root/klp-convert-tree/cross-compiler
>     [ ... snip ... ]
>   
>   4) Add kernel livepatching configuration options:
>   
>     $ ./cross-dev klp-config
>     
>     Configuring x86_64 ...
>     [ ... snip ... ]
>     
>     $ grep LIVEPATCH "$OUTDIR_PREFIX"-x86_64/.config
>     CONFIG_HAVE_LIVEPATCH=y
>     CONFIG_LIVEPATCH=y
>     CONFIG_SAMPLE_LIVEPATCH=m
>   
>   5) Run the cross-compiler build until it hits a build error on
>   livepatch-extern-symbol.ko:
>   
>     $ ./cross-dev make -j$(nproc)
>     [ ... snip ... ]
>     make: Target '__all' not remade because of errors.
>     [ x86_64 : make -j48 = FAIL ]
>   
>   6) With pre-requisites already built, retry the external symbol sample
>   and add -save-temps to the KCFLAGS to keep the generated assembly file:
>   
>     $ KCFLAGS="-save-temps=obj" ./cross-dev make samples/livepatch/livepatch-extern-symbol.ko
>     [ ... snip ... ]
>     samples/livepatch/livepatch-extern-symbol.s: Assembler messages:
>     samples/livepatch/livepatch-extern-symbol.s:103: Warning: missing closing '"'
>     samples/livepatch/livepatch-extern-symbol.s:103: Warning: missing closing '"'
>     samples/livepatch/livepatch-extern-symbol.s:103: Error: too many memory references for `movq'
>     [ ... snip ... ]
>   
>   7) Which line is that?
>   
>     $ awk 'NR==103' "$OUTDIR_PREFIX"-x86_64/samples/livepatch/livepatch-extern-symbol.s
>             movq    ".klp.sym.rela.vmlinux.vmlinux.saved_command_line,0"(%rip), %rdx
> 
> 
> You could alternatively poke at it through the compiler explorer service
> and toggle the source and binutils versions:
> 
>   (error)   binutils 2.36.1 : https://godbolt.org/z/cGGs6rfWe
>   (success) binutils 2.38   : https://godbolt.org/z/ffzza3vYd

Thank you for those detailed step-by-step instruction to reproduce it!
It helped me a lot to understand the problem.

Lukas