[PATCH v2 0/2] dwarves: enable-SHARED-and-STATIC-lib-creation

[PATCH v2 0/2] dwarves: enable-SHARED-and-STATIC-lib-creation

[Deepak Kumar Mishra]

CMakeLists.txt does not allow creation of static library and link applications
accordingly.

Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS

README file is also ammended to provide information regarding use of
-DBUILD_SHARED_LIBS while using cmake

By default if -DBUILD_SHARED_LIBS cmake option is not provided or
is not set to ON, shared libraries are created.

Deepak Kumar Mishra (2):
  CMakeLists.txt: enable SHARED and STATIC lib creation
  README: add documentation for -DBUILD_SHARED_LIBS

 CMakeLists.txt | 15 ++++++++++-----
 README         | 13 +++++++++++--
 2 files changed, 21 insertions(+), 7 deletions(-)

--
2.17.1

[PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Deepak Kumar Mishra]

CMakeLists.txt does not allow creation of static library and link applications
accordingly.

Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.

Ex:
cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..

Signed-off-by: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
---
 CMakeLists.txt | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5aada20..94f771d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -32,6 +32,11 @@ endif (NOT CMAKE_BUILD_TYPE)
 set(CMAKE_C_FLAGS_DEBUG "-Wall -Werror -ggdb -O0")
 set(CMAKE_C_FLAGS_RELEASE "-Wall -O2")
 
+if (NOT DEFINED BUILD_SHARED_LIBS)
+	set (BUILD_SHARED_LIBS ON)
+	message(STATUS "Setting BUILD_SHARED_LIBS = ${BUILD_SHARED_LIBS}")
+endif (NOT DEFINED BUILD_SHARED_LIBS)
+
 # Just for grepping, DWARVES_VERSION isn't used anywhere anymore
 # add_definitions(-D_GNU_SOURCE -DDWARVES_VERSION="v1.21")
 add_definitions(-D_GNU_SOURCE -DDWARVES_MAJOR_VERSION=1)
