Add initial support for meson build system

[gpollo]

Ref: #189

In theory, all options from the old build system are supported, even cross-compilation as it is built-in meson.

An example build commands could be:

$ cd uftrace/ && mkdir build && cd build
$ meson ..
$ ninja

You can list build options and configure them:

$ cd uftrace/ && mkdir build && cd build
$ meson ..
$ meson configure # list options
$ meson reconfigure -Dwithout-libpython=enabled -Dwithout-schedule=enabled

You can run tests:

$ cd uftrace/ && mkdir build && cd build
$ meson ..
$ ninja
$ ninja check-unit    # executes unit tests
$ ninja check-uftrace # executes runtest.py

For cross-compilation, you can read meson documentation. It involves creating a file with cross-compilation options. For example, this file can cross-compile ARM for the Raspberry-Pi 4.

[binaries]
c = ['/data/toolchain/arm-unknown-linux-gnueabihf/bin/gcc', '--sysroot', '/data/sysroot/rpi4-arm']
strip = ['/data/toolchain/arm-unknown-linux-gnueabihf/bin/strip']

[properties]
sys_root = '/data/sysroot/rpi4-arm'

[host_machine]
system = 'linux'
cpu_family = 'arm'
cpu = 'armv8'
endian = 'little'

If the cross-build libraries are in sys_root, the build will find them. The application can be configured and build:

$ cd uftrace/ && mkdir build && cd build
$ meson .. --cross ../meson_arm.txt \
          -Dwithout-libstdcxx=enabled \
          -Dwithout-perf=enabled \
          -Dwithout-schedule=enabled \
          -Dwithout-tests=enabled \
          -Dwithout-libpython=enabled
$ ninja

Finally, it should be possible to install:

$ cd uftrace/ && mkdir build && cd build
$ meson .. --prefix=/usr/local
$ ninja # can be skipped
$ meson install

I marked the PR as WIP as I believe more testing could be done on it. It is possible that I forgot some compiler flags for certain targets and cross-compiling supports could be improved/tested more.

[namhyung]

Thanks a lot for doing this!

But the first impression is that it looks complicated. Maybe we can have a wrapper Makefile calling meson + ninja internally for the basic usecases?

[honggyukim]

@gpollo Thanks for doing this work! I've tried to build it with meson, but see the following build error.

$ mkdir build && cd build

$ meson ..

