[Lldb-commits] [lldb] d6713ad - Debuginfod Testing & fixes: 3rd times the charm? (#87676)

2024-04-04 Thread via lldb-commits 

Author: Kevin Frei
Date: 2024-04-04T11:43:55-07:00
New Revision: d6713ad80d6907210c629f22babaf12177fa329c

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

LOG: Debuginfod Testing & fixes: 3rd times the charm? (#87676)

I believe I've got the tests properly configured to only run on Linux
x86(_64), as I don't have a Linux AArch64/Arm device to diagnose what's
going wrong with the tests (I suspect there's some issue with generating
`.note.gnu.build-id` sections...)

The actual code fixes have now been reviewed 3 times:
https://github.com/llvm/llvm-project/pull/79181 (moved shell tests to
API tests), https://github.com/llvm/llvm-project/pull/85693 (Changed
some of the testing infra), and
https://github.com/llvm/llvm-project/pull/86812 (didn't get the tests
configured quite right). The Debuginfod integration for symbol
acquisition in LLDB now works with the `executable` and `debuginfo`
Debuginfod network requests working properly for normal, `objcopy
--only-keep-debug` stripped, split-dwarf, and `objcopy
--only-keep-debug` stripped *plus* split-dwarf symbols/binaries.

The reasons for the multiple attempts have been tests on platforms I
don't have access to (Linux AArch64/Arm + MacOS x86_64). I believe I've
got the tests properly disabled for everything except for Linux x86(_64)
now. I've built & tested on MacOS AArch64 and Linux x86_64.

-

Co-authored-by: Kevin Frei 

Added: 
lldb/test/API/debuginfod/Normal/Makefile
lldb/test/API/debuginfod/Normal/TestDebuginfod.py
lldb/test/API/debuginfod/Normal/main.c
lldb/test/API/debuginfod/SplitDWARF/Makefile
lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
lldb/test/API/debuginfod/SplitDWARF/main.c

Modified: 
lldb/packages/Python/lldbsuite/test/make/Makefile.rules
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolLocator/CMakeLists.txt
lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index bfd249ccd43f2e..ee8793fa1b3029 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -51,7 +51,7 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
 #
 # GNUWin32 uname gives "windows32" or "server version windows32" while
 # some versions of MSYS uname return "MSYS_NT*", but most environments
-# standardize on "Windows_NT", so we'll make it consistent here. 
+# standardize on "Windows_NT", so we'll make it consistent here.
 # When running tests from Visual Studio, the environment variable isn't
 # inherited all the way down to the process spawned for make.
 #--
@@ -210,6 +210,12 @@ else
ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
DSYM = $(EXE).debug
endif
+
+   ifeq "$(MAKE_DWP)" "YES"
+   MAKE_DWO := YES
+   DWP_NAME = $(EXE).dwp
+   DYLIB_DWP_NAME = $(DYLIB_NAME).dwp
+   endif
 endif
 
 LIMIT_DEBUG_INFO_FLAGS =
@@ -357,6 +363,7 @@ ifneq "$(OS)" "Darwin"
 
OBJCOPY ?= $(call replace_cc_with,objcopy)
ARCHIVER ?= $(call replace_cc_with,ar)
+   DWP ?= $(call replace_cc_with,dwp)
override AR = $(ARCHIVER)
 endif
 
@@ -527,6 +534,10 @@ ifneq "$(CXX)" ""
endif
 endif
 
+ifeq "$(GEN_GNU_BUILD_ID)" "YES"
+   LDFLAGS += -Wl,--build-id
+endif
+
 #--
 # DYLIB_ONLY variable can be used to skip the building of a.out.
 # See the sections below regarding dSYM file as well as the building of
@@ -565,10 +576,17 @@ else
 endif
 else
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
+ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
+   cp "$(EXE)" "$(EXE).unstripped"
+endif
$(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)"
$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)"
 endif
+ifeq "$(MAKE_DWP)" "YES"
+   $(DWP) -o "$(DWP_NAME)" $(DWOS)
 endif
+endif
+
 
 #--
 # Make the dylib
@@ -610,9 +628,15 @@ endif
 else
