[libFuzzer] Handle unstable edges by using minimum hit counts

[kevinwkt]

Summary

Created unstable_handle flag that takes 1 or 2, depending on the handling type.
Modified RunOne to accommodate the following heuristic:

Use the first CollectFeatures to count how many features there are.
If no new features, CollectFeatures like before.
If there is new feature, we run CB 2 more times,
    Check which edges are unstable per input and we store the least amount of hit counts for each edge.
    Apply these hit counts back to inline8bitcounters so that CollectFeatures can work as intended.

Modified UnstableCounters to 8int_t and created a bitset UnstableSet to tell which edges are unstable.

Patch by Kyungtak Woo (@kevinwkt).

[Dor1s]

lib/fuzzer/FuzzerCorpus.h

241

CheckFeature is a very generalized name. Can we call this like IsFeatureStable? or IsFeatureNew?

lib/fuzzer/FuzzerFlags.def

113

unstable_handle sounds weird to me, if you want to go with "handle", let's do handle_instability IMO. Feel free to ignore this comment unless others also raise this point.

lib/fuzzer/FuzzerLoop.cpp

496

Hm... shouldn't we check Options.UnstableHandle here instead of Options.PrintUnstableStats? If yes, you can just put this inside { } of the condition on line 491.

lib/fuzzer/FuzzerTracePC.cpp

86–97

This line and line 349 do not look good to me, why do we use 1? If it's an important constant, it should be in a constant variable, not a literal value used in different places.

lib/fuzzer/FuzzerTracePC.h

20

nit: use alphabetical order, incude bitset first, then set

150

Wait, this is bad, you're going to assign uint8_t values to int8_t. I think this is not what we've discussed regarding a bit mask. My suggestion was to keep using int16_t, but instead of kUnstableCounter = -1 make it to be something like kUnstableCounterMask = 1 << 8 to utilize higher bits of the UnstableCounters. What's wrong with that approach?

The bitset-based might also work, but I have two concerns:

1. MSan may start reporting false positives again (it's been a case with std::vector)

2. Whenever we decide to have another magic value to mark features with some other purpose, we won't have any room for that and have to either refactor the algorithm or introduce yet another bitset

[metzman]

lib/fuzzer/FuzzerCorpus.h

241

+1

243

Is this logic used elsewhere? I know AFL lets edges collide in the array, but from here it seems that LF does as well?

lib/fuzzer/FuzzerDriver.cpp

622

handle_unstable makes more sense I think

lib/fuzzer/FuzzerFlags.def

118

I can't understand the explanation for 2.
I think we should convey that if 2: we only consider new features for edges observed to be stable, meaning that they can never be non-deterministically reached.

lib/fuzzer/FuzzerLoop.cpp

452

This line is too long.
Please run clang-format-diff from now on.
It would probably help to configure your editor to highlight lines that are too long.

475

Does PoisonUnstable do anything? I'm not sure how it's actually handling unstable edges differently.

491

Please don't use multiline if bodies without curly braces.
Also I don't know why there isn't an || Options.PrintUnstableStats here.
We should only be collecting unstable stat info on interesting testcases right?

lib/fuzzer/FuzzerTracePC.cpp

87

Dumb question: why are we comparing UnstableIdx to MinUnstable, shouldn't the comparison be UnstableMode to MinUnstable? If not, then I find this very confusing.

93

I think the comment is wrong. It moves the counts to ModuleCounters right?

lib/fuzzer/FuzzerTracePC.h

77

HandleUnstableOptions if we are changing unstable_handle to handle_unstable

150

Did matt say to use a bitset? I don't like it since it is optimized for space (which we don't care much about) instead of speed (which we care a lot about).

An array of structs containing uint8_t and bool (so we can use true instead of 1) seems to be the best solution to me.

150

Also I don't know why this became int8_t. I think we decide it can be uint8_t but I think int8_t will lose info (lets say hit count of is UINT8_T_MAX across all runs.