$ ninja
[54/227] Linking target libmcount.so
FAILED: libmcount.so
cc  -o libmcount.so libmcount.so.p/libmcount_dynamic.c.o libmcount.so.p/libmcount_event.c.o libmcount.so.p/libmcount_mcount.c.o libmcount.so.p/libmcount_misc.c.o libmcount.so.p/libmcount_plthook.c.o libmcount.so.p/libmcount_pmu.c.o libmcount.so.p/libmcount_record.c.o libmcount.so.p/libmcount_wrap.c.o libmcount.so.p/utils_argspec.c.o libmcount.so.p/utils_auto-args.c.o libmcount.so.p/utils_debug.c.o libmcount.so.p/utils_demangle.c.o libmcount.so.p/utils_dwarf.c.o libmcount.so.p/utils_filter.c.o libmcount.so.p/utils_hashmap.c.o libmcount.so.p/utils_rbtree.c.o libmcount.so.p/utils_regs.c.o libmcount.so.p/utils_script-luajit.c.o libmcount.so.p/utils_script-python.c.o libmcount.so.p/utils_script.c.o libmcount.so.p/utils_symbol-libelf.c.o libmcount.so.p/utils_symbol-rawelf.c.o libmcount.so.p/utils_symbol.c.o libmcount.so.p/utils_utils.c.o -Wl,--as-needed -Wl,--no-undefined -shared -fPIC -Wl,--start-group -Wl,-soname,libmcount.so -lstdc++ /usr/lib/x86_64-linux-gnu/libcapstone.so -ldl -ldw -lelf /usr/lib/x86_64-linux-gnu/libluajit-5.1.so -lpthread -lrt -Wl,--whole-archive libmcount-arch.a -Wl,--end-group -Wl,--no-whole-archive
/usr/bin/ld: libmcount-arch.a(arch_x86_64_dynamic.S.o): relocation R_X86_64_PC32 against symbol `mcount_entry' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value
collect2: error: ld returned 1 exit status
[59/227] Compiling C object libmcount-fast.so.p/utils_demangle.c.o
ninja: build stopped: subcommand failed.

It seems that the related part is as follows.

libmcount_arch = static_library(
    'mcount-arch', [libmcount_arch_sources, libmcount_arch_asm_sources, version_h],
    include_directories: [libmcount_arch_include],
    c_args: [common_cflags],
    pic: true,
)

libmcount_arch_dep = declare_dependency(
    include_directories: [libmcount_arch_include],
    # we link whole archive to overwrite weak symbols
    link_args: ['-Wl,--whole-archive', 'libmcount-arch.a' , '-Wl,--no-whole-archive']
)

It seems that it already builds libmcount-arch.a with pic enabled, but doesn't work as expected.

By the way, is it really needed to make a static library libmcount-arch.a?

[gpollo]

@namhyung That's a good idea, I'll try to add some basic target in the current Makefile.

@honggyukim Building the arch stuff in a separate lib should not longer be needed. I think I did it this way initially because meson do not supports per-object compilation flags. I though that these files needed specific flags. I'll do some test and probably move them directly in the shared_library where they are needed.

[namhyung]

Also you can change misc/install-deps.sh to have meson and ninja.

[gpollo]

Rebased the branch and removed the intermediate libmcount_entry static library.

@honggyukim Turns out your build error was caused by -fvisibility=hidden not used. I believe that Arch Linux has this option activated by default. When I tested on a Ubuntu 18.04 container, I had the same error as you. I compared the old build system commands with the new ones and I found this missing option. It should work now.

[honggyukim]

@gpollo Thanks for the update. I've tested it again and see that the build is fine this time. But I see the default build generates unittests binary unnecessarily.

And please properly rebase the patch on top of the current master. It seems that your last patch is on top of a wrong commit hash.

[gpollo]

• Rebased on master.
• Optionally build dbginfo and demangler binary with -Dbuild-dbginfo=enabled and -Dbuild-demangler=enabled
• Don't build tests by default. If ninja check-unit or ninja check-uftrace is ran, a message is written to tell to reconfigure the build with -Dwith-tests=enabled.

[honggyukim]

Thanks a lot for the update! It looks good now, but I have a few more requests.

1. I think with-tests, build-dbginfo, and build-demangler don't have to be in meson_options.txt. Rather than that, they can be ninja target using run_target. They can be compiled like ninja unittests, ninja dbginfo and ninja demangler.
2. We also need to handle runtime variables just like we did make DEBUG=1, make ASAN=1, make COVERAGE=1, and etc. They are build options that enable address sanitizer and gcov for code coverage. You can find all the info at https://github.com/namhyung/uftrace/blob/v0.9.4/Makefile#L87-L132.

[honggyukim]

In other words, I think meson command should be the replacement of configure script and ninja should be for the current make command. So ninja is better to accept most of the usage that we provide with make.

[gpollo]

Alright, did some modifications again, hopefully, I didn't broke anything, I'll be testing more the upcoming weeks. Mainly,

• You are right, I remove with-tests, build-dbginfo and build-demangler. They are optional when you run ninja dbginfo or ninja demangler.
• Unit tests can be executed with ninja tests and uftrace tests can be executed with ninja uftrace-tests. Should they both be executed at the same time?
• I also renamed all options like -Dwithout-capstone= (default: disabled) to -Dwith-capstone= (default: auto). Doesn't change much except it makes more sense.
• For make DEBUG=1, make ASAN=1 or make COVERAGE=1, I don't think we can have the equivalent with ninja. I put those at configuration time (e.g. meson -Ddebug=true -Dasan=true -Dcoverage=false, etc.).

[honggyukim]

Thanks for the update. I see version.h missing error.

$ mkdir build && cd build
$ meson ..
      ...
$ ninja
[1/188] Generating gen_version_h with a custom command.
FAILED: version.h
/usr/bin/sh -c '/home/honggyu/work/uftrace/misc/version.sh version.h $(git describe --tags 2> /dev/null || echo v0.9.4) . > /dev/null'
[8/188] Compiling C object 'traceevent@sta/libtraceevent_event-parse.c.o'.
ninja: build stopped: subcommand failed.

I will have a look at the details and your comment when I have more time. Thanks.

[gpollo]

I fixed it, commit 31d89ea added a new argument to version.sh and the generation failed. When I rebased against master. It kept the old one and my build wouldn't failed. Should be good now.

[honggyukim]

Thanks for the update. It's getting closer to the complete set.

You are right, I remove with-tests, build-dbginfo and build-demangler. They are optional when you run ninja dbginfo or ninja demangler.

There is symbols binary in misc so could you add ninja symbols as well? And I think we better build those binaries by default because make does it.

Unit tests can be executed with ninja tests and uftrace tests can be executed with ninja uftrace-tests. Should they both be executed at the same time?

To make the behavior same as make, it'd be better to make ninja unittest does same thing with make unittest, ninja runtest runs same as make runtest and ninja test to run both unittest and runtest.

I also renamed all options like -Dwithout-capstone= (default: disabled) to -Dwith-capstone= (default: auto). Doesn't change much except it makes more sense.

Can we make the value to boolean? I think -Dwith-capstone=false is easier than -Dwith-capstone=disabled because it might be confusing between disabled and disable. In addition, is there any way to list up all the configurations to user? I'd expected meson --help to show it but there must be something else.

For make DEBUG=1, make ASAN=1 or make COVERAGE=1, I don't think we can have the equivalent with ninja. I put those at configuration time (e.g. meson -Ddebug=true -Dasan=true -Dcoverage=false, etc.).

I see that -Dsanitize=address, -Dcoverage=true and other options do this work. But I prefer -Ddebug=true rather than -Ddebugging=true because we use make DEBUG=1. I found that debug is reserved so cannot use.

In addition, meson -Dsanitize=address hits a linking error so we have to add link flag as well.

So I would like to apply the following change although it didn't address all the points I mentioned.

diff --git a/meson.build b/meson.build
index 660e8562..7700d6c0 100644
--- a/meson.build
+++ b/meson.build
@@ -10,6 +10,8 @@ common_ldflags = []
 uftrace_cflags    = []
 libmcount_cflags  = []
 demangler_cflags  = []
+symbols_cflags    = []
+dbginfo_cflags    = []
 traceevent_cflags = []
 test_cflags       = []

@@ -41,6 +43,8 @@ if get_option('trace')
     trace_cflags       = ['-pg', '-fno-omit-frame-pointer']
     uftrace_cflags    += trace_cflags
     demangler_cflags  += trace_cflags
+    symbols_cflags    += trace_cflags
+    dbginfo_cflags    += trace_cflags
     traceevent_cflags += trace_cflags
     test_cflags       += trace_cflags
     # cannot add -pg to libmcount_cflags because mcount() is not reentrant
@@ -55,15 +59,31 @@ libmcount_cflags
     test_cflags      += coverage_cflags
 endif

+if get_option('asan')
+    asan_cflags = ['-O0', '-g', '-fsanitize=address,leak']
+    common_ldflags   += ['-fsanitize=address,leak']
+
+    uftrace_cflags    += asan_cflags
+    demangler_cflags  += asan_cflags
+    symbols_cflags    += asan_cflags
+    dbginfo_cflags    += asan_cflags
+    traceevent_cflags += asan_cflags
+    test_cflags       += asan_cflags
+endif
+
 if get_option('sanitize') != ''
     if get_option('sanitize') == 'all'
         sanitize_cflags = ['-O0', '-g', '-fsanitize=address,leak,undefined']
+        common_ldflags   += ['-fsanitize=address,leak,undefined']
     else
         sanitize_cflags = ['-O0', '-g', '-fsanitize=@0@'.format(get_option('sanitize'))]
+        common_ldflags   += ['-fsanitize=@0@'.format(get_option('sanitize'))]
     endif

     uftrace_cflags    += sanitize_cflags
     demangler_cflags  += sanitize_cflags
+    symbols_cflags    += sanitize_cflags
+    dbginfo_cflags    += sanitize_cflags
     traceevent_cflags += sanitize_cflags
     test_cflags       += sanitize_cflags
 endif
@@ -521,9 +541,9 @@ dbginfo_deps = [
 dbginfo = executable(
     'dbginfo', [dbginfo_sources],
     dependencies: dbginfo_deps,
-    c_args: [common_cflags],
+    c_args: [common_cflags, dbginfo_cflags],
     link_args: [common_ldflags],
-    build_by_default: false,
+    build_by_default: true,
 )

 #############
@@ -545,7 +565,42 @@ demangler = executable(
     dependencies: demangler_deps,
     c_args: [common_cflags, demangler_cflags],
     link_args: [common_ldflags],
-    build_by_default: false,
+    build_by_default: true,
+)
+
+###########
+# symbols #
+###########
+
+symbols_sources = [
+    'misc/symbols.c',
+    'utils/session.c',
+    'utils/demangle.c',
+    'utils/rbtree.c',
+    'utils/utils.c',
+    'utils/debug.c',
+    'utils/filter.c',
+    'utils/dwarf.c',
+    'utils/auto-args.c',
+    'utils/regs.c',
+    'utils/argspec.c',
+    'utils/symbol.c',
+    'utils/symbol-libelf.c',
+    'utils/symbol-rawelf.c',
+]
+symbols_deps = [
+    cxa_demangle_dep,
+    libdl_dep,
+    libdw_dep,
+    libelf_dep,
+]
+
+symbols = executable(
+    'symbols', [symbols_sources],
+    dependencies: symbols_deps,
+    c_args: [common_cflags, symbols_cflags],
+    link_args: [common_ldflags],
+    build_by_default: true,
 )

 #########
diff --git a/meson_options.txt b/meson_options.txt
index b7acf797..d088940d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -112,9 +112,16 @@ option(
     description: 'Enable code coverage',
 )

+option(
+    'asan',
+    type : 'boolean',
+    value : 'false',
+    description: 'Enable address sanitizer',
+)
+
 option(
     'sanitize',
     type : 'string',
     value : '',
     description: 'Specify address and thread sanitizer options',
-)
\ No newline at end of file
+)

@namhyung It'd be better if you could provide how you think.

[honggyukim]

In addition, -Dwith-libelf=disabled has to disable with-libdw as well. So the following cannot be compiled.

$ meson -Dwith-libelf=disabled ../
      ...
Message:            libdw: [ on  ] - dwarf support
Message:           libelf: [ off ] - more flexible ELF data handling
      ...

$ ninja
      ...
build/../utils/dwarf.c:215: undefined reference to `gelf_getehdr'
collect2: error: ld returned 1 exit status