$(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)"
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
+   ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
+   cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).unstripped"
+   endif
$(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" 
"$(DYLIB_FILENAME).debug"
$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" 
"$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)"
 endif
+ifeq "$(MAKE_DWP)" "YES"
+   $(DWP) -o $(DYLIB_DWP_FILE) $(DYLIB_DWOS)
+endif
 endi

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Jon Roelofs via lldb-commits 


@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_NO_EXPORTED_SYMBOLS

jroelofs wrote:

The double negative is confusing. Maybe invert it, and make the name 
`LLVM_ENABLE_EXPORTED_SYMBOLS`?

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Dave Lee via lldb-commits 


@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_NO_EXPORTED_SYMBOLS

kastiglione wrote:

should the name reflect the opposite of the default? if so maybe 
`LLVM_DISABLE_EXPORTED_SYMBOLS`?

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Dave Lee via lldb-commits 

kastiglione wrote:

Thanks for adding this! I've been using `CMAKE_EXE_LINKER_FLAGS` to 
`-Wl,-no_exported_symbols`, and have had no problems.

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Cyndy Ishida via lldb-commits 

https://github.com/cyndyishida updated 
https://github.com/llvm/llvm-project/pull/87684

>From 3ac6872328334384fa20998541fac841add767d9 Mon Sep 17 00:00:00 2001
From: Cyndy Ishida 
Date: Thu, 4 Apr 2024 12:08:28 -0700
Subject: [PATCH 1/2] [cmake] Build executables with -no_exported_symbols when
 building Apple toolchain

Building the Apple way turns off plugin support, meaning we don't need to be 
exporting unloadable symbols from all executables.
While deadstripping effects aren't expected to change, enabling this across all 
tools prevents the creation of export tries. This saves us ~3.5 MB's in just 
the universal build of `clang`.
---
 clang/cmake/caches/Apple-stage2.cmake   |  1 +
 lldb/cmake/caches/Apple-lldb-base.cmake |  1 +
 llvm/CMakeLists.txt |  3 +++
 llvm/cmake/modules/AddLLVM.cmake| 30 ++---
 llvm/docs/CMake.rst |  4 
 5 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/clang/cmake/caches/Apple-stage2.cmake 
b/clang/cmake/caches/Apple-stage2.cmake
index 72cdedd611bc96..faf61fd1fe9ecb 100644
--- a/clang/cmake/caches/Apple-stage2.cmake
+++ b/clang/cmake/caches/Apple-stage2.cmake
@@ -15,6 +15,7 @@ set(LLVM_ENABLE_ZLIB ON CACHE BOOL "")
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
 set(LLVM_EXTERNALIZE_DEBUGINFO ON CACHE BOOL "")
+set(LLVM_ENABLE_NO_EXPORTED_SYMBOLS ON CACHE BOOL "")
 set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
 set(CLANG_SPAWN_CC1 ON CACHE BOOL "")
 set(BUG_REPORT_URL "http://developer.apple.com/bugreporter/"; CACHE STRING "")
diff --git a/lldb/cmake/caches/Apple-lldb-base.cmake 
b/lldb/cmake/caches/Apple-lldb-base.cmake
index 4d4f02bfae95bd..021538896b2346 100644
--- a/lldb/cmake/caches/Apple-lldb-base.cmake
+++ b/lldb/cmake/caches/Apple-lldb-base.cmake
@@ -3,6 +3,7 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS ON CACHE BOOL "")
 
 set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(LLVM_ENABLE_NO_EXPORTED_SYMBOLS ON CACHE BOOL "")
 
 set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
 set(LIBCXX_ENABLE_STATIC OFF CACHE BOOL "")
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 6f5647d70d8bc1..7e393acacb80d8 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_NO_EXPORTED_SYMBOLS
+  "When building executables, disable any symbol exports (Darwin Only)" OFF)
+
 set(LLVM_CODESIGNING_IDENTITY "" CACHE STRING
   "Sign executables and dylibs with the given identity or skip if empty 
(Darwin Only)")
 
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 745935f1405170..141a97c852e24f 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -258,15 +258,24 @@ if (NOT DEFINED LLVM_LINKER_DETECTED AND NOT WIN32)
 endif()
   endif()
 
-  # Apple's linker complains about duplicate libraries, which CMake likes to do
-  # to support ELF platforms. To silence that warning, we can use
-  # -no_warn_duplicate_libraries, but only in versions of the linker that
-  # support that flag.
-  if(NOT LLVM_USE_LINKER AND ${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+  if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 include(CheckLinkerFlag)
-check_linker_flag(C "-Wl,-no_warn_duplicate_libraries" 
LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES)
-  else()
-set(LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES OFF CACHE INTERNAL "")
+# Linkers that support Darwin allow a setting to internalize all symbol 
exports, 
+# aiding in reducing binary size and often is applicable for executables.
+check_linker_flag(C "-Wl,-no_exported_symbols" 
LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS)
+
+if (NOT LLVM_USE_LINKER) 
+  # Apple's linker complains about duplicate libraries, which CMake likes 
to do
+  # to support ELF platforms. To silence that warning, we can use
+  # -no_warn_duplicate_libraries, but only in versions of the linker that
+  # support that flag.
+  check_linker_flag(C "-Wl,-no_warn_duplicate_libraries" 
LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES)
+else()
+  set(LLVM_LINKER_SUPPORTS_NO_WARN_DUPLICATE_LIBRARIES OFF CACHE INTERNAL 
"")
+endif()
+  
+  else() 
+set(LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS OFF CACHE INTERNAL "")
   endif()
 endif()
 
@@ -1029,6 +1038,11 @@ macro(add_llvm_executable name)
 add_llvm_symbol_exports( ${name} ${LLVM_EXPORTED_SYMBOL_FILE} )
   endif(LLVM_EXPORTED_SYMBOL_FILE)
 
+  if (LLVM_ENABLE_NO_EXPORTED_SYMBOLS AND 
LLVM_LINKER_SUPPORTS_NO_EXPORTED_SYMBOLS)
+set_property(TARGET ${name} APPEND_STRING PROPERTY
+  LINK_FLAGS " -Wl,-no_exported_symbols")
+  endif()
+
   if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB)
 set(USE_SHARED USE_SHARED)
   end

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Cyndy Ishida via lldb-commits 


@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_NO_EXPORTED_SYMBOLS

cyndyishida wrote:

No preference here. Whatever seems easier for others to understand.

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread via lldb-commits 

jimingham wrote:

lldb can and does present its external symbol interface through LLDB.framework. 
 I can't see any reason why it would also need to export symbols from the 
executable.  The lldb driver is just a fairly small convenience wrapper around 
the API's from LLDB.framework, its job is to handle command-line arguments and 
start up a debugger from LLDB.framework.

Jim 

> On Apr 4, 2024, at 2:01 PM, Dave Lee ***@***.***> wrote:
> 
> 
> Thanks for adding this! I've been using CMAKE_EXE_LINKER_FLAGS to 
> -Wl,-no_exported_symbols, and have had no problems.
> 
> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because you are on a team that was mentioned.
> 



https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Jon Roelofs via lldb-commits 


@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_NO_EXPORTED_SYMBOLS

jroelofs wrote:

Flags like this shouldn't have `DISABLE` in their name either. It's 
ambiguous/confusing whether `LLVM_DISABLE_THING=On` means THING=ON or THING=Off.

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][ClangUserExpression][NFCI] Pass the most specific ExecutionContextScope possible into ClangExpressionParser (PR #87657)

2024-04-04 Thread Greg Clayton via lldb-commits 


@@ -553,9 +553,9 @@ bool ClangUserExpression::PrepareForParsing(
 }
 
 bool ClangUserExpression::TryParse(
-DiagnosticManager &diagnostic_manager, ExecutionContextScope *exe_scope,
-ExecutionContext &exe_ctx, lldb_private::ExecutionPolicy execution_policy,
-bool keep_result_in_memory, bool generate_debug_info) {
+DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+lldb_private::ExecutionPolicy execution_policy, bool keep_result_in_memory,
+bool generate_debug_info) {

clayborg wrote:

Jim made comments below. `ExecutionContextScope *` is just as good as an 
ExecutionContext, but it is actually more specific as a `Target`, `Process`, 
`Thread` and `StackFrame` are all `ExecutionContextScope *` objects. They just 
know how to re-create any items needed. So a `StackFrame` can use 
`ExecutionContextScope` APIs (which is virtually overrides) to get the thread, 
process and target. The `Thread` can get the process and target. The `Process` 
can get the target. So it is no better or worse, it just specifies one thing 
would be the same as having an `ExecutionContext` filled out as much as needed. 
We do need to be careful to pass in the right thing as Jim said, where if we 
only want the `Target` to be used, we shouldn't pass in a `StackFrame` as the 
`ExecutionContextScope *`, but this will be exactly the same as passing a 
`ExecutionContext` that has the stack frame filled out when we only want to 
consult the target. So no real difference, and using `ExecutionContextScope *` 
is better since we can control what we pass as the root of where to start 
looking whereas if we pass in a fully filled out ExecutionContext, we will 
always consult the deepest item (Stack, then frame, then thread, then 
process/target) if it is filled out. 

So if the ExecutionContext is always filled out correctly to the right depth, 
then calling the `exe_ctx.GetBestExecutionContextScope()` suggestion will work. 
If not, then we need to pass in the right level, or modify the ExecutionContext 
and remove items from it.

https://github.com/llvm/llvm-project/pull/87657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Debuginfod Testing & fixes: 3rd times the charm? (PR #87676)

2024-04-04 Thread Shubham Sandeep Rastogi via lldb-commits 

rastogishubham wrote:

Hi @kevinfrei it seems like this patch broke greendragon

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/812/

https://github.com/llvm/llvm-project/pull/87676
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Debuginfod Testing & fixes: 3rd times the charm? (PR #87676)

2024-04-04 Thread via lldb-commits 

gulfemsavrun wrote:

It also broke our toolchain builders:
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8751560900790274577/overview

https://github.com/llvm/llvm-project/pull/87676
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Dan Liew via lldb-commits 

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Dan Liew via lldb-commits 


@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_EXPORTED_SYMBOLS

delcypher wrote:

We may want to rename this. The name hints that this option applies to 
everything but actually it only applies to executables and not libraries. E.g. 
`LLVM_ENABLE_TOOLS_WITH_EXPORTED_SYMBOLS`

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Dan Liew via lldb-commits 


@@ -654,6 +654,11 @@ enabled sub-projects. Nearly all of these variable names 
begin with
   Generate dSYM files and strip executables and libraries (Darwin Only).
   Defaults to OFF.
 
+**LLVM_ENABLE_EXPORTED_SYMBOLS**:BOOL
+  When building executables, preserve symbol exports. Defaults to ON. 
+  You can use this option to disable exported symbols on all executable build

delcypher wrote:

```suggestion
  Setting this option to ``OFF`` removes exported symbols from all executables.
```

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Dan Liew via lldb-commits 

https://github.com/delcypher requested changes to this pull request.

Generally looks good. I have some minor nits. I'd like to see 
`LLVM_ENABLE_EXPORTED_SYMBOLS` renamed to avoid ambiguity.

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Dan Liew via lldb-commits 


@@ -654,6 +654,11 @@ enabled sub-projects. Nearly all of these variable names 
begin with
   Generate dSYM files and strip executables and libraries (Darwin Only).
   Defaults to OFF.
 
+**LLVM_ENABLE_EXPORTED_SYMBOLS**:BOOL
+  When building executables, preserve symbol exports. Defaults to ON. 
+  You can use this option to disable exported symbols on all executable build

delcypher wrote:

Nit: Documentation tends not to use "You" as the subject.

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)

2024-04-04 Thread Jonas Devlieghere via lldb-commits 

JDevlieghere wrote:

Alright, so this happens when `listener` in `TypeCategoryMap` is the 
`TypeCategory`. Please update the commit message and add a comment before the 
call to `Changed` to say why we're doing it outside the scope of the lock. With 
those changes this should be good to go. 

https://github.com/llvm/llvm-project/pull/87540
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Cyndy Ishida via lldb-commits 


@@ -654,6 +654,11 @@ enabled sub-projects. Nearly all of these variable names 
begin with
   Generate dSYM files and strip executables and libraries (Darwin Only).
   Defaults to OFF.
 
+**LLVM_ENABLE_EXPORTED_SYMBOLS**:BOOL
+  When building executables, preserve symbol exports. Defaults to ON. 
+  You can use this option to disable exported symbols on all executable build

cyndyishida wrote:

That's what I initially thought as well but a lot of the LLVM_INCLUDE* 
variables on the lines below follow a "You can use this option..." And they 
also have similar behavior which defaults to ON. 

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Jonas Devlieghere via lldb-commits 


@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_EXPORTED_SYMBOLS

JDevlieghere wrote:

"tools" has a pretty specific meaning for LLVM so I think that will actually 
cause more confusion (i.e. I would expect that option to apply to 
`llvm-dwarfdump`, but not to `clang`). I'm personally fine with the current 
concise name. 

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [cmake] Build executables with -no_exported_symbols when building Apple toolchain (PR #87684)

2024-04-04 Thread Cyndy Ishida via lldb-commits 


@@ -673,6 +673,9 @@ option(LLVM_USE_OPROFILE
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
+option(LLVM_ENABLE_EXPORTED_SYMBOLS

cyndyishida wrote:

Would `LLVM_ENABLE_EXECUTABLES_WITH_EXPORTED_SYMBOLS` be too long?

https://github.com/llvm/llvm-project/pull/87684
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)

2024-04-04 Thread Vincent Belliard via lldb-commits 

https://github.com/v-bulle edited 
https://github.com/llvm/llvm-project/pull/87540
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)

2024-04-04 Thread Vincent Belliard via lldb-commits 

https://github.com/v-bulle updated 
https://github.com/llvm/llvm-project/pull/87540

>From 5234f6873d894dd80b2c1fef40fd18e4b722a2c9 Mon Sep 17 00:00:00 2001
From: Vincent Belliard 
Date: Wed, 3 Apr 2024 11:31:06 -0700
Subject: [PATCH 1/2] [lldb] fix dead lock in TypeCategoryMap.cpp

FormatManager::GetCategoryForLanguage and
FormatManager::GetCategory(can_create = true) can be called concurently
and they both take the TypeCategory::m_map_mutex and the
FormatManager::m_language_categories_mutex but in reverse order.

Without the patch, we can have a dead lock.
---
 .../source/DataFormatters/TypeCategoryMap.cpp | 28 +++
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp 
b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index fd76bab95826af..0d1f55fff473d1 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -25,19 +25,23 @@ TypeCategoryMap::TypeCategoryMap(IFormatChangeListener *lst)
 }
 
 void TypeCategoryMap::Add(KeyType name, const TypeCategoryImplSP &entry) {
-  std::lock_guard guard(m_map_mutex);
-  m_map[name] = entry;
+  {
+std::lock_guard guard(m_map_mutex);
+m_map[name] = entry;
+  }
   if (listener)
 listener->Changed();
 }
 
 bool TypeCategoryMap::Delete(KeyType name) {
-  std::lock_guard guard(m_map_mutex);
-  MapIterator iter = m_map.find(name);
-  if (iter == m_map.end())
-return false;
-  m_map.erase(name);
-  Disable(name);
+  {
+std::lock_guard guard(m_map_mutex);
+MapIterator iter = m_map.find(name);
+if (iter == m_map.end())
+  return false;
+m_map.erase(name);
+Disable(name);
+  }
   if (listener)
 listener->Changed();
   return true;
@@ -123,9 +127,11 @@ void TypeCategoryMap::DisableAllCategories() {
 }
 
 void TypeCategoryMap::Clear() {
-  std::lock_guard guard(m_map_mutex);
-  m_map.clear();
-  m_active_categories.clear();
+  {
+std::lock_guard guard(m_map_mutex);
+m_map.clear();
+m_active_categories.clear();
+  }
   if (listener)
 listener->Changed();
 }

>From 7305de06ab0425631fdf554cd1ad0498ac9fc7ed Mon Sep 17 00:00:00 2001
From: Vincent Belliard 
Date: Thu, 4 Apr 2024 15:20:59 -0700
Subject: [PATCH 2/2] Adds comments

---
 lldb/source/DataFormatters/TypeCategoryMap.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp 
b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index 0d1f55fff473d1..ecbdcde98d07d3 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -29,6 +29,7 @@ void TypeCategoryMap::Add(KeyType name, const 
TypeCategoryImplSP &entry) {
 std::lock_guard guard(m_map_mutex);
 m_map[name] = entry;
   }
+  // The lock is now released for the eventual call to Changed.
   if (listener)
 listener->Changed();
 }
@@ -42,6 +43,7 @@ bool TypeCategoryMap::Delete(KeyType name) {
 m_map.erase(name);
 Disable(name);
   }
+  // The lock is now released for the eventual call to Changed.
   if (listener)
 listener->Changed();
   return true;
@@ -132,6 +134,7 @@ void TypeCategoryMap::Clear() {
 m_map.clear();
 m_active_categories.clear();
   }
+  // The lock is now released for the eventual call to Changed.
   if (listener)
 listener->Changed();
 }

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ca55ee8 - Revert "Debuginfod Testing & fixes: 3rd times the charm? (#87676)"

2024-04-04 Thread Shubham Rastogi via lldb-commits 

Author: Shubham Rastogi
Date: 2024-04-04T15:37:07-07:00
New Revision: ca55ee88263e5b190965c3f14fd3b2647efab26a

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

LOG: Revert "Debuginfod Testing & fixes: 3rd times the charm? (#87676)"

This reverts commit d6713ad80d6907210c629f22babaf12177fa329c.

This changed was reverted because of greendragon failures such
as

Unresolved Tests (2):
  lldb-api :: debuginfod/Normal/TestDebuginfod.py
  lldb-api :: debuginfod/SplitDWARF/TestDebuginfodDWP.py

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/make/Makefile.rules
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolLocator/CMakeLists.txt
lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp

Removed: 
lldb/test/API/debuginfod/Normal/Makefile
lldb/test/API/debuginfod/Normal/TestDebuginfod.py
lldb/test/API/debuginfod/Normal/main.c
lldb/test/API/debuginfod/SplitDWARF/Makefile
lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
lldb/test/API/debuginfod/SplitDWARF/main.c



diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index ee8793fa1b3029..bfd249ccd43f2e 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -51,7 +51,7 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
 #
 # GNUWin32 uname gives "windows32" or "server version windows32" while
 # some versions of MSYS uname return "MSYS_NT*", but most environments
-# standardize on "Windows_NT", so we'll make it consistent here.
+# standardize on "Windows_NT", so we'll make it consistent here. 
 # When running tests from Visual Studio, the environment variable isn't
 # inherited all the way down to the process spawned for make.
 #--
@@ -210,12 +210,6 @@ else
ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
DSYM = $(EXE).debug
endif
-
-   ifeq "$(MAKE_DWP)" "YES"
-   MAKE_DWO := YES
-   DWP_NAME = $(EXE).dwp
-   DYLIB_DWP_NAME = $(DYLIB_NAME).dwp
-   endif
 endif
 
 LIMIT_DEBUG_INFO_FLAGS =
@@ -363,7 +357,6 @@ ifneq "$(OS)" "Darwin"
 
OBJCOPY ?= $(call replace_cc_with,objcopy)
ARCHIVER ?= $(call replace_cc_with,ar)
-   DWP ?= $(call replace_cc_with,dwp)
override AR = $(ARCHIVER)
 endif
 
@@ -534,10 +527,6 @@ ifneq "$(CXX)" ""
endif
 endif
 
-ifeq "$(GEN_GNU_BUILD_ID)" "YES"
-   LDFLAGS += -Wl,--build-id
-endif
-
 #--
 # DYLIB_ONLY variable can be used to skip the building of a.out.
 # See the sections below regarding dSYM file as well as the building of
@@ -576,17 +565,10 @@ else
 endif
 else
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
-ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
-   cp "$(EXE)" "$(EXE).unstripped"
-endif
$(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)"
$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)"
 endif
-ifeq "$(MAKE_DWP)" "YES"
-   $(DWP) -o "$(DWP_NAME)" $(DWOS)
 endif
-endif
-
 
 #--
 # Make the dylib
@@ -628,15 +610,9 @@ endif
 else
$(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)"
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
-   ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
-   cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).unstripped"
-   endif
$(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" 
"$(DYLIB_FILENAME).debug"
$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" 
"$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)"
 endif
-ifeq "$(MAKE_DWP)" "YES"
-   $(DWP) -o $(DYLIB_DWP_FILE) $(DYLIB_DWOS)
-endif
 endif
 
 #--

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index dafdf241f9db0c..49f13d2c89e380 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4378,38 +4378,26 @@ const std::shared_ptr 
&SymbolFileDWARF::GetDwpSymbolFile() {
 FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
 ModuleSpec module_spec;
 module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
-FileSpec dwp_filespec;
 for (const auto &symfile : symfiles.files()) {
   module_spec.GetSymbolFileSpec() =
   FileSpec(symfile.GetPath() + ".dwp", symfile.GetPathStyle());
   LLDB_LOG(log, "Searching for DWP using: \"{0}\"",
module

[Lldb-commits] [lldb] Debuginfod Testing & fixes: 3rd times the charm? (PR #87676)

2024-04-04 Thread Shubham Sandeep Rastogi via lldb-commits 

rastogishubham wrote:

To make sure green dragon stays green, I reverted this change with 
ca55ee88263e5b190965c3f14fd3b2647efab26a

https://github.com/llvm/llvm-project/pull/87676
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][ClangUserExpression][NFCI] Pass the most specific ExecutionContextScope possible into ClangExpressionParser (PR #87657)

2024-04-04 Thread Michael Buch via lldb-commits 


@@ -553,9 +553,9 @@ bool ClangUserExpression::PrepareForParsing(
 }
 
 bool ClangUserExpression::TryParse(
-DiagnosticManager &diagnostic_manager, ExecutionContextScope *exe_scope,
-ExecutionContext &exe_ctx, lldb_private::ExecutionPolicy execution_policy,
-bool keep_result_in_memory, bool generate_debug_info) {
+DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+lldb_private::ExecutionPolicy execution_policy, bool keep_result_in_memory,
+bool generate_debug_info) {

Michael137 wrote:

I removed this parameter because it seemed redundant as it was only used for 
the `ClangExpressionParser`, which wants access to all of 
Process/Target/StackFrame. So if we can get that from the ExecutionContext, 
there's no point in passing the `exe_scope` down the the parser. Maybe I'm 
misunderstanding but my impression was that this is a simplification and 
wouldn't cause any functional change. This is purely about making the 
`ClangExpressionParser` get the `StackFrame` calculation right, which it 
doesn't even currently use apart for logging

https://github.com/llvm/llvm-project/pull/87657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][ClangUserExpression][NFCI] Pass the most specific ExecutionContextScope possible into ClangExpressionParser (PR #87657)

2024-04-04 Thread Michael Buch via lldb-commits 

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/87657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][ClangUserExpression][NFCI] Pass the most specific ExecutionContextScope possible into ClangExpressionParser (PR #87657)

2024-04-04 Thread Michael Buch via lldb-commits 

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/87657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][ClangUserExpression][NFCI] Pass the most specific ExecutionContextScope possible into ClangExpressionParser (PR #87657)

2024-04-04 Thread via lldb-commits 


@@ -553,9 +553,9 @@ bool ClangUserExpression::PrepareForParsing(
 }
 
 bool ClangUserExpression::TryParse(
-DiagnosticManager &diagnostic_manager, ExecutionContextScope *exe_scope,
-ExecutionContext &exe_ctx, lldb_private::ExecutionPolicy execution_policy,
-bool keep_result_in_memory, bool generate_debug_info) {
+DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+lldb_private::ExecutionPolicy execution_policy, bool keep_result_in_memory,
+bool generate_debug_info) {

jimingham wrote:

I was also mostly moved by the fact that there were two things specifying what 
should be one thing.  I was hoping that this was just redundant, not someone 
being overly clever, and Michael showed that it wasn't a used distinction, 
which is good.  As far as which type to use, that's really more convenience, 
right?  If the methods you want to call immediately from TryParse take 
ExecutionContextScopes, use that, if ExecutionContext's, use that.  I don't 
think it makes any other difference.

https://github.com/llvm/llvm-project/pull/87657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

2024-04-04 Thread Greg Clayton via lldb-commits 

https://github.com/clayborg created 
https://github.com/llvm/llvm-project/pull/87740

This patch adds support for the new foreign type unit support in .debug_names. 
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp 
file
- fix crashers that happen due to PeekDIEName() using wrong offsets

>From d69497fc66ce092fd75fcbe7c64460a49a6e2172 Mon Sep 17 00:00:00 2001
From: Greg Clayton 
Date: Sat, 30 Mar 2024 10:50:34 -0700
Subject: [PATCH] Add support for using foreign type units in .debug_names.

This patch adds support for the new foreign type unit support in .debug_names. 
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp 
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
---
 .../SymbolFile/DWARF/DWARFDebugInfo.cpp   | 16 +++-
 .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h |  2 +
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 68 +-
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  5 +-
 .../SymbolFile/DWARF/ManualDWARFIndex.cpp |  6 +-
 .../SymbolFile/DWARF/ManualDWARFIndex.h   |  7 +-
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  | 65 +++--
 .../SymbolFile/DWARF/SymbolFileDWARF.h|  9 ++
 .../DWARF/x86/dwp-foreign-type-units.cpp  | 91 +++
 .../DebugInfo/DWARF/DWARFAcceleratorTable.h   | 11 +++
 .../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 13 +++
 11 files changed, 260 insertions(+), 33 deletions(-)
 create mode 100644 
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 44febcfac3b000..0e111c8ec47f45 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -223,7 +223,17 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section 
section,
 }
 
 DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
-  return GetUnitContainingDIEOffset(die_ref.section(), die_ref.die_offset());
+  // Make sure we get the correct SymbolFileDWARF from the DIERef before
+  // asking for information from a debug info object. We might start with the
+  // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+  // might be pointing to a specific .dwo file or to the .dwp file. So this
+  // makes sure we get the right SymbolFileDWARF instance before finding the
+  // DWARFUnit that contains the offset. If we just use this object to do the
+  // search, we might be using the wrong .debug_info section from the wrong
+  // file with an offset meant for a different section.
+  SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+  return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+   die_ref.die_offset());
 }
 
 DWARFUnit *
@@ -236,6 +246,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section 
section,
   return result;
 }
 
+const std::shared_ptr DWARFDebugInfo::GetDwpSymbolFile() {
+  return m_dwarf.GetDwpSymbolFile();
+}
+
 DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
   auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index c1f0cb0203fb76..b7f99ce6282b1f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -58,6 +58,8 @@ class DWARFDebugInfo {
 
   const DWARFDebugAranges &GetCompileUnitAranges();
 
+  const std::shared_ptr GetDwpSymbolFile();
+
 protected:
   typedef std::vector UnitColl;
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 4da0d56fdcacb4..ba35b3b872e6c6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module &module, 
DWARFDataExtractor debug_names,
   module, std::move(index_up), debug_names, debug_str, dwarf));
 }
 
+
+llvm::DenseSet
+DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames &debug_names) {
+  llvm::DenseSet result;
+  for (const DebugNames::NameIndex &ni : debug_names) {
+const uint32_t num_tus = ni.getForeignTUCount();
+for (uint32_t tu = 0; tu < num_tus; ++tu)
+  result.insert(ni.getForeignTUSignature(tu));
+  }
+  return result;
+}
+
 llvm::DenseSet
 DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   llvm::DenseSet result;