150

Another idea: We don't really need to have UnstableCounters at all right? Couldn't we just use ModuleCounters instead? Don't worry about making this change now, since it is less flexible I think we can hold off on doing it until after the CL lands, if we find that we don't want to change this approach at all.

[Dor1s]

lib/fuzzer/FuzzerTracePC.h

150

Using an array of structs sounds good to me.

[kevinwkt]

PTAL
Ran Clang-Format-Diff

Changed CheckFeature to IsFeatureNew
Changed unstable_handle to handle_unstable
Put brackets around multiline ifs
Removed UnstableSet and created an array of structs

[kevinwkt]

lib/fuzzer/FuzzerCorpus.h

243

This is copied from the AddFeature() method which is the method above this one.

lib/fuzzer/FuzzerLoop.cpp

475

PoisonUnstable is not implemented yet. I wanted the first implementation landed first. Should I also include it in this diff?

491

The new logic is the following:

if (Options.HandleUnstable || Options.PrintUnstableStats) {
  TPC.CollectFeatures([&](size_t Feature) {
    if (Corpus.IsFeatureNew(Feature, Size))
      NumNewFeaturesForUnstable++;
  });
  if (NumNewFeaturesForUnstable)
    CheckForUnstableCounters(Data, Size, Options.HandleUnstable);
}

This is due to that we are allowing print_unstable_stats to be run without handle_unstable.

496

I had it thought that you could either run with flags -handle_unstable and -print_unstable_stats independently and also together. If print_unstable_stats were to be run alone, then we would need to also run CheckForUnstableCounters.

lib/fuzzer/FuzzerTracePC.cpp

87

Bug fixed.

lib/fuzzer/FuzzerTracePC.h

150

We do need UnstableCounters when implementing the MinUnstable since we need somewhere to store the previous counter state to compare it with ModuleCounters (current state).

[kevinwkt]

Removed PoisonUnstable from code.

[Dor1s]

lib/fuzzer/FuzzerTracePC.h

150

This looks a bit weird. Can you separate the struct and the array declarations please?

153

No need for this, a struct would have a default constructor generated by the compiler which initialized fields with the same values.

[kevinwkt]

PTAL
Separated struct from array declaration.
Removed unnecessary Struct constructor.

[Dor1s]

lib/fuzzer/FuzzerTracePC.h

77

If we go with enum, I think we should change functions UpdateUnstableCounters to accept an argument of HandleUnstableOptions type instead of int. Let's see first if Matt is fine with us using enum here.

[Dor1s]

Kevin, can you think of a test that would verify that your handling approach works? We must have a test for this stuff to make sure it'll not get accidentally broken.

lib/fuzzer/FuzzerLoop.cpp

492

Did we consider incorporating this callback into the one used on line 501? I think this would be a bit cleaner and somewhat faster, rather then iterate through the features twice.

[kevinwkt]

Added test for -handle_unstable=1

[kevinwkt]

lib/fuzzer/FuzzerLoop.cpp

492

We use need to run the first one to check for new features in order to run CheckForUnstableCounters (line 498) accordingly before the "real" collectFeatures.

[Dor1s]

Matt, can you please take a look?

[morehouse]

lib/fuzzer/FuzzerCorpus.h

243

Can we reuse this inside AddFeature to reduce code duplication?

lib/fuzzer/FuzzerDriver.cpp

622

I know you intend to add other modes, but since there's only 1 right now, let's simplify this patch.

We can get rid of MinUnstable and just set Options.HandleUnstable = Flags.handle_unstable here. Elsewhere, we can just check if (Options.HandleUnstable) ...

lib/fuzzer/FuzzerInternal.h

71 ↗