[honggyukim]

I found one more. The libmcount*.so has to be installed directly under lib directory rather than lib/x86_64-linux-gnu to make it same as current status.

$ meson -Dprefix=`pwd`/out ..
 
$ ninja install

$ ls -R out/lib/
out/lib/:
x86_64-linux-gnu

out/lib/x86_64-linux-gnu:
libmcount-fast-single.so  libmcount-fast.so  libmcount-nop.so  libmcount-single.so  libmcount.so

@namhyung I actually think it's better structure and can find the libmcount*.so under its arch name as I suggested at #1032. How do you think about it?

[gpollo]

• Applied your patch.
• Renamed ninja tests to ninja unittest, ninja uftrace-tests to ninja runtest.
• Cannot use ninja test for running both since it is a reserved keyword. I use ninja tests for now.

Can we make the value to boolean? I think -Dwith-capstone=false is easier than -Dwith-capstone=disabled because it might be confusing between disabled and disable.

The problem with using boolean type is that it only allows 2 states (true/false). By using feature, there is 3 states (enabled/disabled/auto). By default, they are all auto. It allows us to enable the feature if the library is present on the system or disable the feature if it is not present. By forcing -Dwith-capstone=enabled, the build will fail if the library is not found.

An alternative solution using boolean would be to have the options like -Dwithout-capstone=true which would disable the feature and -Dwithout-capstone=false (e.g. by default) which would only enable the feature if it is the library is present. The difference is that we can't "force" the build to enable a feature and fail if the library is not present (it will only do so if the library is present).

