[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