Received-SPF: pass (google.com: domain of linux-kernel+bounces-204080-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 6 Jun 2024 12:05:42 +0200
From: Lukas Hruska <lhruska@suse.cz>
To: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Lukas Hruska <lhruska@suse.cz>, pmladek@suse.com, mbenes@suse.cz,
	jpoimboe@kernel.org, live-patching@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	mpdesouza@suse.com, Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH v2 2/6] livepatch: Add klp-convert tool
Message-ID: <ZmGJ6zMU-0OG29Uk@localhost.localdomain>
References: <20240516133009.20224-1-lhruska@suse.cz>
 <20240516133009.20224-3-lhruska@suse.cz>
 <ZljceDZ7eqEJtYhQ@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZljceDZ7eqEJtYhQ@redhat.com>

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