userland: make libs work

Working for build, but now test-user-mode-in-tree is not using --in-tree,
TODO fix later on.

README.adoc

@@ -973,7 +973,7 @@ This is the most reproducible and controlled environment, and all examples work
 
 With this setup, we will use the host toolchain and execute executables directly on the host.
 
-No installation or toolchain build is reuqired, so you can just jump straight into it.
+No installation or toolchain build is required, so you can just jump straight into it.
 
 Build, run and example, and clean it in-tree with:
 
@@ -1002,13 +1002,22 @@ cd userland/c
 ./test
 ....
 
-You can install those libraries and do the build in one go with:
+As mentioned at <<user-mode-tests>>, tests under link:userland/libs[] require certain optional libraries to be installed, and are not built or tested by default.
+
+You can install those libraries with:
 
 ....
 cd linux-kernel-module-cheat
 ./build --download-dependencies userland-host
 ....
 
+and then build the examples and test with:
+
+....
+./build --package-all
+./test --package-all
+....
+
 Pass custom compiler options:
 
 ....
@@ -3536,7 +3545,7 @@ This script skips a manually configured list of tests, notably:
 * tests that take perceptible ammounts of time
 * known bugs we didn't have time to fix ;-)
 
-Tests under link:userland/libs/[] depend on certain libraries being available on the target, e.g. <<blas>> for link:userland/libs/blas[]. They are not run by default, but can be enabled with `--has-package` and `--has-all-packages`.
+Tests under link:userland/libs/[] depend on certain libraries being available on the target, e.g. <<blas>> for link:userland/libs/blas[]. They are not run by default, but can be enabled with `--package` and `--package-all`.
 
 The gem5 tests require building statically with build id `static`, see also: <<gem5-syscall-emulation-mode>>. TODO automate this better.
 
@@ -3695,7 +3704,7 @@ So programs that rely on those libraries might not compile as GCC can't find the
 For example, if we try to build <<blas>> statically:
 
 ....
-./build-userland --has-package openblas --static -- userland/libs/openblas/hello.c
+./build-userland --package openblas --static -- userland/libs/openblas/hello.c
 ....
 
 it fails with:
@@ -8302,7 +8311,7 @@ DRM / DRI is the new interface that supersedes `fbdev`:
 
 ....
 ./build-buildroot --config 'BR2_PACKAGE_LIBDRM=y'
-./build-userland --has-package libdrm -- userland/libs/libdrm/modeset.c
+./build-userland --package libdrm -- userland/libs/libdrm/modeset.c
 ./run --eval-after './libs/libdrm/modeset.out' --graphic
 ....
 
@@ -8314,7 +8323,7 @@ TODO not working for `aarch64`, it takes over the screen for a few seconds and t
 
 ....
 ./build-buildroot --config 'BR2_PACKAGE_LIBDRM=y'
-./build-userland --has-package libdrm
+./build-userland --package libdrm
 ./build-buildroot
 ./run --eval-after './libs/libdrm/modeset.out' --graphic
 ....
@@ -10061,7 +10070,7 @@ Buildroot supports it, which makes everything just trivial:
 
 ....
 ./build-buildroot --config 'BR2_PACKAGE_OPENBLAS=y'
-./build-userland --has-package openblas -- userland/libs/openblas/hello.c
+./build-userland --package openblas -- userland/libs/openblas/hello.c
 ./run --eval-after './libs/openblas/hello.out; echo $?'
 ....
 
@@ -10101,7 +10110,7 @@ Header only linear algebra library with a mainline Buildroot package:
 
 ....
 ./build-buildroot --config 'BR2_PACKAGE_EIGEN=y'
-./build-userland --has-package eigen -- userland/libs/eigen/hello.cpp
+./build-userland --package eigen -- userland/libs/eigen/hello.cpp
 ....
 
 Just create an array and print it:
@@ -13351,7 +13360,7 @@ We have link:https://buildroot.org/downloads/manual/manual.html#ccache[enabled c
 * absolute paths are used and GDB can find source files
 * but builds are not reused across separated LKMC directories
 
-=== Rebuild buildroot while running
+=== Rebuild Buildroot while running
 
 It is not possible to rebuild the root filesystem while running QEMU because QEMU holds the file qcow2 file:
 
