From 26cab92bfcc8e29affec05e931f8078da8a68056 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: Mon, 6 May 2019 00:00:01 +0000 Subject: [PATCH] baremetal: allow arbitrary exit status with the magic string test-baremetal: fix missing setting x0 return value Examples were just returning on ret without setting x0, which led to failures... those were not noticed because of how broken the testing system was ;-) --- README.adoc | 37 ++++++++--- baremetal/arch/aarch64/add.S | 7 +- baremetal/arch/aarch64/c_from_as.S | 2 +- baremetal/arch/aarch64/fadd.S | 20 +++--- baremetal/arch/aarch64/multicore.S | 3 +- .../aarch64/no_bootloader/semihost_exit.S | 2 +- baremetal/arch/aarch64/regs.S | 18 ++--- baremetal/arch/aarch64/return.S | 1 + baremetal/arch/aarch64/svc_asm.S | 1 + baremetal/arch/arm/add.S | 1 + baremetal/arch/arm/multicore.S | 1 + baremetal/arch/arm/regs.S | 1 + baremetal/lib/syscalls.c | 14 +++- baremetal/return2.c | 1 + build-baremetal | 1 + common.py | 27 +++++++- path_properties.py | 1 + rootfs_overlay/lkmc/test_fail.sh | 2 +- run | 65 ++++++++----------- test-baremetal | 5 +- 20 files changed, 133 insertions(+), 77 deletions(-) create mode 100644 baremetal/return2.c diff --git a/README.adoc b/README.adoc index 06c0073..80e1cbb 100644 --- a/README.adoc +++ b/README.adoc @@ -3524,7 +3524,7 @@ To stop at the very first instruction of a freestanding program, just use `--no- === User mode tests -Automatically run userland tests that can be run in user mode simulation, and check that they exit with status 0: +Automatically run all userland tests that can be run in user mode simulation, and check that they exit with status 0: .... ./build --all-archs test-user-mode @@ -12608,14 +12608,25 @@ We then found out that QEMU starts in EL1, and so we kept just the EL1 part, and === Baremetal tests -Automatically run non-interactive baremetal tests: +Automatically run all non-interactive baremetal tests: .... -./test-baremetal +./test-baremetal --arch aarch64 .... Source: link:test-baremetal[] +Analogously to <>, we can select individual tests or directories with: + +.... +./test-baremetal --arch aarch64 baremetal/hello.c baremetal/arch/aarch64/no_bootloader/ +.... + +which would run all of: + +* link:baremetal/hello.c[] +* all tests under the directory: link:baremetal/arch/aarch64/no_bootloader/[] + We detect if tests failed by parsing logs for the <>. We also skip tests that cannot work on certain conditions based on their basenames, e.g.: @@ -14003,19 +14014,27 @@ To debug GDB problems on gem5, you might want to enable the following <> works on all archs +* user mode: QEMU forwards exit status, gem5 we do some log parsing: <> + +For this reason, we just parse the terminal output for a magic failure string to check if tests failed. + +In order to cover all archs, our run scripts parse the serial output looking for a line line containing only exactly the magic regular expression: .... -lkmc_test_fail +lkmc_exit_status_(\d+) .... -to the terminal, then our run scripts detect that and exit with status `1`. +and then exit with the given regular expression. -This magic output string is notably used by: +This magic output string is notably generated by: -* the `lkmc_assert_fail()` function, which is used by <> +* the `exit()` baremetal function when `status != 1`. This is in turn called by failed assertions for example from `lkmc_assert_fail`, which is used by <> * link:rootfs_overlay/lkmc/test_fail.sh[], which is used by <> ==== Non-automated tests diff --git a/baremetal/arch/aarch64/add.S b/baremetal/arch/aarch64/add.S index 6c08fe8..caaffae 100644 --- a/baremetal/arch/aarch64/add.S +++ b/baremetal/arch/aarch64/add.S @@ -1,12 +1,13 @@ .global main main: /* 1 + 2 == 3 */ - mov x0, #1 + mov x0, 1 /* test-gdb-op1 */ - add x1, x0, #2 + add x1, x0, 2 /* test-gdb-result */ - cmp x1, #3 + cmp x1, 3 beq 1f bl lkmc_assert_fail 1: + mov x0, 0 ret diff --git a/baremetal/arch/aarch64/c_from_as.S b/baremetal/arch/aarch64/c_from_as.S index 8359afb..c6cef1f 100644 --- a/baremetal/arch/aarch64/c_from_as.S +++ b/baremetal/arch/aarch64/c_from_as.S @@ -1,5 +1,5 @@ /* Call a C function. */ .global main main: - mov x0, #0 + mov x0, 0 bl exit diff --git a/baremetal/arch/aarch64/fadd.S b/baremetal/arch/aarch64/fadd.S index f9259f7..e974e6c 100644 --- a/baremetal/arch/aarch64/fadd.S +++ b/baremetal/arch/aarch64/fadd.S @@ -3,43 +3,45 @@ .global main main: /* 1.5 + 2.5 == 4.0 */ - fmov d0, #1.5 + fmov d0, 1.5 /* test-gdb-d0 */ - fmov d1, #2.5 + fmov d1, 2.5 /* test-gdb-d1 */ fadd d2, d0, d1 /* test-gdb-d2 */ - fmov d3, #4.0 + fmov d3, 4.0 fcmp d2, d3 beq 1f bl lkmc_assert_fail 1: /* Now in 32-bit. */ - fmov s0, #1.5 + fmov s0, 1.5 /* test-gdb-s0 */ - fmov s1, #2.5 + fmov s1, 2.5 /* test-gdb-s1 */ fadd s2, s0, s1 /* test-gdb-s2 */ fadd s2, s0, s1 - fmov s3, #4.0 + fmov s3, 4.0 fcmp s2, s3 beq 1f bl lkmc_assert_fail 1: /* Higher registers. */ - fmov d28, #1.5 + fmov d28, 1.5 /* test-gdb-d28 */ - fmov d29, #2.5 + fmov d29, 2.5 /* test-gdb-d29 */ fadd d30, d28, d29 /* test-gdb-d30 */ - fmov d31, #4.0 + fmov d31, 4.0 /* test-gdb-d31 */ fcmp d30, d31 beq 1f bl lkmc_assert_fail 1: + + mov x0, 0 ret diff --git a/baremetal/arch/aarch64/multicore.S b/baremetal/arch/aarch64/multicore.S index 45d8f8f..742366a 100644 --- a/baremetal/arch/aarch64/multicore.S +++ b/baremetal/arch/aarch64/multicore.S @@ -3,7 +3,7 @@ .global main main: /* Reset spinlock. */ - mov x0, #0 + mov x0, 0 ldr x1, =spinlock str x0, [x1] @@ -66,6 +66,7 @@ spinlock_start: wfe cbz x0, spinlock_start + mov x0, 0 ret spinlock: diff --git a/baremetal/arch/aarch64/no_bootloader/semihost_exit.S b/baremetal/arch/aarch64/no_bootloader/semihost_exit.S index 5d87c96..634880a 100644 --- a/baremetal/arch/aarch64/no_bootloader/semihost_exit.S +++ b/baremetal/arch/aarch64/no_bootloader/semihost_exit.S @@ -6,7 +6,7 @@ mystart: movk x1, 2, lsl 16 ldr x2, =semihost_args str x1, [x2, 0] - mov x0, #0 + mov x0, 0 str x0, [x2, 8] mov x1, x2 mov w0, 0x18 diff --git a/baremetal/arch/aarch64/regs.S b/baremetal/arch/aarch64/regs.S index 894e0d1..6015239 100644 --- a/baremetal/arch/aarch64/regs.S +++ b/baremetal/arch/aarch64/regs.S @@ -4,26 +4,26 @@ */ .global main main: - mov x0, #1 + mov x0, 1 /* test-gdb-x0 */ - mov x1, #2 + mov x1, 2 /* test-gdb-x1 */ - mov x29, #1 + mov x29, 1 /* test-gdb-x29 */ - mov x30, #2 + mov x30, 2 /* test-gdb-x30 */ - fmov d0, #1.5 + fmov d0, 1.5 /* test-gdb-d0 */ - fmov d1, #2.5 + fmov d1, 2.5 /* test-gdb-d1 */ - fmov d30, #1.5 + fmov d30, 1.5 /* test-gdb-d30 */ - fmov d31, #2.5 + fmov d31, 2.5 /* test-gdb-d31 */ /* Exit required since we messed up with x30 which is the lr. */ - mov x0, #0 + mov x0, 0 bl exit diff --git a/baremetal/arch/aarch64/return.S b/baremetal/arch/aarch64/return.S index b16d907..25a43ec 100644 --- a/baremetal/arch/aarch64/return.S +++ b/baremetal/arch/aarch64/return.S @@ -1,4 +1,5 @@ /* Return to ensure that the post main works. */ .global main main: + mov x0, 0 ret diff --git a/baremetal/arch/aarch64/svc_asm.S b/baremetal/arch/aarch64/svc_asm.S index 95f1df4..2803a2f 100644 --- a/baremetal/arch/aarch64/svc_asm.S +++ b/baremetal/arch/aarch64/svc_asm.S @@ -16,6 +16,7 @@ main: 1: /* Go home. */ + mov x0, 0 ret LKMC_GLOBAL(lkmc_vector_trap_handler) diff --git a/baremetal/arch/arm/add.S b/baremetal/arch/arm/add.S index bb9eb0f..254ae1a 100644 --- a/baremetal/arch/arm/add.S +++ b/baremetal/arch/arm/add.S @@ -9,4 +9,5 @@ main: beq 1f bl lkmc_assert_fail 1: + mov r0, #0 bx lr diff --git a/baremetal/arch/arm/multicore.S b/baremetal/arch/arm/multicore.S index a051de4..b56ebbb 100644 --- a/baremetal/arch/arm/multicore.S +++ b/baremetal/arch/arm/multicore.S @@ -32,6 +32,7 @@ spinlock_start: wfe cmp r0, #0 beq spinlock_start + mov r0, #0 bx lr spinlock: .skip 4 diff --git a/baremetal/arch/arm/regs.S b/baremetal/arch/arm/regs.S index dfca10c..4b38b82 100644 --- a/baremetal/arch/arm/regs.S +++ b/baremetal/arch/arm/regs.S @@ -5,4 +5,5 @@ main: /* test-gdb-r0 */ mov r1, #2 /* test-gdb-r1 */ + mov r0, #0 bx lr diff --git a/baremetal/lib/syscalls.c b/baremetal/lib/syscalls.c index 5914731..3203832 100644 --- a/baremetal/lib/syscalls.c +++ b/baremetal/lib/syscalls.c @@ -62,13 +62,23 @@ int _write(int file, char *ptr, int len) { return len; } -/* Only 0 is supported for now, arm semihosting cannot handle other values. */ void _exit(int status) { + if (status != 0) { + /* https://github.com/cirosantilli/linux-kernel-module-cheat#magic-failure-string */ + printf("lkmc_exit_status_%d\n", status); + } #if defined(GEM5) LKMC_M5OPS_EXIT; #else #if defined(__arm__) - __asm__ __volatile__ ("mov r0, #0x18; ldr r1, =#0x20026; svc 0x00123456" : : : "r0", "r1"); + __asm__ __volatile__ ( + "mov r0, #0x18\n" + "ldr r1, =#0x20026\n" + "svc 0x00123456\n" + : + : + : "r0", "r1" + ); #elif defined(__aarch64__) /* TODO actually use the exit value here, just for fun. */ __asm__ __volatile__ ( diff --git a/baremetal/return2.c b/baremetal/return2.c new file mode 100644 index 0000000..51f253b --- /dev/null +++ b/baremetal/return2.c @@ -0,0 +1 @@ +int main(void) { return 2; } diff --git a/build-baremetal b/build-baremetal index 6388bcd..b32325a 100755 --- a/build-baremetal +++ b/build-baremetal @@ -136,6 +136,7 @@ Build the baremetal examples with crosstool-NG. in_ext in (self.env['c_ext'], self.env['asm_ext']) ): out = os.path.join(out_dir, in_name + self.env['baremetal_build_ext']) + print(out) if self.need_rebuild( common_objs_bootloader + [self.env['baremetal_link_script'] + self.env['common_h']], out diff --git a/common.py b/common.py index d654632..be4b42b 100644 --- a/common.py +++ b/common.py @@ -115,7 +115,7 @@ consts['userland_out_exts'] = [ consts['obj_ext'], ] consts['default_config_file'] = os.path.join(consts['data_dir'], 'config.py') -consts['magic_fail_string'] = b'lkmc_test_fail' +consts['serial_magic_exit_status_regexp_string'] = b'lkmc_exit_status_(\d+)' consts['baremetal_lib_basename'] = 'lib' consts['emulator_userland_only_short_to_long_dict'] = collections.OrderedDict([ ('n', 'native'), @@ -482,6 +482,25 @@ CLI arguments to pass to the userland executable. ) # Run. + self.add_argument( + '--background', + default=False, + help='''\ +Make programs that would take over the terminal such as QEMU full system run on the +background instead. + +Currently only implemented for ./run. + +Interactive input cannot be given. + +Send QEMU serial output to a file instead of the host terminal. + +TODO: use a port instead. If only there was a way to redirect a serial to multiple +places, both to a port and a file? We use the file currently to be able to have +any output at all. +https://superuser.com/questions/1373226/how-to-redirect-qemu-serial-output-to-both-a-file-and-the-terminal-or-a-port +''' + ) self.add_argument( '--in-tree', default=False, @@ -763,7 +782,10 @@ Incompatible archs are skipped. env['executable'] = env['qemu_executable'] env['run_dir'] = env['qemu_run_dir'] env['termout_file'] = env['qemu_termout_file'] - env['guest_terminal_file'] = env['qemu_termout_file'] + if env['background']: + env['guest_terminal_file'] = env['qemu_background_serial_file'] + else: + env['guest_terminal_file'] = env['qemu_termout_file'] env['trace_txt_file'] = env['qemu_trace_txt_file'] env['run_cmd_file'] = join(env['run_dir'], 'run.sh') @@ -1353,7 +1375,6 @@ https://github.com/cirosantilli/linux-kernel-module-cheat#gem5-debug-build **self._build_arguments[argument_name] ) - def _build_one( self, in_path, diff --git a/path_properties.py b/path_properties.py index 3584304..f77313f 100644 --- a/path_properties.py +++ b/path_properties.py @@ -236,6 +236,7 @@ path_properties_tuples = ( ), 'getchar.c': {'interactive': True}, 'return1.c': {'exit_status': 1}, + 'return2.c': {'exit_status': 2}, } ), 'userland': ( diff --git a/rootfs_overlay/lkmc/test_fail.sh b/rootfs_overlay/lkmc/test_fail.sh index 682d36b..87931c8 100755 --- a/rootfs_overlay/lkmc/test_fail.sh +++ b/rootfs_overlay/lkmc/test_fail.sh @@ -1,3 +1,3 @@ #!/bin/sh # https://github.com/cirosantilli/linux-kernel-module-cheat#magic-failure-string -echo lkmc_test_fail +echo lkmc_exit_status_1 diff --git a/run b/run index 5a77ad5..3e1466e 100755 --- a/run +++ b/run @@ -15,18 +15,6 @@ class Main(common.LkmcCliFunction): super().__init__( description='''\ Run some content on an emulator. -''' - ) - self.add_argument( - '--background', - default=False, - help='''\ -Send QEMU serial output to a file instead of the terminal so it does not require a -terminal attached to run on the background. Interactive input cannot be given. -TODO: use a port instead. If only there was a way to redirect a serial to multiple -places, both to a port and a file? We use the file currently to be able to have -any output at all. -https://superuser.com/questions/1373226/how-to-redirect-qemu-serial-output-to-both-a-file-and-the-terminal-or-a-port ''' ) self.add_argument( @@ -241,10 +229,11 @@ Setup a kernel init parameter that makes the emulator quit immediately after boo '--terminal', default=False, help='''\ -Output to the terminal, don't pipe to tee as the default. -Does not save the output to a file, but allows you to use debuggers. -Set automatically by --debug-vm, but you still need this option to debug -gem5 Python scripts with pdb. +Output directly to the terminal, don't pipe to tee as the default. +With this, we don't not save the output to a file as is done by default, +but we are able to do things that require not having a pipe suh as you to +using debuggers. This option issSet automatically by --debug-vm, but you still need +it to debug gem5 Python scripts with pdb. ''' ) self.add_argument( @@ -570,7 +559,7 @@ Extra options to append at the end of the emulator command line. serial_monitor = [] else: if self.env['background']: - serial_monitor = ['-serial', 'file:{}'.format(self.env['qemu_background_serial_file']), LF] + serial_monitor = ['-serial', 'file:{}'.format(self.env['guest_terminal_file']), LF] if self.env['quiet']: show_stdout = False else: @@ -752,34 +741,36 @@ Extra options to append at the end of the emulator command line. show_stdout=show_stdout, ) if exit_status == 0: - # Check if guest panicked. - if self.env['emulator'] == 'gem5': - # We have to do some parsing here because gem5 exits with status 0 even when panic happens. - # Grepping for '^panic: ' does not work because some errors don't show that message. - panic_msg = b'--- BEGIN LIBC BACKTRACE ---$' - else: - panic_msg = b'Kernel panic - not syncing' - panic_re = re.compile(panic_msg) error_string_found = False exit_status = 0 if out_file is not None and not self.env['dry_run']: - with open(self.env['termout_file'], 'br') as logfile: - line = None - for line in logfile: - if panic_re.search(line): - exit_status = 1 - if line is not None: - last_line = line.rstrip() - match = re.search(b'Simulated exit code not 0! Exit code is (\d+)', last_line) - if match: - exit_status = int(match.group(1)) + if self.env['emulator'] == 'gem5': + with open(self.env['termout_file'], 'br') as logfile: + # We have to do some parsing here because gem5 exits with status 0 even when panic happens. + # Grepping for '^panic: ' does not work because some errors don't show that message... + gem5_panic_re = re.compile(b'--- BEGIN LIBC BACKTRACE ---$') + line = None + for line in logfile: + if gem5_panic_re.search(line): + exit_status = 1 + if self.env['userland']: + if line is not None: + last_line = line.rstrip() + match = re.search(b'Simulated exit code not 0! Exit code is (\d+)', last_line) + if match: + exit_status = int(match.group(1)) if not self.env['userland']: if os.path.exists(self.env['guest_terminal_file']): with open(self.env['guest_terminal_file'], 'br') as logfile: + linux_panic_re = re.compile(b'Kernel panic - not syncing') + serial_magic_exit_status_regexp = re.compile(self.env['serial_magic_exit_status_regexp_string']) for line in logfile.readlines(): - if line.rstrip() == self.env['magic_fail_string']: + line = line.rstrip() + if not self.env['baremetal'] and linux_panic_re.search(line): exit_status = 1 - break + match = serial_magic_exit_status_regexp.match(line) + if match: + exit_status = int(match.group(1)) if exit_status != 0 and self.env['show_stdout']: self.log_error('simulation error detected by parsing logs') return exit_status diff --git a/test-baremetal b/test-baremetal index 5fc55b9..18fcea7 100755 --- a/test-baremetal +++ b/test-baremetal @@ -4,6 +4,8 @@ import os import sys import common +import path_properties +from thread_pool import ThreadPool class Main(common.TestCliFunction): def __init__(self): @@ -26,6 +28,7 @@ If given, run only the given tests. Otherwise, run all tests. def timed_main(self): run_args = self.get_common_args() + rootdir_abs_len = len(self.env['root_dir']) with ThreadPool( self.run_test, nthreads=self.env['nproc'], @@ -49,7 +52,7 @@ If given, run only the given tests. Otherwise, run all tests. error = thread_pool.submit({ 'expected_exit_status': my_path_properties['exit_status'], 'run_args': cur_run_args, - 'run_obj': self.import_path_main('run'), + 'run_obj': self.import_path_main('run'), 'test_id': path_relative_root, }) if error is not None: