[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py:5
+
+class StepIntoNamespace(TestBase):
+    mydir = TestBase.compute_mydir(__file__)
----------------
Am I correct in assuming that the "namespace" part here comes from the fact that global functions (e.g. `_Z3foov` aka `foo()`) would be found according to their base name (`foo`) or something?

If yes, that's fine, but it would still be nice to mention (say in a comment or something) that the shared library is an important aspect of the test ("TestStepIntoNamespace" does not convey that notion).

If this can also be reproduced with regular functions, then it'd be better to rename the test into something else (which includes the word "trampoline").


================
Comment at: lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py:8
+
+    def test(self):
+        self.build()
----------------
mdaniels wrote:
> clayborg wrote:
> > Do we want to limit this to linux? I am not sure this will pass the windows buildbots if we don't restrict it
> Looking at other tests that load a shared library, it seems I should have at least
> 
> ```
> @skipIfRemote
> @skipIfWindows
> ```
> But I can also just restrict it to the platforms I am able to test on locally, linux and darwin, if there is still concern that other platforms might fail.
Neither of these is strictly necessary. Remote shared library tests need additional care as one has to copy the shared library to the remote system, but afaik, you're using the correct incantation which should do that automatically.

Windows shared library support is not at the level of other platforms, but a simple setup like this should work. Or rather: if it fails, it will probably be due to the trampoline aspect, and not the shared library loading. So I think you don't need to add any decorators right now. We can add xfail-windows if it turns out to be failing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127999/new/

https://reviews.llvm.org/D127999