[Lldb-commits] [lldb] 7c178fd - [lldb] Correct byte order check for 128 bit integer registers

2024-04-03 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2024-04-03T15:05:13Z
New Revision: 7c178fdf0094afbf4757d71b792bc159ddcac72f

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

LOG: [lldb] Correct byte order check for 128 bit integer registers

Size was clearly not correct here. This call has been here since
the initial reformat of all of lldb so it has likely always been
incorrect.

(although registers don't typically have an endian, they are
just values, in the remote protocol register data is in target
endian)

This might have been a problem for Neon registers on big endian
AArch64, but only if the debug server describes them as integers.

lldb-server does not, they've always been vectors which doesn't
take this code path.

Not adding a test because the way I've mocked up a big endian
target in the past is using s390x as the architecture. This
apparently has some form of vector extension that may be 128 bit
but lldb doesn't support it.

Added: 


Modified: 
lldb/source/Utility/RegisterValue.cpp

Removed: 




diff  --git a/lldb/source/Utility/RegisterValue.cpp 
b/lldb/source/Utility/RegisterValue.cpp
index fa92ba8a8f9236..cbf840258302d2 100644
--- a/lldb/source/Utility/RegisterValue.cpp
+++ b/lldb/source/Utility/RegisterValue.cpp
@@ -199,7 +199,7 @@ Status RegisterValue::SetValueFromData(const RegisterInfo 
®_info,
 else if (reg_info.byte_size <= 16) {
   uint64_t data1 = src.GetU64(&src_offset);
   uint64_t data2 = src.GetU64(&src_offset);
-  if (src.GetByteSize() == eByteOrderBig) {
+  if (src.GetByteOrder() == eByteOrderBig) {
 int128.x[0] = data1;
 int128.x[1] = data2;
   } else {



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


[Lldb-commits] [lldb] a8425d2 - DebugInfoD issues, take 2 (#86812)

2024-04-03 Thread via lldb-commits

Author: Kevin Frei
Date: 2024-04-03T12:15:41-07:00
New Revision: a8425d2fa2e0d29b83d16eac008441ecb9516320

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

LOG: DebugInfoD issues, take 2 (#86812)

The previous diff (and it's subsequent fix) were reverted as the tests
didn't work properly on the AArch64 & ARM LLDB buildbots. I made a
couple more minor changes to tests (from @clayborg's feedback) and
disabled them for non Linux-x86(_64) builds, as I don't have the ability
do anything about an ARM64 Linux failure. If I had to guess, I'd say the
toolchain on the buildbots isn't respecting the `-Wl,--build-id` flag.
Maybe, one day, when I have a Linux AArch64 system I'll dig in to it.

>From the reverted PR:

I've migrated the tests in my
https://github.com/llvm/llvm-project/pull/79181 from shell to API (at
@JDevlieghere's suggestion) and addressed a couple issues that were
exposed during testing.

The tests first test the "normal" situation (no DebugInfoD involvement,
just normal debug files sitting around), then the "no debug info"
situation (to make sure the test is seeing failure properly), then it
tests to validate that when DebugInfoD returns the symbols, things work
properly. This is duplicated for DWP/split-dwarf scenarios.

-

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
 endif
 
 #--

diff  --git a/lldb/source/Plugin

[Lldb-commits] [lldb] 20433e9 - Revert "DebugInfoD issues, take 2" (#87583)

2024-04-03 Thread via lldb-commits

Author: Chelsea Cassanova
Date: 2024-04-03T16:34:03-07:00
New Revision: 20433e9b2483d64843310e97062541dd73f54436

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

LOG: Revert "DebugInfoD issues, take 2" (#87583)

Reverts llvm/llvm-project#86812. 

This commit caused a regression on the x86_64 MacOS buildbot:
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/784/

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_spec.GetSymbolFileSpec());
-  dwp_filespec =
+  FileSpec dwp_filespec =
   PluginManager::Locate

[Lldb-commits] [lldb] 622851a - [lldb] Set static Module's load addresses via ObjectFile (#87439)

2024-04-03 Thread via lldb-commits

Author: Jason Molenda
Date: 2024-04-03T16:40:34-07:00
New Revision: 622851a9059694487811a7f6078312fc2cce5486

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

LOG: [lldb] Set static Module's load addresses via ObjectFile (#87439)

This is a followup to
https://github.com/llvm/llvm-project/pull/86359
"[lldb] [ObjectFileMachO] LLVM_COV is not mapped into firmware memory
(#86359)"

where I treat LLVM_COV segments in a Mach-O binary as non-loadable.
There is another codepath in
`DynamicLoaderStatic::LoadAllImagesAtFileAddresses` which is called to
set the load addresses for a Module to the file addresses. It has no
logic to detect a segment that is not loaded in virtual memory
(ObjectFileMachO::SectionIsLoadable), so it would set the load address
for this LLVM_COV segment to the file address and shadow actual code,
breaking lldb behavior.

This method currently sets the load address for any section that doesn't
have a load address set already. This presumes that a Module was added
to the Target, some mechanism set the correct load address for SOME
segments, and then this method is going to set the other segments to a
no-slide value, assuming they were forgotten.

ObjectFile base class doesn't, today, vend a SectionIsLoadable method,
but we do have ObjectFile::SetLoadAddress and at a higher level,
Module::SetLoadAddress, when we're setting the same slide to all
segments.

That's the behavior we want in this method. If any section has a load
address, we don't touch this Module. Otherwise we set all sections to
have a load address that is the same as the file address.

I also audited the other parts of lldb that are calling
SectionList::SectionLoadAddress and looked if they should be more
correctly using Module::SetLoadAddress for the entire binary. But in
most cases, we have the potential for different slides for different
sections so this section-by-section approach must be taken.

rdar://125800290

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp

Removed: 




diff  --git a/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp 
b/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
index a39aa2280ab86d..545998123dda1b 100644
--- a/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
+++ b/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
@@ -84,51 +84,43 @@ void DynamicLoaderStatic::LoadAllImagesAtFileAddresses() {
   // Disable JIT for static dynamic loader targets
   m_process->SetCanJIT(false);
 
+  Target &target = m_process->GetTarget();
   for (ModuleSP module_sp : module_list.Modules()) {
 if (module_sp) {
   bool changed = false;
+  bool no_load_addresses = true;
+  // If this module has a section with a load address set in
+  // the target, assume all necessary work is already done. There
+  // may be sections without a load address set intentionally
+  // and we don't want to mutate that.
+  // For a module with no load addresses set, set the load addresses
+  // to slide == 0, the same as the file addresses, in the target.
   ObjectFile *image_object_file = module_sp->GetObjectFile();
   if (image_object_file) {
 SectionList *section_list = image_object_file->GetSectionList();
 if (section_list) {
-  // All sections listed in the dyld image info structure will all
-  // either be fixed up already, or they will all be off by a single
-  // slide amount that is determined by finding the first segment that
-  // is at file offset zero which also has bytes (a file size that is
-  // greater than zero) in the object file.
-
-  // Determine the slide amount (if any)
   const size_t num_sections = section_list->GetSize();
-  size_t sect_idx = 0;
-  for (sect_idx = 0; sect_idx < num_sections; ++sect_idx) {
-// Iterate through the object file sections to find the first
-// section that starts of file offset zero and that has bytes in
-// the file...
+  for (size_t sect_idx = 0; sect_idx < num_sections; ++sect_idx) {
 SectionSP section_sp(section_list->GetSectionAtIndex(sect_idx));
 if (section_sp) {
-  // If this section already has a load address set in the target,
-  // don't re-set it to the file address.  Something may have
-  // set it to a more correct value already.
-  if (m_process->GetTarget()
-  .GetSectionLoadList()
-  .GetSectionLoadAddress(section_sp) !=
-  LLDB_INVALID_ADDRESS) {
-continue;
+  if (target.GetSectionLoadList().GetSectionLoadAddress(
+