[PATCH] D17639: [asan] Fix recvfrom.cc testcase failure in large parallel tests run.

Yury Gribov via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 26 06:38:25 PST 2016


ygribov added inline comments.

================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:20
@@ +19,3 @@
+    exit(1);                \
+  }
+
----------------
Maybe wrap in do-while(0) just in case?

================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:30
@@ -23,4 +29,3 @@
   int sockfd = socket(AF_INET, SOCK_DGRAM, 0);
-  if (sockfd < 0)
-    fprintf(stderr, "ERROR opening socket\n");
+  CHECK_ERROR(sockfd < 0, "ERROR opening socket.\n");
 
----------------
Still no agreement on ERROR vs. Error. I suggest to move prefix and newline to CHECK_ERROR macro.

================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:35
@@ -29,2 +34,3 @@
   serveraddr.sin_addr.s_addr = htonl(INADDR_ANY);
-  serveraddr.sin_port = htons(kPortNum);
+  serveraddr.sin_port = htons(0);
+
----------------
Perhaps avoid htons?

================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:45
@@ -35,1 +44,3 @@
+  // Now it's safe to unblock client thread.
+  pthread_mutex_unlock(&server_ready_mu);
   recvfrom(sockfd, buf, kBufSize, 0, NULL, NULL); // BOOM
----------------
Shouldn't mutexes be checked as well? BTW you could probably avoid mutexes alltogether if you moved server to main thread (and start client thread from it after socket is initialized).

================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:55
@@ -43,4 +54,3 @@
 int main() {
-  char buf[kBufSize] = "123456789";
-  struct sockaddr_in serveraddr; // server's addr
-
+  char buf[kBufSize];
+  struct sockaddr_in serveraddr;
----------------
Should probably initialialize to avoid UB.

================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:64
@@ +63,3 @@
+  int succeeded = pthread_create(&server_thread, NULL, server_thread_udp,
+                                 (void *)&serveraddr);
+  CHECK_ERROR(succeeded, "Error creating thread.\n");
----------------
I always thought that there's no need for explicit casts to starred voids (ditto for memset, memcpy, etc.).

================
Comment at: test/asan/TestCases/Linux/recvfrom.cc:72
@@ -71,1 +71,3 @@
+  pthread_mutex_lock(&server_ready_mu);
+  sendto(sockfd, buf, sizeof(buf), 0, (struct sockaddr *)&serveraddr,
          sizeof(serveraddr));
----------------
Should this be checked?


Repository:
  rL LLVM

http://reviews.llvm.org/D17639





More information about the llvm-commits mailing list