[Lldb-commits] [lldb] 7c178fd - [lldb] Correct byte order check for 128 bit integer registers
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)
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)
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)
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( +