[PATCH] D154275: [llvm-exegesis] Support older kernel versions in subprocess executor
Clement Courbet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 17 00:17:42 PDT 2023
courbet added inline comments.
================
Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp:205
+
+ *((int *)CMSG_DATA(ControlMessage)) = FD;
+
----------------
I think this is technically UB in C++ (it violates strict aliasing). I'd be more comfortable with `memcpy(CMSG_DATA(ControlMessage)), FD, sizeof(FD));`, which does the same thing while having defined semantics.
================
Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp:232
+ unsigned char *RawData = CMSG_DATA(ControlMessage);
+
+ return *((int *)RawData);
----------------
I'm not sure you cant just read 4 bytes from the message unconditionally, you need to check that they are indeed available by checking `cmsg_len`.
```
struct cmsghdr {
socklen_t cmsg_len; /* data byte count, including header */
...
};
```
================
Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp:233
+
+ return *((int *)RawData);
+ }
----------------
Ditto:
```
int FD;
mempcy(&FD, RawData, sizeof(FD));
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154275/new/
https://reviews.llvm.org/D154275
More information about the llvm-commits
mailing list