@@ -91,7 +96,7 @@ target_include_directories(bpf PRIVATE
 set(dwarves_LIB_SRCS dwarves.c dwarves_fprintf.c gobuffer.c strings.c
 		     ctf_encoder.c ctf_loader.c libctf.c btf_encoder.c btf_loader.c libbtf.c
 		     dwarf_loader.c dutil.c elf_symtab.c rbtree.c)
-add_library(dwarves SHARED ${dwarves_LIB_SRCS} $<TARGET_OBJECTS:bpf>)
+add_library(dwarves ${dwarves_LIB_SRCS} $<TARGET_OBJECTS:bpf>)
 set_target_properties(dwarves PROPERTIES VERSION 1.0.0 SOVERSION 1)
 set_target_properties(dwarves PROPERTIES INTERFACE_LINK_LIBRARIES "")
 target_include_directories(dwarves PRIVATE
@@ -99,12 +104,12 @@ target_include_directories(dwarves PRIVATE
 target_link_libraries(dwarves ${DWARF_LIBRARIES} ${ZLIB_LIBRARIES})
 
 set(dwarves_emit_LIB_SRCS dwarves_emit.c)
-add_library(dwarves_emit SHARED ${dwarves_emit_LIB_SRCS})
+add_library(dwarves_emit ${dwarves_emit_LIB_SRCS})
 set_target_properties(dwarves_emit PROPERTIES VERSION 1.0.0 SOVERSION 1)
 target_link_libraries(dwarves_emit dwarves)
 
 set(dwarves_reorganize_LIB_SRCS dwarves_reorganize.c)
-add_library(dwarves_reorganize SHARED ${dwarves_reorganize_LIB_SRCS})
+add_library(dwarves_reorganize ${dwarves_reorganize_LIB_SRCS})
 set_target_properties(dwarves_reorganize PROPERTIES VERSION 1.0.0 SOVERSION 1)
 target_link_libraries(dwarves_reorganize dwarves)
 
@@ -151,8 +156,8 @@ target_link_libraries(syscse dwarves)
 install(TARGETS codiff ctracer dtagnames pahole pdwtags
 		pfunct pglobal prefcnt scncopy syscse RUNTIME DESTINATION
 		${CMAKE_INSTALL_PREFIX}/bin)
-install(TARGETS dwarves LIBRARY DESTINATION ${LIB_INSTALL_DIR})
-install(TARGETS dwarves dwarves_emit dwarves_reorganize LIBRARY DESTINATION ${LIB_INSTALL_DIR})
+install(TARGETS dwarves LIBRARY DESTINATION ${LIB_INSTALL_DIR} ARCHIVE DESTINATION ${LIB_INSTALL_DIR})
+install(TARGETS dwarves dwarves_emit dwarves_reorganize LIBRARY DESTINATION ${LIB_INSTALL_DIR} ARCHIVE DESTINATION ${LIB_INSTALL_DIR})
 install(FILES dwarves.h dwarves_emit.h dwarves_reorganize.h
 	      dutil.h gobuffer.h list.h rbtree.h pahole_strings.h
 	      btf_encoder.h config.h ctf_encoder.h ctf.h
-- 
2.17.1

[PATCH v2 2/2] README: add documentation for -DBUILD_SHARED_LIBS

[Deepak Kumar Mishra]

added a section in README to give details about use of -DBUILD_SHARED_LIBS
cmake option and existing documentation reformatted to accommodate this.

Signed-off-by: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
---
 README | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/README b/README
index 576cd42..c9f1737 100644
--- a/README
+++ b/README
@@ -6,8 +6,17 @@ Build instructions:
 4. cmake -D__LIB=lib ..
 5. make install
 
-Default is to install to /usr/local, use -DCMAKE_INSTALL_PREFIX=
-when invoking cmake to specify another install location.
+cmake Options:
+  -DBUILD_SHARED_LIBS
+    By default SHARED libraries are created and applications are linked to it.
+    Use -DBUILD_SHARED_LIBS=OFF while invoking cmake to create STATIC libraries
+    and link applications to it.
+
+    Ex. cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
+
+  -DCMAKE_INSTALL_PREFIX
+    Default is to install to /usr/local, use -DCMAKE_INSTALL_PREFIX=
+    when invoking cmake to specify another install location.
 
 Known to work scenarios:
 
-- 
2.17.1

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Arnaldo Carvalho de Melo]

Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra escreveu:
> CMakeLists.txt does not allow creation of static library and link applications
> accordingly.
> 
> Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
> If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
> 
> Ex:
> cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..

Had to do some fixups due to a previous patch touching CMakeLists.txt,
please check below.

I tested it and added some performance notes.

Thanks!

- Arnaldo

commit aa2027708659f172780f85698f14303c7de6a1d2
Author: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
Date:   Tue Jun 8 00:50:13 2021 +0530

    CMakeLists.txt: Enable SHARED and STATIC lib creation
    
    CMakeLists.txt does not allow creation of static library and link applications
    accordingly.
    
    Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
    If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
    
    Ex:
    
      $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
      $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
    
    Committer testing:
    
    I had to fixup it a bit due to changes related to allowing building with
    libbpf-devel, test resuts:
    
    With the default, i.e. creating libdwarves.so and using it:
    
      ⬢[acme@toolbox pahole]$ rm -f vmlinux.btf ; perf stat -r5 pahole -j vmlinux.btf vmlinux && perf stat -r5 btfdiff vmlinux vmlinux.btf
    
       Performance counter stats for 'pahole -j vmlinux.btf vmlinux' (5 runs):
    
                8,612.69 msec task-clock:u              #    1.014 CPUs utilized            ( +-  0.68% )
                       0      context-switches:u        #    0.000 /sec
                       0      cpu-migrations:u          #    0.000 /sec
                 775,702      page-faults:u             #   91.330 K/sec                    ( +-  0.00% )
          33,720,048,514      cycles:u                  #    3.970 GHz                      ( +-  0.38% )  (83.33%)
             689,752,139      stalled-cycles-frontend:u #    2.05% frontend cycles idle     ( +-  2.57% )  (83.33%)
           5,070,723,369      stalled-cycles-backend:u  #   15.04% backend cycles idle      ( +-  1.20% )  (83.34%)
          77,270,640,084      instructions:u            #    2.29  insn per cycle
                                                        #    0.07  stalled cycles per insn  ( +-  0.01% )  (83.34%)
          18,164,028,242      branches:u                #    2.139 G/sec                    ( +-  0.04% )  (83.33%)
             150,194,338      branch-misses:u           #    0.83% of all branches          ( +-  0.17% )  (83.32%)
    
                  8.4979 +- 0.0590 seconds time elapsed  ( +-  0.69% )
    
       Performance counter stats for 'btfdiff vmlinux vmlinux.btf' (5 runs):
    
                7,001.79 msec task-clock:u              #    1.006 CPUs utilized            ( +-  1.02% )
                       0      context-switches:u        #    0.000 /sec
                       0      cpu-migrations:u          #    0.000 /sec
                 727,980      page-faults:u             #  104.736 K/sec                    ( +-  0.00% )
          26,912,307,448      cycles:u                  #    3.872 GHz                      ( +-  0.31% )  (83.33%)
             509,467,882      stalled-cycles-frontend:u #    1.90% frontend cycles idle     ( +-  2.44% )  (83.33%)
           3,602,777,263      stalled-cycles-backend:u  #   13.43% backend cycles idle      ( +-  1.34% )  (83.32%)
          66,192,815,701      instructions:u            #    2.47  insn per cycle
                                                        #    0.06  stalled cycles per insn  ( +-  0.01% )  (83.35%)
          15,753,663,095      branches:u                #    2.267 G/sec                    ( +-  0.02% )  (83.35%)
              98,345,696      branch-misses:u           #    0.62% of all branches          ( +-  0.14% )  (83.34%)
    
                  6.9586 +- 0.0720 seconds time elapsed  ( +-  1.03% )
    
      ⬢[acme@toolbox pahole]$
    
    Then building with:
    
      ⬢[acme@toolbox pahole]$ rm -rf build ; mkdir build ; cd build ; cmake -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=Release .. ; cd .. ; make -C build
      ⬢[acme@toolbox pahole]$ ldd build/pahole
            linux-vdso.so.1 (0x00007ffff0172000)
            libdw.so.1 => /lib64/libdw.so.1 (0x00007fdd3905d000)
            libelf.so.1 => /lib64/libelf.so.1 (0x00007fdd39042000)
            libz.so.1 => /lib64/libz.so.1 (0x00007fdd39028000)
            libc.so.6 => /lib64/libc.so.6 (0x00007fdd38e59000)
            libdl.so.2 => /lib64/libdl.so.2 (0x00007fdd38e52000)
            libzstd.so.1 => /lib64/libzstd.so.1 (0x00007fdd38d5c000)
            liblzma.so.5 => /lib64/liblzma.so.5 (0x00007fdd38d2e000)
            libbz2.so.1 => /lib64/libbz2.so.1 (0x00007fdd38d1b000)
            libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fdd38cfa000)
            /lib64/ld-linux-x86-64.so.2 (0x00007fdd390ff000)
      ⬢[acme@toolbox pahole]$
    
    We get some performance improvement:
    
    - First is encoding detached BTF, i.e. read DWARF, generate BTF.
    - Second is about loading both DWARF and BTF, producing output for both, that must match.
    
      ⬢[acme@toolbox pahole]$ rm -f vmlinux.btf ; perf stat -r5 pahole -j vmlinux.btf vmlinux && perf stat -r5 btfdiff vmlinux vmlinux.btf
    
       Performance counter stats for 'pahole -j vmlinux.btf vmlinux' (5 runs):
    
                8,566.34 msec task-clock:u              #    1.025 CPUs utilized            ( +-  0.85% )
                       0      context-switches:u        #    0.000 /sec
                       0      cpu-migrations:u          #    0.000 /sec
                 775,685      page-faults:u             #   92.865 K/sec                    ( +-  0.00% )
          33,333,991,512      cycles:u                  #    3.991 GHz                      ( +-  0.23% )  (83.34%)
             799,187,919      stalled-cycles-frontend:u #    2.42% frontend cycles idle     ( +-  1.95% )  (83.33%)
           5,157,722,792      stalled-cycles-backend:u  #   15.61% backend cycles idle      ( +-  1.25% )  (83.33%)
          76,273,972,066      instructions:u            #    2.31  insn per cycle
                                                        #    0.07  stalled cycles per insn  ( +-  0.03% )  (83.33%)
          17,843,388,470      branches:u                #    2.136 G/sec                    ( +-  0.05% )  (83.33%)
             150,507,690      branch-misses:u           #    0.84% of all branches          ( +-  0.46% )  (83.33%)
    
                  8.3561 +- 0.0729 seconds time elapsed  ( +-  0.87% )
    
       Performance counter stats for 'btfdiff vmlinux vmlinux.btf' (5 runs):
    
                6,685.39 msec task-clock:u              #    0.986 CPUs utilized            ( +-  0.54% )
                       0      context-switches:u        #    0.000 /sec
                       0      cpu-migrations:u          #    0.000 /sec
                 727,931      page-faults:u             #  107.475 K/sec                    ( +-  0.00% )
          26,149,371,139      cycles:u                  #    3.861 GHz                      ( +-  0.15% )  (83.36%)
             525,918,808      stalled-cycles-frontend:u #    2.00% frontend cycles idle     ( +-  2.96% )  (83.33%)
           3,547,589,546      stalled-cycles-backend:u  #   13.51% backend cycles idle      ( +-  1.28% )  (83.35%)
          65,389,507,702      instructions:u            #    2.49  insn per cycle
                                                        #    0.06  stalled cycles per insn  ( +-  0.03% )  (83.29%)
          15,553,175,605      branches:u                #    2.296 G/sec                    ( +-  0.02% )  (83.34%)
              97,812,971      branch-misses:u           #    0.63% of all branches          ( +-  0.23% )  (83.34%)
    
                  6.7784 +- 0.0359 seconds time elapsed  ( +-  0.53% )
    
      ⬢[acme@toolbox pahole]$
    
    Signed-off-by: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
    Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    Cc: Qais Yousef <qais.yousef@arm.com>
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/CMakeLists.txt b/CMakeLists.txt
index df16b8ec3031f3e6..669ea59167c7ef65 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -47,6 +47,11 @@ endif (NOT CMAKE_BUILD_TYPE)
 set(CMAKE_C_FLAGS_DEBUG "-Wall -Werror -ggdb -O0")
 set(CMAKE_C_FLAGS_RELEASE "-Wall -O2")
 
+if (NOT DEFINED BUILD_SHARED_LIBS)
+	set (BUILD_SHARED_LIBS ON)
+	message(STATUS "Setting BUILD_SHARED_LIBS = ${BUILD_SHARED_LIBS}")
+endif (NOT DEFINED BUILD_SHARED_LIBS)
+
 # Just for grepping, DWARVES_VERSION isn't used anywhere anymore
 # add_definitions(-D_GNU_SOURCE -DDWARVES_VERSION="v1.21")
 add_definitions(-D_GNU_SOURCE -DDWARVES_MAJOR_VERSION=1)
@@ -111,18 +116,18 @@ set(dwarves_LIB_SRCS dwarves.c dwarves_fprintf.c gobuffer.c strings.c
 if (NOT LIBBPF_FOUND)
 	list(APPEND dwarves_LIB_SRCS $<TARGET_OBJECTS:bpf>)
 endif()
-add_library(dwarves SHARED ${dwarves_LIB_SRCS})
+add_library(dwarves ${dwarves_LIB_SRCS})
 set_target_properties(dwarves PROPERTIES VERSION 1.0.0 SOVERSION 1)
 set_target_properties(dwarves PROPERTIES INTERFACE_LINK_LIBRARIES "")
 target_link_libraries(dwarves ${DWARF_LIBRARIES} ${ZLIB_LIBRARIES} ${LIBBPF_LIBRARIES})
 
 set(dwarves_emit_LIB_SRCS dwarves_emit.c)
-add_library(dwarves_emit SHARED ${dwarves_emit_LIB_SRCS})
+add_library(dwarves_emit ${dwarves_emit_LIB_SRCS})
 set_target_properties(dwarves_emit PROPERTIES VERSION 1.0.0 SOVERSION 1)
 target_link_libraries(dwarves_emit dwarves)
 
 set(dwarves_reorganize_LIB_SRCS dwarves_reorganize.c)
-add_library(dwarves_reorganize SHARED ${dwarves_reorganize_LIB_SRCS})
+add_library(dwarves_reorganize ${dwarves_reorganize_LIB_SRCS})
 set_target_properties(dwarves_reorganize PROPERTIES VERSION 1.0.0 SOVERSION 1)
 target_link_libraries(dwarves_reorganize dwarves)
 
@@ -169,8 +174,8 @@ target_link_libraries(syscse dwarves)
 install(TARGETS codiff ctracer dtagnames pahole pdwtags
 		pfunct pglobal prefcnt scncopy syscse RUNTIME DESTINATION
 		${CMAKE_INSTALL_PREFIX}/bin)
-install(TARGETS dwarves LIBRARY DESTINATION ${LIB_INSTALL_DIR})
-install(TARGETS dwarves dwarves_emit dwarves_reorganize LIBRARY DESTINATION ${LIB_INSTALL_DIR})
+install(TARGETS dwarves LIBRARY DESTINATION ${LIB_INSTALL_DIR} ARCHIVE DESTINATION ${LIB_INSTALL_DIR})
+install(TARGETS dwarves dwarves_emit dwarves_reorganize LIBRARY DESTINATION ${LIB_INSTALL_DIR} ARCHIVE DESTINATION ${LIB_INSTALL_DIR})
 install(FILES dwarves.h dwarves_emit.h dwarves_reorganize.h
 	      dutil.h gobuffer.h list.h rbtree.h pahole_strings.h
 	      btf_encoder.h config.h ctf_encoder.h ctf.h

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Andrii Nakryiko]

On Thu, Jun 10, 2021 at 10:31 AM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra escreveu:
> > CMakeLists.txt does not allow creation of static library and link applications
> > accordingly.
> >
> > Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
> > If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
> >
> > Ex:
> > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
>
> Had to do some fixups due to a previous patch touching CMakeLists.txt,
> please check below.
>
> I tested it and added some performance notes.

Hey Arnaldo, Deepak,

I think this commit actually breaks libbpf's CI (see [0]) and my local
setup as well (see output below). It seems like now we are using
system-wide libbpf headers, while still building local libbpf sources.
This is pretty bad because system-wide headers might be too old or
just missing.

Is it possible to make sure that we always use local libbpf headers
when building pahole with libbpf built from sources (the default case,
right?). It's also important to use UAPI headers distributed with
libbpf when building libbpf itself, I don't know if that's what is
done right now or not.

Note how libbpf CI case shows that system-wide bpf/btf.h is not
available at all because we don't have system-wide libbpf installed.
In my local case, you can see that my system-wide header is outdated
and doesn't have BTF_LITTLE_ENDIAN/BTF_BIG_ENDIAN constants defined in
libbpf.h.

BTW, I tried -D__LIB=lib -DBUILD_SHARED_LIBS=OFF options and they
didn't help. Maybe I'm doing something wrong.

  [0] https://travis-ci.com/github/kernel-patches/bpf/builds/228673352


$ make -j60
-- Setting BUILD_SHARED_LIBS = ON
-- Checking availability of DWARF and ELF development libraries
-- Checking availability of DWARF and ELF development libraries - done
-- Configuring done
-- Generating done
-- Build files have been written to: /home/andriin/local/pahole/build

....

/home/andriin/local/pahole/btf_encoder.c:900:28: error:
‘BTF_LITTLE_ENDIAN’ undeclared (first use in this function)
   btf__set_endianness(btf, BTF_LITTLE_ENDIAN);
                            ^
/home/andriin/local/pahole/btf_encoder.c:900:28: note: each undeclared
identifier is reported only once for each function it appears in
/home/andriin/local/pahole/btf_encoder.c:903:28: error:
‘BTF_BIG_ENDIAN’ undeclared (first use in this function)
   btf__set_endianness(btf, BTF_BIG_ENDIAN);
                            ^
...


>
> Thanks!
>
> - Arnaldo
>
> commit aa2027708659f172780f85698f14303c7de6a1d2
> Author: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
> Date:   Tue Jun 8 00:50:13 2021 +0530
>
>     CMakeLists.txt: Enable SHARED and STATIC lib creation
>
>     CMakeLists.txt does not allow creation of static library and link applications
>     accordingly.
>
>     Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
>     If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
>
>     Ex:
>
>       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
>       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
>

[...]

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Andrii Nakryiko]

On Fri, Jun 11, 2021 at 12:34 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Jun 10, 2021 at 10:31 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra escreveu:
> > > CMakeLists.txt does not allow creation of static library and link applications
> > > accordingly.
> > >
> > > Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
> > > If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
> > >
> > > Ex:
> > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> >
> > Had to do some fixups due to a previous patch touching CMakeLists.txt,
> > please check below.
> >
> > I tested it and added some performance notes.
>
> Hey Arnaldo, Deepak,

Should have added Luca as well, it might be his patch (ae2581647e84
("libbpf: Allow to use packaged version")) that might have introduced
this breakage in the first place.

>
> I think this commit actually breaks libbpf's CI (see [0]) and my local
> setup as well (see output below). It seems like now we are using
> system-wide libbpf headers, while still building local libbpf sources.
> This is pretty bad because system-wide headers might be too old or
> just missing.
>
> Is it possible to make sure that we always use local libbpf headers
> when building pahole with libbpf built from sources (the default case,
> right?). It's also important to use UAPI headers distributed with
> libbpf when building libbpf itself, I don't know if that's what is
> done right now or not.
>
> Note how libbpf CI case shows that system-wide bpf/btf.h is not
> available at all because we don't have system-wide libbpf installed.
> In my local case, you can see that my system-wide header is outdated
> and doesn't have BTF_LITTLE_ENDIAN/BTF_BIG_ENDIAN constants defined in
> libbpf.h.
>
> BTW, I tried -D__LIB=lib -DBUILD_SHARED_LIBS=OFF options and they
> didn't help. Maybe I'm doing something wrong.
>
>   [0] https://travis-ci.com/github/kernel-patches/bpf/builds/228673352
>
>
> $ make -j60
> -- Setting BUILD_SHARED_LIBS = ON
> -- Checking availability of DWARF and ELF development libraries
> -- Checking availability of DWARF and ELF development libraries - done
> -- Configuring done
> -- Generating done
> -- Build files have been written to: /home/andriin/local/pahole/build
>
> ....
>
> /home/andriin/local/pahole/btf_encoder.c:900:28: error:
> ‘BTF_LITTLE_ENDIAN’ undeclared (first use in this function)
>    btf__set_endianness(btf, BTF_LITTLE_ENDIAN);
>                             ^
> /home/andriin/local/pahole/btf_encoder.c:900:28: note: each undeclared
> identifier is reported only once for each function it appears in
> /home/andriin/local/pahole/btf_encoder.c:903:28: error:
> ‘BTF_BIG_ENDIAN’ undeclared (first use in this function)
>    btf__set_endianness(btf, BTF_BIG_ENDIAN);
>                             ^
> ...
>
>
> >
> > Thanks!
> >
> > - Arnaldo
> >
> > commit aa2027708659f172780f85698f14303c7de6a1d2
> > Author: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
> > Date:   Tue Jun 8 00:50:13 2021 +0530
> >
> >     CMakeLists.txt: Enable SHARED and STATIC lib creation
> >
> >     CMakeLists.txt does not allow creation of static library and link applications
> >     accordingly.
> >
> >     Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
> >     If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
> >
> >     Ex:
> >
> >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> >
>
> [...]

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Arnaldo Carvalho de Melo]

Em Fri, Jun 11, 2021 at 12:34:13PM -0700, Andrii Nakryiko escreveu:
> On Thu, Jun 10, 2021 at 10:31 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra escreveu:
> > > CMakeLists.txt does not allow creation of static library and link applications
> > > accordingly.
> > >
> > > Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
> > > If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
> > >
> > > Ex:
> > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> >
> > Had to do some fixups due to a previous patch touching CMakeLists.txt,
> > please check below.
> >
> > I tested it and added some performance notes.
> 
> Hey Arnaldo, Deepak,
> 
> I think this commit actually breaks libbpf's CI (see [0]) and my local
> setup as well (see output below). It seems like now we are using
> system-wide libbpf headers, while still building local libbpf sources.
> This is pretty bad because system-wide headers might be too old or
> just missing.

I can't check this right now, but isn't this related to this one
instead?

commit ae2581647e84948810ba209f3891359dd4540110 (quaco/master, quaco/HEAD, acme/tmp.master)
Author: Luca Boccassi <bluca@debian.org>
Date:   Mon Jan 4 22:16:22 2021 +0000

    libbpf: Allow to use packaged version

    Add a new CMake option, LIBBPF_EMBEDDED, to switch between the embedded
    version and the system version (searched via pkg-config) of libbpf. Set
    the embedded version as the default.

 -------

I can't look at this right now, will try probably tomorrow.

Andrii, I would love to be able to stage this somewhere, like I did with
tmp.master, so that it could go thru your CI before I moved to master,
is that possible?

- Arnaldo
 
> Is it possible to make sure that we always use local libbpf headers
> when building pahole with libbpf built from sources (the default case,
> right?). It's also important to use UAPI headers distributed with
> libbpf when building libbpf itself, I don't know if that's what is
> done right now or not.
> 
> Note how libbpf CI case shows that system-wide bpf/btf.h is not
> available at all because we don't have system-wide libbpf installed.
> In my local case, you can see that my system-wide header is outdated
> and doesn't have BTF_LITTLE_ENDIAN/BTF_BIG_ENDIAN constants defined in
> libbpf.h.
> 
> BTW, I tried -D__LIB=lib -DBUILD_SHARED_LIBS=OFF options and they
> didn't help. Maybe I'm doing something wrong.
> 
>   [0] https://travis-ci.com/github/kernel-patches/bpf/builds/228673352
> 
> 
> $ make -j60
> -- Setting BUILD_SHARED_LIBS = ON
> -- Checking availability of DWARF and ELF development libraries
> -- Checking availability of DWARF and ELF development libraries - done
> -- Configuring done
> -- Generating done
> -- Build files have been written to: /home/andriin/local/pahole/build
> 
> ....
> 
> /home/andriin/local/pahole/btf_encoder.c:900:28: error:
> ‘BTF_LITTLE_ENDIAN’ undeclared (first use in this function)
>    btf__set_endianness(btf, BTF_LITTLE_ENDIAN);
>                             ^
> /home/andriin/local/pahole/btf_encoder.c:900:28: note: each undeclared
> identifier is reported only once for each function it appears in
> /home/andriin/local/pahole/btf_encoder.c:903:28: error:
> ‘BTF_BIG_ENDIAN’ undeclared (first use in this function)
>    btf__set_endianness(btf, BTF_BIG_ENDIAN);
>                             ^
> ...
> 
> 
> >
> > Thanks!
> >
> > - Arnaldo
> >
> > commit aa2027708659f172780f85698f14303c7de6a1d2
> > Author: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
> > Date:   Tue Jun 8 00:50:13 2021 +0530
> >
> >     CMakeLists.txt: Enable SHARED and STATIC lib creation
> >
> >     CMakeLists.txt does not allow creation of static library and link applications
> >     accordingly.
> >
> >     Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
> >     If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
> >
> >     Ex:
> >
> >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> >
> 
> [...]

-- 

- Arnaldo

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Andrii Nakryiko]

