From 9cd48d5184f5bc2c3d9d58149298c4575ed7ee6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ciro=20Santilli=20=E5=85=AD=E5=9B=9B=E4=BA=8B=E4=BB=B6=20?= =?UTF-8?q?=E6=B3=95=E8=BD=AE=E5=8A=9F?= Date: Sun, 5 May 2019 00:00:00 +0000 Subject: [PATCH] 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 | 52 +++++++++--- build-userland | 110 +++++++++---------------- common.py | 17 ++++ lkmc.c | 15 +--- lkmc.h | 1 - lkmc/math.h | 20 +++++ path_properties.py | 141 +++++++++++++++++++-------------- test-user-mode | 2 +- userland/cpp/bst_vs_heap.cpp | 2 +- userland/cpp/hello.cpp | 2 +- userland/libs/eigen/hello.cpp | 5 +- userland/libs/openblas/hello.c | 2 +- 12 files changed, 207 insertions(+), 162 deletions(-) create mode 100644 lkmc/math.h diff --git a/README.adoc b/README.adoc index 40d562c..701236f 100644 --- a/README.adoc +++ b/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 <>, 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. <> 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. <> 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: <>. 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 <> 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/[] diff --git a/build-userland b/build-userland index c095d94..a9b3d2e 100755 --- a/build-userland +++ b/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['pedantic']: + if my_path_properties.should_be_built(self.env): + 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, diff --git a/common.py b/common.py index c091d1e..fbc6285 100644 --- a/common.py +++ b/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']) diff --git a/lkmc.c b/lkmc.c index 23d56d7..2ceac96 100644 --- a/lkmc.c +++ b/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)) { \ diff --git a/lkmc.h b/lkmc.h index ed7df9e..6ba10b9 100644 --- a/lkmc.h +++ b/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 */ diff --git a/lkmc/math.h b/lkmc/math.h new file mode 100644 index 0000000..80e42ca --- /dev/null +++ b/lkmc/math.h @@ -0,0 +1,20 @@ +#ifndef LKMC_MATH_H +#define LKMC_MATH_H + +#include +#include + +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 diff --git a/path_properties.py b/path_properties.py index 2f719ed..c040f96 100644 --- a/path_properties.py +++ b/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 = { - 'allowed_archs', - '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: + for key in properties: + if not key in self.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) - for path_component in test_path.split(os.sep): + path_components = 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, - 'cc_flags': [], + 'c_std': default_c_std, + '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': ( {}, diff --git a/test-user-mode b/test-user-mode index 17f176c..ab34a57 100755 --- a/test-user-mode +++ b/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, diff --git a/userland/cpp/bst_vs_heap.cpp b/userland/cpp/bst_vs_heap.cpp index 18eedcd..dca4028 100644 --- a/userland/cpp/bst_vs_heap.cpp +++ b/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 #include diff --git a/userland/cpp/hello.cpp b/userland/cpp/hello.cpp index 2a435e9..b9ca976 100644 --- a/userland/cpp/hello.cpp +++ b/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 diff --git a/userland/libs/eigen/hello.cpp b/userland/libs/eigen/hello.cpp index dc29adc..3d8d00c 100644 --- a/userland/libs/eigen/hello.cpp +++ b/userland/libs/eigen/hello.cpp @@ -1,6 +1,5 @@ -/* https://github.com/cirosantilli/linux-kernel-module-cheat#eigen - * Adapted from: https://eigen.tuxfamily.org/dox/GettingStarted.html - */ +// https://github.com/cirosantilli/linux-kernel-module-cheat#eigen +// Adapted from: https://eigen.tuxfamily.org/dox/GettingStarted.html #include diff --git a/userland/libs/openblas/hello.c b/userland/libs/openblas/hello.c index 765fa89..16e63d0 100644 --- a/userland/libs/openblas/hello.c +++ b/userland/libs/openblas/hello.c @@ -5,7 +5,7 @@ #include #include -#include +#include int main(void) { double A[6] = {