@@ -48,6 +60,15 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames 
&d

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

2024-04-04 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)


Changes

This patch adds support for the new foreign type unit support in .debug_names. 
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp 
file
- fix crashers that happen due to PeekDIEName() using wrong offsets

---

Patch is 22.47 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/87740.diff


11 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp (+15-1) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h (+2) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
(+67-1) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h (+4-1) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp (+4-2) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h (+5-2) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+39-26) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (+9) 
- (added) lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp (+91) 
- (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h (+11) 
- (modified) llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp (+13) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 44febcfac3b000..0e111c8ec47f45 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -223,7 +223,17 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section 
section,
 }
 
 DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
-  return GetUnitContainingDIEOffset(die_ref.section(), die_ref.die_offset());
+  // Make sure we get the correct SymbolFileDWARF from the DIERef before
+  // asking for information from a debug info object. We might start with the
+  // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+  // might be pointing to a specific .dwo file or to the .dwp file. So this
+  // makes sure we get the right SymbolFileDWARF instance before finding the
+  // DWARFUnit that contains the offset. If we just use this object to do the
+  // search, we might be using the wrong .debug_info section from the wrong
+  // file with an offset meant for a different section.
+  SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+  return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+   die_ref.die_offset());
 }
 
 DWARFUnit *