On Fri, Jun 11, 2021 at 1:00 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Fri, Jun 11, 2021 at 12:34:13PM -0700, Andrii Nakryiko escreveu:
> > On Thu, Jun 10, 2021 at 10:31 AM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra escreveu:
> > > > CMakeLists.txt does not allow creation of static library and link applications
> > > > accordingly.
> > > >
> > > > Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
> > > > If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
> > > >
> > > > Ex:
> > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> > >
> > > Had to do some fixups due to a previous patch touching CMakeLists.txt,
> > > please check below.
> > >
> > > I tested it and added some performance notes.
> >
> > Hey Arnaldo, Deepak,
> >
> > I think this commit actually breaks libbpf's CI (see [0]) and my local
> > setup as well (see output below). It seems like now we are using
> > system-wide libbpf headers, while still building local libbpf sources.
> > This is pretty bad because system-wide headers might be too old or
> > just missing.
>
> I can't check this right now, but isn't this related to this one
> instead?

Heh, I beat you by 5 minutes ;)

>
> commit ae2581647e84948810ba209f3891359dd4540110 (quaco/master, quaco/HEAD, acme/tmp.master)
> Author: Luca Boccassi <bluca@debian.org>
> Date:   Mon Jan 4 22:16:22 2021 +0000
>
>     libbpf: Allow to use packaged version
>
>     Add a new CMake option, LIBBPF_EMBEDDED, to switch between the embedded
>     version and the system version (searched via pkg-config) of libbpf. Set
>     the embedded version as the default.
>
>  -------
>
> I can't look at this right now, will try probably tomorrow.
>
> Andrii, I would love to be able to stage this somewhere, like I did with
> tmp.master, so that it could go thru your CI before I moved to master,
> is that possible?

