On Mon, Sep 02, 2024 at 08:59:43PM -0700, Josh Poimboeuf wrote:
> Hi,
> 
> Here's a new way to build livepatch modules called klp-build.
> 
> I started working on it when I realized that objtool already does 99% of
> the work needed for detecting function changes.
> 
> This is similar in concept to kpatch-build, but the implementation is
> much cleaner.
> 
> Personally I still have reservations about the "source-based" approach
> (klp-convert and friends), including the fragility and performance
> concerns of -flive-patching.  I would submit that klp-build might be
> considered the "official" way to make livepatch modules.
> 
> Please try it out and let me know what you think.  Based on v6.10.
> 
> Also avaiable at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git klp-build-rfc
> 
> [ ... snip ... ]

Hi Josh,

A few minor build complaints on my system:

  $ make tools/objtool/check.o
    CALL    scripts/checksyscalls.sh
    DESCEND objtool
    INSTALL libsubcmd_headers
    CC      /home/jolawren/src/linux/tools/objtool/check.o
  check.c: In function ‘is_livepatch_module’:
  check.c:661:16: error: implicit declaration of function ‘memmem’; did you mean ‘memset’? [-Werror=implicit-function-declaration]
    661 |         return memmem(sec->data->d_buf, sec_size(sec), "livepatch=Y", 12);
        |                ^~~~~~
        |                memset
  In function ‘is_first_func_insn’,
      inlined from ‘add_jump_destinations’ at check.c:1640:7,
      inlined from ‘decode_sections’ at check.c:2461:2:
  check.c:1498:17: error: ‘dest_insn’ may be used uninitialized [-Werror=maybe-uninitialized]
   1498 |         if (insn->offset == func->offset)
        |             ~~~~^~~~~~~~
  check.c: In function ‘decode_sections’:
  check.c:1522:37: note: ‘dest_insn’ was declared here
   1522 |                 struct instruction *dest_insn;
        |                                     ^~~~~~~~~
  cc1: all warnings being treated as errors
  make[4]: *** [/home/jolawren/src/linux/tools/build/Makefile.build:106: /home/jolawren/src/linux/tools/objtool/check.o] Error 1
  make[3]: *** [Makefile:78: /home/jolawren/src/linux/tools/objtool/objtool-in.o] Error 2
  make[2]: *** [Makefile:72: objtool] Error 2
  make[1]: *** [/home/jolawren/src/linux/Makefile:1378: tools/objtool] Error 2
  make: *** [Makefile:240: __sub-make] Error 2


  $ make tools/objtool/klp-diff.o
    CALL    scripts/checksyscalls.sh
    DESCEND objtool
    INSTALL libsubcmd_headers
    CC      /home/jolawren/src/linux/tools/objtool/check.o
  check.c: In function ‘is_livepatch_module’:
  check.c:661:16: error: implicit declaration of function ‘memmem’; did you mean ‘memset’? [-Werror=implicit-function-declaration]
    661 |         return memmem(sec->data->d_buf, sec_size(sec), "livepatch=Y", 12);
        |                ^~~~~~
        |                memset
  In function ‘is_first_func_insn’,
      inlined from ‘add_jump_destinations’ at check.c:1640:7,
      inlined from ‘decode_sections’ at check.c:2461:2:
  check.c:1498:17: error: ‘dest_insn’ may be used uninitialized [-Werror=maybe-uninitialized]
   1498 |         if (insn->offset == func->offset)
        |             ~~~~^~~~~~~~
  check.c: In function ‘decode_sections’:
  check.c:1522:37: note: ‘dest_insn’ was declared here
   1522 |                 struct instruction *dest_insn;
        |                                     ^~~~~~~~~
  cc1: all warnings being treated as errors
  make[4]: *** [/home/jolawren/src/linux/tools/build/Makefile.build:106: /home/jolawren/src/linux/tools/objtool/check.o] Error 1
  make[3]: *** [Makefile:78: /home/jolawren/src/linux/tools/objtool/objtool-in.o] Error 2
  make[2]: *** [Makefile:72: objtool] Error 2
  make[1]: *** [/home/jolawren/src/linux/Makefile:1378: tools/objtool] Error 2
  make: *** [Makefile:240: __sub-make] Error 2


In the case of klp-diff.c, adding #include <string.h> will provide the
memmem prototype.  For both files, I needed to #define _GNU_SOURCE for
that prototype though.

For the other complaint, I just set struct instruction *dest_insn = NULL
at the top of the for loop, but perhaps the code could be refactored to
clarify the situation to the compiler if you prefer not to do that.

More info:

  $ gcc --version
  gcc (GCC) 12.3.1 20230508 (Red Hat 12.3.1-1)

  And kernel .config is attached.

--
Joe