@@ -236,6 +246,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section 
section,
   return result;
 }
 
+const std::shared_ptr DWARFDebugInfo::GetDwpSymbolFile() {
+  return m_dwarf.GetDwpSymbolFile();
+}
+
 DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
   auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index c1f0cb0203fb76..b7f99ce6282b1f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -58,6 +58,8 @@ class DWARFDebugInfo {
 
   const DWARFDebugAranges &GetCompileUnitAranges();
 
+  const std::shared_ptr GetDwpSymbolFile();
+
 protected:
   typedef std::vector UnitColl;
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 4da0d56fdcacb4..ba35b3b872e6c6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module &module, 
DWARFDataExtractor debug_names,
   module, std::move(index_up), debug_names, debug_str, dwarf));
 }
 
+
+llvm::DenseSet
+DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames &debug_names) {
+  llvm::DenseSet result;
+  for (const DebugNames::NameIndex &ni : debug_names) {
+const uint32_t num_tus = ni.getForeignTUCount();
+for (uint32_t tu = 0; tu < num_tus; ++tu)
+  result.insert(ni.getForeignTUSignature(tu));
+  }
+  return result;
+}
+
 llvm::DenseSet
 DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   llvm::DenseSet result;
@@ -48,6 +60,15 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames 
&debug_names) {
   return result;
 }
 