Yes, absolutely, we can pick whatever branch and use that to checkout
and build pahole. It would be great, though, if you can keep an eye on
kernel CI and/or libbpf CI breakages when you are pushing new changes
to pahole. That would save everyone time and will shorten the downtime
for our CIs.

Here are the links where all the builds can be seen in real-time:

  - kernel CI: https://travis-ci.com/github/kernel-patches/bpf/pull_requests
  - libbpf CI: https://travis-ci.com/github/libbpf/libbpf


Let me know which branch we should hard-code for staging.

>
> - Arnaldo
>
> > Is it possible to make sure that we always use local libbpf headers
> > when building pahole with libbpf built from sources (the default case,
> > right?). It's also important to use UAPI headers distributed with
> > libbpf when building libbpf itself, I don't know if that's what is
> > done right now or not.
> >
> > Note how libbpf CI case shows that system-wide bpf/btf.h is not
> > available at all because we don't have system-wide libbpf installed.
> > In my local case, you can see that my system-wide header is outdated
> > and doesn't have BTF_LITTLE_ENDIAN/BTF_BIG_ENDIAN constants defined in
> > libbpf.h.
> >
> > BTW, I tried -D__LIB=lib -DBUILD_SHARED_LIBS=OFF options and they
> > didn't help. Maybe I'm doing something wrong.
> >
> >   [0] https://travis-ci.com/github/kernel-patches/bpf/builds/228673352
> >
> >
> > $ make -j60
> > -- Setting BUILD_SHARED_LIBS = ON
> > -- Checking availability of DWARF and ELF development libraries
> > -- Checking availability of DWARF and ELF development libraries - done
> > -- Configuring done
> > -- Generating done
> > -- Build files have been written to: /home/andriin/local/pahole/build
> >
> > ....
> >
> > /home/andriin/local/pahole/btf_encoder.c:900:28: error:
> > ‘BTF_LITTLE_ENDIAN’ undeclared (first use in this function)
> >    btf__set_endianness(btf, BTF_LITTLE_ENDIAN);
> >                             ^
> > /home/andriin/local/pahole/btf_encoder.c:900:28: note: each undeclared
> > identifier is reported only once for each function it appears in
> > /home/andriin/local/pahole/btf_encoder.c:903:28: error:
> > ‘BTF_BIG_ENDIAN’ undeclared (first use in this function)
> >    btf__set_endianness(btf, BTF_BIG_ENDIAN);
> >                             ^
> > ...
> >
> >
> > >
> > > Thanks!
> > >
> > > - Arnaldo
> > >
> > > commit aa2027708659f172780f85698f14303c7de6a1d2
> > > Author: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
> > > Date:   Tue Jun 8 00:50:13 2021 +0530
> > >
> > >     CMakeLists.txt: Enable SHARED and STATIC lib creation
> > >
> > >     CMakeLists.txt does not allow creation of static library and link applications
> > >     accordingly.
> > >
> > >     Creation of SHARED and STATIC should be allowed using -DBUILD_SHARED_LIBS
> > >     If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt sets it to ON.
> > >
> > >     Ex:
> > >
> > >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> > >
> >
> > [...]
>
> --
>
> - Arnaldo

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 6556 bytes --]

