[BOLT][Instrumentation] Keep profile open in WatchProcess

[treapster]

Summary

When a binary is instrumented with --instrumentation-sleep-time and instrumentation-wait-forks options and lauched, the profile is periodically written until all the forks die. The problem is that we cannot wait for the whole process tree, and we have no way to tell when it's safe to read the profile. Hovewer, if we keep profile open throughout the life of the process tree, we can use fuser to determine when writing is finished. It will help us to write robust tests for instrumentation, as mentioned in https://reviews.llvm.org/D153771.

[treapster]

SEEK_END -> SEEK_SET

[treapster]

Add diff context

[rafauler]

Thanks for working on this @treapster . Can you add to your diff stack a diff that adds a test that uses this improved mechanism?

[treapster]

In D154436#4474610, @rafauler wrote:

Thanks for working on this @treapster . Can you add to your diff stack a diff that adds a test that uses this improved mechanism?

I added a test in D154121, but it looks like it won't work until D153771 is fully relanded, and adding it as parent is not working because it's closed. If you or Amir approve D153771, i'll reland and things will get easier.

[treapster]

Add return value checks

[rafauler]

Thanks! LGTM

bolt/runtime/common.h

   __asm__ __volatile__("movq $8, %%rax\n"
                        "syscall\n"
                        : "=a"(ret)
                        : "D"(fd), "S"(pos), "d"(whence)
                        : "cc", "rcx", "r11", "memory");
   return ret;
 }
 
+int __ftruncate(uint64_t fd, uint64_t length) {
+  int ret;
+  __asm__ __volatile__("movq $77, %%rax\n"
+                       "syscall\n"
+                       : "=a"(ret)
+                       : "D"(fd), "S"(length)
+                       : "cc", "rcx", "r11", "memory");
+  return ret;
+}
+
 int __close(uint64_t fd) {
   uint64_t ret;
   __asm__ __volatile__("movq $3, %%rax\n"
                        "syscall\n"
                        : "=a"(ret)
                        : "D"(fd)
                        : "cc", "rcx", "r11", "memory");
   return ret;

bolt/runtime/instr.cpp

 ///    hooked into FINI from your binary dynamic section;
 ///  * You used the sleep timer option and during initialization we forked
 ///    a separete process that will call this function periodically;
 ///  * BOLT prints this function address so you can attach a debugger and
 ///    call this function directly to get your profile written to disk
 ///    on demand.
 ///
 extern "C" void __attribute((force_align_arg_pointer))
-__bolt_instr_data_dump() {
+__bolt_instr_data_dump(int FD) {
   // Already dumping
   if (!GlobalWriteProfileMutex->acquire())
     return;
 
+  __lseek(FD, 0, SEEK_END);
+  __ftruncate(FD, 0);
   BumpPtrAllocator HashAlloc;
   HashAlloc.setMaxSize(0x6400000);
   ProfileWriterContext Ctx = readDescriptions();
   Ctx.CallFlowTable = new (HashAlloc, 0) CallFlowHashTable(HashAlloc);
 
   DEBUG(printStats(Ctx));
 
-  int FD = openProfile();
-
   BumpPtrAllocator Alloc;
   Alloc.setMaxSize(0x6400000);
   const uint8_t *FuncDesc = Ctx.FuncDescriptions;
   for (int I = 0, E = __bolt_instr_num_funcs; I < E; ++I) {
     FuncDesc = writeFunctionProfile(FD, Ctx, FuncDesc, Alloc);
     Alloc.clear();
     DEBUG(reportNumber("FuncDesc now: ", (uint64_t)FuncDesc, 16));
   }
   assert(FuncDesc == (void *)Ctx.Strings,
          "FuncDesc ptr must be equal to stringtable");
 
   writeIndirectCallProfile(FD, Ctx);
   Ctx.CallFlowTable->forEachElement(visitCallFlowEntry, FD, &Ctx);
 
   __fsync(FD);
-  __close(FD);
   __munmap(Ctx.MMapPtr, Ctx.MMapSize);
   __close(Ctx.FileDesc);
   HashAlloc.destroy();
   GlobalWriteProfileMutex->release();
   DEBUG(report("Finished writing profile.\n"));
 }
 
 /// Event loop for our child process spawned during setup to dump profile data
 /// at user-specified intervals
 void watchProcess() {
   timespec ts, rem;
   uint64_t Ellapsed = 0ull;
+  int FD = openProfile();
   uint64_t ppid;
   if (__bolt_instr_wait_forks) {
     // Store parent pgid
     ppid = -__getpgid(0);
     // And leave parent process group
     __setpgid(0, 0);
   } else {
     // Store parent pid
     ppid = __getppid();
     if (ppid == 1) {
       // Parent already dead
-      __bolt_instr_data_dump();
+      __bolt_instr_data_dump(FD);
       goto out;
     }
   }
 
   ts.tv_sec = 1;
   ts.tv_nsec = 0;
   while (1) {
     __nanosleep(&ts, &rem);
     // This means our parent process or all its forks are dead,
     // so no need for us to keep dumping.
     if (__kill(ppid, 0) < 0) {
       if (__bolt_instr_no_counters_clear)
-        __bolt_instr_data_dump();
+        __bolt_instr_data_dump(FD);
       break;
     }
 
     if (++Ellapsed < __bolt_instr_sleep_time)
       continue;
 
     Ellapsed = 0;
-    __bolt_instr_data_dump();
+    __bolt_instr_data_dump(FD);
     if (__bolt_instr_no_counters_clear == false)
       __bolt_instr_clear_counters();
   }
 
 out:;
   DEBUG(report("My parent process is dead, bye!\n"));
+  __close(FD);
   __exit(0);
 }
 
 extern "C" void __bolt_instr_indirect_call();
 extern "C" void __bolt_instr_indirect_tailcall();
 
 /// Initialization code
 extern "C" void __attribute((force_align_arg_pointer)) __bolt_instr_setup() {

                        RESTORE_ALL
                        "jmp __bolt_start_trampoline\n"
                        :::);
 }
 
 /// This is hooking into ELF's DT_FINI
 extern "C" void __bolt_instr_fini() {
   __bolt_fini_trampoline();
-  if (__bolt_instr_sleep_time == 0)
-    __bolt_instr_data_dump();
+  if (__bolt_instr_sleep_time == 0) {
+    int FD = openProfile();
+    __bolt_instr_data_dump(FD);
+    __close(FD);
+  }
   DEBUG(report("Finished.\n"));
 }
 
 #endif
 
 #if defined(__APPLE__)
 
 extern "C" void __bolt_instr_data_dump() {