[PATCH] Add user-defined callback on write() calls to labeled data.

Peter Collingbourne peter at pcc.me.uk
Wed Apr 16 16:24:53 PDT 2014



================
Comment at: lib/dfsan/dfsan_custom.cc:854
@@ +853,3 @@
+  }
+  *ret_label = result_label;
+
----------------
We haven't traditionally labelled the return value of system call functions according to their inputs, even with strict data dependencies turned on. The general justification for this is that system calls involve so much process-external state that we shouldn't even attempt to model their data flow beyond simply zero labeling their outputs.

I'm also not sure if this labelling is correct (for example, it reads the buffer's labels, even though the success or failure of a write is typically not dependent on the data being written).

================
Comment at: test/dfsan/write_callback.c:82
@@ +81,3 @@
+
+static int compare_file_to_string(char *filePath, char *string) {
+  // Read one more byte than the length of the expected string.  If the file has
----------------
I was imagining that the test could use something like FileCheck to check the program's output, rather than having the program check its own output. This would reduce the complexity of the test program and permit external validation of the program's behavior -- the wrappers could conceivably be lying to the program about the file system state.

================
Comment at: lib/dfsan/dfsan_custom.cc:821
@@ +820,3 @@
+    write_trampoline_t write_callback_trampoline,
+    dfsan_write_callback_t write_callback,
+    dfsan_label write_callback_label,
----------------
This isn't the correct type for write_callback -- I would use void* here.

================
Comment at: lib/dfsan/dfsan_custom.cc:832
@@ +831,3 @@
+
+  return previous_write_callback;
+}
----------------
This isn't necessarily always going to be the function pointer that was originally passed to dfsan_set_write_callback -- in theory, if the callee is known, we could generate the trampoline such that it calls the callee directly and the second argument is null. It would probably be simplest not to return anything from this function.


http://reviews.llvm.org/D3268






More information about the llvm-commits mailing list