[PATCH] D31345: [XRay] [compiler-rt] Unwriting FDR mode buffers when functions are short.

[Dean Michael Berris via Phabricator via llvm-commits]

dberris accepted this revision.
dberris added a comment.
This revision is now accepted and ready to land.

LGTM -- but please see one important comment on correctness.



================
Comment at: lib/xray/xray_fdr_logging_impl.h:511-513
+      AlignedFuncStorage AlignedFuncRecordBuffer;
+      const auto &FuncRecord = *reinterpret_cast<FunctionRecord *>(
+          std::memcpy(&AlignedFuncRecordBuffer, RecordPtr, FunctionRecSize));
----------------
kpw wrote:
> dberris wrote:
> > Why are we copying from the log buffer into the local aligned record buffer? This seems unused now.
> I should probably reinterpret cast directly from the buffer. I can make that fix easily enough.
Actually, now that I understand what you were doing, I think it's actually better that you copy to a local aligned buffer. This incarnation is actually running into undefined behaviour, since it's technically unsafe to alias a void* as something other than a char*. For correctness and efficiency, I recommend going back to the aligned buffer local.

Sorry I missed the intent the first time around. :(


https://reviews.llvm.org/D31345