[PATCH] D30630: [XRay][compiler-rt] Runtime changes to support custom event logging

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added inline comments.


================
Comment at: lib/xray/xray_fdr_logging.cc:508
+  case XRayEntryType::CUSTOM_EVENT:
+    // FIXME: This never should occur. This would imply a patching bug, so at
+    // this point we proceed as if we're still doing the right thing but really
----------------
timshen wrote:
> Do you want to assert here?
I thought about it, but then figured it would be bad form to be crashing the process if there was a runtime implementation bug. Is it OK to have assertions in runtime implementations?

I thought about using llvm_unreachable(...) but wasn't sure that was available in compiler-rt.


================
Comment at: lib/xray/xray_trampoline_x86_64.S:205
+  .cfi_startproc
+	pushq %rbp
+	.cfi_def_cfa_offset 16
----------------
timshen wrote:
> Why stash rbp, if we don't clobber it?
I can't fully explain it, but if I don't stash this (seems to be the frame pointer) then I've run into issues in the past. I can try removing it, and see how that goes.


https://reviews.llvm.org/D30630