[Lldb-commits] [lldb] 8f549c5 - [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

Med Ismail Bennani via lldb-commits lldb-commits at lists.llvm.org
Thu Jan 12 19:21:20 PST 2023


Author: Med Ismail Bennani
Date: 2023-01-12T19:20:51-08:00
New Revision: 8f549c5329275293ced1d5eb87a1cf8b3d52a794

URL: https://github.com/llvm/llvm-project/commit/8f549c5329275293ced1d5eb87a1cf8b3d52a794
DIFF: https://github.com/llvm/llvm-project/commit/8f549c5329275293ced1d5eb87a1cf8b3d52a794.diff

LOG: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

This patch should fix an nondeterministic error in TestStackCoreScriptedProcess.

In order to test both the multithreading capability and shared library
loading in Scripted Processes, the test would create multiple threads
that would take the same variable as a reference.

The first thread would alter the value and the second thread would
monitor the value until it gets altered. This assumed a certain ordering
regarding the `std::thread` spawning, however the ordering was not
always guaranteed at runtime.

To fix that, the test now makes use of a `std::condition_variable`
shared between the each thread. On the former, it will notify the other
thread when the variable gets initialized or updated and on the latter,
it will wait until the variable it receives a new notification.

This should fix the data racing issue while preserving the testing
coverage.

rdar://98678134

Differential Revision: https://reviews.llvm.org/D139484

Signed-off-by: Med Ismail Bennani <medismail.bennani at gmail.com>

Added: 
    lldb/test/API/functionalities/scripted_process/baz.cpp

Modified: 
    lldb/test/API/functionalities/scripted_process/Makefile
    lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
    lldb/test/API/functionalities/scripted_process/baz.h
    lldb/test/API/functionalities/scripted_process/main.cpp

Removed: 
    lldb/test/API/functionalities/scripted_process/baz.c


################################################################################
diff  --git a/lldb/test/API/functionalities/scripted_process/Makefile b/lldb/test/API/functionalities/scripted_process/Makefile
index ca7f8f7d6d82..c23310226d87 100644
--- a/lldb/test/API/functionalities/scripted_process/Makefile
+++ b/lldb/test/API/functionalities/scripted_process/Makefile
@@ -6,8 +6,8 @@ override ARCH := $(shell uname -m)
 
 all: libbaz.dylib a.out
 
-libbaz.dylib: baz.c
+libbaz.dylib: baz.cpp
 	$(MAKE) -f $(MAKEFILE_RULES) ARCH=$(ARCH) \
-		DYLIB_ONLY=YES DYLIB_NAME=baz DYLIB_C_SOURCES=baz.c
+		DYLIB_ONLY=YES DYLIB_NAME=baz DYLIB_CXX_SOURCES=baz.cpp
 
 include Makefile.rules

diff  --git a/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py b/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
index 7a6a7ddb4b77..8555faf67226 100644
--- a/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
+++ b/lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
@@ -17,7 +17,7 @@ class StackCoreScriptedProcesTestCase(TestBase):
     def create_stack_skinny_corefile(self, file):
         self.build()
         target, process, thread, _ = lldbutil.run_to_source_breakpoint(self, "// break here",
-                                                                       lldb.SBFileSpec("baz.c"))
+                                                                       lldb.SBFileSpec("baz.cpp"))
         self.assertTrue(process.IsValid(), "Process is invalid.")
         # FIXME: Use SBAPI to save the process corefile.
         self.runCmd("process save-core -s stack  " + file)
@@ -109,9 +109,9 @@ def cleanup():
         self.assertTrue(func, "Invalid function.")
 
         self.assertIn("baz", frame.GetFunctionName())
-        self.assertEqual(frame.vars.GetSize(), 2)
-        self.assertEqual(int(frame.vars.GetFirstValueByName('j').GetValue()), 42 * 42)
+        self.assertGreater(frame.vars.GetSize(), 0)
         self.assertEqual(int(frame.vars.GetFirstValueByName('k').GetValue()), 42)