@@ -13640,6 +13649,31 @@ We chose this awkward name so that our includes will have an `lkmc/` prefix.
 
 Another option would have been to name it as `includes/lkmc`, but that would make paths longer, and we might want to store source code in that directory as well in the future.
 
+===== Userland objects vs header-only
+
+When factoring out functionality across userland examples, there are two main options:
+
+* use header-only implementations
+* use separate C files and link to separate objects.
+
+The downsides of the header-only implementation are:
+
+* slower compilation time, especially for C++
+* cannot call C implementations from assembly files
+
+The advantages of header-only implementations are:
+
+* easier to use, just `#include` and you are done, no need to modify build metadata.
+
+As a result, we are currently using the following rule:
+
+* if something is only going to be used from C and not assembly, define it in a header which is easier to use
++
+The slower compilation should be OK as long as split functionality amongst different headers and only include the required ones.
++
+Also we don't have a choice in the case of C++ template, which must stay in headers.
+* if the functionality will be called from assembly, then we don't have a choice, and must add it to a separate source file and link against it.
+
 ==== buildroot_packages directory
 
 Source: link:buildroot_packages/[]

build-userland

@@ -17,21 +17,6 @@ class Main(common.BuildCliFunction):
 Build our compiled userland examples.
 '''
         super().__init__(*args, **kwargs)
-        self.add_argument(
-            '--has-package',
-            action='append',
-            help='''\
-Indicate that a given package is present in the root filesystem, which
-allows us to build examples that rely on it.
-''',
-        )
-        self.add_argument(
-            '--has-all-packages',
-            action='store_true',
-            help='''\
-Indicate that all packages from --has-package are available.
-''',
-        )
         self.add_argument(
             'targets',
             default=[],
@@ -49,7 +34,7 @@ Default: build all examples that have their package dependencies met, e.g.:
 -   userland/arch/ programs only build if the target arch matches
 -   an OpenBLAS example can only be built if the target root filesystem
     has the OpenBLAS libraries and headers installed, which you must inform
-    with --has-package
+    with --package
 ''',
             nargs='*',
         )
@@ -87,9 +72,6 @@ Default: build all examples that have their package dependencies met, e.g.:
                     std = path_properties.default_c_std
                 else:
                     std = c_std
-                cc_flags.extend([
-                    '-fopenmp', LF,
-                ])
             elif in_ext == self.env['cxx_ext']:
                 cc = self.env['gxx']
                 if cxx_std is None:
@@ -109,10 +91,6 @@ Default: build all examples that have their package dependencies met, e.g.:
                         in_path, LF,
                     ] +
                     self.sh.add_newlines(extra_objs) +
-                    [
-                        '-lm', LF,
-                        '-pthread', LF,
-                    ] +
                     cc_flags_after
                 ),
                 extra_paths=[self.env['ccache_dir']],
@@ -121,16 +99,9 @@ Default: build all examples that have their package dependencies met, e.g.:
 
     def build(self):
         build_dir = self.get_build_dir()
-        has_packages = set(self.env['has_package'])
-        has_all_packages = self.env['has_all_packages']
         cc_flags = [
             '-I', self.env['root_dir'], LF,
             '-O{}'.format(self.env['optimization_level']), LF,
-            '-Wall', LF,
-            '-Werror', LF,
-            '-Wextra', LF,
-            '-Wno-unused-function', LF,
-            '-ggdb3', LF,
         ] + self.sh.shlex_split(self.env['ccflags'])
         if self.env['static']:
             cc_flags.extend(['-static', LF])
@@ -168,7 +139,7 @@ Default: build all examples that have their package dependencies met, e.g.:
             eigen_root = '/'
         else:
             eigen_root = self.env['buildroot_staging_dir']
