[PATCH] D153771: [BOLT][Instrumentation] Fix hash table memory corruption and append-pid option

[Rafael Auler via Phabricator via llvm-commits]

rafauler added inline comments.


================
Comment at: bolt/runtime/instr.cpp:216
+// instrumentation-file-append-pid.
+char *GlobalMetadataStorage;
+
----------------
void *


================
Comment at: bolt/runtime/instr.cpp:1627-1629
+  GlobalMetadataStorage = reinterpret_cast<char *>(
+      __mmap(0, 4096, PROT_READ | PROT_WRITE,
+             (Shared ? MAP_SHARED : MAP_PRIVATE) | MAP_ANONYMOUS, -1, 0));
----------------
Drop the cast
  GlobalMetadataStorage = 
      __mmap(0, 4096, PROT_READ | PROT_WRITE,
             (Shared ? MAP_SHARED : MAP_PRIVATE) | MAP_ANONYMOUS, -1, 0);


================
Comment at: bolt/test/runtime/instrumentation-indirect-2.c:111
+# Wait till output is fully written in case child outlives parent
+RUN: sleep 1
+# Make sure all functions were called
----------------
Do you really need that? (Lines 111 and 66)

I'm testing locally with the sleep timers removed and the test is not failing. If line 65 " Wait for profile and output to be fully written"  is the problem, I don't know, perhaps something like https://linux.die.net/man/1/sync?  (I don't know because I'm not sure what's happening in your machine, but it looks odd to me that the process will finish but your test script will access a half-finished output file).

If the problem is the one described in line 110 "in case child outlives parent", then perhaps using bash's  "wait" command? https://phoenixnap.com/kb/bash-wait-command


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153771/new/

https://reviews.llvm.org/D153771