On Fri, 2021-06-11 at 13:08 -0700, Andrii Nakryiko wrote:
On Fri, Jun 11, 2021 at 1:00 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
> 
> Em Fri, Jun 11, 2021 at 12:34:13PM -0700, Andrii Nakryiko escreveu:
> > On Thu, Jun 10, 2021 at 10:31 AM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > > 
> > > Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra
> > > escreveu:
> > > > CMakeLists.txt does not allow creation of static library and
> > > > link applications
> > > > accordingly.
> > > > 
> > > > Creation of SHARED and STATIC should be allowed using -
> > > > DBUILD_SHARED_LIBS
> > > > If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt
> > > > sets it to ON.
> > > > 
> > > > Ex:
> > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> > > 
> > > Had to do some fixups due to a previous patch touching
> > > CMakeLists.txt,
> > > please check below.
> > > 
> > > I tested it and added some performance notes.
> > 
> > Hey Arnaldo, Deepak,
> > 
> > I think this commit actually breaks libbpf's CI (see [0]) and my
> > local
> > setup as well (see output below). It seems like now we are using
> > system-wide libbpf headers, while still building local libbpf
> > sources.
> > This is pretty bad because system-wide headers might be too old
> > or
> > just missing.
> 
> I can't check this right now, but isn't this related to this one
> instead?

Heh, I beat you by 5 minutes ;)


Hi,

This should not be the case - the local paths are added to CMake and
should win, unless something is going wrong - which is of course
possible. A quick build of the current tip of the master branch would
seem to confirm things are working - building with -
DLIBBPF_EMBEDDED=off (which does force to use the system library, and
defaults to on) the build fails, while building without any options on
a new tree the build succeeds.

I'll fetch the script and try to reproduce, as it might be using other
options - I assume it's this one, right?