-        pkgs = {
+        packages = {
             'eigen': {
                 # TODO: was failing with:
                 # fatal error: Eigen/Dense: No such file or directory as of
@@ -187,8 +158,6 @@ Default: build all examples that have their package dependencies met, e.g.:
                 # Header only.
                 'cc_flags_after': [],
             },
-            'libdrm': {},
-            'openblas': {},
         }
         rootdir_abs_len = len(self.env['userland_source_dir'])
         with ThreadPool(
@@ -206,42 +175,10 @@ Default: build all examples that have their package dependencies met, e.g.:
                             build_dir,
                             dirpath_relative_root
                         )
-                        common_objs_dir = []
-                        cc_flags_after = []
-                        cc_flags_dir = cc_flags.copy()
-                        if dirpath_relative_root_components_len > 0:
-                            if dirpath_relative_root_components[0] == 'arch':
-                                cc_flags_dir.extend([
-                                    '-I', os.path.join(self.env['userland_source_arch_arch_dir']), LF,
-                                    '-I', os.path.join(self.env['userland_source_arch_dir']), LF,
-                                ])
-                            elif dirpath_relative_root_components[0] == 'libs':
-                                if dirpath_relative_root_components_len > 1:
-                                    pkg_key = dirpath_relative_root_components[1]
-                                    if not (has_all_packages or pkg_key in has_packages):
-                                        continue
-                                    pkg = pkgs[pkg_key]
-                                    if 'cc_flags' in pkg:
-                                        cc_flags_dir.extend(pkg['cc_flags'])
-                                    else:
-                                        pkg_config_output = subprocess.check_output([
-                                            self.env['pkg_config'],
-                                            '--cflags',
-                                            pkg_key
-                                        ]).decode()
-                                        cc_flags_dir.extend(self.sh.shlex_split(pkg_config_output))
-                                    if 'cc_flags_after' in pkg:
-                                        cc_flags_dir.extend(pkg['cc_flags_after'])
-                                    else:
-                                        pkg_config_output = subprocess.check_output([
-                                            self.env['pkg_config'],
-                                            '--libs',
-                                            pkg_key
-                                        ]).decode()
-                                        cc_flags_after.extend(self.sh.shlex_split(pkg_config_output))
                         for in_filename in in_filenames:
                             in_path = os.path.join(path, in_filename)
-                            cc_flags_file = cc_flags_dir.copy()
+                            cc_flags_file = cc_flags.copy()
+                            cc_flags_after = []
                             in_ext = os.path.splitext(in_filename)[1]
                             if not in_ext in self.env['userland_in_exts']:
                                 continue
@@ -250,10 +187,41 @@ Default: build all examples that have their package dependencies met, e.g.:
                                 dirpath_relative_root,
                                 in_filename
                             ))
-                            if my_path_properties.should_be_built(self.env['arch']):
+                            if my_path_properties.should_be_built(self.env):
-                                if my_path_properties['pedantic']:
+                                if dirpath_relative_root_components_len > 0:
+                                    if dirpath_relative_root_components[0] == 'arch':
+                                        cc_flags_file.extend([
+                                            '-I', os.path.join(self.env['userland_source_arch_arch_dir']), LF,
+                                            '-I', os.path.join(self.env['userland_source_arch_dir']), LF,
+                                        ])
+                                    elif dirpath_relative_root_components[0] == 'libs':
+                                        if dirpath_relative_root_components_len > 1:
+                                            package_key = dirpath_relative_root_components[1]
+                                            if package_key in packages:
+                                                package = packages[package_key]
+                                            else:
+                                                package = {}
+                                            if 'cc_flags' in package:
+                                                cc_flags_file.extend(package['cc_flags'])
+                                            else:
+                                                pkg_config_output = subprocess.check_output([
+                                                    self.env['pkg_config'],
+                                                    '--cflags',
+                                                    package_key
+                                                ]).decode()
+                                                cc_flags_file.extend(self.sh.shlex_split(pkg_config_output))
+                                            if 'cc_flags_after' in package:
+                                                cc_flags_file.extend(package['cc_flags_after'])
+                                            else:
+                                                pkg_config_output = subprocess.check_output([
+                                                    self.env['pkg_config'],
+                                                    '--libs',
+                                                    package_key
+                                                ]).decode()
+                                                cc_flags_after.extend(self.sh.shlex_split(pkg_config_output))
+                                if my_path_properties['cc_pedantic']:
                                     cc_flags_file.extend(['-pedantic', LF])
-                                common_objs_file = common_objs_dir.copy()
+                                common_objs_file = []
                                 if my_path_properties['extra_objs_lkmc_common']:
                                     common_objs_file.append(common_obj)
                                 if my_path_properties['extra_objs_userland_asm']:
