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

Sam Kerner skerner at chromium.org
Mon Apr 21 11:10:37 PDT 2014



================
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,
----------------
Peter Collingbourne wrote:
> This isn't the correct type for write_callback -- I would use void* here.
Done.

================
Comment at: lib/dfsan/dfsan_custom.cc:832
@@ +831,3 @@
+
+  return previous_write_callback;
+}
----------------
Peter Collingbourne wrote:
> 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.
Changed to not return the previous callback.

================
Comment at: lib/dfsan/dfsan_custom.cc:854
@@ +853,3 @@
+  }
+  *ret_label = result_label;
+
----------------
Peter Collingbourne wrote:
> 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).
> 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.

Makes sense.  Removed.



================
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
----------------
Peter Collingbourne wrote:
> 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.
Good points.  Changed the code to use FileCheck.


http://reviews.llvm.org/D3268






More information about the llvm-commits mailing list