[llvm-exegesis] Make SubprocessMemoryTest use PIDs

[aidengrossman]

Summary

This patch makes SubprocessMemoryTest use process PIDs during creation of the SubprocessMemory objects within the tests so that there isn't interference between multiple instances of the test running at the same time which could potentially occur in multi-user environments.

[aidengrossman]

Remove unnecessary headers and debug statements.

[aidengrossman]

This patch is ready when reviewers have a chance. Thanks!

[bjope]

llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp

42

Multiplying a pid with a number seems a bit weird to me. It is not really guaranteed to result in a number unique to the process (pid=1 and TestNumber=2 gives the same value as pid=2 and TestNumber=1). And I don't know if this multiplication potentially could overflow as well.

[bjope]

llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp

42

Any news about this? Do you agree that the multiplication is weird?

[aidengrossman]

llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp

42

Sorry for the delay. Planning on working on this over the weekend. I do agree that multiplication is weird the way I have it implemented here. However, I do think it's the most elegant solution available without changing the library code just to expose more testing hooks. I think doing 2D array indexing should work. We can do getpid() * NumberOfTests + TestNumber which should make everything unique and also won't overflow on 32 bit arithmetic as PIDs are limited to 2^22.

[aidengrossman]

Abandoning in favor of https://github.com/llvm/llvm-project/pull/65245.

llvm/unittests/tools/llvm-exegesis/X86/SubprocessMemoryTest.cpp

 #include "X86/TestBase.h"
 #include "gtest/gtest.h"
 #include <unordered_map>
 
 #ifdef __linux__
 #include <endian.h>
 #include <fcntl.h>
 #include <sys/mman.h>
