[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

In D108061#2947526 <https://reviews.llvm.org/D108061#2947526>, @rdhindsa wrote:

> Reverted code changes from ObjectFileELF as suggested and added directly to DYLDRendezvous.cpp.
>
> Is there a better place to place the test, or is it okay to keep under ObjectFile.

I think a test would be better added to the python test area of LLDB.

The test will look something like:

  """
  Test that LLDB can launch a linux executable through the dynamic loader and still hit a breakpoint.
  """
  
  import lldb
  
  from lldbsuite.test.decorators import *
  from lldbsuite.test.lldbtest import *
  from lldbsuite.test import lldbutil
  
  class TestLinux64LaunchingViaDynamicLoader(TestBase):
      mydir = TestBase.compute_mydir(__file__)
  
      def test(self):
          """Test we can launch and hit a breakpoint when we run our program through the dynamic loader"""
          self.build()
          exe = "/lib64/ld-linux-x86-64.so.2"
          launch_info = lldb.SBLaunchInfo([exe, self.getBuildArtifact("a.out")])
          launch_info.SetWorkingDirectory(self.get_process_working_directory())
          lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"), exe=exe, in_cwd=False, launch_info=launch_info)

This should do everything that is needed!

So the steps to create the test are:

- make a new directory in "lldb/test/API/functionalities/dyld-launch-linux"
  - create the main.cpp as noted above
  - add a TestDyldLaunchLinux.py and put the above test case python into it
  - copy a Makefile from another test like "lldb/test/API/commands/expression/anonymous-struct/Makefile"  into this directory

To run the test, build "check-lldb" first and kill it once it starts running the test suite. Then you can type this from the llvm build directory:

  $ ./bin/llvm-lit -sv ../lldb/test/API/functionalities/dyld-launch-linux

Let me know if there are any questions. You can then remove all of the test files  from lldb/test/Shell/ObjectFile/ELF



================
Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:356-361
+            &containingModules, nullptr /* containingSourceFiles */,
+            DebugStateCandidates, eFunctionNameTypeFull, eLanguageTypeC,
+            0,           /* offset */
+            eLazyBoolNo, /* skip_prologue */
+            true,        /* internal */
+            false /* request_hardware */);
----------------
I know you copied this from below, but LLVM coding conventions say to do comments like:

https://llvm.org/docs/CodingStandards.html#id13

And might be nice to fix the ones below too. 


================
Comment at: lldb/test/Shell/ObjectFile/ELF/ld_test.test:15
+#
+# RUN: %lldb -b -s %t.in -- /lib64/ld-linux-x86-64.so.2 --library-path %t  %tmain 2>&1 | FileCheck %s 
+#
----------------
So I worry the "/lib64/ld-linux-x86-64.so.2" path won't be portable across different linux distributions. Anyone know if it is the same on every linux?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061