Page MenuHomePhabricator

[gn build] Add build files for LLDB
ClosedPublic

Authored by thakis on Sep 2 2021, 12:47 PM.

Details

Summary

This is enough to get the lit-based tests to pass on macOS.

Doesn't yet add build targets for:

  • Any LLDB unit tests
  • swig bindings
  • various targets not needed by lit tests

LLDB has many dependency cycles, something GN doesn't allow. For
that reason, I've omitted some dependency edges. Hopefully we can
clean up the cycles one day.

LLDB has a public/private header distinction, but mostly ignores it.
Many libraries include private headers from other modules.

Since LLDB is the first target the LLVM/GN build that uses Objective-C++
code, add some machinery to the toolchain file to handle that.

Diff Detail

Event Timeline

thakis created this revision.Sep 2 2021, 12:47 PM
thakis requested review of this revision.Sep 2 2021, 12:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 2 2021, 12:47 PM
thakis edited the summary of this revision. (Show Details)Sep 2 2021, 12:49 PM

minor tweaks

llvm/utils/gn/secondary/lldb/tools/lldb-test/BUILD.gn
27

This is probably something I should figure out before landing this.

pcc added a comment.Sep 2 2021, 3:12 PM

Thanks for working on this!

I developed a Linux port as a patch on top of yours:


Feel free to fold it in.

With that lldb seems to work and check-lldb looks like this for me on Linux:

Failed Tests (1):
  lldb-shell :: Breakpoint/ppc64-localentry.test


Testing Time: 12.20s
  Unsupported      : 1046
  Passed           :  334
  Expectedly Failed:    5
  Failed           :    1

I'm not sure what's going on with that test. I tried including the PowerPC ABI plugin, but that didn't help.

I developed a Linux port as a patch on top of yours:

Very cool! That was admittedly more involved than I had naively assumed :)

I'm not sure what's going on with that test. I tried including the PowerPC ABI plugin, but that didn't help.

Hey that test fails over here too (on macOS, that is – but also on my Linux box with your patch), I just never noticed since I never build with the PPC target enabled :) I'll see if I can debug it a bit when I have some time.

This fixes that one PPC64 test. I guess the other plugins in Architecture/ aren't covered by shell tests...