In addition, is there any way to list up all the configurations to user? I'd expected meson --help to show it but there must be something else.

You can use meson configure to list the options.

[gpollo]

Sorry, I forgot some points.

In addition, -Dwith-libelf=disabled has to disable with-libdw as well. So the following cannot be compiled.

Fixed it.

I found one more. The libmcount*.so has to be installed directly under lib directory rather than lib/x86_64-linux-gnu to make it same as current status.

I think it depends on your system. For me (Arch Linux), the installation location was lib.

[honggyukim]

@gpollo Thanks very much for the update. I have two more requests.

1. Could you add meson -Dcc=clang or meson -Dcompiler=clang so that we can select the compiler to gcc or clang?
2. Please add more descriptive commit message for your work with Signed-off-by. You can refer to other commits with git log. It'd be better to show the usage for build options. I think your first message in this PR is a good example.

I think it's almost done so @namhyung can take a look at the final review. I appreciate your work!

[gpollo]

Could you add meson -Dcc=clang or meson -Dcompiler=clang so that we can select the compiler to gcc or clang?

Turns out that it can be done using CC=clang meson .. during the build configuration. See this page.

Please add more descriptive commit message for your work with Signed-off-by. You can refer to other commits with git log. It'd be better to show the usage for build options. I think your first message in this PR is a good example.

