From f063dfd966f00f90fbae94d179f26cf36fea3f5b Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Mon, 22 Jul 2024 09:59:09 +0200 Subject: executor: fix writing of remote coverage We never reset remote coverage, so if there is one block, we will write it after every call and multiple times at the end. It can lead to "too many calls in output" and just writes quadratic amount of coverage/signal. Reset remote coverage after writing. --- executor/common_test.h | 11 +++++++++++ executor/executor.cc | 5 ++++- executor/executor_test.h | 13 +++++++++++-- pkg/runtest/run_test.go | 24 ++++++++++++++++++++++-- sys/test/exec.txt | 1 + 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/executor/common_test.h b/executor/common_test.h index dc39fa326..f5f54d9ee 100644 --- a/executor/common_test.h +++ b/executor/common_test.h @@ -180,6 +180,17 @@ static long syz_inject_cover(volatile long a, volatile long b) #endif #endif +#if SYZ_EXECUTOR || __NR_syz_inject_remote_cover +static long syz_inject_remote_cover(volatile long a, volatile long b) +#if SYZ_EXECUTOR + ; // defined in executor_test.h +#else +{ + return 0; +} +#endif +#endif + #if SYZ_EXECUTOR || SYZ_SYSCTL static void setup_sysctl() { diff --git a/executor/executor.cc b/executor/executor.cc index 3a1ce78bd..17de4e87d 100644 --- a/executor/executor.cc +++ b/executor/executor.cc @@ -1001,7 +1001,9 @@ void execute_one() // that we were killed on timeout before we write any. // Check for extra coverage is very cheap, effectively a memory load. const uint64 kSleepMs = 100; - for (uint64 i = 0; i < prog_extra_cover_timeout / kSleepMs; i++) { + for (uint64 i = 0; i < prog_extra_cover_timeout / kSleepMs && + output_data->completed.load(std::memory_order_relaxed) < kMaxCalls; + i++) { sleep_ms(kSleepMs); write_extra_output(); } @@ -1267,6 +1269,7 @@ void write_extra_output() if (!extra_cov.size) return; write_output(-1, &extra_cov, rpc::CallFlag::NONE, 997, all_extra_signal); + cover_reset(&extra_cov); } flatbuffers::span finish_output(OutputData* output, int proc_id, uint64 req_id, uint64 elapsed, diff --git a/executor/executor_test.h b/executor/executor_test.h index 5e128d851..c94de09a2 100644 --- a/executor/executor_test.h +++ b/executor/executor_test.h @@ -121,9 +121,8 @@ static void cover_unprotect(cover_t* cov) { } -static long syz_inject_cover(volatile long a, volatile long b) +static long inject_cover(cover_t* cov, long a, long b) { - cover_t* cov = ¤t_thread->cov; if (cov->data == nullptr) return ENOENT; uint32 size = std::min((uint32)b, cov->mmap_alloc_size); @@ -132,6 +131,16 @@ static long syz_inject_cover(volatile long a, volatile long b) return 0; } +static long syz_inject_cover(volatile long a, volatile long b) +{ + return inject_cover(¤t_thread->cov, a, b); +} + +static long syz_inject_remote_cover(volatile long a, volatile long b) +{ + return inject_cover(&extra_cov, a, b); +} + static const char* setup_fault() { return nullptr; diff --git a/pkg/runtest/run_test.go b/pkg/runtest/run_test.go index 3da87dd6c..0861b7e2b 100644 --- a/pkg/runtest/run_test.go +++ b/pkg/runtest/run_test.go @@ -144,6 +144,7 @@ func TestCover(t *testing.T) { type CoverTest struct { Is64Bit bool + ExtraCoverage bool Input []byte MaxSignal []uint64 CoverFilter []uint64 @@ -356,6 +357,15 @@ func testCover(t *testing.T, target *prog.Target) { 0xc0000020, 0xc0000010, 0xc0000001}, Signal: []uint64{0xc0001100, 0xc0000140, 0xc0000011, 0xc0000001}, }, + // Extra coverage. + { + Is64Bit: true, + ExtraCoverage: true, + Input: makeCover64(0xc0dec0dec0000001, 0xc0dec0dec0000010), + Flags: flatrpc.ExecFlagCollectSignal | flatrpc.ExecFlagCollectCover, + Cover: []uint64{0xc0dec0dec0000010, 0xc0dec0dec0000001}, + Signal: []uint64{0xc0dec0dec0000011, 0xc0dec0dec0000001}, + }, } executor := csource.BuildExecutor(t, target, "../../") for i, test := range tests { @@ -382,7 +392,11 @@ func testCover(t *testing.T, target *prog.Target) { } func testCover1(t *testing.T, ctx context.Context, target *prog.Target, test CoverTest, source *queue.PlainQueue) { - text := fmt.Sprintf(`syz_inject_cover(&AUTO="%s", AUTO)`, hex.EncodeToString(test.Input)) + callName := "syz_inject_cover" + if test.ExtraCoverage { + callName = "syz_inject_remote_cover" + } + text := fmt.Sprintf(`%s(&AUTO="%s", AUTO)`, callName, hex.EncodeToString(test.Input)) p, err := target.Deserialize([]byte(text), prog.Strict) if err != nil { t.Fatal(err) @@ -390,7 +404,7 @@ func testCover1(t *testing.T, ctx context.Context, target *prog.Target, test Cov req := &queue.Request{ Prog: p, ExecOpts: flatrpc.ExecOpts{ - EnvFlags: flatrpc.ExecEnvSignal | flatrpc.ExecEnvSandboxNone, + EnvFlags: flatrpc.ExecEnvSignal | flatrpc.ExecEnvExtraCover | flatrpc.ExecEnvSandboxNone, ExecFlags: test.Flags, }, } @@ -403,6 +417,12 @@ func testCover1(t *testing.T, ctx context.Context, target *prog.Target, test Cov t.Fatalf("program execution failed: status=%v err=%v\n%s", res.Status, res.Err, res.Output) } call := res.Info.Calls[0] + if test.ExtraCoverage { + call = res.Info.Extra + if call == nil { + t.Fatalf("got no extra coverage info") + } + } if test.Cover == nil { test.Cover = []uint64{} } diff --git a/sys/test/exec.txt b/sys/test/exec.txt index fb895bd18..0faef9820 100644 --- a/sys/test/exec.txt +++ b/sys/test/exec.txt @@ -13,6 +13,7 @@ syz_compare_zlib(data ptr[in, array[int8]], size bytesize[data], zdata ptr[in, c # Copies the data into KCOV buffer verbatim. syz_inject_cover(ptr ptr[in, array[int8]], size bytesize[ptr]) +syz_inject_remote_cover(ptr ptr[in, array[int8]], size bytesize[ptr]) (prog_timeout[1000], remote_cover) compare_data [ align0 align0 -- cgit mrf-deployment