Skip to content

Commit

Permalink
Stop model_init() overriding coverage filename
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
elliotb-lowrisc authored and Alasdair committed Apr 24, 2024
1 parent 0f29616 commit a3f4047
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 14 deletions.
11 changes: 10 additions & 1 deletion lib/rts.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions src/sail_c_backend/c_backend.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ^ ", " | _ -> ""

Expand Down Expand Up @@ -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"))])
Expand Down
2 changes: 0 additions & 2 deletions src/sail_c_backend/c_backend.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions src/sail_c_backend/sail_plugin_c.ml
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ let c_options =
Arg.String (fun str -> C_backend.opt_branch_coverage := Some (open_out str)),
"<file> 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),
"<file> The generated C will output coverage at runtime to the filename provided (default sail_coverage)"
);
( "-O",
Arg.Tuple
[
Expand Down
4 changes: 2 additions & 2 deletions test/sailcov/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit a3f4047

Please sign in to comment.