(On Diff #156533)

We shouldn't need to pass UnstableMode since we have access to Options.

lib/fuzzer/FuzzerLoop.cpp

453

Rather than passing UnstableMode around, let's check Options.

489

I think this can be a bool.

492

This iterates over all counters once. Then we iterate again with CheckForUnstableCounters if there are any new features. Then we iterate again in ApplyUnstableCounters. Then we iterate again with the below call to CollectFeatures.

If we get this down to 2 or 1 pass, the effectiveness of this feature may increase significantly.

test/fuzzer/handle_unstable_minunstable.test

6

Is the reason for not including t[0-4] that we could get lucky and hit the same one all three times?

[kevinwkt]

Got rid of enum and FuzzerTracePC::MinUnstable.

[kevinwkt]

ptal

lib/fuzzer/FuzzerCorpus.h

243

We could, the only thing I could think of was the following:

1. template function where IsFeatureNew would send an empty lambda, and AddFeature would send everything in between
2. Call IsFeatureNew within AddFeature, but we would need to get Idx and OldSize in AddFeature anyways.

Do you have any suggestions?

lib/fuzzer/FuzzerLoop.cpp

492

Currently:

Iterate to check if new features
Iterate to save Counters to UnstableCounters

Run CB
Iterate to compare and save Counters to UnstableCounters (Update)

RunCB
Iterate to compare and save Counters to UnstableCounters(Update)

Copy UnstableCounters to Counters (Apply, only if handle_unstable=1)

I dont think we can unite things when there's CB in between.

1. We could bind the "check if new features" with "iterate to save Counters to UnstableCounters" but I dont think this is beneficial given that we would be doing this even if no new features are found.
2. ApplyUnstableCounters could be done at same time as Update, if we have an if-else condition. But this would make the code more complicated. I don't know if it would be worth it.

I might be missing something. What do you think?

[morehouse]

lib/fuzzer/FuzzerCorpus.h

243

No, looking closer I don't think there's a good way to reuse this... Ignore my previous suggestion.

244

Can combine previous two lines into

uint32_t OldSize = GetFeature(Idx % kFeatureSetSize);

245

We probably want to check Shrink here too.

Can simplify this to

return OldSize == 0 || (Shrink && OldSize > NewSize);

lib/fuzzer/FuzzerLoop.cpp

489

If it's a bool, we should rename it to something like NewFeaturesUnstable.

492

We can probably eliminate two iterations by remembering the new features that are stable and directly adding them to the corpus rather than modifying the counters.

[kevinwkt]

ptal
Changed IsFeatureNew like suggested.
Improvements to the amount of runs will be optimized if needed in the future.

[morehouse]

lib/fuzzer/FuzzerCorpus.h

244

Does this compile? I don't think Shrink is declared anywhere.

lib/fuzzer/FuzzerLoop.cpp

489

This comment was NOT addressed.

[kevinwkt]

ptal
Passes tests, fixed naming issue as previously asked.

[Dor1s]

Matt, thanks a lot for the review!

Kostya, we'll land this on Monday morning and enable as a strategy on CF / OSS-Fuzz

[Dor1s]

Kevin, a bunch of tests seems to be failing for me with your change, but everything works perfectly without it. Do the tests pass for you?

[Dor1s]

Kevin, don't panic, ninja clean && ninja && ninja check-fuzzer made things work for me. Committing now.

[Dor1s]

Rebase and getting ready to commit.

[Dor1s]

Kevin, looks like test is not fully deterministic :(

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/5509

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/5509/steps/ninja%20check%202/logs/FAIL%3A%20libFuzzer%3A%3A%20handle_unstable_minunstable.test

This is the first testbot failure I've seen so far, so am not reverting the change yet, but please take a look into why the test might be failing sometimes.

[thopre]

In D49525#1173010, @Dor1s wrote:

Kevin, looks like test is not fully deterministic :(

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/5509

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/5509/steps/ninja%20check%202/logs/FAIL%3A%20libFuzzer%3A%3A%20handle_unstable_minunstable.test

This is the first testbot failure I've seen so far, so am not reverting the change yet, but please take a look into why the test might be failing sometimes.

It seems to have be failing for aarch64 since it was introduced:

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/5509 is failing and covers the range which added this test
http://lab.llvm.org:8011/waterfall?show=clang-cmake-aarch64-full shows that the test has been failing since that build onwards, except for build 5514 which was interrupted before stage2 tests were run.

[Dor1s]

In D49525#1175242, @thopre wrote:

In D49525#1173010, @Dor1s wrote:

Kevin, looks like test is not fully deterministic :(

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/5509

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/5509/steps/ninja%20check%202/logs/FAIL%3A%20libFuzzer%3A%3A%20handle_unstable_minunstable.test

This is the first testbot failure I've seen so far, so am not reverting the change yet, but please take a look into why the test might be failing sometimes.

It seems to have be failing for aarch64 since it was introduced:

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/5509 is failing and covers the range which added this test
http://lab.llvm.org:8011/waterfall?show=clang-cmake-aarch64-full shows that the test has been failing since that build onwards, except for build 5514 which was interrupted before stage2 tests were run.

@thopre, thanks for the heads up and sorry about the breakage. Kevin has prepared a fix, we'll land it soon.

lib/fuzzer/FuzzerCorpus.h

if (OldSize == 0 || (Shrink && OldSize > NewSize)) {
         Printf("ADD FEATURE %zd sz %d\n", Idx, NewSize);
       SmallestElementPerFeature[Idx] = Inputs.size();
       InputSizesPerFeature[Idx] = NewSize;
       return true;
     }
     return false;
   }
 
+  bool IsFeatureNew(size_t Idx, uint32_t NewSize, bool Shrink) {
+    assert(NewSize);
+    uint32_t OldSize = GetFeature(Idx % kFeatureSetSize);
+    return OldSize == 0 || (Shrink && OldSize > NewSize);
+  }
+
   size_t NumFeatures() const { return NumAddedFeatures; }
   size_t NumFeatureUpdates() const { return NumUpdatedFeatures; }
 
 private:
 
   static const bool FeatureDebug = false;
 
   size_t GetFeature(size_t Idx) const { return InputSizesPerFeature[Idx]; }

lib/fuzzer/FuzzerDriver.cpp

Options.SaveArtifacts =
       !DoPlainRun || Flags.minimize_crash_internal_step;
   Options.PrintNewCovPcs = Flags.print_pcs;
   Options.PrintNewCovFuncs = Flags.print_funcs;
   Options.PrintFinalStats = Flags.print_final_stats;
   Options.PrintMutationStats = Flags.print_mutation_stats;
   Options.PrintCorpusStats = Flags.print_corpus_stats;
   Options.PrintCoverage = Flags.print_coverage;
   Options.PrintUnstableStats = Flags.print_unstable_stats;
+  if (Flags.handle_unstable)
+    Options.HandleUnstable = Flags.handle_unstable;
   Options.DumpCoverage = Flags.dump_coverage;
   if (Flags.exit_on_src_pos)
     Options.ExitOnSrcPos = Flags.exit_on_src_pos;
   if (Flags.exit_on_item)
     Options.ExitOnItem = Flags.exit_on_item;
   if (Flags.focus_function)
     Options.FocusFunction = Flags.focus_function;
   if (Flags.data_flow_trace)

lib/fuzzer/FuzzerFlags.def

 FUZZER_FLAG_INT(print_final_stats, 0, "If 1, print statistics at exit.")
 FUZZER_FLAG_INT(print_corpus_stats, 0,
   "If 1, print statistics on corpus elements at exit.")
 FUZZER_FLAG_INT(print_coverage, 0, "If 1, print coverage information as text"
                                    " at exit.")
 FUZZER_FLAG_INT(dump_coverage, 0, "Deprecated."
                                   " If 1, dump coverage information as a"
                                   " .sancov file at exit.")
+FUZZER_FLAG_INT(handle_unstable, 0, "Experimental."
+                   " Executes every input 3 times in total if a unique feature"
+                   " is found during the first execution."
+                   " If 1, we only use the minimum hit count from the 3 runs"
+                   " to determine whether an input is interesting.")
 FUZZER_FLAG_INT(print_unstable_stats, 0, "Experimental."
           " If 1, print unstable statistics at exit.")
 FUZZER_FLAG_INT(handle_segv, 1, "If 1, try to intercept SIGSEGV.")
 FUZZER_FLAG_INT(handle_bus, 1, "If 1, try to intercept SIGBUS.")
 FUZZER_FLAG_INT(handle_abrt, 1, "If 1, try to intercept SIGABRT.")
 FUZZER_FLAG_INT(handle_ill, 1, "If 1, try to intercept SIGILL.")
 FUZZER_FLAG_INT(handle_fpe, 1, "If 1, try to intercept SIGFPE.")
 FUZZER_FLAG_INT(handle_int, 1, "If 1, try to intercept SIGINT.")
 FUZZER_FLAG_INT(handle_term, 1, "If 1, try to intercept SIGTERM.")

lib/fuzzer/FuzzerLoop.cpp

void Fuzzer::PrintPulseAndReportSlowInput(const uint8_t *Data, size_t Size) {
   if (TimeOfUnit > TimeOfLongestUnitInSeconds * 1.1 &&
       TimeOfUnit >= Options.ReportSlowUnits) {
     TimeOfLongestUnitInSeconds = TimeOfUnit;
     Printf("Slowest unit: %zd s:\n", TimeOfLongestUnitInSeconds);
     WriteUnitToFileWithPrefix({Data, Data + Size}, "slow-unit-");
   }
 }
 
 void Fuzzer::CheckForUnstableCounters(const uint8_t *Data, size_t Size) {
   auto CBSetupAndRun = [&]() {
     ScopedEnableMsanInterceptorChecks S;
     UnitStartTime = system_clock::now();
     TPC.ResetMaps();
     RunningUserCallback = true;
     CB(Data, Size);
     RunningUserCallback = false;
     UnitStopTime = system_clock::now();
   };
 
   // Copy original run counters into our unstable counters
   TPC.InitializeUnstableCounters();
 
   // First Rerun
   CBSetupAndRun();
-  TPC.UpdateUnstableCounters();
+  TPC.UpdateUnstableCounters(Options.HandleUnstable);
 
   // Second Rerun
   CBSetupAndRun();
-  TPC.UpdateUnstableCounters();
+  TPC.UpdateUnstableCounters(Options.HandleUnstable);
+
+  // Move minimum hit counts back to ModuleInline8bitCounters
+  if (Options.HandleUnstable)
+    TPC.ApplyUnstableCounters();
 }
 
 bool Fuzzer::RunOne(const uint8_t *Data, size_t Size, bool MayDeleteFile,
                     InputInfo *II, bool *FoundUniqFeatures) {
   if (!Size)
     return false;
 
   ExecuteCallback(Data, Size);
 
   UniqFeatureSetTmp.clear();
   size_t FoundUniqFeaturesOfII = 0;
   size_t NumUpdatesBefore = Corpus.NumFeatureUpdates();
+  bool NewFeaturesUnstable = false;
+
+  if (Options.HandleUnstable || Options.PrintUnstableStats) {
+    TPC.CollectFeatures([&](size_t Feature) {
+      if (Corpus.IsFeatureNew(Feature, Size, Options.Shrink))
+        NewFeaturesUnstable = true;
+    });
+    if (NewFeaturesUnstable)
+      CheckForUnstableCounters(Data, Size);
+  }
+
   TPC.CollectFeatures([&](size_t Feature) {
     if (Corpus.AddFeature(Feature, Size, Options.Shrink))
       UniqFeatureSetTmp.push_back(Feature);
     if (Options.ReduceInputs && II)
       if (std::binary_search(II->UniqFeatureSet.begin(),
                              II->UniqFeatureSet.end(), Feature))
         FoundUniqFeaturesOfII++;
   });
+
   if (FoundUniqFeatures)
     *FoundUniqFeatures = FoundUniqFeaturesOfII;
   PrintPulseAndReportSlowInput(Data, Size);
   size_t NumNewFeatures = Corpus.NumFeatureUpdates() - NumUpdatesBefore;
 
-  // If print_unstable_stats, execute the same input two more times to detect
-  // unstable edges.
-  if (NumNewFeatures && Options.PrintUnstableStats)
-    CheckForUnstableCounters(Data, Size);
-
   if (NumNewFeatures) {
     TPC.UpdateObservedPCs();
     Corpus.AddToCorpus({Data, Data + Size}, NumNewFeatures, MayDeleteFile,
                        TPC.ObservedFocusFunction(), UniqFeatureSetTmp, DFT, II);
     return true;
   }
   if (II && FoundUniqFeaturesOfII &&
       II->DataFlowTraceForFocusFunction.empty() &&

lib/fuzzer/FuzzerOptions.h

struct FuzzingOptions {
   bool PrintNEW = true; // Print a status line when new units are found;
   bool PrintNewCovPcs = false;
   int PrintNewCovFuncs = 0;
   bool PrintFinalStats = false;
   bool PrintMutationStats = false;
   bool PrintCorpusStats = false;
   bool PrintCoverage = false;
   bool PrintUnstableStats = false;
+  int HandleUnstable = 0;
   bool DumpCoverage = false;
   bool DetectLeaks = true;
   int PurgeAllocatorIntervalSec = 1;
   int  TraceMalloc = 0;
   bool HandleAbrt = false;
   bool HandleBus = false;
   bool HandleFpe = false;
   bool HandleIll = false;

lib/fuzzer/FuzzerTracePC.h

 #ifndef LLVM_FUZZER_TRACE_PC
 #define LLVM_FUZZER_TRACE_PC
 
 #include "FuzzerDefs.h"
 #include "FuzzerDictionary.h"
 #include "FuzzerValueBitMap.h"
 
 #include <set>
 #include <unordered_map>
 
 namespace fuzzer {
 
 // TableOfRecentCompares (TORC) remembers the most recently performed
 // comparisons of type T.
 // We record the arguments of CMP instructions in this table unconditionally
 // because it seems cheaper this way than to compute some expensive
 // conditions inside __sanitizer_cov_trace_cmp*.

 };
 
 class TracePC {
  public:
   static const size_t kNumPCs = 1 << 21;
   // How many bits of PC are used from __sanitizer_cov_trace_pc.
   static const size_t kTracePcBits = 18;
 
   void HandleInit(uint32_t *Start, uint32_t *Stop);
   void HandleInline8bitCountersInit(uint8_t *Start, uint8_t *Stop);
   void HandlePCsInit(const uintptr_t *Start, const uintptr_t *Stop);
   void HandleCallerCallee(uintptr_t Caller, uintptr_t Callee);
   template <class T> void HandleCmp(uintptr_t PC, T Arg1, T Arg2);
   size_t GetTotalPCCoverage();
   void SetUseCounters(bool UC) { UseCounters = UC; }
   void SetUseValueProfileMask(uint32_t VPMask) { UseValueProfileMask = VPMask; }
   void SetPrintNewPCs(bool P) { DoPrintNewPCs = P; }
void ForEachObservedPC(CallBack CB) {
     for (auto PC : ObservedPCs)
       CB(PC);
   }
 
   void SetFocusFunction(const std::string &FuncName);
   bool ObservedFocusFunction();
 
   void InitializeUnstableCounters();
-  void UpdateUnstableCounters();
+  void UpdateUnstableCounters(int UnstableMode);
+  void ApplyUnstableCounters();
 
 private:
-  // Value used to represent unstable edge.
-  static constexpr int16_t kUnstableCounter = -1;
+  struct UnstableEdge {
+    uint8_t Counter;
+    bool IsUnstable;
+  };
 
-  // Uses 16-bit signed type to be able to accommodate any possible value from
-  // uint8_t counter and -1 constant as well.
-  int16_t UnstableCounters[kNumPCs];
+  UnstableEdge UnstableCounters[kNumPCs];
 
   bool UseCounters = false;
   uint32_t UseValueProfileMask = false;
   bool DoPrintNewPCs = false;
   size_t NumPrintNewFuncs = 0;
 
   struct Module {
     uint32_t *Start, *Stop;
   };
 
   Module Modules[4096];

lib/fuzzer/FuzzerTracePC.cpp

if (NumInline8bitCounters && NumInline8bitCounters == NumPCsInPCTables) {
     }
   }
 }
 
 // Initializes unstable counters by copying Inline8bitCounters to unstable
 // counters.
 void TracePC::InitializeUnstableCounters() {
   IterateInline8bitCounters([&](int i, int j, int UnstableIdx) {
-    if (UnstableCounters[UnstableIdx] != kUnstableCounter)
-      UnstableCounters[UnstableIdx] = ModuleCounters[i].Start[j];
+    UnstableCounters[UnstableIdx].Counter = ModuleCounters[i].Start[j];
   });
 }
 
 // Compares the current counters with counters from previous runs
 // and records differences as unstable edges.
-void TracePC::UpdateUnstableCounters() {
+void TracePC::UpdateUnstableCounters(int UnstableMode) {
   IterateInline8bitCounters([&](int i, int j, int UnstableIdx) {
-    if (ModuleCounters[i].Start[j] != UnstableCounters[UnstableIdx])
-      UnstableCounters[UnstableIdx] = kUnstableCounter;
+    if (ModuleCounters[i].Start[j] != UnstableCounters[UnstableIdx].Counter)
+      UnstableCounters[UnstableIdx].IsUnstable = true;
+    if (UnstableMode &&
+        ModuleCounters[i].Start[j] < UnstableCounters[UnstableIdx].Counter)
+      UnstableCounters[UnstableIdx].Counter = ModuleCounters[i].Start[j];
+  });
+}
+
+// Moves the minimum hit counts to ModuleCounters.
+void TracePC::ApplyUnstableCounters() {
+  IterateInline8bitCounters([&](int i, int j, int UnstableIdx) {
+    ModuleCounters[i].Start[j] = UnstableCounters[UnstableIdx].Counter;
   });
 }
 
 void TracePC::HandleInline8bitCountersInit(uint8_t *Start, uint8_t *Stop) {
   if (Start == Stop) return;
   if (NumModulesWithInline8bitCounters &&
       ModuleCounters[NumModulesWithInline8bitCounters-1].Start == Start) return;
   assert(NumModulesWithInline8bitCounters <
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] == kUnstableCounter)
+    if (UnstableCounters[i].IsUnstable)
       count++;
   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.

test/fuzzer/handle_unstable_minunstable.test

+RUN: %cpp_compiler %S/PrintUnstableStatsTest.cpp -o %t-HandleUnstableMinUnstableTest
+RUN: %run %t-HandleUnstableMinUnstableTest -print_coverage=1 -handle_unstable=1 -runs=100000 2>&1 | FileCheck %s --check-prefix=UNSTABLE
+UNSTABLE-NOT: ini0()
+UNSTABLE-NOT: ini1()
+UNSTABLE-NOT: ini2()
+
+RUN: %run %t-HandleUnstableMinUnstableTest -print_coverage=1 -runs=100000 2>&1 | FileCheck %s --check-prefix=NORMAL
+NORMAL-DAG: ini0()
+NORMAL-DAG: ini1()
+NORMAL-DAG: ini2()
+
+