[Lldb-commits] [PATCH] Add new test for stress testing stack unwinding

[Pavel Labath]

I think having tests like these will help our unwinding reliability. However, I think it can be done without python introspection. Also, I'm not sure if it make sense to commit this, while we don't have any tests to run this on.


================
Comment at: test/functionalities/unwind/standard/TestStandardUnwind.py:32
@@ +31,3 @@
+        #                         instruction because it is special for some reason.) For these
+        #                         functions we will immediately do a step-out when we it them.
+
----------------
s/it/hit/?

================
Comment at: test/functionalities/unwind/standard/TestStandardUnwind.py:56
@@ +55,3 @@
+
+        if not process:
+            self.fail("SBTarget.Launch() failed")
----------------
AssertTrue(process != None) ?

================
Comment at: test/functionalities/unwind/standard/TestStandardUnwind.py:64
@@ +63,3 @@
+        # cases when we stuck in an infinite loop waiting for an event from the kernel while
+        # stepping (e.g.: in case of standard IO)
+        # TODO: Use better heuristic to detect when we have to do a step-out to increase the
----------------
I am confused by this comment. How will using step-out instead of step-inst help us unstuck a program which is waiting for STDIO? If it is waiting for an input event, it will keep waiting no matter how we step it...

================
Comment at: test/functionalities/unwind/standard/TestStandardUnwind.py:73
@@ +72,3 @@
+            for i in range(process.GetNumThreads()):
+                thread = process.GetThreadAtIndex(0)
+
----------------
s/0/i/ ?

================
Comment at: test/functionalities/unwind/standard/TestStandardUnwind.py:98
@@ +97,3 @@
+# Generate test cases based on the cpp sources in this directory
+for f in os.listdir(os.path.dirname(__file__)):
+    if f.endswith(".cpp") or f.endswith(".c"):
----------------
This design, besides being confusing to non-python experts, makes it impossible for someone to selectively enable tests. I think this is important, as there sure will be many failures once somebody starts bringing this up on a new type of platform. I would prefer refactoring this so that every source file can be defined using two lines:
```
  def test_unwind_foo(self):
    run_test("foo.cpp")
```
These lines can be generated by a script on the initial import.

================
Comment at: test/functionalities/unwind/standard/TestStandardUnwind.py:115
@@ +114,3 @@
+            except:
+                self.skipTest("Inferior not supported")
+            self.standard_unwind_tests()
----------------
What kind and how many compile errors were you encountering? I'm a bit worried that these skips everywhere will make it hard to figure out which tests are actually being run.

http://reviews.llvm.org/D10454

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/