@@ -261,7 +229,7 @@ Default: build all examples that have their package dependencies met, e.g.:
                                 error = thread_pool.submit({
                                     'c_std': my_path_properties['c_std'],
                                     'cc_flags': cc_flags_file + my_path_properties['cc_flags'],
-                                    'cc_flags_after': cc_flags_after,
+                                    'cc_flags_after': cc_flags_after + my_path_properties['cc_flags_after'],
                                     'cxx_std': my_path_properties['cxx_std'],
                                     'extra_objs': common_objs_file,
                                     'in_path': in_path,

common.py

@@ -436,6 +436,22 @@ Machine type:
         )
 
         # Userland.
+        self.add_argument(
+            '--package',
+            action='append',
+            help='''\
+Request to install a package in the target root filesystem, or indicate that it is present
+when building examples that rely on it or running tests for those examples.
+''',
+        )
+        self.add_argument(
+            '--package-all',
+            action='store_true',
+            help='''\
+Indicate that all packages used by our userland/ examples with --package
+are available.
+''',
+        )
         self.add_argument(
             '--static',
             default=False,
@@ -772,6 +788,7 @@ Incompatible archs are skipped.
             env['userland_build_dir'] = self.env['userland_source_dir']
         else:
             env['userland_build_dir'] = join(env['out_dir'], 'userland', env['userland_build_id'], env['arch'])
+        env['package'] = set(env['package'])
 
         # Kernel modules.
         env['kernel_modules_build_dir'] = join(env['kernel_modules_build_base_dir'], env['arch'])

lkmc.c

@@ -11,24 +11,11 @@ void lkmc_assert(bool condition) {
         lkmc_assert_fail();
 }
 
-void lkmc_assert_fail() {
+void lkmc_assert_fail(void) {
     puts("lkmc_test_fail");
     exit(1);
 }
 
-bool lkmc_vector_equal(size_t n, double *v1, double *v2, double max_err) {
-    double sum = 0.0;
-    double diff;
-    size_t i;
-    for (i = 0; i < n; ++i) {
-        diff = v1[i] - v2[i];
-        sum += diff * diff;
-    }
-    if (sqrt(sum)/n > max_err)
-        return false;
-    return true;
-}
-
 #if defined(__aarch64__)
 #define LKMC_SYSREG_READ_WRITE(type, name) \
     type LKMC_CONCAT(LKMC_CONCAT(LKMC_SYSREG_SYMBOL_PREFIX, name), _read(void)) { \

lkmc.h

@@ -11,7 +11,6 @@
 
 void lkmc_assert(bool);
 void lkmc_assert_fail();
-bool lkmc_vector_equal(size_t n, double *v1, double *v2, double max_err);
 #endif
 
 /* https://stackoverflow.com/questions/1489932/how-to-concatenate-twice-with-the-c-preprocessor-and-expand-a-macro-as-in-arg */

lkmc/math.h

@@ -0,0 +1,20 @@
+#ifndef LKMC_MATH_H
+#define LKMC_MATH_H
+
+#include <math.h>
+#include <stdbool.h>
+
+bool lkmc_vector_equal(size_t n, double *v1, double *v2, double max_err) {
+    double sum = 0.0;
+    double diff;
+    size_t i;
+    for (i = 0; i < n; ++i) {
+        diff = v1[i] - v2[i];
+        sum += diff * diff;
+    }
+    if (sqrt(sum)/n > max_err)
+        return false;
+    return true;
+}
+
+#endif

path_properties.py

@@ -5,6 +5,33 @@ import os
 from shell_helpers import LF
 
 class PathProperties:
+    property_keys = {
+        'allowed_archs',
+        'c_std',
+        'cc_flags',
+        'cc_flags_after',
+        'cc_pedantic',
+        'cxx_std',
+        'exit_status',
+        'interactive',
+        # We should get rid of this if we ever properly implement dependency graphs.
+        'extra_objs_lkmc_common',
+        'extra_objs_userland_asm',
+        # We were lazy to properly classify why we are skipping these tests.
+        # TODO get it done.
+        'skip_run_unclassified',
+        'more_than_1s',
+        # The path does not generate an executable in itself, e.g.
+        # it only generates intermediate object files.
+        'no_executable',
+        # the test receives a signal. We skip those tests for now,
+        # on userland because we are lazy to figure out the exact semantics
+        # of how Python + QEMU + gem5 determine the exit status of signals.
+        'receives_signal',
+        'requires_kernel_modules',
+        'uses_dynamic_library',
+    }
+
     '''
     Encodes properties of userland and baremetal paths.
     For directories, it applies to all files under the directory.
@@ -12,37 +39,12 @@ class PathProperties:
     '''
     def __init__(
         self,
-        **kwargs
+        properties
     ):
-        property_keys = {
+        for key in properties:
-            'allowed_archs',
+            if not key in self.property_keys:
-            'c_std',
-            'cc_flags',
-            'cc_pedantic',
-            'cxx_std',
-            'exit_status',
-            'interactive',
-            # We should get rid of this if we ever properly implement dependency graphs.
-            'extra_objs_lkmc_common',
-            'extra_objs_userland_asm',
-            # We were lazy to properly classify why we are skipping these tests.
-            # TODO get it done.
-            'skip_run_unclassified',
-            'more_than_1s',
-            # The path does not generate an executable in itself, e.g.
-            # it only generates intermediate object files.
-            'no_executable',
-            'pedantic',
-            # the test receives a signal. We skip those tests for now,
-            # on userland because we are lazy to figure out the exact semantics
-            # of how Python + QEMU + gem5 determine the exit status of signals.
-            'receives_signal',
-            'requires_kernel_modules',
-        }
-        for key in kwargs:
-            if not key in property_keys:
                 raise ValueError('Unknown key: {}'.format(key))
-        self.properties = kwargs
+        self.properties = properties.copy()
 
     def __getitem__(self, key):
         return self.properties[key]
@@ -50,29 +52,38 @@ class PathProperties:
     def __repr__(self):
         return str(self.properties)
 
+    def set_path_components(self, path_components):
+        self.path_components = path_components
+
+    def should_be_built(self, env):
+        if len(self.path_components) > 1 and \
+                self.path_components[1] == 'libs' and \
+                not env['package_all'] and \
+                not self.path_components[2] in env['package']:
+            return False
+        return \
+            not self['no_executable'] and \
+            (
+                self['allowed_archs'] is None or
+                env['arch'] in self['allowed_archs']
+            )
+
+    def should_be_tested(self, env):
+        return \
+            self.should_be_built(env) and \
+            not self['interactive'] and \
+            not self['more_than_1s'] and \
+            not self['receives_signal'] and \
+            not self['requires_kernel_modules'] and \
+            not self['skip_run_unclassified'] and \
+            not (self['uses_dynamic_library'] and env['emulator'] == 'gem5')
+
     def update(self, other):
         other_tmp_properties = other.properties.copy()
         if 'cc_flags' in self.properties and 'cc_flags' in other_tmp_properties:
             other_tmp_properties['cc_flags'] = self.properties['cc_flags'] + other_tmp_properties['cc_flags']
         return self.properties.update(other_tmp_properties)
 
-    def should_be_built(self, arch):
-        return \
-            not self['no_executable'] and \
-            (
-                self['allowed_archs'] is None or
-                arch in self['allowed_archs']
-            )
-
-    def should_be_tested(self, arch):
-        return \
-            self.should_be_built(arch) and \
-            not self['interactive'] and \
-            not self['more_than_1s'] and \
-            not self['receives_signal'] and \
-            not self['requires_kernel_modules'] and \
-            not self['skip_run_unclassified']
-
 class PrefixTree:
     def __init__(self, path_properties_dict=None, children=None):
         if path_properties_dict is None:
@@ -80,7 +91,7 @@ class PrefixTree:
         if children is None:
             children = {}
         self.children = children
-        self.path_properties = PathProperties(**path_properties_dict)
+        self.path_properties = PathProperties(path_properties_dict)
 
     @staticmethod
     def make_from_tuples(tuples):
@@ -100,15 +111,17 @@ class PrefixTree:
                 todo_trees.append(new_tree)
         return top_tree
 
-def get(test_path):
+def get(path):
     cur_node = path_properties_tree
-    path_properties = PathProperties(**cur_node.path_properties.properties)
+    path_components = path.split(os.sep)
-    for path_component in test_path.split(os.sep):
+    path_properties = PathProperties(cur_node.path_properties.properties.copy())
+    for path_component in path_components:
         if path_component in cur_node.children:
             cur_node = cur_node.children[path_component]
             path_properties.update(cur_node.path_properties)
         else:
             break
+    path_properties.set_path_components(path_components)
     return path_properties
 
 default_c_std = 'c11'
@@ -128,24 +141,32 @@ freestanding_properties = {
 }
 path_properties_tuples = (
     {
-        'c_std': default_c_std,
-        'cxx_std': default_cxx_std,
-        'pedantic': True,
         'allowed_archs': None,
-        'c_std': None,
+        'c_std': default_c_std,
-        'cc_flags': [],
+        'cc_flags': [
+            '-Wall', LF,
+            '-Werror', LF,
+            '-Wextra', LF,
+            '-Wno-unused-function', LF,
+            '-fopenmp', LF,
+            '-ggdb3', LF,
+        ],
+        'cc_flags_after': [
+            '-lm', LF,
+            '-pthread', LF,
+        ],
         'cc_pedantic': True,
-        'cxx_std': None,
+        'cxx_std': default_cxx_std,
         'exit_status': 0,
         'extra_objs_lkmc_common': False,
         'extra_objs_userland_asm': False,
         'interactive': False,
-        'skip_run_unclassified': False,
         'more_than_1s': False,
         'no_executable': False,
-        'pedantic': False,
         'receives_signal': False,
         'requires_kernel_modules': False,
+        'skip_run_unclassified': False,
+        'uses_dynamic_library': False,
     },
     {
         'userland': (
@@ -240,10 +261,10 @@ path_properties_tuples = (
                 'lkmc': (
                     {'extra_objs_lkmc_common': True},
                     {
-                        'assert_fail.c': {'exit_status': 1}
+                        'assert_fail.c': {'exit_status': 1},
                     }
                 ),
-                'libs': {'skip_run_unclassified': True},
+                'libs': {'uses_dynamic_library': True},
                 'linux': {**gnu_extension_properties, **{'skip_run_unclassified': True}},
                 'posix': (
                     {},

test-user-mode

@@ -56,7 +56,7 @@ If given, run only the given tests. Otherwise, run all tests.
                             if os.path.splitext(in_filename)[1] in self.env['userland_in_exts']:
                                 path_relative_root = os.path.join(dirpath_relative_root, in_filename)
                                 my_path_properties = path_properties.get(path_relative_root)
-                                if my_path_properties.should_be_tested(self.env['arch']):
+                                if my_path_properties.should_be_tested(self.env):
                                     cur_run_args = run_args.copy()
                                     cur_run_args.update({
                                         'background': True,

userland/cpp/bst_vs_heap.cpp

@@ -1,4 +1,4 @@
-/* https://github.com/cirosantilli/linux-kernel-module-cheat#bst-vs-heap */
+// https://github.com/cirosantilli/linux-kernel-module-cheat#bst-vs-heap
 
 #include <algorithm>
 #include <iostream>

userland/cpp/hello.cpp

@@ -1,4 +1,4 @@
-/* https://github.com/cirosantilli/linux-kernel-module-cheat#sanity-checks */
+// https://github.com/cirosantilli/linux-kernel-module-cheat#sanity-checks
 
 #include <iostream>
 

userland/libs/eigen/hello.cpp

@@ -1,6 +1,5 @@
-/* https://github.com/cirosantilli/linux-kernel-module-cheat#eigen
+// https://github.com/cirosantilli/linux-kernel-module-cheat#eigen
- * Adapted from: https://eigen.tuxfamily.org/dox/GettingStarted.html
+// Adapted from: https://eigen.tuxfamily.org/dox/GettingStarted.html
- */
 
 #include <iostream>
 

userland/libs/openblas/hello.c

@@ -5,7 +5,7 @@
 #include <assert.h>
 #include <cblas.h>
 
-#include <lkmc.h>
+#include <lkmc/math.h>
 
 int main(void) {
     double A[6] = {