[PATCH] D43278: Add Xray instrumentation support to FreeBSD

[Kamil Rytarowski via Phabricator via llvm-commits]

krytarowski added inline comments.


================
Comment at: lib/xray/xray_fdr_logging_impl.h:240
   TLD.RecordPtr = static_cast<char *>(B.Data);
-  pid_t Tid = syscall(SYS_gettid);
+  pid_t Tid = getTid();
   timespec TS{0, 0};
----------------
devnexen wrote:
> krytarowski wrote:
> > krytarowski wrote:
> > > krytarowski wrote:
> > > > devnexen wrote:
> > > > > krytarowski wrote:
> > > > > > krytarowski wrote:
> > > > > > > Here `Tid` should be of type `Tid_t`.
> > > > > > There are more `pid_t` out there:
> > > > > > 
> > > > > > ```
> > > > > > static void writeNewBufferPreamble(pid_t Tid, timespec TS);
> > > > > > ```
> > > > > > 
> > > > > > ```
> > > > > > static void writeNewBufferPreamble(pid_t Tid, 
> > > > > >                                    timespec TS) XRAY_NEVER_INSTRUMENT {
> > > > > > ```
> > > > > > 
> > > > > > ```
> > > > > >     // point we only write down the following bytes:
> > > > > >     //   - Thread ID (pid_t, 4 bytes)
> > > > > > ```
> > > > > Careful here writeNewBufferPreamble expects internally a size of pid_t (so 4 bytes)
> > > > Please adjust the code for `Tid_t` of 8bytes.
> > > Alternatively we can squash `Tid_t` to 4 bytes, it will be fine for Linux, FreeBSD and NetBSD. I have no idea about Darwin.
> > By a squash I mean to typedef `Tid_t` to `int32_t`.
> I honestly prefer it fits 4 bytes indeed struct using them has strict alignment
Same here, it can be extended in future when an OS with 64-bit Thread ID range will need to be supported.

For now please alter the typedef of `Tid_t` to `int32_t` and please replace `pid_t` with `Tid_t` in this file for Thread ID.


https://reviews.llvm.org/D43278