From a3f4047a9b66db736e4e0192f2170d0d443310dc Mon Sep 17 00:00:00 2001 From: Elliot Baptist Date: Wed, 24 Apr 2024 12:19:20 +0100 Subject: [PATCH] Stop `model_init()` overriding coverage filename The addition of a `sail_set_coverage_file()` call into `model_init()` in commit 28ef9e9 seems to have unintentionally restricted the ability to use `sail_set_coverage_file()` elsewhere. Any program that calls Sail's `model_init()` after performing its own `sail_set_coverage_file()` call will now behave differently. The coverage filename specified at runtime will be overridden, even if the sail `-c_coverage_output` argument was not used. This issue was found when using the c_emulator/cheri_riscv_rvfi_RV32 binary generated by the CTSRD-CHERI/sail-cheri-riscv repository. It calls `model_init()` as part of resetting between tests when running in a particular multi-test mode. The addition of the `sail_set_coverage_file()` call into `model_init()` seems to have been to allow test/sailcov/run_tests.py to use a coverage filename other than the default ("sail_coverage") specified in lib/coverage/libsail_coverage.a (generated from lib/coverage/src/lib.rs). This can alternately be achieved by adding a `-c` (coverage) argument to the argument parser included in the "simple_if.bin" binary built as part of the test, and removing the changes to `model_init()`. This solution seems to satisfy the original requirement without the adverse change in behaviour in other programs. --- lib/rts.c | 11 ++++++++++- src/sail_c_backend/c_backend.ml | 5 ----- src/sail_c_backend/c_backend.mli | 2 -- src/sail_c_backend/sail_plugin_c.ml | 4 ---- test/sailcov/run_tests.py | 4 ++-- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/rts.c b/lib/rts.c index b07457628..1b58658af 100644 --- a/lib/rts.c +++ b/lib/rts.c @@ -631,6 +631,7 @@ static struct option options[] = { {"elf", required_argument, 0, 'e'}, {"entry", required_argument, 0, 'n'}, {"image", required_argument, 0, 'i'}, + {"coverage", required_argument, 0, 'c'}, {"verbosity", required_argument, 0, 'v'}, {"help", no_argument, 0, 'h'}, {0, 0, 0, 0} @@ -654,7 +655,7 @@ int process_arguments(int argc, char *argv[]) while (true) { int option_index = 0; - c = getopt_long(argc, argv, "e:n:i:b:l:C:v:h", options, &option_index); + c = getopt_long(argc, argv, "e:n:i:b:l:C:c:v:h", options, &option_index); if (c == -1) break; @@ -721,6 +722,14 @@ int process_arguments(int argc, char *argv[]) } break; + case 'c': +#ifdef SAIL_COVERAGE_H + sail_set_coverage_file(optarg); +#else + fprintf(stderr, "Ignoring flag -c %s. Requires sail arg: -c_include sail_coverage.h", optarg); +#endif + break; + case 'v': if (!sscanf(optarg, "0x%" PRIx64, &g_verbosity)) { fprintf(stderr, "Could not parse verbosity flags %s\n", optarg); diff --git a/src/sail_c_backend/c_backend.ml b/src/sail_c_backend/c_backend.ml index 59d187159..7e10f4b59 100644 --- a/src/sail_c_backend/c_backend.ml +++ b/src/sail_c_backend/c_backend.ml @@ -90,7 +90,6 @@ let opt_prefix = ref "z" let opt_extra_params = ref None let opt_extra_arguments = ref None let opt_branch_coverage = ref None -let opt_branch_coverage_output = ref "sail_coverage" let extra_params () = match !opt_extra_params with Some str -> str ^ ", " | _ -> "" @@ -2058,10 +2057,6 @@ let compile_ast env effect_info output_chan c_includes ast = separate hardline (List.map string ([Printf.sprintf "%svoid model_init(void)" (static ()); "{"; " setup_rts();"] - @ ( if Option.is_some !opt_branch_coverage then - [Printf.sprintf " sail_set_coverage_file(\"%s\");" !opt_branch_coverage_output] - else [] - ) @ fst exn_boilerplate @ startup cdefs @ letbind_initializers @ List.concat (List.map (fun r -> fst (register_init_clear r)) regs) @ (if regs = [] then [] else [Printf.sprintf " %s(UNIT);" (sgen_function_id (mk_id "initialize_registers"))]) diff --git a/src/sail_c_backend/c_backend.mli b/src/sail_c_backend/c_backend.mli index 4d9bccdf8..863a66a88 100644 --- a/src/sail_c_backend/c_backend.mli +++ b/src/sail_c_backend/c_backend.mli @@ -111,8 +111,6 @@ val opt_extra_arguments : string option ref val opt_branch_coverage : out_channel option ref -val opt_branch_coverage_output : string ref - (** Optimization flags *) val optimize_primops : bool ref diff --git a/src/sail_c_backend/sail_plugin_c.ml b/src/sail_c_backend/sail_plugin_c.ml index 31f4a8f78..6d9c4f4ff 100644 --- a/src/sail_c_backend/sail_plugin_c.ml +++ b/src/sail_c_backend/sail_plugin_c.ml @@ -104,10 +104,6 @@ let c_options = Arg.String (fun str -> C_backend.opt_branch_coverage := Some (open_out str)), " Turn on coverage tracking and output information about all branches and functions to a file" ); - ( "-c_coverage_output", - Arg.String (fun str -> C_backend.opt_branch_coverage_output := str), - " The generated C will output coverage at runtime to the filename provided (default sail_coverage)" - ); ( "-O", Arg.Tuple [ diff --git a/test/sailcov/run_tests.py b/test/sailcov/run_tests.py index 2a8edcbec..b9d8409cc 100755 --- a/test/sailcov/run_tests.py +++ b/test/sailcov/run_tests.py @@ -38,9 +38,9 @@ def test_sailcov(): basename = os.path.splitext(os.path.basename(filename))[0] tests[filename] = os.fork() if tests[filename] == 0: - step('{} -no_warn -no_memo_z3 -c -c_include sail_coverage.h -c_coverage {}.branches -c_coverage_output {}.taken {} -o {}'.format(sail, basename, basename, filename, basename)) + step('{} -no_warn -no_memo_z3 -c -c_include sail_coverage.h -c_coverage {}.branches {} -o {}'.format(sail, basename, filename, basename)) step('cc {}.c {}/lib/*.c {}/lib/coverage/libsail_coverage.a -lgmp -lz -lpthread -ldl -I {}/lib -o {}.bin'.format(basename, sail_dir, sail_dir, sail_dir, basename)) - step('./{}.bin'.format(basename)) + step('./{}.bin -c {}.taken'.format(basename, basename)) step('{} --all {}.branches --taken {}.taken {}'.format(sailcov, basename, basename, filename)) step('diff {}.html {}.expect'.format(basename, basename)) print_ok(filename)