https://github.com/libbpf/libbpf/blob/master/travis-ci/vmtest/build_pahole.sh

> > 
> > commit ae2581647e84948810ba209f3891359dd4540110 (quaco/master,
> > quaco/HEAD, acme/tmp.master)
> > Author: Luca Boccassi <bluca@debian.org>
> > Date:   Mon Jan 4 22:16:22 2021 +0000
> > 
> >     libbpf: Allow to use packaged version
> > 
> >     Add a new CMake option, LIBBPF_EMBEDDED, to switch between the
> > embedded
> >     version and the system version (searched via pkg-config) of
> > libbpf. Set
> >     the embedded version as the default.
> > 
> >  -------
> > 
> > I can't look at this right now, will try probably tomorrow.
> > 
> > Andrii, I would love to be able to stage this somewhere, like I did
> > with
> > tmp.master, so that it could go thru your CI before I moved to
> > master,
> > is that possible?
> 
> Yes, absolutely, we can pick whatever branch and use that to checkout
> and build pahole. It would be great, though, if you can keep an eye
> on
> kernel CI and/or libbpf CI breakages when you are pushing new changes
> to pahole. That would save everyone time and will shorten the
> downtime
> for our CIs.
> 
> Here are the links where all the builds can be seen in real-time:
> 
>   - kernel CI: 
> https://travis-ci.com/github/kernel-patches/bpf/pull_requests
>   - libbpf CI: https://travis-ci.com/github/libbpf/libbpf
> 
> 
> Let me know which branch we should hard-code for staging.
> 
> > 
> > - Arnaldo
> > 
> > > Is it possible to make sure that we always use local libbpf
> > > headers
> > > when building pahole with libbpf built from sources (the default
> > > case,
> > > right?). It's also important to use UAPI headers distributed with
> > > libbpf when building libbpf itself, I don't know if that's what
> > > is
> > > done right now or not.
> > > 
> > > Note how libbpf CI case shows that system-wide bpf/btf.h is not
> > > available at all because we don't have system-wide libbpf
> > > installed.
> > > In my local case, you can see that my system-wide header is
> > > outdated
> > > and doesn't have BTF_LITTLE_ENDIAN/BTF_BIG_ENDIAN constants
> > > defined in
> > > libbpf.h.
> > > 
> > > BTW, I tried -D__LIB=lib -DBUILD_SHARED_LIBS=OFF options and they
> > > didn't help. Maybe I'm doing something wrong.
> > > 
> > >   [0] 
> > > https://travis-ci.com/github/kernel-patches/bpf/builds/228673352
> > > 
> > > 
> > > $ make -j60
> > > -- Setting BUILD_SHARED_LIBS = ON
> > > -- Checking availability of DWARF and ELF development libraries
> > > -- Checking availability of DWARF and ELF development libraries -
> > > done
> > > -- Configuring done
> > > -- Generating done
> > > -- Build files have been written to:
> > > /home/andriin/local/pahole/build
> > > 
> > > ....
> > > 
> > > /home/andriin/local/pahole/btf_encoder.c:900:28: error:
> > > ‘BTF_LITTLE_ENDIAN’ undeclared (first use in this function)
> > >    btf__set_endianness(btf, BTF_LITTLE_ENDIAN);
> > >                             ^
> > > /home/andriin/local/pahole/btf_encoder.c:900:28: note: each
> > > undeclared
> > > identifier is reported only once for each function it appears in
> > > /home/andriin/local/pahole/btf_encoder.c:903:28: error:
> > > ‘BTF_BIG_ENDIAN’ undeclared (first use in this function)
> > >    btf__set_endianness(btf, BTF_BIG_ENDIAN);
> > >                             ^
> > > ...
> > > 
> > > 
> > > > 
> > > > Thanks!
> > > > 
> > > > - Arnaldo
> > > > 
> > > > commit aa2027708659f172780f85698f14303c7de6a1d2
> > > > Author: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
> > > > Date:   Tue Jun 8 00:50:13 2021 +0530
> > > > 
> > > >     CMakeLists.txt: Enable SHARED and STATIC lib creation
> > > > 
> > > >     CMakeLists.txt does not allow creation of static library
> > > > and link applications
> > > >     accordingly.
> > > > 
> > > >     Creation of SHARED and STATIC should be allowed using -
> > > > DBUILD_SHARED_LIBS
> > > >     If -DBUILD_SHARED_LIBS option is not supplied,
> > > > CMakeLists.txt sets it to ON.
> > > > 
> > > >     Ex:
> > > > 
> > > >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> > > > 
> > > 
> > > [...]
> > 
> > --
> > 
> > - Arnaldo

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]

On Fri, 2021-06-11 at 23:17 +0100, Luca Boccassi wrote:
> On Fri, 2021-06-11 at 13:08 -0700, Andrii Nakryiko wrote:
> On Fri, Jun 11, 2021 at 1:00 PM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> > 
> > Em Fri, Jun 11, 2021 at 12:34:13PM -0700, Andrii Nakryiko escreveu:
> > > On Thu, Jun 10, 2021 at 10:31 AM Arnaldo Carvalho de Melo
> > > <arnaldo.melo@gmail.com> wrote:
> > > > 
> > > > Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra
> > > > escreveu:
> > > > > CMakeLists.txt does not allow creation of static library and
> > > > > link applications
> > > > > accordingly.
> > > > > 
> > > > > Creation of SHARED and STATIC should be allowed using -
> > > > > DBUILD_SHARED_LIBS
> > > > > If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt
> > > > > sets it to ON.
> > > > > 
> > > > > Ex:
> > > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> > > > 
> > > > Had to do some fixups due to a previous patch touching
> > > > CMakeLists.txt,
> > > > please check below.
> > > > 
> > > > I tested it and added some performance notes.
> > > 
> > > Hey Arnaldo, Deepak,
> > > 
> > > I think this commit actually breaks libbpf's CI (see [0]) and my
> > > local
> > > setup as well (see output below). It seems like now we are using
> > > system-wide libbpf headers, while still building local libbpf
> > > sources.
> > > This is pretty bad because system-wide headers might be too old
> > > or
> > > just missing.
> > 
> > I can't check this right now, but isn't this related to this one
> > instead?
> 
> Heh, I beat you by 5 minutes ;)
> 
> 
> Hi,
> 
> This should not be the case - the local paths are added to CMake and
> should win, unless something is going wrong - which is of course
> possible. A quick build of the current tip of the master branch would
> seem to confirm things are working - building with -
> DLIBBPF_EMBEDDED=off (which does force to use the system library, and
> defaults to on) the build fails, while building without any options
> on
> a new tree the build succeeds.
> 
> I'll fetch the script and try to reproduce, as it might be using
> other
> options - I assume it's this one, right?
> 
> https://github.com/libbpf/libbpf/blob/master/travis-ci/vmtest/build_pahole.sh

Right, so this script is using the CMake option "-D__LIB=lib", and the
issue is reproducible when that happens, on a clean tree without any
other options, which is indeed wrong. I was not aware of that so didn't
test with it, apologies.
I'll see if I can come up with a fix, if noone else beats me to it.

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 3150 bytes --]

On Fri, 2021-06-11 at 23:20 +0100, Luca Boccassi wrote:
> On Fri, 2021-06-11 at 23:17 +0100, Luca Boccassi wrote:
> > On Fri, 2021-06-11 at 13:08 -0700, Andrii Nakryiko wrote:
> > On Fri, Jun 11, 2021 at 1:00 PM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > > 
> > > Em Fri, Jun 11, 2021 at 12:34:13PM -0700, Andrii Nakryiko
> > > escreveu:
> > > > On Thu, Jun 10, 2021 at 10:31 AM Arnaldo Carvalho de Melo
> > > > <arnaldo.melo@gmail.com> wrote:
> > > > > 
> > > > > Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra
> > > > > escreveu:
> > > > > > CMakeLists.txt does not allow creation of static library
> > > > > > and
> > > > > > link applications
> > > > > > accordingly.
> > > > > > 
> > > > > > Creation of SHARED and STATIC should be allowed using -
> > > > > > DBUILD_SHARED_LIBS
> > > > > > If -DBUILD_SHARED_LIBS option is not supplied,
> > > > > > CMakeLists.txt
> > > > > > sets it to ON.
> > > > > > 
> > > > > > Ex:
> > > > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> > > > > 
> > > > > Had to do some fixups due to a previous patch touching
> > > > > CMakeLists.txt,
> > > > > please check below.
> > > > > 
> > > > > I tested it and added some performance notes.
> > > > 
> > > > Hey Arnaldo, Deepak,
> > > > 
> > > > I think this commit actually breaks libbpf's CI (see [0]) and
> > > > my
> > > > local
> > > > setup as well (see output below). It seems like now we are
> > > > using
> > > > system-wide libbpf headers, while still building local libbpf
> > > > sources.
> > > > This is pretty bad because system-wide headers might be too old
> > > > or
> > > > just missing.
> > > 
> > > I can't check this right now, but isn't this related to this one
> > > instead?
> > 
> > Heh, I beat you by 5 minutes ;)
> > 
> > 
> > Hi,
> > 
> > This should not be the case - the local paths are added to CMake
> > and
> > should win, unless something is going wrong - which is of course
> > possible. A quick build of the current tip of the master branch
> > would
> > seem to confirm things are working - building with -
> > DLIBBPF_EMBEDDED=off (which does force to use the system library,
> > and
> > defaults to on) the build fails, while building without any options
> > on
> > a new tree the build succeeds.
> > 
> > I'll fetch the script and try to reproduce, as it might be using
> > other
> > options - I assume it's this one, right?
> > 
> > https://github.com/libbpf/libbpf/blob/master/travis-ci/vmtest/build_pahole.sh
> 
> Right, so this script is using the CMake option "-D__LIB=lib", and
> the
> issue is reproducible when that happens, on a clean tree without any
> other options, which is indeed wrong. I was not aware of that so
> didn't
> test with it, apologies.
> I'll see if I can come up with a fix, if noone else beats me to it.
> 
Actually that was my mistake, used the wrong build tree (sorry, it's
late!). I can however reproduce the issue in a chroot running the
libbpf CI script. Still looking.

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Dominique Martinet]

Luca Boccassi wrote on Fri, Jun 11, 2021 at 11:45:25PM +0100:
> Actually that was my mistake, used the wrong build tree (sorry, it's
> late!). I can however reproduce the issue in a chroot running the
> libbpf CI script. Still looking.

with the ci script I get

$ /usr/lib64/ccache/cc -DDWARVES_MAJOR_VERSION=1 -DDWARVES_MINOR_VERSION=21 -D_GNU_SOURCE -Ddwarves_EXPORTS -I/path/to/pahole/build -I/path/to/pahole -I/path/to/pahole/lib/include -I/path/to/pahole/lib/bpf/include/uapi -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -g -DNDEBUG -fPIC -MD -MT CMakeFiles/dwarves.dir/btf_encoder.c.o -MF CMakeFiles/dwarves.dir/btf_encoder.c.o.d -o CMakeFiles/dwarves.dir/btf_encoder.c.o -c /path/to/pahole/btf_encoder.c
/path/to/pahole/btf_encoder.c: In function ‘btf_encoder__add_float’:
/path/to/pahole/btf_encoder.c:224:22: warning: implicit declaration of function ‘btf__add_float’; did you mean ‘btf__add_var’? [-Wimplicit-function-declaration]
  224 |         int32_t id = btf__add_float(encoder->btf, name, BITS_ROUNDUP_BYTES(bt->bit_size));
      |                      ^~~~~~~~~~~~~~
      |                      btf__add_var



with btf__add_float defined in .../pahole/lib/bpf/src/btf.h
and btf_encoder.c including linux/btf.h


changing btf_loader.c to include bpf/btf.h instead fixes the issue for me:

diff --git a/btf_loader.c b/btf_loader.c
index 75ec674b3b3e..272c73bca7fe 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -20,7 +20,7 @@
 #include <string.h>
 #include <limits.h>
 #include <libgen.h>
-#include <linux/btf.h>
+#include <bpf/btf.h>
 #include <bpf/libbpf.h>
 #include <zlib.h>
 


-- 
Dominique

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Dominique Martinet]

Dominique Martinet wrote on Sat, Jun 12, 2021 at 07:53:51AM +0900:
> changing btf_loader.c to include bpf/btf.h instead fixes the issue for me:

Sorry that was a bit quick on my part, it's not the proper solution if
there are worse conflicts and I'm actually not sure why that worked...
It still includes the wrong btf.h -- Luca's patch is much better

/me goes back to bed
-- 
Dominique

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 3409 bytes --]