+        self.assertEqual(int(frame.vars.GetFirstValueByName('j').Dereference().GetValue()), 42 * 42)
 
         corefile_dylib = self.get_module_with_name(corefile_target, 'libbaz.dylib')
         self.assertTrue(corefile_dylib, "Dynamic library libbaz.dylib not found.")

diff  --git a/lldb/test/API/functionalities/scripted_process/baz.c b/lldb/test/API/functionalities/scripted_process/baz.c
deleted file mode 100644
index cfd452e7779c..000000000000
--- a/lldb/test/API/functionalities/scripted_process/baz.c
+++ /dev/null
@@ -1,8 +0,0 @@
-#include "baz.h"
-
-#include <math.h>
-
-int baz(int j) {
-  int k = sqrt(j);
-  return k; // break here
-}

diff  --git a/lldb/test/API/functionalities/scripted_process/baz.cpp b/lldb/test/API/functionalities/scripted_process/baz.cpp
new file mode 100644
index 000000000000..e94b99454009
--- /dev/null
+++ b/lldb/test/API/functionalities/scripted_process/baz.cpp
@@ -0,0 +1,10 @@
+#include "baz.h"
+
+#include <math.h>
+
+int baz(int &j, std::mutex &mutex, std::condition_variable &cv) {
+  std::unique_lock<std::mutex> lock(mutex);
+  cv.wait(lock, [&j] { return j == 42 * 42; });
+  int k = sqrt(j);
+  return k; // break here
+}

diff  --git a/lldb/test/API/functionalities/scripted_process/baz.h b/lldb/test/API/functionalities/scripted_process/baz.h
index aa667d233392..eafc42ba01eb 100644
--- a/lldb/test/API/functionalities/scripted_process/baz.h
+++ b/lldb/test/API/functionalities/scripted_process/baz.h
@@ -1,3 +1,6 @@
 #pragma once
 
-int baz(int j);
+#include <condition_variable>
+#include <mutex>
+
+int baz(int &j, std::mutex &mutex, std::condition_variable &cv);

diff  --git a/lldb/test/API/functionalities/scripted_process/main.cpp b/lldb/test/API/functionalities/scripted_process/main.cpp
index 41d137eeff8f..202402d852ad 100644
--- a/lldb/test/API/functionalities/scripted_process/main.cpp
+++ b/lldb/test/API/functionalities/scripted_process/main.cpp
@@ -1,10 +1,9 @@
-#include <iostream>
-#include <mutex>
 #include <thread>
 
-extern "C" {
-int baz(int);
-}
+#include "baz.h"
+
+std::condition_variable cv;
+std::mutex mutex;
 
 int bar(int i) {
   int j = i * i;
@@ -13,23 +12,20 @@ int bar(int i) {
 
 int foo(int i) { return bar(i); }
 
-void call_and_wait(int &n) {
-  std::cout << "waiting for computation!" << std::endl;
-  while (baz(n) != 42)
-    ;
-  std::cout << "finished computation!" << std::endl;
+void compute_pow(int &n) {
+  std::unique_lock<std::mutex> lock(mutex);
+  n = foo(n);
+  lock.unlock();
+  cv.notify_one(); // waiting thread is notified with n == 42 * 42, cv.wait
+                   // returns
 }
 
-void compute_pow(int &n) { n = foo(n); }
+void call_and_wait(int &n) { baz(n, mutex, cv); }
 
 int main() {
   int n = 42;
-  std::mutex mutex;
-  std::unique_lock<std::mutex> lock(mutex);
-
   std::thread thread_1(call_and_wait, std::ref(n));
   std::thread thread_2(compute_pow, std::ref(n));
-  lock.unlock();
 
   thread_1.join();
   thread_2.join();


        


More information about the lldb-commits mailing list