$ git diff HEAD
diff --git a/llvm/utils/gn/secondary/lldb/source/Plugins/Architecture/PPC64/BUILD.gn b/llvm/utils/gn/secondary/lldb/source/Plugins/Architecture/PPC64/BUILD.gn
new file mode 100644
index 000000000000..177620783e8d
--- /dev/null
+++ b/llvm/utils/gn/secondary/lldb/source/Plugins/Architecture/PPC64/BUILD.gn
@@ -0,0 +1,14 @@
+static_library("PPC64") {
+  output_name = "lldbPluginArchitecturePPC64"
+  configs += [ "//llvm/utils/gn/build:lldb_code" ]
+  deps = [
+    "//lldb/source/Core",
+    "//lldb/source/Target",
+    "//lldb/source/Utility",
+    "//lldb/source/Plugins/Process/Utility",
+    "//llvm/lib/Support",
+  ]
+  # Uses source-relative paths for own includes.
+  include_dirs = [ "//lldb/source" ]
+  sources = [ "ArchitecturePPC64.cpp" ]
+}
diff --git a/llvm/utils/gn/secondary/lldb/source/Plugins/BUILD.gn b/llvm/utils/gn/secondary/lldb/source/Plugins/BUILD.gn
index 35149476aff6..219b0732bc25 100644
--- a/llvm/utils/gn/secondary/lldb/source/Plugins/BUILD.gn
+++ b/llvm/utils/gn/secondary/lldb/source/Plugins/BUILD.gn
@@ -5,6 +5,7 @@ import("//llvm/utils/gn/build/write_cmake_config.gni")
 plugins = [
   "ABIPowerPC",
   "ABIX86",
+  "ArchitecturePPC64",
   "DisassemblerLLVMC",
   "DynamicLoaderDarwinKernel",
   "DynamicLoaderMacOSXDYLD",
diff --git a/llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn b/llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
index 22dc74e0b98b..8455ba4751c9 100644
--- a/llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
+++ b/llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
@@ -20,6 +20,7 @@ executable("lldb") {
     # FIXME: in the cmake build these come in from elsewhere
     "//lldb/source/Plugins/ABI/PowerPC",
     "//lldb/source/Plugins/ABI/X86",
+    "//lldb/source/Plugins/Architecture/PPC64",
     "//lldb/source/Plugins/Disassembler/LLVMC",
     "//lldb/source/Plugins/DynamicLoader/MacOSX-DYLD",
     "//lldb/source/Plugins/DynamicLoader/POSIX-DYLD",
diff --git a/llvm/utils/gn/secondary/lldb/tools/lldb-test/BUILD.gn b/llvm/utils/gn/secondary/lldb/tools/lldb-test/BUILD.gn
index 7f67be94ad1c..1e2a7f67bb79 100644
--- a/llvm/utils/gn/secondary/lldb/tools/lldb-test/BUILD.gn
+++ b/llvm/utils/gn/secondary/lldb/tools/lldb-test/BUILD.gn
@@ -24,6 +24,7 @@ executable("lldb-test") {
 
     # FIXME: in the cmake build these come in from elsewhere
     # FIXME: also, don't duplicate this in driver and lldb-test
+    "//lldb/source/Plugins/Architecture/PPC64",
     "//lldb/source/Plugins/ABI/PowerPC",
     "//lldb/source/Plugins/ABI/X86",
     "//lldb/source/Plugins/Disassembler/LLVMC",
diff --git a/llvm/utils/gn/secondary/lldb/tools/lldb-vscode/BUILD.gn b/llvm/utils/gn/secondary/lldb/tools/lldb-vscode/BUILD.gn
index 1ba744912d4e..deebb31fbaf5 100644
--- a/llvm/utils/gn/secondary/lldb/tools/lldb-vscode/BUILD.gn
+++ b/llvm/utils/gn/secondary/lldb/tools/lldb-vscode/BUILD.gn
@@ -15,6 +15,7 @@ executable("lldb-vscode") {
 
     # FIXME: in the cmake build these come in from elsewhere
     # FIXME: also, don't duplicate this in driver and lldb-test and lldb-vscode
+    "//lldb/source/Plugins/Architecture/PPC64",
     "//lldb/source/Plugins/ABI/PowerPC",
     "//lldb/source/Plugins/ABI/X86",
     "//lldb/source/Plugins/Disassembler/LLVMC",
thakis updated this revision to Diff 371081.Sep 7 2021, 8:12 AM
  • don't repeat plugin list in 4 places
  • run gn format on all new files
thakis added inline comments.Sep 7 2021, 8:13 AM
lldb/source/Plugins/ObjectFile/CMakeLists.txt
3 ↗(On Diff #371081)

Oh sorry, these weren't meant for this patch, I'll send them out separately. (They aren't needed for anything, just seems nice to keep them alphabetized -- and it makes it a bit easier to manually compare Plugins.def between the two builds.)

  • rebase
  • add -sectcreate flags for embedding Info.plist files into lldb and lldb-vscode

This is probably in shape enough to iterate in in-tree. I didn't squash in your linux change; let's land that separately :)

minor tweaks

pcc accepted this revision.Sep 7 2021, 11:59 AM

LGTM

  • rebase
  • add -sectcreate flags for embedding Info.plist files into lldb and lldb-vscode

This is probably in shape enough to iterate in in-tree. I didn't squash in your linux change; let's land that separately :)

Okay, sounds good.

llvm/utils/gn/secondary/lldb/source/Host/BUILD.gn
12

Maybe add the missing dependency on //lldb/include/lldb/Host:Config from my patch here? That seems more like a generic dependency problem than something Linux-specific.

This revision is now accepted and ready to land.Sep 7 2021, 11:59 AM
thakis marked an inline comment as done.Sep 7 2021, 12:25 PM

Thanks!

This revision was landed with ongoing or failed builds.Sep 7 2021, 12:25 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 7 2021, 12:25 PM
teemperor added a comment.EditedSep 8 2021, 1:30 AM

Thanks! Out of curiosity, is there a public GN bot that is testing this?

LLDB has many dependency cycles, something GN doesn't allow. For that reason, I've omitted some dependency edges. Hopefully we can clean up the cycles one day.

No idea about GN, but I would assume this would break the build in some form? Or does this just mean ninja doesn't know the dependencies and the build process has a race condition?

LLDB has a public/private header distinction, but mostly ignores it.
Many libraries include private headers from other modules.

Is this referring to some private-*.h public-*.h files or what are those public/private headers that are incorrectly included?

Thanks! Out of curiosity, is there a public GN bot that is testing this?

The GN bots at http://45.33.8.238/ do the compile step. They don't yet run lldb tests. Maybe I'll add that, but maybe I'll wait until more of the test suite is brought up.

LLDB has many dependency cycles, something GN doesn't allow. For that reason, I've omitted some dependency edges. Hopefully we can clean up the cycles one day.

No idea about GN, but I would assume this would break the build in some form? Or does this just mean ninja doesn't know the dependencies and the build process has a race condition?

The CMake build works around this with the set_target_properties(lldbCore PROPERTIES LINK_INTERFACE_MULTIPLICITY 5) hack in lldb/source/Core/CMakeLists.txt.

In the GN build, it means you can't really build the libraries independently. But the binaries build fine (…at least with lld and ld64. It's possible BFD ld needs a --start-group / --end-group around libraries? Haven't tried that yet.)

LLDB has a public/private header distinction, but mostly ignores it.
Many libraries include private headers from other modules.

Is this referring to some private-*.h public-*.h files or what are those public/private headers that are incorrectly included?

LLDB has lldb/include/lldb/Foo which (like in the other LLVM projects) are supposed to define a module's public interface from what I understand, while headers in lldb/source/Foo are Foo's internal headers. At least that's how it works in clang and llvm. In lldb, many modules include another module's private headers (in that sense that they're in lldb/source/Foo instead of in lldb/include/lldb/Foo), while that's much less common in llvm and clang.