On Sat, 2021-06-12 at 07:53 +0900, Dominique Martinet wrote:
> Luca Boccassi wrote on Fri, Jun 11, 2021 at 11:45:25PM +0100:
> > Actually that was my mistake, used the wrong build tree (sorry,
> > it's
> > late!). I can however reproduce the issue in a chroot running the
> > libbpf CI script. Still looking.
> 
> with the ci script I get
> 
> $ /usr/lib64/ccache/cc -DDWARVES_MAJOR_VERSION=1 -
> DDWARVES_MINOR_VERSION=21 -D_GNU_SOURCE -Ddwarves_EXPORTS -
> I/path/to/pahole/build -I/path/to/pahole -
> I/path/to/pahole/lib/include -I/path/to/pahole/lib/bpf/include/uapi -
> D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -g -DNDEBUG -fPIC -MD
> -MT CMakeFiles/dwarves.dir/btf_encoder.c.o -MF
> CMakeFiles/dwarves.dir/btf_encoder.c.o.d -o
> CMakeFiles/dwarves.dir/btf_encoder.c.o -c
> /path/to/pahole/btf_encoder.c
> /path/to/pahole/btf_encoder.c: In function ‘btf_encoder__add_float’:
> /path/to/pahole/btf_encoder.c:224:22: warning: implicit declaration
> of function ‘btf__add_float’; did you mean ‘btf__add_var’? [-
> Wimplicit-function-declaration]
>   224 |         int32_t id = btf__add_float(encoder->btf, name,
> BITS_ROUNDUP_BYTES(bt->bit_size));
>       |                      ^~~~~~~~~~~~~~
>       |                      btf__add_var
> 
> 
> 
> with btf__add_float defined in .../pahole/lib/bpf/src/btf.h
> and btf_encoder.c including linux/btf.h
> 
> 
> changing btf_loader.c to include bpf/btf.h instead fixes the issue
> for me:
> 
> diff --git a/btf_loader.c b/btf_loader.c
> index 75ec674b3b3e..272c73bca7fe 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -20,7 +20,7 @@
>  #include <string.h>
>  #include <limits.h>
>  #include <libgen.h>
> -#include <linux/btf.h>
> +#include <bpf/btf.h>
>  #include <bpf/libbpf.h>
>  #include <zlib.h>

I've just sent a patch - the issue is that the original commit included
a symlink lib/include/bpf -> ../bpf/src as suggested by Andrii here:

https://www.spinics.net/lists/dwarves/msg00738.html

git show 82749180b23d3c9c060108bc290ae26507fc324e -- lib/include
    commit 82749180b23d3c9c060108bc290ae26507fc324e
    Author: Luca Boccassi <bluca@debian.org>
    Date:   Mon Jan 4 22:16:22 2021 +0000
    
        libbpf: allow to use packaged version
    
        Add a new CMake option, LIBBPF_EMBEDDED, to switch between the
        embedded version and the system version (searched via pkg-config)
        of libbpf. Set the embedded version as the default.
    
        Signed-off-by: Luca Boccassi <bluca@debian.org>
        Cc: dwarves@vger.kernel.org
        Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    
    diff --git a/lib/include/bpf b/lib/include/bpf
    new file mode 120000
    index 0000000..4c41b71
    --- /dev/null
    +++ b/lib/include/bpf
    @@ -0,0 +1 @@
    +../bpf/src
    \ No newline at end of file

When the patch was reverted and re-added, the symlink was dropped.

It stayed in my local tree, and I completely missed it - that's why the
build was working fine for me! D'oh!
Adding the symlink back fixes the build with the libbpf CI script. I
would be grateful if folks who are seeing the issue could apply the
patch (or create the symlink) and confirm whether it fixes the problem
or not. Thanks!

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Andrii Nakryiko]

On Fri, Jun 11, 2021 at 4:07 PM Luca Boccassi <bluca@debian.org> wrote:
>
> On Sat, 2021-06-12 at 07:53 +0900, Dominique Martinet wrote:
> > Luca Boccassi wrote on Fri, Jun 11, 2021 at 11:45:25PM +0100:
> > > Actually that was my mistake, used the wrong build tree (sorry,
> > > it's
> > > late!). I can however reproduce the issue in a chroot running the
> > > libbpf CI script. Still looking.
> >
> > with the ci script I get
> >
> > $ /usr/lib64/ccache/cc -DDWARVES_MAJOR_VERSION=1 -
> > DDWARVES_MINOR_VERSION=21 -D_GNU_SOURCE -Ddwarves_EXPORTS -
> > I/path/to/pahole/build -I/path/to/pahole -
> > I/path/to/pahole/lib/include -I/path/to/pahole/lib/bpf/include/uapi -
> > D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -g -DNDEBUG -fPIC -MD
> > -MT CMakeFiles/dwarves.dir/btf_encoder.c.o -MF
> > CMakeFiles/dwarves.dir/btf_encoder.c.o.d -o
> > CMakeFiles/dwarves.dir/btf_encoder.c.o -c
> > /path/to/pahole/btf_encoder.c
> > /path/to/pahole/btf_encoder.c: In function ‘btf_encoder__add_float’:
> > /path/to/pahole/btf_encoder.c:224:22: warning: implicit declaration
> > of function ‘btf__add_float’; did you mean ‘btf__add_var’? [-
> > Wimplicit-function-declaration]
> >   224 |         int32_t id = btf__add_float(encoder->btf, name,
> > BITS_ROUNDUP_BYTES(bt->bit_size));
> >       |                      ^~~~~~~~~~~~~~
> >       |                      btf__add_var
> >
> >
> >
> > with btf__add_float defined in .../pahole/lib/bpf/src/btf.h
> > and btf_encoder.c including linux/btf.h
> >
> >
> > changing btf_loader.c to include bpf/btf.h instead fixes the issue
> > for me:
> >
> > diff --git a/btf_loader.c b/btf_loader.c
> > index 75ec674b3b3e..272c73bca7fe 100644
> > --- a/btf_loader.c
> > +++ b/btf_loader.c
> > @@ -20,7 +20,7 @@
> >  #include <string.h>
> >  #include <limits.h>
> >  #include <libgen.h>
> > -#include <linux/btf.h>
> > +#include <bpf/btf.h>
> >  #include <bpf/libbpf.h>
> >  #include <zlib.h>
>
> I've just sent a patch - the issue is that the original commit included
> a symlink lib/include/bpf -> ../bpf/src as suggested by Andrii here:
>
> https://www.spinics.net/lists/dwarves/msg00738.html
>
> git show 82749180b23d3c9c060108bc290ae26507fc324e -- lib/include
>     commit 82749180b23d3c9c060108bc290ae26507fc324e
>     Author: Luca Boccassi <bluca@debian.org>
>     Date:   Mon Jan 4 22:16:22 2021 +0000
>
>         libbpf: allow to use packaged version
>
>         Add a new CMake option, LIBBPF_EMBEDDED, to switch between the
>         embedded version and the system version (searched via pkg-config)
>         of libbpf. Set the embedded version as the default.
>
>         Signed-off-by: Luca Boccassi <bluca@debian.org>
>         Cc: dwarves@vger.kernel.org
>         Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
>     diff --git a/lib/include/bpf b/lib/include/bpf
>     new file mode 120000
>     index 0000000..4c41b71
>     --- /dev/null
>     +++ b/lib/include/bpf
>     @@ -0,0 +1 @@
>     +../bpf/src
>     \ No newline at end of file
>
> When the patch was reverted and re-added, the symlink was dropped.
>
> It stayed in my local tree, and I completely missed it - that's why the
> build was working fine for me! D'oh!
> Adding the symlink back fixes the build with the libbpf CI script. I
> would be grateful if folks who are seeing the issue could apply the
> patch (or create the symlink) and confirm whether it fixes the problem
> or not. Thanks!

It does fix it for me locally (apart from another unrelated build
problem), thanks! I can't really test CI beyond what you did, so this
will need to be applied to pahole master for us to know for sure.

>
> --
> Kind regards,
> Luca Boccassi