Re: [PATCH] livepatch-build-tools: allow livepatching version.c

["Roger Pau Monné" <roger.pau@citrix.com>]

On Tue, Dec 05, 2023 at 02:15:05PM +0000, Andrew Cooper wrote:
> On 05/12/2023 12:34 pm, Roger Pau Monne wrote:
> > Currently version.o is explicitly ignored as the file would change as a result
> > of the orignal and the patched build having possibly different dates and
> > times.
> >
> > Fix such difference by exporting the date and time from the build script, so
> > that both builds share the same build time.  This allows checking for changes
> > in version.c, since the rest of fields need to be manually changed in order to
> > produce different output.
> >
> > Setting XEN_BUILD_{DATE,TIME} as an environment variable has been supported
> > since before livepatch support was added to Xen, so it's safe to export those
> > variables unconditionally.
> >
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> >  livepatch-build | 4 ++++
> >  livepatch-gcc   | 2 --
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/livepatch-build b/livepatch-build
> > index e2ccce4f7fd7..f622683fc56c 100755
> > --- a/livepatch-build
> > +++ b/livepatch-build
> > @@ -417,6 +417,10 @@ if [ "${SKIP}" != "build" ]; then
> >  
> >      export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc "
> >  
> > +    # Force same date and time to prevent unwanted changes in version.c
> > +    export XEN_BUILD_DATE=`LC_ALL=C date`
> > +    export XEN_BUILD_TIME=`LC_ALL=C date +%T`
> 
> Date is the one that goes wrong every time, but everything else in
> compile.h can go wrong in a way that causes version.o to change.

I've attempted to reflect that in "since the rest of fields need to be
manually changed in order to produce different output".

For those to change there must be some kind of environment change
between the original and the patched version build, and hence I don't
think that would be supported.

> Ideally, the pristine source for building livepatches would include a
> generated compile.h, and livepatch would have a way to force no
> regeneration of the header.  But I've got no idea how nice that would be
> to arrange.

Yes, no idea how fragile that would be either.  IMO the proposed
approach is not that bad.

> That way, you're using the same details as the Xen being patched, rather
> than hoping that two identical different details will cancel out in the
> binary diff.

Another option is to set all the env variables to disable any
build time probing.  However things like compiler or version changing
between the original and the patched builds likely point out to issues
elsewhere, unless it's intentional modification of the helpers.

> > +
> >      echo "Perform full initial build with ${CPUS} CPU(s)..."
> >      build_full
> >  
> > diff --git a/livepatch-gcc b/livepatch-gcc
> > index fcad80551aa0..e4cb6fb59029 100755
> > --- a/livepatch-gcc
> > +++ b/livepatch-gcc
> > @@ -33,7 +33,6 @@ if [[ "$TOOLCHAINCMD" =~ $GCC_RE ]] ; then
> >              obj=$2
> >              [[ $2 = */.tmp_*.o ]] && obj=${2/.tmp_/}
> >              case "$(basename $obj)" in
> > -            version.o|\
> >              debug.o|\
> >              check.o|\
> 
> Tangential question.  check.o is excluded because it's a toolchain test,
> but any idea what debug.o is doing in this list?
> 
> It can't possibly be the debug.c I've recently added to x86 (which we'll
> want to be able to livepatch), so I guess it's got something to do the
> ARM debug.S's, but I can't see anything in those that are worthy of
> exemption either...

Hm, that comes from the first commit that imported the wrapper to the
repository, and at that point only x86 had livepatch support.

I'm tempted to think this was inherited from the original xsplice
tooling, and so debug.o needs to be removed from the list.

Thanks, Roger.