[libFuzzer] Add unstable function printing to print_unstable_stats flag

[kevinwkt]

Summary

There may be cases in which a user wants to know which part of their code is unstable.
We use ObservedFuncs and UnstableCounters to print at exit which of the ObservedFunctions
are unstable under the -print_unstable_stats flag.

Patch by Kyungtak Woo (@kevinwkt).

[Dor1s]

Very nice!

lib/fuzzer/FuzzerFlags.def

105 ↗

(On Diff #159045)

Let's re-phrase a bit, maybe: "If 1, at exit print functions that have unstable coverage edges. Requires -handle_unstable!=0"

Still sounds bad to me though, hopefully Jonathan can help :)

lib/fuzzer/FuzzerLoop.cpp

361 ↗

(On Diff #159045)

but we need some assertion mechanism to make sure it's not possible to use -print_unstable_funcs without -handle_unstable.

Another idea I have: can we re-use PrintUnstableStats flag for that? I think it doesn't need handle_unstable. Let's just print functions under that flag as well.

test/fuzzer/print-unstable-funcs.test

1 ↗

(On Diff #159045)

It would be nice if we could have at least one function with arguments, just to make sure it also gets printed properly.

[metzman]

lib/fuzzer/FuzzerFlags.def

105 ↗

(On Diff #159045)

Sounds pretty good actually. I would just change the ordering slightly:
"If 1, print functions that have unstable coverage edges at exit. Requires -handle_unstable = 1 or 2".
Will it work with print_unstable_stats?
I think an option that depends on another is bad, we should avoid this.

[kevinwkt]

PTAL
Merged -print_unstable_stats with -print_unstable_funcs.
Added better test cases with arguments.

[Dor1s]

Please update the CL summary and description. LGTM, otherwise.

[metzman]

LGTM with Max's suggested fix.

[morehouse]

lib/fuzzer/FuzzerTracePC.cpp

358

Since we always print this along with PrintUnstableStats, let's move the implementation there.

test/fuzzer/PrintUnstableStatsTest.cpp

26

Why are these changes necessary?

test/fuzzer/print_unstable_stats.test

6

I thought IsUnstable is set if there's any mismatch at all. Which would mean the ini functions should have IsUnstable set.

20

Are these {{^}} and {{$}} necessary?

[morehouse]

lib/fuzzer/FuzzerTracePC.cpp

384

Can we put the counter increment inside this lambda as well?

test/fuzzer/PrintUnstableStatsTest.cpp

26

Why are these changes necessary?

test/fuzzer/print_unstable_stats.test

6

I thought IsUnstable is set if there's any mismatch at all. Which would mean the ini functions should have IsUnstable set.

[kevinwkt]

test/fuzzer/PrintUnstableStatsTest.cpp

26

These changes were requested by Max so that we could verify that a function with parameters will be printed correctly. We can revert if needed.

test/fuzzer/print_unstable_stats.test

6

IsUnstable is set but because the hit count for ini functions are set to 0, they are not picked up for ObservedFunctions. Because those functions were technically "not observed", they are not printed. Basically, this would only print unstable functions that are not caused by "initialization".

20

During fuzzing libFuzzer prints information containing function names such as:
#11 NEW cov: 18 ft: 19 corp: 5/6b lim: 4 exec/s: 0 rss: 28Mb L: 1/2 MS: 1 ChangeBit-
NEW_FUNC[1/1]: 0x584710 in t3() /.../llvm2/projects/compiler-rt/test/fuzzer/PrintUnstableStatsTest.cpp:24
#48 NEW cov: 20 ft: 21 corp: 6/8b lim: 4 exec/s: 0 rss: 28Mb L: 2/2 MS: 2 EraseBytes-CrossOver-

I wanted to make sure it was getting the functions I was checking for and not for the examples above.

[kevinwkt]

ptal
Included counter increase for stability stats within the lambda.

[morehouse]

test/fuzzer/print_unstable_stats.test

20

The check for UNSTABLE_FUNCTIONS: ensures any following matches will come after the UNSTABLE_FUNCTIONS: line, so there shouldn't be any other output from libFuzzer that will match.

[kevinwkt]

ptal
Fixed print_unstable_stats tests to simplify regex.

test/fuzzer/print_unstable_stats.test

20

Yeah, you're right. Will take out.

[Dor1s]

LGTM, will land soon

[Dor1s]

rebase and getting ready to land

lib/fuzzer/FuzzerTracePC.cpp

if (EF->__sanitizer_dump_coverage) {
     for (size_t i = 0; i < GetNumPCs(); i++)
       PCsCopy[i] = PCs()[i] ? GetPreviousInstructionPc(PCs()[i]) : 0;
     EF->__sanitizer_dump_coverage(PCsCopy.data(), PCsCopy.size());
   }
 }
 
 void TracePC::PrintUnstableStats() {
   size_t count = 0;
-  for (size_t i = 0; i < NumInline8bitCounters; i++)
-    if (UnstableCounters[i].IsUnstable)
+  Printf("UNSTABLE_FUNCTIONS:\n");
+  IterateInline8bitCounters([&](int i, int j, int UnstableIdx) {
+    const PCTableEntry &TE = ModulePCTable[i].Start[j];
+    if (UnstableCounters[UnstableIdx].IsUnstable) {
       count++;
+      if (ObservedFuncs.count(TE.PC)) {
+        auto VisualizePC = GetNextInstructionPc(TE.PC);
+        std::string FunctionStr = DescribePC("%F", VisualizePC);
+        if (FunctionStr.find("in ") == 0)
+          FunctionStr = FunctionStr.substr(3);
+        Printf("%s\n", FunctionStr.c_str());
+      }
+    }
+  });
+
   Printf("stat::stability_rate: %.2f\n",
          100 - static_cast<float>(count * 100) / NumInline8bitCounters);
 }
 
 // Value profile.
 // We keep track of various values that affect control flow.
 // These values are inserted into a bit-set-based hash map.
 // Every new bit in the map is treated as a new coverage.
 //
 // For memcmp/strcmp/etc the interesting value is the length of the common
 // prefix of the parameters.
 // For cmp instructions the interesting value is a XOR of the parameters.
 // The interesting value is mixed up with the PC and is then added to the map.
 
 ATTRIBUTE_NO_SANITIZE_ALL
 void TracePC::AddValueForMemcmp(void *caller_pc, const void *s1, const void *s2,
                                 size_t n, bool StopAtZero) {
   if (!n) return;
   size_t Len = std::min(n, Word::GetMaxSize());
   const uint8_t *A1 = reinterpret_cast<const uint8_t *>(s1);
   const uint8_t *A2 = reinterpret_cast<const uint8_t *>(s2);
   uint8_t B1[Word::kMaxSize];
   uint8_t B2[Word::kMaxSize];
   // Copy the data into locals in this non-msan-instrumented function

test/fuzzer/PrintUnstableStatsTest.cpp

 __attribute__((noinline)) void det2() { x++; }
 __attribute__((noinline)) void det3() { x++; }
 __attribute__((noinline)) void det4() { x++; }
 
 __attribute__((noinline)) void ini0() { x++; }
 __attribute__((noinline)) void ini1() { x++; }
 __attribute__((noinline)) void ini2() { x++; }
 
-__attribute__((noinline)) void t0() { x++; }
+__attribute__((noinline)) void t0(int a) { x += a; }
 __attribute__((noinline)) void t1() { x++; }
-__attribute__((noinline)) void t2() { x++; }
+__attribute__((noinline)) void t2(int a, int b) { x += a + b; }
 __attribute__((noinline)) void t3() { x++; }
-__attribute__((noinline)) void t4() { x++; }
+__attribute__((noinline)) void t4(int a, int b, int c) { x += a + b +c; }
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   if (Size == 1 && Data[0] == 'A' && !skip0) {
     skip0 = true;
     ini0();
   }
   if (Size == 1 && Data[0] == 'B' && !skip1) {
     skip1 = true;
     ini1();
   }
   if (Size == 1 && Data[0] == 'C' && !skip2) {
     skip2 = true;
     ini2();
   }
 
   det0();
   det1();
   int a = rand();
   det2();
 
   switch (a % 5) {
   case 0:
-    t0();
+    t0(a);
     break;
   case 1:
     t1();
     break;
   case 2:
-    t2();
+    t2(a, a);
     break;
   case 3:
     t3();
     break;
   case 4:
-    t4();
+    t4(a, a, a);
     break;
   default:
     assert(false);
   }
 
   det3();
   det4();
   return 0;
 }

test/fuzzer/handle-unstable.test

 # Tests -handle_unstable
 UNSUPPORTED: aarch64
 
 RUN: %cpp_compiler %S/PrintUnstableStatsTest.cpp -o %t-HandleUnstableTest
 
 ; Normal
 RUN: %run %t-HandleUnstableTest -print_coverage=1 -runs=100000 2>&1 | FileCheck %s --check-prefix=NORMAL
 NORMAL-DAG: det0()
 NORMAL-DAG: det1()
 NORMAL-DAG: det2()
 NORMAL-DAG: det3()
 NORMAL-DAG: det4()
 NORMAL-DAG: ini0()
 NORMAL-DAG: ini1()
 NORMAL-DAG: ini2()
-NORMAL-DAG: t0()
+NORMAL-DAG: t0(int)
 NORMAL-DAG: t1()
-NORMAL-DAG: t2()
+NORMAL-DAG: t2(int, int)
 NORMAL-DAG: t3()
-NORMAL-DAG: t4()
+NORMAL-DAG: t4(int, int, int)
 
 ; MinUnstable
 RUN: %run %t-HandleUnstableTest -print_coverage=1 -handle_unstable=1 -runs=100000 2>&1 | FileCheck %s --check-prefix=MIN
 MIN-NOT: ini0()
 MIN-NOT: ini1()
 MIN-NOT: ini2()
 MIN: det0()
 MIN: det1()

test/fuzzer/print_unstable_stats.test

 RUN: %cpp_compiler %S/PrintUnstableStatsTest.cpp -o %t-PrintUnstableStatsTest
+
 RUN: %run %t-PrintUnstableStatsTest -print_unstable_stats=1 -runs=100000 2>&1 | FileCheck %s --check-prefix=LONG
-LONG: stat::stability_rate: 27.59
+; We do not observe ini functions since we take the minimum hit counts, and minimum hit counts for ini is 0.
+LONG: UNSTABLE_FUNCTIONS:
+LONG-NOT: det0()
+LONG-NOT: det1()
+LONG-NOT: det2()
+LONG-NOT: det3()
+LONG-NOT: det4()
+LONG-NOT: ini0()
+LONG-NOT: ini1()
+LONG-NOT: ini2()
+LONG-DAG: t0(int)
+LONG-DAG: t1()
+LONG-DAG: t2(int, int)
+LONG-DAG: t3()
+LONG-DAG: t4(int, int, int)
+
+LONG-DAG: stat::stability_rate: 27.59
+