+DWARFTypeUnit *
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const 
{
+  std::optional type_sig = entry.getForeignTUTypeSignature();
+  if (type_sig)
+if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())
+  return

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

2024-04-04 Thread via lldb-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 6aa53888a8e8a6e3f0bd279539703f4d4701b4e7 
d69497fc66ce092fd75fcbe7c64460a49a6e2172 -- 
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h 
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h 
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h 
llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h 
llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index ba35b3b872..7200421b6f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,7 +34,6 @@ DebugNamesDWARFIndex::Create(Module &module, 
DWARFDataExtractor debug_names,
   module, std::move(index_up), debug_names, debug_str, dwarf));
 }
 
-
 llvm::DenseSet
 DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames &debug_names) {
   llvm::DenseSet result;
@@ -301,7 +300,6 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
 if (!isType(entry.tag()))
   continue;
 
-
 DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
 if (foreign_tu) {
   // If this entry represents a foreign type unit, we need to verify that
@@ -325,8 +323,8 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
   // unit came from a specific .dwo file.
   if (name_index->getCUCount() == 1) {
 dw_offset_t cu_offset = name_index->getCUOffset(0);
-DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::DebugInfo,
- cu_offset);
+DWARFUnit *cu =
+m_debug_info.GetUnitAtOffset(DIERef::DebugInfo, cu_offset);
 if (cu) {
   DWARFBaseDIE cu_die = cu->GetUnitDIEOnly();
   DWARFBaseDIE tu_die = foreign_tu->GetUnitDIEOnly();
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 6b48ce4eea..c316884180 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -99,7 +99,8 @@ private:
   llvm::StringRef name);
 
   static llvm::DenseSet GetUnits(const DebugNames &debug_names);
-  static llvm::DenseSet GetTypeUnitSigs(const DebugNames 
&debug_names);
+  static llvm::DenseSet
+  GetTypeUnitSigs(const DebugNames &debug_names);
 };
 
 } // namespace dwarf
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 103e157d3c..e1a7cfc794 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -60,7 +60,8 @@ void ManualDWARFIndex::Index() {
   }
   if (dwp_info && dwp_info->ContainsTypeUnits()) {
 for (size_t U = 0; U < dwp_info->GetNumUnits(); ++U) {
-  if (auto *tu = 
llvm::dyn_cast(dwp_info->GetUnitAtIndex(U))) {
+  if (auto *tu =
+  llvm::dyn_cast(dwp_info->GetUnitAtIndex(U))) {
 if (m_type_sigs_to_avoid.count(tu->GetTypeHash()) == 0)
   units_to_index.push_back(tu);
   }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index b828b56cc3..355fcfb2b6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1751,7 +1751,7 @@ SymbolFileDWARF 
*SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
   if (file_index) {
 SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile();
 if (debug_map) {
-// We have a SymbolFileDWARFDebugMap, so let it find the right file
+  // We have a SymbolFileDWARFDebugMap, so let it find the right file
   return debug_map->GetSymbolFileByOSOIndex(*file_index);
 } else {
   // Handle the .dwp file case correctly
@@ -1759,8 +1759,9 @@ SymbolFileDWARF 
*SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
 return GetDwpSymbolFile().get(); // DWP case
 
   // Handle the .dwo file case correctly
-  return DebugInfo().GetUnitAtIndex(*die_ref.file_index())
-->GetDwoSymbolFile(); // DWO case
+  return DebugInfo()
+  .