added reset feature to dfsan

[farahhariri]

Summary

Adding reset feature to dfsan. This is especially needed in the context of fuzzing. Otherwise, we would run out of labels very fast.

[kcc]

also, would like to hear from @pcc

include/sanitizer/dfsan_interface.h

55

Explain thread-[un]safety
I assume this function is *not* thread safe and thus the comment must be clear on this topic.
Add '.' at the end

lib/dfsan/dfsan.cc

179

Add some code to check-fail in case __dfsan_last_label has changed (i.e. to check that no other threads are messing around)

Then, add a test with threads that will trigger that check-fail.

[pcc]

include/sanitizer/dfsan_interface.h

55

Please also document that there should also be no active stack frames which may be processing values with non-zero labels.

test/dfsan/reset.cc

18

Is this safe, given the constraint I mentioned above? I'd prefer to move the code that creates labels into a separate function.

[farahhariri]

lib/dfsan/dfsan.cc

179

Then, add a test with threads that will trigger that check-fail.

That means I will have to add a flaky test that sometimes passes and sometimes fails.
I cannot guarantee that the scheduler will switch context between the atomic store and
the check-fail to another thread, and then switch back to the check-fail. That's the only
way to guarantee it will fail..

[kcc]

lib/dfsan/dfsan.cc

179

You can create a test that runs a very long loop in a thread and thus guarantee that the test will crash 9999999999999999 our of 100000000000000 times.

[farahhariri]

addressed concerns related to multithreaded code

[farahhariri]

lib/dfsan/dfsan.cc

179

I tried that and let it run for 12+ hours and could not trigger the assertion after the atomic store to fail. But what I did is control the thread scheduling from the test and make an assertion in that test fail (check the update).

[pcc]

lib/dfsan/dfsan.cc

174

Is this part necessary? I believe that the mmap call above will also reset the union table to zero.

test/dfsan/reset_multithread.cc

20 ↗

(On Diff #108683)

Is this really testing for the property that Kostya mentioned? It seems like it is trying to reproduce the scenario where the labels are created in the other thread *after* (i.e. not parallel with) the reset. You might have better luck reproducing if you change the dfsan_reset function to do this:

void dfsan_reset() {
  atomic store to dfsan_last_label;
  mmap;
  assert(atomic load from dfan_last_label == 0);
}

[farahhariri]

removed unecessary reset, and fixed multithreaded test.

[farahhariri]

lib/dfsan/dfsan.cc

174

You're right it's not. Thanks!

[farahhariri]

This comment was removed by farahhariri.

[pcc]

Please add the other hooks in a separate patch.

test/dfsan/reset_multithread.cc

19 ↗

(On Diff #109186)

Is this assert necessary? It's basically testing for the same thing as the assert in dfsan_reset, isn't it?

[farahhariri]

This comment was removed by farahhariri.

include/sanitizer/dfsan_interface.h

 dfsan_label dfsan_union(dfsan_label l1, dfsan_label l2);
 
 /// Creates and returns a base label with the given description and user data.
 dfsan_label dfsan_create_label(const char *desc, void *userdata);
 
 /// Sets the label for each address in [addr,addr+size) to \c label.
 void dfsan_set_label(dfsan_label label, void *addr, size_t size);
 
+/// In process reset of dfsan
+void dfsan_reset(void);
+
 /// Sets the label for each address in [addr,addr+size) to the union of the
 /// current label for that address and \c label.
 void dfsan_add_label(dfsan_label label, void *addr, size_t size);
 
 /// Retrieves the label associated with the given data.
 ///
 /// The type of 'data' is arbitrary.  The function accepts a value of any type,
 /// which can be truncated or extended (implicitly or explicitly) as necessary.

lib/dfsan/dfsan.h

struct dfsan_label_info {
   dfsan_label l2;
   const char *desc;
   void *userdata;
 };
 
 extern "C" {
 void dfsan_add_label(dfsan_label label, void *addr, uptr size);
 void dfsan_set_label(dfsan_label label, void *addr, uptr size);
+void dfsan_reset(void);
 dfsan_label dfsan_read_label(const void *addr, uptr size);
 dfsan_label dfsan_union(dfsan_label l1, dfsan_label l2);
 }  // extern "C"
 
 template <typename T>
 void dfsan_set_label(dfsan_label label, T &data) {  // NOLINT
   dfsan_set_label(label, (void *)&data, sizeof(T));
 }

lib/dfsan/dfsan.cc

 // Checks we do not run out of labels.
 static void dfsan_check_label(dfsan_label label) {
   if (label == kInitializingLabel) {
     Report("FATAL: DataFlowSanitizer: out of labels\n");
     Die();
   }
 }
 
+// Reset labels and shadow memory for dfsan to restart from clean.
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE
+void dfsan_reset(void) {
+  // reset shadow memory
+  MmapFixedNoReserve(ShadowAddr(), UnusedAddr() - ShadowAddr());
+
+  dfsan_label last_label =
+      atomic_load(&__dfsan_last_label, memory_order_relaxed);
+
+  // reset potentially polluted union table entries
+  for (uptr l = 0; l <= last_label; ++l) {
+    for (uptr m = 0; m <= last_label; ++m) {
+      atomic_dfsan_label *table_ent = union_table(l, m);
+      dfsan_label label = 0;
+      atomic_store(table_ent, label, memory_order_acquire);
+    }
+  }
+  // reset label count
+  atomic_store(&__dfsan_last_label, 0, memory_order_relaxed);
+}
+
 // Resolves the union of two unequal labels.  Nonequality is a precondition for
 // this function (the instrumentation pass inlines the equality test).
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 dfsan_label __dfsan_union(dfsan_label l1, dfsan_label l2) {
   DCHECK_NE(l1, l2);
 
   if (l1 == 0)
     return l2;

lib/dfsan/done_abilist.txt

 fun:main=uninstrumented
 fun:main=discard
 
 ###############################################################################
 # DFSan interface functions
 ###############################################################################
 fun:dfsan_union=uninstrumented
 fun:dfsan_union=discard
+fun:dfsan_reset=uninstrumented
+fun:dfsan_reset=discard
 fun:dfsan_create_label=uninstrumented
 fun:dfsan_create_label=discard
 fun:dfsan_set_label=uninstrumented
 fun:dfsan_set_label=discard
 fun:dfsan_add_label=uninstrumented
 fun:dfsan_add_label=discard
 fun:dfsan_get_label=uninstrumented
 fun:dfsan_get_label=custom

test/dfsan/reset.cc

+// RUN: %clang_dfsan %s -o %t && %run %t
+// RUN: %clang_dfsan -mllvm -dfsan-args-abi %s -o %t && %run %t
+
+// Tests that dfsan runtime is reset correctly.
+
+#include <sanitizer/dfsan_interface.h>
+#include <assert.h>
+
+int main(void) {
+  int i = 1;
+  int j = 1;
+  dfsan_label i_label = dfsan_create_label("i", 0);
+  dfsan_set_label(i_label, &i, sizeof(i));
+  dfsan_label j_label = dfsan_create_label("j", 0);
+  dfsan_add_label(j_label, &j, sizeof(j));
+  assert(dfsan_get_label_count() == 2);
+
+  dfsan_reset();
+  assert(dfsan_get_label_count() == 0);
+
+  return 0;
+}