+#include <unistd.h>
 #endif // __linux__
 
 namespace llvm {
 namespace exegesis {
 
 #if defined(__linux__) && !defined(__ANDROID__)
 
 class SubprocessMemoryTest : public X86TestBase {
 protected:
+  // In this function and below, TestNumber is a one-based index of the test
+  // rather than a zero-based index of the test so that the zero-case doesn't
+  // cancel out the differentiation provided by the process PID.
   void
   testCommon(std::unordered_map<std::string, MemoryValue> MemoryDefinitions,
-             const int MainProcessPID) {
-    EXPECT_FALSE(SM.initializeSubprocessMemory(MainProcessPID));
-    EXPECT_FALSE(SM.addMemoryDefinition(MemoryDefinitions, MainProcessPID));
+             const unsigned TestNumber) {
+    EXPECT_FALSE(SM.initializeSubprocessMemory(getpid() * TestNumber));
+    EXPECT_FALSE(
+        SM.addMemoryDefinition(MemoryDefinitions, getpid() * TestNumber));
+  }
+
+  std::string getSharedMemoryName(const unsigned TestNumber,
+                                  const unsigned DefinitionNumber) {
+    return "/" + std::to_string(getpid() * TestNumber) + "memdef" +
+           std::to_string(DefinitionNumber);
   }
 
   void checkSharedMemoryDefinition(const std::string &DefinitionName,
                                    size_t DefinitionSize,
                                    std::vector<uint8_t> ExpectedValue) {
     int SharedMemoryFD =
         shm_open(DefinitionName.c_str(), O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
     uint8_t *SharedMemoryMapping = (uint8_t *)mmap(

 // memory calls not working in some cases, so they have been disabled.
 // TODO(boomanaiden154): Investigate and fix this issue on PPC.
 
 #if defined(__powerpc__) || defined(__s390x__)
 TEST_F(SubprocessMemoryTest, DISABLED_OneDefinition) {
 #else
 TEST_F(SubprocessMemoryTest, OneDefinition) {
 #endif
-  testCommon({{"test1", {APInt(8, 0xff), 4096, 0}}}, 0);
-  checkSharedMemoryDefinition("/0memdef0", 4096, {0xff});
+  testCommon({{"test1", {APInt(8, 0xff), 4096, 0}}}, 1);
+  checkSharedMemoryDefinition(getSharedMemoryName(1, 0), 4096, {0xff});
 }
 
 #if defined(__powerpc__) || defined(__s390x__)
 TEST_F(SubprocessMemoryTest, DISABLED_MultipleDefinitions) {
 #else
 TEST_F(SubprocessMemoryTest, MultipleDefinitions) {
 #endif
   testCommon({{"test1", {APInt(8, 0xaa), 4096, 0}},
               {"test2", {APInt(8, 0xbb), 4096, 1}},
               {"test3", {APInt(8, 0xcc), 4096, 2}}},
-             1);
-  checkSharedMemoryDefinition("/1memdef0", 4096, {0xaa});
-  checkSharedMemoryDefinition("/1memdef1", 4096, {0xbb});
-  checkSharedMemoryDefinition("/1memdef2", 4096, {0xcc});
+             2);
+  checkSharedMemoryDefinition(getSharedMemoryName(2, 0), 4096, {0xaa});
+  checkSharedMemoryDefinition(getSharedMemoryName(2, 1), 4096, {0xbb});
+  checkSharedMemoryDefinition(getSharedMemoryName(2, 2), 4096, {0xcc});
 }
 
 #if defined(__powerpc__) || defined(__s390x__)
 TEST_F(SubprocessMemoryTest, DISABLED_DefinitionFillsCompletely) {
 #else
 TEST_F(SubprocessMemoryTest, DefinitionFillsCompletely) {
 #endif
   testCommon({{"test1", {APInt(8, 0xaa), 4096, 0}},
               {"test2", {APInt(16, 0xbbbb), 4096, 1}},
               {"test3", {APInt(24, 0xcccccc), 4096, 2}}},
-             2);
+             3);
   std::vector<uint8_t> Test1Expected(512, 0xaa);
   std::vector<uint8_t> Test2Expected(512, 0xbb);
   std::vector<uint8_t> Test3Expected(512, 0xcc);
-  checkSharedMemoryDefinition("/2memdef0", 4096, Test1Expected);
-  checkSharedMemoryDefinition("/2memdef1", 4096, Test2Expected);
-  checkSharedMemoryDefinition("/2memdef2", 4096, Test3Expected);
+  checkSharedMemoryDefinition(getSharedMemoryName(3, 0), 4096, Test1Expected);
+  checkSharedMemoryDefinition(getSharedMemoryName(3, 1), 4096, Test2Expected);
+  checkSharedMemoryDefinition(getSharedMemoryName(3, 2), 4096, Test3Expected);
 }
 
 // The following test is only supported on little endian systems.
 #if defined(__powerpc__) || defined(__s390x__) || __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
 TEST_F(SubprocessMemoryTest, DISABLED_DefinitionEndTruncation) {
 #else
 TEST_F(SubprocessMemoryTest, DefinitionEndTruncation) {
 #endif
-  testCommon({{"test1", {APInt(48, 0xaabbccddeeff), 4096, 0}}}, 3);
+  testCommon({{"test1", {APInt(48, 0xaabbccddeeff), 4096, 0}}}, 4);
   std::vector<uint8_t> Test1Expected(512, 0);
   // order is reversed since we're assuming a little endian system.
   for (size_t I = 0; I < Test1Expected.size(); ++I) {
     switch (I % 6) {
     case 0:
       Test1Expected[I] = 0xff;
       break;
     case 1:
       Test1Expected[I] = 0xee;
       break;
     case 2:
       Test1Expected[I] = 0xdd;
       break;
     case 3:
       Test1Expected[I] = 0xcc;
       break;
     case 4:
       Test1Expected[I] = 0xbb;
       break;
     case 5:
       Test1Expected[I] = 0xaa;
     }
   }
-  checkSharedMemoryDefinition("/3memdef0", 4096, Test1Expected);
+  checkSharedMemoryDefinition(getSharedMemoryName(4, 0), 4096, Test1Expected);
 }
 
 #endif // defined(__linux__) && !defined(__ANDROID__)
 
 } // namespace exegesis
 } // namespace llvm