I didn't list the build options, be the command to list them is in the commit message.

[gpollo]

Fixed the python version.

[honggyukim]

Thanks. I see that python version is fixed, but build: fix version string in when building with Meson breaks the original build system because UFTRACE_HAVE_LIBDW and other variables are not defined there.

It even shows some errors misc/version.sh: 34: [: unexpected operator.

[gpollo]

That's weird, the original build system still seems to work here. Maybe there's some configurations in your shell?

[honggyukim]

It's because my /bin/sh is a soft link to dash.

$ ls -l /bin/sh
lrwxrwxrwx 1 root root 4 Feb 17  2016 /bin/sh -> dash

Maybe we should explicitly specify shell name to bash. It's okay with the following change.

diff --git a/misc/version.sh b/misc/version.sh
index 51b01b51..8a309463 100755
--- a/misc/version.sh
+++ b/misc/version.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash

 VERSION_FILE=$1

[namhyung]

Thanks for working on this! I think we should change the logic in the check-deps to generate files under the build directory. Please check the check/build-dep-v1 branch.

Also I think it'd be nice if it can work with any shell.

[eli-schwartz]

Nice to see people looking into meson! I have some suggestions for your build files, hope this helps.

[honggyukim]

@eli-schwartz Thanks very much for your review in details. @gpollo Could you please have a look? If it takes a long time to address all the comments, maybe we can finalize the current PR and revisit addressing @eli-schwartz's comments in the follow up patch.

[gpollo]

Thank you @eli-schwartz for your detailed review! @honggyukim I think I'll leave the current commit as is since it replicated as much as possible the old build system. When transitionning to the build-in meson options, we can document the changes in the commit. I've already started on a few, they'll be in separate commits.

[eli-schwartz]

Well, please at least fix the /bin/sh syntax errors :p

And several other of my review comments pertained to using e.g. meson in-process string replacement rather than sed string replacement which is just all around good.

[gpollo]

I've fix most of @eli-schwartz comments. Again, thanks a lot!

I've tried cross-compiling again (last time was before submitting this PR...). Removed the -Darch option and I had to change cc.run() to cc.compiles() when checking for some dependencies, but it's working okay. I've put a warning saying that it was still experimental.

[eli-schwartz]

Could be potentially rewritten

fs_mod = import('fs')
if fs_mod.is_dir('arch' / arch)
    libmcount_arch_include = include_directories('arch' / arch)
    subdir('arch' / arch)
else
     error('target architecture `@0@` is not supported'.format(arch))
endif

inside arch/aarch64/meson.build

libmcount_arch_sources = files(
    'cpuinfo.c',
    'mcount-dynamic.c',
    'mcount-insn.c',
    'mcount-support.c',
)
libmcount_arch_asm_sources = files(
    'dynamic.S',
    'mcount.S',
    'plthook.S',
)

[eli-schwartz]

cf. mesonbuild/meson@dc51740

In meson 0.60 you will be able to do this instead:

unittest = run_target(...)
runtest = run_target(...)
alias_target('tests', unittest, runtest)

[qpakzk]

@honggyukim I want to keep going this feature.

[honggyukim]

@qpakzk thanks but you should address all the comments given by @eli-schwartz here.