[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -108,6 +108,9 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { lldb_private::ConstString GetDIEClassTemplateParams( const lldb_private::plugin::dwarf::DWARFDIE ) override; labath wrote: delete `lldb_private::plugin::dwarf::` (and elsewhere in this file) https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1921,38 +1970,33 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext , GetClangASTImporter().SetRecordLayout(record_decl, layout); } } -} else if (clang_type_was_created) { - // Start the definition if the class is not objective C since the - // underlying decls respond to isCompleteDefinition(). Objective - // C decls don't respond to isCompleteDefinition() so we can't - // start the declaration definition right away. For C++ - // class/union/structs we want to start the definition in case the - // class is needed as the declaration context for a contained class - // or type without the need to complete that type.. - - if (attrs.class_language != eLanguageTypeObjC && - attrs.class_language != eLanguageTypeObjC_plus_plus) -TypeSystemClang::StartTagDeclarationDefinition(clang_type); - - // Leave this as a forward declaration until we need to know the - // details of the type. lldb_private::Type will automatically call - // the SymbolFile virtual function - // "SymbolFileDWARF::CompleteType(Type *)" When the definition - // needs to be defined. - assert(!dwarf->GetForwardDeclCompilerTypeToDIE().count( - ClangUtil::RemoveFastQualifiers(clang_type) - .GetOpaqueQualType()) && - "Type already in the forward declaration map!"); - // Can't assume m_ast.GetSymbolFile() is actually a - // SymbolFileDWARF, it can be a SymbolFileDWARFDebugMap for Apple - // binaries. - dwarf->GetForwardDeclCompilerTypeToDIE().try_emplace( - ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType(), - *die.GetDIERef()); - m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true); } +// Start the definition if the class is not objective C since the +// underlying decls respond to isCompleteDefinition(). Objective +// C decls don't respond to isCompleteDefinition() so we can't +// start the declaration definition right away. For C++ +// class/union/structs we want to start the definition in case the +// class is needed as the declaration context for a contained class +// or type without the need to complete that type.. + +if (attrs.class_language != eLanguageTypeObjC && +attrs.class_language != eLanguageTypeObjC_plus_plus) + TypeSystemClang::StartTagDeclarationDefinition(clang_type); labath wrote: Is it a problem that `clang_type` can already have its definition started (and completed) on line 1944 (if the DIE has no children)? Should this maybe be in some sort of an else branch? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE ) { return qualified_name; } +bool DWARFASTParserClang::FindDefinitionDIE(const DWARFDIE , +bool _forward_declaration) { labath wrote: I'm having trouble finding where is this by-ref argument actually used in the caller. Can we get rid of it (perhaps by moving the `IsForwardDeclaration` call to the caller)? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/labath commented: Modulo comments, this makes sense to me (as much as that can ever be said about this code), but it could definitely use a second (third?) pair of eyes. Michael, what do you make of this? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -249,11 +270,10 @@ static void ForcefullyCompleteType(CompilerType type) { /// This function serves a similar purpose as RequireCompleteType above, but it /// avoids completing the type if it is not immediately necessary. It only /// ensures we _can_ complete the type later. -static void PrepareContextToReceiveMembers(TypeSystemClang , - ClangASTImporter _importer, - clang::DeclContext *decl_ctx, - DWARFDIE die, - const char *type_name_cstr) { +void DWARFASTParserClang::PrepareContextToReceiveMembers( +TypeSystemClang , clang::DeclContext *decl_ctx, labath wrote: This arg is not necessary now that this is a member https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1632,6 +1669,96 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE ) { return qualified_name; } labath wrote: I am bothered by this name. I would expect that something called `FindDefinitionDIE` returns a DWARFDIE (or something along those lines). Instead it returns bool, and appears to do a lot more than simply finding a DIE (it constructs the full clang ast for it, iiuc). How about, mirroring the SymbolFileDWARF apis this wraps, we call this something like `FindDefinitionTypeForDIE` (and then actually have the function return the parsed Type object)? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed unresolved test lldb-api python_api/debugger/TestDebuggerAPI.py on x86_64 host (PR #90580)
labath wrote: FWIW, I agree with @bulbazord. If the user specifies an explicit platform in the CreateTarget function, that platform should really take precedence over anything else. (If it were up to me, I would not even attempt matching other platforms in this case (and just refuse to create the target if the provided platform does not work), but that might be going too far in terms of changing existing behavior). https://github.com/llvm/llvm-project/pull/90580 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix Scalar::GetData for non-multiple-of-8-bits values (PR #90846)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/90846 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f8fedfb - [lldb] Fix TestSharedLibStrippedSymbols for #90622
Author: Pavel Labath Date: 2024-05-03T06:30:49Z New Revision: f8fedfb6802173372ec923f99f31d4af810fbcb0 URL: https://github.com/llvm/llvm-project/commit/f8fedfb6802173372ec923f99f31d4af810fbcb0 DIFF: https://github.com/llvm/llvm-project/commit/f8fedfb6802173372ec923f99f31d4af810fbcb0.diff LOG: [lldb] Fix TestSharedLibStrippedSymbols for #90622 `ifeq` needs to be at the beginning of a line, otherwise it's interpreted as part of the recipe. Added: Modified: lldb/packages/Python/lldbsuite/test/make/Makefile.rules Removed: diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index 2cbc918ebbaeb1..b2f2516d8423f8 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -629,9 +629,9 @@ endif else $(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)" ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" - ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" +ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).unstripped" - endif +endif $(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).debug" $(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)" endif ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
@@ -0,0 +1,183 @@ +import os +import shutil +import tempfile + +import lldb +from lldbsuite.test.decorators import * +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.lldbtest import * + + +""" +Test support for the DebugInfoD network symbol acquisition protocol. +This one is for simple / no split-dwarf scenarios. + +For no-split-dwarf scenarios, there are 2 variations: +1 - A stripped binary with it's corresponding unstripped binary: +2 - A stripped binary with a corresponding --only-keep-debug symbols file +""" + + +class DebugInfodTests(TestBase): +# No need to try every flavor of debug inf. +NO_DEBUG_INFO_TESTCASE = True + +def setUp(self): +TestBase.setUp(self) +# Don't run these tests if we don't have Debuginfod support +if "Debuginfod" not in configuration.enabled_plugins: +self.skipTest("The Debuginfod SymbolLocator plugin is not enabled") + +def test_normal_no_symbols(self): +""" +Validate behavior with no symbols or symbol locator. +('baseline negative' behavior) +""" +test_root = self.config_test(["a.out"]) +self.try_breakpoint(False) + +def test_normal_default(self): +""" +Validate behavior with symbols, but no symbol locator. +('baseline positive' behavior) +""" +test_root = self.config_test(["a.out", "a.out.debug"]) +self.try_breakpoint(True) + +def test_debuginfod_symbols(self): +""" +Test behavior with the full binary available from Debuginfod as +'debuginfo' from the plug-in. +""" +test_root = self.config_test(["a.out"], "a.out.unstripped") +self.try_breakpoint(True) + +def test_debuginfod_executable(self): +""" +Test behavior with the full binary available from Debuginfod as +'executable' from the plug-in. +""" +test_root = self.config_test(["a.out"], None, "a.out.unstripped") +self.try_breakpoint(True) + +def test_debuginfod_okd_symbols(self): +""" +Test behavior with the 'only-keep-debug' symbols available from Debuginfod. +""" +test_root = self.config_test(["a.out"], "a.out.debug") +self.try_breakpoint(True) + +def try_breakpoint(self, should_have_loc): +""" +This function creates a target from self.aout, sets a function-name +breakpoint, and checks to see if we have a file/line location, +as a way to validate that the symbols have been loaded. +should_have_loc specifies if we're testing that symbols have or +haven't been loaded. +""" +target = self.dbg.CreateTarget(self.aout) +self.assertTrue(target and target.IsValid(), "Target is valid") + +bp = target.BreakpointCreateByName("func") +self.assertTrue(bp and bp.IsValid(), "Breakpoint is valid") +self.assertEqual(bp.GetNumLocations(), 1) + +loc = bp.GetLocationAtIndex(0) +self.assertTrue(loc and loc.IsValid(), "Location is valid") +addr = loc.GetAddress() +self.assertTrue(addr and addr.IsValid(), "Loc address is valid") +line_entry = addr.GetLineEntry() +self.assertEqual( +should_have_loc, +line_entry != None and line_entry.IsValid(), +"Loc line entry is valid", +) +if should_have_loc: +self.assertEqual(line_entry.GetLine(), 4) +self.assertEqual( +line_entry.GetFileSpec().GetFilename(), +self.main_source_file.GetFilename(), +) +self.dbg.DeleteTarget(target) +shutil.rmtree(self.tmp_dir) + +def config_test(self, local_files, debuginfo=None, executable=None): +""" +Set up a test with local_files[] copied to a different location +so that we control which files are, or are not, found in the file system. +Also, create a stand-alone file-system 'hosted' debuginfod server with the +provided debuginfo and executable files (if they exist) + +Make the filesystem look like: + +/tmp//test/[local_files] + +/tmp//cache (for lldb to use as a temp cache) + +/tmp//buildid//executable -> +/tmp//buildid//debuginfo -> +Returns the /tmp/ path +""" + +self.build() + +uuid = self.getUUID("a.out") +if not uuid: +self.fail("Could not get UUID for a.out") +return +self.main_source_file = lldb.SBFileSpec("main.c") +self.tmp_dir = tempfile.mkdtemp() +test_dir = os.path.join(self.tmp_dir, "test") +os.makedirs(test_dir) + +self.aout = "" +# Copy the files used by the test: +for f in local_files: +shutil.copy(self.getBuildArtifact(f), test_dir) +# The first item is the binary to be used for the test +if self.aout == "":
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
@@ -0,0 +1,183 @@ +import os +import shutil +import tempfile + +import lldb +from lldbsuite.test.decorators import * +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.lldbtest import * + + +""" +Test support for the DebugInfoD network symbol acquisition protocol. +This one is for simple / no split-dwarf scenarios. + +For no-split-dwarf scenarios, there are 2 variations: +1 - A stripped binary with it's corresponding unstripped binary: +2 - A stripped binary with a corresponding --only-keep-debug symbols file +""" + + +class DebugInfodTests(TestBase): +# No need to try every flavor of debug inf. +NO_DEBUG_INFO_TESTCASE = True + +def setUp(self): +TestBase.setUp(self) +# Don't run these tests if we don't have Debuginfod support +if "Debuginfod" not in configuration.enabled_plugins: +self.skipTest("The Debuginfod SymbolLocator plugin is not enabled") + +def test_normal_no_symbols(self): +""" +Validate behavior with no symbols or symbol locator. +('baseline negative' behavior) +""" +test_root = self.config_test(["a.out"]) +self.try_breakpoint(False) + +def test_normal_default(self): +""" +Validate behavior with symbols, but no symbol locator. +('baseline positive' behavior) +""" +test_root = self.config_test(["a.out", "a.out.debug"]) +self.try_breakpoint(True) + +def test_debuginfod_symbols(self): +""" +Test behavior with the full binary available from Debuginfod as +'debuginfo' from the plug-in. +""" +test_root = self.config_test(["a.out"], "a.out.unstripped") +self.try_breakpoint(True) + +def test_debuginfod_executable(self): +""" +Test behavior with the full binary available from Debuginfod as +'executable' from the plug-in. +""" +test_root = self.config_test(["a.out"], None, "a.out.unstripped") +self.try_breakpoint(True) + +def test_debuginfod_okd_symbols(self): +""" +Test behavior with the 'only-keep-debug' symbols available from Debuginfod. +""" +test_root = self.config_test(["a.out"], "a.out.debug") +self.try_breakpoint(True) + +def try_breakpoint(self, should_have_loc): +""" +This function creates a target from self.aout, sets a function-name +breakpoint, and checks to see if we have a file/line location, +as a way to validate that the symbols have been loaded. +should_have_loc specifies if we're testing that symbols have or +haven't been loaded. +""" +target = self.dbg.CreateTarget(self.aout) +self.assertTrue(target and target.IsValid(), "Target is valid") + +bp = target.BreakpointCreateByName("func") +self.assertTrue(bp and bp.IsValid(), "Breakpoint is valid") +self.assertEqual(bp.GetNumLocations(), 1) + +loc = bp.GetLocationAtIndex(0) +self.assertTrue(loc and loc.IsValid(), "Location is valid") +addr = loc.GetAddress() +self.assertTrue(addr and addr.IsValid(), "Loc address is valid") +line_entry = addr.GetLineEntry() +self.assertEqual( +should_have_loc, +line_entry != None and line_entry.IsValid(), +"Loc line entry is valid", +) +if should_have_loc: +self.assertEqual(line_entry.GetLine(), 4) +self.assertEqual( +line_entry.GetFileSpec().GetFilename(), +self.main_source_file.GetFilename(), +) +self.dbg.DeleteTarget(target) +shutil.rmtree(self.tmp_dir) + +def config_test(self, local_files, debuginfo=None, executable=None): +""" +Set up a test with local_files[] copied to a different location +so that we control which files are, or are not, found in the file system. +Also, create a stand-alone file-system 'hosted' debuginfod server with the +provided debuginfo and executable files (if they exist) + +Make the filesystem look like: + +/tmp//test/[local_files] + +/tmp//cache (for lldb to use as a temp cache) + +/tmp//buildid//executable -> +/tmp//buildid//debuginfo -> +Returns the /tmp/ path +""" + +self.build() + +uuid = self.getUUID("a.out") +if not uuid: +self.fail("Could not get UUID for a.out") +return +self.main_source_file = lldb.SBFileSpec("main.c") +self.tmp_dir = tempfile.mkdtemp() +test_dir = os.path.join(self.tmp_dir, "test") +os.makedirs(test_dir) + +self.aout = "" +# Copy the files used by the test: +for f in local_files: +shutil.copy(self.getBuildArtifact(f), test_dir) +# The first item is the binary to be used for the test +if self.aout == "":
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
@@ -44,6 +44,10 @@ lldb_build_intel_pt = '@LLDB_BUILD_INTEL_PT@' if lldb_build_intel_pt == '1': config.enabled_plugins.append('intel-pt') +llvm_enable_curl = '@LLVM_ENABLE_CURL@' labath wrote: Emulating the curses dependency approach would probably be more consistent (grep for `skipIfCursesSupportMissing`). Intel-pt does something different because it lives outside of (lib)lldb. https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] LLDB Debuginfod tests and a fix or two (PR #90622)
labath wrote: f8fedfb6802173372ec923f99f31d4af810fbcb0 ought to fix `TestSharedLibStrippedSymbols.py`. The rest might be due to the test being too strict with what it expects of a UUID. https://github.com/llvm/llvm-project/pull/90622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1654,6 +1660,99 @@ bool SymbolFileDWARF::CompleteType(CompilerType _type) { return false; } +DWARFDIE SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE ) { + auto def_die_it = GetDeclarationDIEToDefinitionDIE().find(die.GetDIE()); + if (def_die_it != GetDeclarationDIEToDefinitionDIE().end()) +return GetDIE(def_die_it->getSecond()); + + ParsedDWARFTypeAttributes attrs(die); + const dw_tag_t tag = die.Tag(); + TypeSP type_sp; + Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); + if (log) { +GetObjectFile()->GetModule()->LogMessage( +log, +"SymbolFileDWARF({0:p}) - {1:x16}: {2} type \"{3}\" is a " +"forward declaration DIE, trying to find definition DIE", +static_cast(this), die.GetOffset(), DW_TAG_value_to_name(tag), +attrs.name.GetCString()); + } + // We haven't parse definition die for this type, starting to search for it. + // After we found the definition die, the GetDeclarationDIEToDefinitionDIE() + // map will have the new mapping from this declaration die to definition die. + if (attrs.class_language == eLanguageTypeObjC || labath wrote: I realize this is a bit fuzzy, and there are already some arguably clang-specific pieces of code in SymbolFileDWARF, but the main reason I am bringing this up is because this code (some of it at least) has previously been in DWARFASTParserClang, and now it isn't -- which seems like a regression to me. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)
labath wrote: > So no need to change the structure of this PR, but do correct me if any of my > assumptions are incorrect there. The `DynamicLoaderPOSIXDYLD` is only used on ELF systems anyway (we might as well rename it do `DynamicLoaderELF`), so this actually lines up fairly well. https://github.com/llvm/llvm-project/pull/90783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1654,6 +1660,99 @@ bool SymbolFileDWARF::CompleteType(CompilerType _type) { return false; } +DWARFDIE SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE ) { + auto def_die_it = GetDeclarationDIEToDefinitionDIE().find(die.GetDIE()); + if (def_die_it != GetDeclarationDIEToDefinitionDIE().end()) +return GetDIE(def_die_it->getSecond()); + + ParsedDWARFTypeAttributes attrs(die); + const dw_tag_t tag = die.Tag(); + TypeSP type_sp; + Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); + if (log) { +GetObjectFile()->GetModule()->LogMessage( +log, +"SymbolFileDWARF({0:p}) - {1:x16}: {2} type \"{3}\" is a " +"forward declaration DIE, trying to find definition DIE", +static_cast(this), die.GetOffset(), DW_TAG_value_to_name(tag), +attrs.name.GetCString()); + } + // We haven't parse definition die for this type, starting to search for it. + // After we found the definition die, the GetDeclarationDIEToDefinitionDIE() + // map will have the new mapping from this declaration die to definition die. + if (attrs.class_language == eLanguageTypeObjC || labath wrote: Definitely anything that is gated on `language == eLanguageType*C*` checks, but a case could also be made that the whole declaration-to-definition mapping should be within the purview of the DWARFASTParser instance responsible for the specific language. Having the logic split between an ASTParser (a language-specific class) and SymbolFileDWARF (language-agnostic) forces all languages to work in the same way. I don't know how much of a problem would that be in practice, but it would definitely make the code nicer (single responsibility principle, and all) if the logic lived in a central place. https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix Scalar::GetData for non-multiple-of-8-bits values (PR #90846)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/90846 >From 258654adb8ae34626e58c35f04e1b63fa9100f4a Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 2 May 2024 10:49:13 + Subject: [PATCH] [lldb] Fix Scalar::GetData for non-multiple-of-8-bits values It was aligning the byte size down. Now it aligns up. This manifested itself as SBTypeStaticField::GetConstantValue returning a zero-sized value for `bool` fields (because clang represents bool as a 1-bit value). I've changed the code for float Scalars as well, although I'm not aware of floating point values that are not multiples of 8 bits. --- lldb/source/Utility/Scalar.cpp| 4 +-- lldb/test/API/python_api/type/TestTypeList.py | 13 lldb/test/API/python_api/type/main.cpp| 1 + lldb/unittests/Utility/ScalarTest.cpp | 30 +++ 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp index e94fd459623665..c70c5e10799187 100644 --- a/lldb/source/Utility/Scalar.cpp +++ b/lldb/source/Utility/Scalar.cpp @@ -134,9 +134,9 @@ size_t Scalar::GetByteSize() const { case e_void: break; case e_int: -return (m_integer.getBitWidth() / 8); +return (m_integer.getBitWidth() + 7) / 8; case e_float: -return m_float.bitcastToAPInt().getBitWidth() / 8; +return (m_float.bitcastToAPInt().getBitWidth() + 7) / 8; } return 0; } diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py index 81c44f7a39d61a..17e27b624511cf 100644 --- a/lldb/test/API/python_api/type/TestTypeList.py +++ b/lldb/test/API/python_api/type/TestTypeList.py @@ -52,6 +52,19 @@ def _find_static_field_in_Task_pointer(self, task_pointer): self.DebugSBValue(value) self.assertEqual(value.GetValueAsSigned(), 47) +static_constexpr_bool_field = task_type.GetStaticFieldWithName( +"static_constexpr_bool_field" +) +self.assertTrue(static_constexpr_bool_field) +self.assertEqual( +static_constexpr_bool_field.GetName(), "static_constexpr_bool_field" +) +self.assertEqual(static_constexpr_bool_field.GetType().GetName(), "const bool") + +value = static_constexpr_bool_field.GetConstantValue(self.target()) +self.DebugSBValue(value) +self.assertEqual(value.GetValueAsUnsigned(), 1) + static_mutable_field = task_type.GetStaticFieldWithName("static_mutable_field") self.assertTrue(static_mutable_field) self.assertEqual(static_mutable_field.GetName(), "static_mutable_field") diff --git a/lldb/test/API/python_api/type/main.cpp b/lldb/test/API/python_api/type/main.cpp index c86644d918279a..7384a3d8da16fb 100644 --- a/lldb/test/API/python_api/type/main.cpp +++ b/lldb/test/API/python_api/type/main.cpp @@ -28,6 +28,7 @@ class Task { union U { } u; static constexpr long static_constexpr_field = 47; +static constexpr bool static_constexpr_bool_field = true; static int static_mutable_field; Task(int i, Task *n): id(i), diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp index 8d957d16593ee7..500cb8bb2286e0 100644 --- a/lldb/unittests/Utility/ScalarTest.cpp +++ b/lldb/unittests/Utility/ScalarTest.cpp @@ -13,8 +13,11 @@ #include "lldb/Utility/Scalar.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/StreamString.h" +#include "lldb/lldb-enumerations.h" +#include "llvm/ADT/APSInt.h" #include "llvm/Testing/Support/Error.h" +#include #include using namespace lldb_private; @@ -163,6 +166,33 @@ TEST(ScalarTest, GetBytes) { ASSERT_EQ(0, memcmp(f, Storage, sizeof(f))); } +TEST(ScalarTest, GetData) { + auto get_data = [](llvm::APSInt v) { +DataExtractor data; +Scalar(v).GetData(data); +return data.GetData().vec(); + }; + + auto vec = [](std::initializer_list l) { +std::vector v(l.begin(), l.end()); +if (endian::InlHostByteOrder() == lldb::eByteOrderLittle) + std::reverse(v.begin(), v.end()); +return v; + }; + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/1, /*Unsigned=*/true)), + vec({0x01})); + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/8, /*Unsigned=*/true)), + vec({0xff})); + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/9, /*Unsigned=*/true)), + vec({0x01, 0xff})); +} + TEST(ScalarTest, SetValueFromData) { uint8_t a[] = {1, 2, 3, 4}; Scalar s; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix Scalar::GetData for non-multiple-of-8-bits values (PR #90846)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/90846 It was aligning the byte size down. Now it aligns up. This manifested itself as SBTypeStaticField::GetConstantValue returning a zero-sized value for `bool` fields (because clang represents bool as a 1-bit value). I've changed the code for float Scalars as well, although I'm not aware of floating point values that are not multiples of 8 bits. >From 7336344b98881a5158d9d4d72af5c51ebd1e74e2 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 2 May 2024 10:49:13 + Subject: [PATCH] [lldb] Fix Scalar::GetData for non-multiple-of-8-bits values It was aligning the byte size down. Now it aligns up. This manifested itself as SBTypeStaticField::GetConstantValue returning a zero-sized value for `bool` fields (because clang represents bool as a 1-bit value). I've changed the code for float Scalars as well, although I'm not aware of floating point values that are not multiples of 8 bits. --- lldb/source/Utility/Scalar.cpp| 4 +-- lldb/test/API/python_api/type/TestTypeList.py | 11 +++ lldb/test/API/python_api/type/main.cpp| 1 + lldb/unittests/Utility/ScalarTest.cpp | 30 +++ 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp index e94fd459623665..c70c5e10799187 100644 --- a/lldb/source/Utility/Scalar.cpp +++ b/lldb/source/Utility/Scalar.cpp @@ -134,9 +134,9 @@ size_t Scalar::GetByteSize() const { case e_void: break; case e_int: -return (m_integer.getBitWidth() / 8); +return (m_integer.getBitWidth() + 7) / 8; case e_float: -return m_float.bitcastToAPInt().getBitWidth() / 8; +return (m_float.bitcastToAPInt().getBitWidth() + 7) / 8; } return 0; } diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py index 81c44f7a39d61a..4dbfb06f8fc58c 100644 --- a/lldb/test/API/python_api/type/TestTypeList.py +++ b/lldb/test/API/python_api/type/TestTypeList.py @@ -52,6 +52,17 @@ def _find_static_field_in_Task_pointer(self, task_pointer): self.DebugSBValue(value) self.assertEqual(value.GetValueAsSigned(), 47) +static_constexpr_bool_field = task_type.GetStaticFieldWithName( +"static_constexpr_bool_field" +) +self.assertTrue(static_constexpr_bool_field) +self.assertEqual(static_constexpr_bool_field.GetName(), "static_constexpr_bool_field") +self.assertEqual(static_constexpr_bool_field.GetType().GetName(), "const bool") + +value = static_constexpr_bool_field.GetConstantValue(self.target()) +self.DebugSBValue(value) +self.assertEqual(value.GetValueAsUnsigned(), 1) + static_mutable_field = task_type.GetStaticFieldWithName("static_mutable_field") self.assertTrue(static_mutable_field) self.assertEqual(static_mutable_field.GetName(), "static_mutable_field") diff --git a/lldb/test/API/python_api/type/main.cpp b/lldb/test/API/python_api/type/main.cpp index c86644d918279a..7384a3d8da16fb 100644 --- a/lldb/test/API/python_api/type/main.cpp +++ b/lldb/test/API/python_api/type/main.cpp @@ -28,6 +28,7 @@ class Task { union U { } u; static constexpr long static_constexpr_field = 47; +static constexpr bool static_constexpr_bool_field = true; static int static_mutable_field; Task(int i, Task *n): id(i), diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp index 8d957d16593ee7..500cb8bb2286e0 100644 --- a/lldb/unittests/Utility/ScalarTest.cpp +++ b/lldb/unittests/Utility/ScalarTest.cpp @@ -13,8 +13,11 @@ #include "lldb/Utility/Scalar.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/StreamString.h" +#include "lldb/lldb-enumerations.h" +#include "llvm/ADT/APSInt.h" #include "llvm/Testing/Support/Error.h" +#include #include using namespace lldb_private; @@ -163,6 +166,33 @@ TEST(ScalarTest, GetBytes) { ASSERT_EQ(0, memcmp(f, Storage, sizeof(f))); } +TEST(ScalarTest, GetData) { + auto get_data = [](llvm::APSInt v) { +DataExtractor data; +Scalar(v).GetData(data); +return data.GetData().vec(); + }; + + auto vec = [](std::initializer_list l) { +std::vector v(l.begin(), l.end()); +if (endian::InlHostByteOrder() == lldb::eByteOrderLittle) + std::reverse(v.begin(), v.end()); +return v; + }; + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/1, /*Unsigned=*/true)), + vec({0x01})); + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/8, /*Unsigned=*/true)), + vec({0xff})); + + EXPECT_THAT( + get_data(llvm::APSInt::getMaxValue(/*numBits=*/9, /*Unsigned=*/true)), + vec({0x01, 0xff})); +} + TEST(ScalarTest, SetValueFromData) { uint8_t a[] = {1, 2, 3, 4}; Scalar s; ___ lldb-commits
[Lldb-commits] [lldb] [lldb] fix step in AArch64 trampoline (PR #90783)
@@ -506,9 +506,29 @@ DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread , Target = thread.GetProcess()->GetTarget(); const ModuleList = target.GetImages(); - images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols); - if (!target_symbols.GetSize()) -return thread_plan_sp; + llvm::StringRef target_name = sym_name.GetStringRef(); + // On AArch64, the trampoline name has a prefix (__AArch64ADRPThunk_ or + // __AArch64AbsLongThunk_) added to the function name. If we detect a + // trampoline with the prefix, we need to remove the prefix to find the + // function symbol. + if (target_name.consume_front("__AArch64ADRPThunk_")) { labath wrote: How about `if (target_name.consume_front("__AArch64ADRPThunk_") || target_name.consume_front("__AArch64AbsLongThunk_"))` https://github.com/llvm/llvm-project/pull/90783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -533,9 +540,16 @@ class SymbolFileDWARF : public SymbolFileCommon { NameToOffsetMap m_function_scope_qualified_name_map; std::unique_ptr m_ranges; UniqueDWARFASTTypeMap m_unique_ast_type_map; + // A map from DIE to lldb_private::Type. For record type, the key might be + // either declaration DIE or definition DIE. DIEToTypePtr m_die_to_type; DIEToVariableSP m_die_to_variable_sp; + // A map from CompilerType to the struct/class/union/enum DIE (might be a + // declaration or a definition) that is used to construct it. CompilerTypeToDIE m_forward_decl_compiler_type_to_die; + // A map from a struct/class/union/enum DIE (might be a declaration or a + // definition) to its definition DIE. + DIEToDIE m_die_to_def_die; labath wrote: Would it be possible to avoid creating the extra map, for example by modifying the `m_forward_decl_compiler_type_to_die` to point to the definition DIE -- after that DIE has been located? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -533,9 +540,16 @@ class SymbolFileDWARF : public SymbolFileCommon { NameToOffsetMap m_function_scope_qualified_name_map; std::unique_ptr m_ranges; UniqueDWARFASTTypeMap m_unique_ast_type_map; + // A map from DIE to lldb_private::Type. For record type, the key might be + // either declaration DIE or definition DIE. DIEToTypePtr m_die_to_type; DIEToVariableSP m_die_to_variable_sp; + // A map from CompilerType to the struct/class/union/enum DIE (might be a + // declaration or a definition) that is used to construct it. CompilerTypeToDIE m_forward_decl_compiler_type_to_die; + // A map from a struct/class/union/enum DIE (might be a declaration or a + // definition) to its definition DIE. + DIEToDIE m_die_to_def_die; labath wrote: (I also can't help but wonder why do we need that `m_forward_decl_compiler_type_to_die` in the first place, given that the `ClangASTMetadata` associated with the type already contains a user ID, which should allow us to retrieve original DIE. I know this isn't related to your patch, I'm just writing it in case you or someone has any thoughts on this.) https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1654,6 +1660,99 @@ bool SymbolFileDWARF::CompleteType(CompilerType _type) { return false; } +DWARFDIE SymbolFileDWARF::FindDefinitionDIE(const DWARFDIE ) { + auto def_die_it = GetDeclarationDIEToDefinitionDIE().find(die.GetDIE()); + if (def_die_it != GetDeclarationDIEToDefinitionDIE().end()) +return GetDIE(def_die_it->getSecond()); + + ParsedDWARFTypeAttributes attrs(die); + const dw_tag_t tag = die.Tag(); + TypeSP type_sp; + Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); + if (log) { +GetObjectFile()->GetModule()->LogMessage( +log, +"SymbolFileDWARF({0:p}) - {1:x16}: {2} type \"{3}\" is a " +"forward declaration DIE, trying to find definition DIE", +static_cast(this), die.GetOffset(), DW_TAG_value_to_name(tag), +attrs.name.GetCString()); + } + // We haven't parse definition die for this type, starting to search for it. + // After we found the definition die, the GetDeclarationDIEToDefinitionDIE() + // map will have the new mapping from this declaration die to definition die. + if (attrs.class_language == eLanguageTypeObjC || labath wrote: I'm not sure how big of a problem this is (as `CompleteType` above already contains some clang-specific code), but moving some clang-specific code out of `DWARFASTParserClang` and into `SymbolFileDWARF` is less than ideal, as the latter is also used with non-clang/non-C languages. Would it be possible to keep this code `DWARFASTParserClang` somehow? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #90663)
@@ -1631,13 +1631,19 @@ bool SymbolFileDWARF::CompleteType(CompilerType _type) { return true; } - DWARFDIE dwarf_die = GetDIE(die_it->getSecond()); + DWARFDIE dwarf_die = FindDefinitionDIE(GetDIE(die_it->getSecond())); if (dwarf_die) { // Once we start resolving this type, remove it from the forward // declaration map in case anyone child members or other types require this // type to get resolved. The type will get resolved when all of the calls // to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done. -GetForwardDeclCompilerTypeToDIE().erase(die_it); +// Need to get a new iterator because FindDefinitionDIE might add new labath wrote: Would it be possible to erase the iterator before calling FindDefinitionDIE? https://github.com/llvm/llvm-project/pull/90663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ELF] Fix section unification to not just use names. (PR #90099)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/90099 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ELF] Fix section unification to not just use names. (PR #90099)
labath wrote: > > I saw that, but a textual test is definitely preferable, particularly after > > the linux xz fiasco. This wouldn't be the first test binary in our repo, > > but in this case, I think it actually adds a lot of value, since yaml2obj > > operates on the same level as what you are testing (so you can see the > > input of the test eyeball-verify that the expected output makes sense). > > :-) Fair point (though this is very much not like the xz incident — I'm a > colleague of Jonas's at Apple, albeit in a different team, with a fairly > longstanding history of contributing to OSS projects, and doing something > like that would be… very career limiting). > > I'll have a go at changing things to use `yaml2obj`. The new test looks great. https://github.com/llvm/llvm-project/pull/90099 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ELF] Fix section unification to not just use names. (PR #90099)
@@ -1854,6 +1854,49 @@ class VMAddressProvider { }; } +namespace { + // We have to do this because ELF doesn't have section IDs, and also + // doesn't require section names to be unique. (We use the section index + // for section IDs, but that isn't guaranteed to be the same in separate + // debug images.) + SectionSP FindMatchingSection(const SectionList _list, +SectionSP section) { +SectionSP sect_sp; + +addr_t vm_addr = section->GetFileAddress(); +ConstString name = section->GetName(); +offset_t file_size = section->GetFileSize(); +offset_t byte_size = section->GetByteSize(); +SectionType type = section->GetType(); +bool thread_specific = section->IsThreadSpecific(); +uint32_t permissions = section->GetPermissions(); +uint32_t alignment = section->GetLog2Align(); + +for (auto sect_iter = section_list.begin(); + sect_iter != section_list.end(); + ++sect_iter) { + if ((*sect_iter)->GetName() == name + && (*sect_iter)->GetType() == type + && (*sect_iter)->IsThreadSpecific() == thread_specific + && (*sect_iter)->GetPermissions() == permissions + && (*sect_iter)->GetFileSize() == file_size + && (*sect_iter)->GetByteSize() == byte_size + && (*sect_iter)->GetFileAddress() == vm_addr + && (*sect_iter)->GetLog2Align() == alignment) { labath wrote: The section type can also change. You can see that with .text, because there we have a name-based fallback, but with anything else, it changes from "code" to "regular": ```$ bin/lldb-test object-file /tmp/a.debug | grep -e foo -C 3 Showing 1 subsections Index: 0 ID: 0x6 Name: .foo Type: regular Permissions: r-x Thread specific: no $ bin/lldb-test object-file /tmp/a.out | grep -e foo -C 3 Showing 1 subsections Index: 0 ID: 0x6 Name: .foo Type: code Permissions: r-x Thread specific: no ``` The other fields are _probably_ fine. https://github.com/llvm/llvm-project/pull/90099 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ELF] Fix section unification to not just use names. (PR #90099)
labath wrote: > > [this](https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/yaml2obj/ELF/duplicate-section-names.yaml) > > test seems to indicate that's possible (the trick appears to me in `(n)` > > name tag suffixes). Can you give that a shot? > > I've actually just included a binary file that exhibits the problem. I saw that, but a textual test is definitely preferable, particularly after the linux xz fiasco. This wouldn't be the first test binary in our repo, but in this case, I think it actually adds a lot of value, since yaml2obj operates on the same level as what you are testing (so you can see the input of the test eyeball-verify that the expected output makes sense). https://github.com/llvm/llvm-project/pull/90099 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ELF] Fix section unification to not just use names. (PR #90099)
@@ -1854,6 +1854,49 @@ class VMAddressProvider { }; } +namespace { + // We have to do this because ELF doesn't have section IDs, and also + // doesn't require section names to be unique. (We use the section index + // for section IDs, but that isn't guaranteed to be the same in separate + // debug images.) + SectionSP FindMatchingSection(const SectionList _list, +SectionSP section) { +SectionSP sect_sp; + +addr_t vm_addr = section->GetFileAddress(); +ConstString name = section->GetName(); +offset_t file_size = section->GetFileSize(); +offset_t byte_size = section->GetByteSize(); +SectionType type = section->GetType(); +bool thread_specific = section->IsThreadSpecific(); +uint32_t permissions = section->GetPermissions(); +uint32_t alignment = section->GetLog2Align(); + +for (auto sect_iter = section_list.begin(); + sect_iter != section_list.end(); + ++sect_iter) { + if ((*sect_iter)->GetName() == name + && (*sect_iter)->GetType() == type + && (*sect_iter)->IsThreadSpecific() == thread_specific + && (*sect_iter)->GetPermissions() == permissions + && (*sect_iter)->GetFileSize() == file_size + && (*sect_iter)->GetByteSize() == byte_size + && (*sect_iter)->GetFileAddress() == vm_addr + && (*sect_iter)->GetLog2Align() == alignment) { labath wrote: Am I correct in understanding that this code is used when unifying the sections of a separate debug file with a stripped file? If so, then I believe this comparison is too strict. An object file produced by `objcopy --only-keep-debug` will only have placeholder sections instead of the sections that contained actual code. That means (at least) their file size and file offset will be different from that in the original file. I think it would be sufficient to use the file address (called virtual address in ELF), possibly confirmed by section name, as the unification key. https://github.com/llvm/llvm-project/pull/90099 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ELF] Fix section unification to not just use names. (PR #90099)
labath wrote: > > Can we add a test for this with obj2yaml? > > Not sure what you're thinking here. We can't use `yaml2obj` to generate an > object with this problem because that tool assumes symbols are in a uniquely > named section [this](https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/yaml2obj/ELF/duplicate-section-names.yaml) test seems to indicate that's possible (the trick appears to me in `(n)` name tag suffixes). Can you give that a shot? https://github.com/llvm/llvm-project/pull/90099 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 0d56d88 - [lldb] Update eh-frame-dwarf-unwind-abort.test for a change in llvm assembler
Author: Pavel Labath Date: 2024-04-26T07:13:33Z New Revision: 0d56d88d9fc48f1b97a641422ee23cc6eabcd6ef URL: https://github.com/llvm/llvm-project/commit/0d56d88d9fc48f1b97a641422ee23cc6eabcd6ef DIFF: https://github.com/llvm/llvm-project/commit/0d56d88d9fc48f1b97a641422ee23cc6eabcd6ef.diff LOG: [lldb] Update eh-frame-dwarf-unwind-abort.test for a change in llvm assembler The jump instruction now gets encoded as a near jump, which changes some offsets. Added: Modified: lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test Removed: diff --git a/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test b/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test index 477a656a711f90..d5e66ca5e26341 100644 --- a/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test +++ b/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-abort.test @@ -9,12 +9,12 @@ process launch # CHECK: stop reason = signal SIGTRAP thread backtrace -# CHECK: frame #0: {{.*}}`asm_main + 23 +# CHECK: frame #0: {{.*}}`asm_main + 19 # CHECK: frame #1: {{.*}}`main + {{.*}} target modules show-unwind -n asm_main # CHECK: eh_frame UnwindPlan: # CHECK: row[0]:0: CFA=rsp +8 => rip=[CFA-8] -# CHECK: row[1]: 14: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8] -# CHECK: row[2]: 17: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8] -# CHECK: row[3]: 22: CFA=rsp +8 => rip=[CFA-8] +# CHECK: row[1]: 10: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8] +# CHECK: row[2]: 13: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8] +# CHECK: row[3]: 18: CFA=rsp +8 => rip=[CFA-8] ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SB API to access static constexpr member values (PR #89730)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/89730 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SB API to access static constexpr member values (PR #89730)
@@ -325,6 +330,79 @@ lldb::SBTypeMemberFunction SBType::GetMemberFunctionAtIndex(uint32_t idx) { return sb_func_type; } +SBTypeStaticField::SBTypeStaticField() { LLDB_INSTRUMENT_VA(this); } + +SBTypeStaticField::SBTypeStaticField(lldb_private::CompilerDecl decl) +: m_opaque_up(decl ? std::make_unique(decl) : nullptr) {} + +SBTypeStaticField::SBTypeStaticField(const SBTypeStaticField ) { + LLDB_INSTRUMENT_VA(this, rhs); + + m_opaque_up = clone(rhs.m_opaque_up); +} + +SBTypeStaticField ::operator=(const SBTypeStaticField ) { + LLDB_INSTRUMENT_VA(this, rhs); + + m_opaque_up = clone(rhs.m_opaque_up); + return *this; +} + +SBTypeStaticField::~SBTypeStaticField() { LLDB_INSTRUMENT_VA(this); } + +SBTypeStaticField::operator bool() const { + LLDB_INSTRUMENT_VA(this); + + return IsValid(); +} + +bool SBTypeStaticField::IsValid() const { + LLDB_INSTRUMENT_VA(this); + + return m_opaque_up != nullptr; +} + +const char *SBTypeStaticField::GetName() { + LLDB_INSTRUMENT_VA(this); + + if (!IsValid()) +return ""; + return m_opaque_up->GetName().GetCString(); +} + +const char *SBTypeStaticField::GetMangledName() { + LLDB_INSTRUMENT_VA(this); + + if (!IsValid()) +return ""; + return m_opaque_up->GetMangledName().GetCString(); +} + +SBType SBTypeStaticField::GetType() { + LLDB_INSTRUMENT_VA(this); + + if (!IsValid()) +return SBType(); + return SBType(m_opaque_up->GetType()); +} + +SBValue SBTypeStaticField::GetConstantValue(lldb::SBTarget target) { + LLDB_INSTRUMENT_VA(this); labath wrote: Done (that's because this function, when I first wrote it, did not have a target argument :P) https://github.com/llvm/llvm-project/pull/89730 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SB API to access static constexpr member values (PR #89730)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/89730 >From 091db4a89de2678fbdcc2db3051f34eeb8cc0cf2 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 23 Apr 2024 08:50:31 + Subject: [PATCH 1/3] [lldb] Add SB API to access static constexpr member values The main change is the addition of a new SBTypeStaticField class, representing a static member of a class. It can be retrieved created through SBType::GetStaticFieldWithName it contains several methods (GetName, GetMangledName, etc.) whose meaning is hopefully obvious. The most interesting method is lldb::SBValue GetConstantValue(lldb::SBTarget) which returns a the value of the field -- if it is a compile time constant. The reason for that is that only constants have their values represented in the clang AST. For non-constants, we need to go back to the module containing that constant, and ask retrive the associated ValueObjectVariable. That's easy enough if the we are still in the type system of the module (because then the type system will contain the pointer to the module symbol file), but it's hard when the type has been copied into another AST (e.g. during expression evaluation). To do that we would need to walk the ast import chain backwards to find the source TypeSystem, and I haven't found a nice way to do that. Another possibility would be to use the mangled name of the variable to perform a lookup (in all modules). That is sort of what happens when evaluating the variable in an expression (which does work), but I did not want to commit to that implementation as it's not necessary for my use case (and if anyone wants to, he can use the GetMangledName function and perform the lookup manually). The patch adds a couple of new TypeSystem functions to surface the information needed to implement this functionality. --- lldb/include/lldb/API/SBTarget.h | 1 + lldb/include/lldb/API/SBType.h| 32 +++ lldb/include/lldb/API/SBValue.h | 1 + lldb/include/lldb/Symbol/CompilerDecl.h | 7 ++ lldb/include/lldb/Symbol/CompilerType.h | 2 + lldb/include/lldb/Symbol/TypeSystem.h | 9 ++ lldb/source/API/SBType.cpp| 88 +++ .../TypeSystem/Clang/TypeSystemClang.cpp | 52 +++ .../TypeSystem/Clang/TypeSystemClang.h| 8 ++ lldb/source/Symbol/CompilerDecl.cpp | 9 ++ lldb/source/Symbol/CompilerType.cpp | 6 ++ lldb/test/API/python_api/type/TestTypeList.py | 26 ++ lldb/test/API/python_api/type/main.cpp| 1 + 13 files changed, 242 insertions(+) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 3644ac056da3dc..823615e6a36df5 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -954,6 +954,7 @@ class LLDB_API SBTarget { friend class SBSection; friend class SBSourceManager; friend class SBSymbol; + friend class SBTypeStaticField; friend class SBValue; friend class SBVariablesOptions; diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h index 9980fe1218305b..7767eace0cebfe 100644 --- a/lldb/include/lldb/API/SBType.h +++ b/lldb/include/lldb/API/SBType.h @@ -107,6 +107,35 @@ class SBTypeMemberFunction { lldb::TypeMemberFunctionImplSP m_opaque_sp; }; +class LLDB_API SBTypeStaticField { +public: + SBTypeStaticField(); + + SBTypeStaticField(const lldb::SBTypeStaticField ); + lldb::SBTypeStaticField =(const lldb::SBTypeStaticField ); + + ~SBTypeStaticField(); + + explicit operator bool() const; + + bool IsValid() const; + + const char *GetName(); + + const char *GetMangledName(); + + lldb::SBType GetType(); + + lldb::SBValue GetConstantValue(lldb::SBTarget target); + +protected: + friend class SBType; + + SBTypeStaticField(lldb_private::CompilerDecl decl); + + std::unique_ptr m_opaque_up; +}; + class SBType { public: SBType(); @@ -182,6 +211,8 @@ class SBType { lldb::SBTypeMember GetVirtualBaseClassAtIndex(uint32_t idx); + lldb::SBTypeStaticField GetStaticFieldWithName(const char *name); + lldb::SBTypeEnumMemberList GetEnumMembers(); uint32_t GetNumberOfTemplateArguments(); @@ -242,6 +273,7 @@ class SBType { friend class SBTypeNameSpecifier; friend class SBTypeMember; friend class SBTypeMemberFunction; + friend class SBTypeStaticField; friend class SBTypeList; friend class SBValue; friend class SBWatchpoint; diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h index bbcccaab51aaee..67f55ce7da2877 100644 --- a/lldb/include/lldb/API/SBValue.h +++ b/lldb/include/lldb/API/SBValue.h @@ -426,6 +426,7 @@ class LLDB_API SBValue { friend class SBModule; friend class SBTarget; friend class SBThread; + friend class SBTypeStaticField; friend class SBTypeSummary; friend class SBValueList; diff --git a/lldb/include/lldb/Symbol/CompilerDecl.h
[Lldb-commits] [lldb] [lldb] Add SB API to access static constexpr member values (PR #89730)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/89730 >From 091db4a89de2678fbdcc2db3051f34eeb8cc0cf2 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 23 Apr 2024 08:50:31 + Subject: [PATCH 1/2] [lldb] Add SB API to access static constexpr member values The main change is the addition of a new SBTypeStaticField class, representing a static member of a class. It can be retrieved created through SBType::GetStaticFieldWithName it contains several methods (GetName, GetMangledName, etc.) whose meaning is hopefully obvious. The most interesting method is lldb::SBValue GetConstantValue(lldb::SBTarget) which returns a the value of the field -- if it is a compile time constant. The reason for that is that only constants have their values represented in the clang AST. For non-constants, we need to go back to the module containing that constant, and ask retrive the associated ValueObjectVariable. That's easy enough if the we are still in the type system of the module (because then the type system will contain the pointer to the module symbol file), but it's hard when the type has been copied into another AST (e.g. during expression evaluation). To do that we would need to walk the ast import chain backwards to find the source TypeSystem, and I haven't found a nice way to do that. Another possibility would be to use the mangled name of the variable to perform a lookup (in all modules). That is sort of what happens when evaluating the variable in an expression (which does work), but I did not want to commit to that implementation as it's not necessary for my use case (and if anyone wants to, he can use the GetMangledName function and perform the lookup manually). The patch adds a couple of new TypeSystem functions to surface the information needed to implement this functionality. --- lldb/include/lldb/API/SBTarget.h | 1 + lldb/include/lldb/API/SBType.h| 32 +++ lldb/include/lldb/API/SBValue.h | 1 + lldb/include/lldb/Symbol/CompilerDecl.h | 7 ++ lldb/include/lldb/Symbol/CompilerType.h | 2 + lldb/include/lldb/Symbol/TypeSystem.h | 9 ++ lldb/source/API/SBType.cpp| 88 +++ .../TypeSystem/Clang/TypeSystemClang.cpp | 52 +++ .../TypeSystem/Clang/TypeSystemClang.h| 8 ++ lldb/source/Symbol/CompilerDecl.cpp | 9 ++ lldb/source/Symbol/CompilerType.cpp | 6 ++ lldb/test/API/python_api/type/TestTypeList.py | 26 ++ lldb/test/API/python_api/type/main.cpp| 1 + 13 files changed, 242 insertions(+) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 3644ac056da3dc..823615e6a36df5 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -954,6 +954,7 @@ class LLDB_API SBTarget { friend class SBSection; friend class SBSourceManager; friend class SBSymbol; + friend class SBTypeStaticField; friend class SBValue; friend class SBVariablesOptions; diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h index 9980fe1218305b..7767eace0cebfe 100644 --- a/lldb/include/lldb/API/SBType.h +++ b/lldb/include/lldb/API/SBType.h @@ -107,6 +107,35 @@ class SBTypeMemberFunction { lldb::TypeMemberFunctionImplSP m_opaque_sp; }; +class LLDB_API SBTypeStaticField { +public: + SBTypeStaticField(); + + SBTypeStaticField(const lldb::SBTypeStaticField ); + lldb::SBTypeStaticField =(const lldb::SBTypeStaticField ); + + ~SBTypeStaticField(); + + explicit operator bool() const; + + bool IsValid() const; + + const char *GetName(); + + const char *GetMangledName(); + + lldb::SBType GetType(); + + lldb::SBValue GetConstantValue(lldb::SBTarget target); + +protected: + friend class SBType; + + SBTypeStaticField(lldb_private::CompilerDecl decl); + + std::unique_ptr m_opaque_up; +}; + class SBType { public: SBType(); @@ -182,6 +211,8 @@ class SBType { lldb::SBTypeMember GetVirtualBaseClassAtIndex(uint32_t idx); + lldb::SBTypeStaticField GetStaticFieldWithName(const char *name); + lldb::SBTypeEnumMemberList GetEnumMembers(); uint32_t GetNumberOfTemplateArguments(); @@ -242,6 +273,7 @@ class SBType { friend class SBTypeNameSpecifier; friend class SBTypeMember; friend class SBTypeMemberFunction; + friend class SBTypeStaticField; friend class SBTypeList; friend class SBValue; friend class SBWatchpoint; diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h index bbcccaab51aaee..67f55ce7da2877 100644 --- a/lldb/include/lldb/API/SBValue.h +++ b/lldb/include/lldb/API/SBValue.h @@ -426,6 +426,7 @@ class LLDB_API SBValue { friend class SBModule; friend class SBTarget; friend class SBThread; + friend class SBTypeStaticField; friend class SBTypeSummary; friend class SBValueList; diff --git a/lldb/include/lldb/Symbol/CompilerDecl.h
[Lldb-commits] [lldb] [lldb] Add SB API to access static constexpr member values (PR #89730)
@@ -1156,6 +1159,10 @@ CompilerType TypeSystemClang::GetTypeForDecl(ObjCInterfaceDecl *decl) { return GetType(getASTContext().getObjCInterfaceType(decl)); } +CompilerType TypeSystemClang::GetTypeForDecl(clang::ValueDecl *value_decl) { labath wrote: ValueDecl is a base of VarDecl. For this patch I only need to know the types of `VarDecl`s, but I figured I could use the lowest class that does the trick (defines `getType`), in case that's useful in the future. https://github.com/llvm/llvm-project/pull/89730 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SB API to access static constexpr member values (PR #89730)
@@ -27,6 +27,7 @@ class Task { enum E : unsigned char {} e; union U { } u; +static constexpr long static_constexpr_field = 47; labath wrote: Good idea. https://github.com/llvm/llvm-project/pull/89730 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SB API to access static constexpr member values (PR #89730)
@@ -107,6 +107,35 @@ class SBTypeMemberFunction { lldb::TypeMemberFunctionImplSP m_opaque_sp; }; +class LLDB_API SBTypeStaticField { +public: + SBTypeStaticField(); + + SBTypeStaticField(const lldb::SBTypeStaticField ); + lldb::SBTypeStaticField =(const lldb::SBTypeStaticField ); + + ~SBTypeStaticField(); + + explicit operator bool() const; + + bool IsValid() const; + + const char *GetName(); + + const char *GetMangledName(); + + lldb::SBType GetType(); + + lldb::SBValue GetConstantValue(lldb::SBTarget target); + +protected: + friend class SBType; + + SBTypeStaticField(lldb_private::CompilerDecl decl); labath wrote: No, I did not. One could say it was implicitly implicit. :P https://github.com/llvm/llvm-project/pull/89730 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SB API to access static constexpr member values (PR #89730)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/89730 The main change is the addition of a new SBTypeStaticField class, representing a static member of a class. It can be retrieved created through SBType::GetStaticFieldWithName. It contains several methods (GetName, GetMangledName, etc.) whose meaning is hopefully obvious. The most interesting method is lldb::SBValue GetConstantValue(lldb::SBTarget) which returns a the value of the field -- if it is a compile time constant. The reason for that is that only constants have their values represented in the clang AST. For non-constants, we need to go back to the module containing that constant, and ask retrieve the associated ValueObjectVariable. That's easy enough if the we are still in the type system of the module (because then the type system will contain the pointer to the module symbol file), but it's hard when the type has been copied into another AST (e.g. during expression evaluation). To do that we would need to walk the ast import chain backwards to find the source TypeSystem, and I haven't found a nice way to do that. Another possibility would be to use the mangled name of the variable to perform a lookup (in all modules). That is sort of what happens when evaluating the variable in an expression (which does work), but I did not want to commit to that implementation as it's not necessary for my use case (and if anyone wants to, he can use the GetMangledName function and perform the lookup manually). The patch adds a couple of new TypeSystem functions to surface the information needed to implement this functionality. >From 091db4a89de2678fbdcc2db3051f34eeb8cc0cf2 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 23 Apr 2024 08:50:31 + Subject: [PATCH] [lldb] Add SB API to access static constexpr member values The main change is the addition of a new SBTypeStaticField class, representing a static member of a class. It can be retrieved created through SBType::GetStaticFieldWithName it contains several methods (GetName, GetMangledName, etc.) whose meaning is hopefully obvious. The most interesting method is lldb::SBValue GetConstantValue(lldb::SBTarget) which returns a the value of the field -- if it is a compile time constant. The reason for that is that only constants have their values represented in the clang AST. For non-constants, we need to go back to the module containing that constant, and ask retrive the associated ValueObjectVariable. That's easy enough if the we are still in the type system of the module (because then the type system will contain the pointer to the module symbol file), but it's hard when the type has been copied into another AST (e.g. during expression evaluation). To do that we would need to walk the ast import chain backwards to find the source TypeSystem, and I haven't found a nice way to do that. Another possibility would be to use the mangled name of the variable to perform a lookup (in all modules). That is sort of what happens when evaluating the variable in an expression (which does work), but I did not want to commit to that implementation as it's not necessary for my use case (and if anyone wants to, he can use the GetMangledName function and perform the lookup manually). The patch adds a couple of new TypeSystem functions to surface the information needed to implement this functionality. --- lldb/include/lldb/API/SBTarget.h | 1 + lldb/include/lldb/API/SBType.h| 32 +++ lldb/include/lldb/API/SBValue.h | 1 + lldb/include/lldb/Symbol/CompilerDecl.h | 7 ++ lldb/include/lldb/Symbol/CompilerType.h | 2 + lldb/include/lldb/Symbol/TypeSystem.h | 9 ++ lldb/source/API/SBType.cpp| 88 +++ .../TypeSystem/Clang/TypeSystemClang.cpp | 52 +++ .../TypeSystem/Clang/TypeSystemClang.h| 8 ++ lldb/source/Symbol/CompilerDecl.cpp | 9 ++ lldb/source/Symbol/CompilerType.cpp | 6 ++ lldb/test/API/python_api/type/TestTypeList.py | 26 ++ lldb/test/API/python_api/type/main.cpp| 1 + 13 files changed, 242 insertions(+) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 3644ac056da3dc..823615e6a36df5 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -954,6 +954,7 @@ class LLDB_API SBTarget { friend class SBSection; friend class SBSourceManager; friend class SBSymbol; + friend class SBTypeStaticField; friend class SBValue; friend class SBVariablesOptions; diff --git a/lldb/include/lldb/API/SBType.h b/lldb/include/lldb/API/SBType.h index 9980fe1218305b..7767eace0cebfe 100644 --- a/lldb/include/lldb/API/SBType.h +++ b/lldb/include/lldb/API/SBType.h @@ -107,6 +107,35 @@ class SBTypeMemberFunction { lldb::TypeMemberFunctionImplSP m_opaque_sp; }; +class LLDB_API SBTypeStaticField { +public: +
[Lldb-commits] [lldb] dbcfb43 - [lldb/test] Rename a function
Author: Pavel Labath Date: 2024-04-23T09:11:58Z New Revision: dbcfb434a9c7fea194c7b1f7f04f0947f88dbc85 URL: https://github.com/llvm/llvm-project/commit/dbcfb434a9c7fea194c7b1f7f04f0947f88dbc85 DIFF: https://github.com/llvm/llvm-project/commit/dbcfb434a9c7fea194c7b1f7f04f0947f88dbc85.diff LOG: [lldb/test] Rename a function I misunderstood what is the function looking up Added: Modified: lldb/test/API/python_api/type/TestTypeList.py Removed: diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py index 09c1dee80ef6c4..c647c2bcdccb6f 100644 --- a/lldb/test/API/python_api/type/TestTypeList.py +++ b/lldb/test/API/python_api/type/TestTypeList.py @@ -18,7 +18,7 @@ def setUp(self): self.source = "main.cpp" self.line = line_number(self.source, "// Break at this line") -def _find_nested_type_in_Task_pointer(self, pointer_type): +def _find_nested_type_in_Pointer_template_arg(self, pointer_type): self.assertTrue(pointer_type) self.DebugSBType(pointer_type) pointer_info_type = pointer_type.template_args[1] @@ -168,8 +168,10 @@ def test(self): # Check that FindDirectNestedType works with types from module and # expression ASTs. - self._find_nested_type_in_Task_pointer(frame0.FindVariable("pointer").GetType()) -self._find_nested_type_in_Task_pointer( +self._find_nested_type_in_Pointer_template_arg( +frame0.FindVariable("pointer").GetType() +) +self._find_nested_type_in_Pointer_template_arg( frame0.EvaluateExpression("pointer").GetType() ) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/89183 >From 80ba4f24cdfe8b5f2aa44a016ea69ad08f56d558 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 18 Apr 2024 07:34:45 + Subject: [PATCH 1/2] [lldb] Make SBType::FindDirectNestedType work with expression ASTs The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- lldb/include/lldb/Symbol/TypeSystem.h | 7 .../TypeSystem/Clang/TypeSystemClang.cpp | 30 +++ .../TypeSystem/Clang/TypeSystemClang.h| 3 ++ lldb/source/Symbol/Type.cpp | 16 +--- lldb/test/API/python_api/type/TestTypeList.py | 37 +++ 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector _indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..68b82e9688f12b 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,9 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector _indexes) override; + CompilerType GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..d9894ac355c6f0 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,8 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) { if (name.empty()) return CompilerType(); auto type_system = GetTypeSystem(/*prefer_dynamic*/ false); - auto *symbol_file = type_system->GetSymbolFile(); - if (!symbol_file) -return CompilerType(); - auto decl_context = type_system->GetCompilerDeclContextForType(m_static_type); - if (!decl_context.IsValid()) -return CompilerType(); - TypeQuery query(decl_context, ConstString(name),
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
labath wrote: > @Michael137 suggested that I check that performance of `FindDirectNestedType` > doesn't regress (at least for my use case), since I have custom > instrumentation in my formatter. I can confirm that 3 versions of this > function (#68705, #74786, and this PR) exhibit the same level of performance. > Raw numbers (in nanoseconds) fluctuate a lot, but I don't see anything > concerning: > > ``` > This PR > --- > 2,611,025 > 1,988,893 > 2,878,981 > 1,873,220 > > main branch (#74786) > > 1,973,071 > 2,542,073 > 1,509,624 > > Initial implementation (#68705) > -- > 2,029,233 > 2,477,041 > 1,315,462 > ``` Cool. Thanks for confirming that. https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/89183 >From 80ba4f24cdfe8b5f2aa44a016ea69ad08f56d558 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 18 Apr 2024 07:34:45 + Subject: [PATCH 1/2] [lldb] Make SBType::FindDirectNestedType work with expression ASTs The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- lldb/include/lldb/Symbol/TypeSystem.h | 7 .../TypeSystem/Clang/TypeSystemClang.cpp | 30 +++ .../TypeSystem/Clang/TypeSystemClang.h| 3 ++ lldb/source/Symbol/Type.cpp | 16 +--- lldb/test/API/python_api/type/TestTypeList.py | 37 +++ 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector _indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..68b82e9688f12b 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,9 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector _indexes) override; + CompilerType GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..d9894ac355c6f0 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,8 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) { if (name.empty()) return CompilerType(); auto type_system = GetTypeSystem(/*prefer_dynamic*/ false); - auto *symbol_file = type_system->GetSymbolFile(); - if (!symbol_file) -return CompilerType(); - auto decl_context = type_system->GetCompilerDeclContextForType(m_static_type); - if (!decl_context.IsValid()) -return CompilerType(); - TypeQuery query(decl_context, ConstString(name),
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
labath wrote: > > > One thing I'd like to get feedback on is whether this should be looking > > > into base classes. The discussion on the original PR focuses on lexically > > > nested types, but going through base classes (following usual language > > > rules) is another form of nesting. Both the existing and the new > > > implementations don't do that, but I think it would be more natural to do > > > it. Of course, then the function can definitely return more than one > > > result... > > > > > > What you're describing here is in line with member lookup described in the > > Standard. While it might be a useful facility, it's at odds with the intent > > of `FindDirectNestedTypes()` to be a small building block for efficient AST > > traversal. For the use case I have for this function, any amount of lookup > > is going to regress the performance of the formatter, and subsequently > > responsiveness of the debugger. So if you're going to pursue this, I > > suggest exposing this as a new function. > > I don't expect the simple `DeclContext` lookup proposed here to be expensive. > What _is_ expensive from the data-formatters point of view is either > full-blown expression evaluation or a global search in DWARF for some type. Agreed. This should be the same lookup that clang does every time is sees a `::`. Going through (potentially many) calls to FindTypes would be a different story. The reason I was asking is because I was not sure if this behavior was just a side-effect of how the original implementation worked, or if it was the intention all along. Judging by the response, it's the latter. > > Re. looking at base classes, don't have a strong opinion on it. There's some > precedent for looking at base classes already in `TypeSystemClang` (e.g., > `GetIndexOfChildWithName`). But as you say @labath, that would require an API > change That's makes sense. Even if it was not the intention, the API makes this the only reasonable implementation. I have no use for the base-class traversal (may change in the future). If there's is a need for it, we can add a different API. https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/89183 >From 80ba4f24cdfe8b5f2aa44a016ea69ad08f56d558 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 18 Apr 2024 07:34:45 + Subject: [PATCH] [lldb] Make SBType::FindDirectNestedType work with expression ASTs The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- lldb/include/lldb/Symbol/TypeSystem.h | 7 .../TypeSystem/Clang/TypeSystemClang.cpp | 30 +++ .../TypeSystem/Clang/TypeSystemClang.h| 3 ++ lldb/source/Symbol/Type.cpp | 16 +--- lldb/test/API/python_api/type/TestTypeList.py | 37 +++ 5 files changed, 63 insertions(+), 30 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector _indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..68b82e9688f12b 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,9 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector _indexes) override; + CompilerType GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..d9894ac355c6f0 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,8 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) { if (name.empty()) return CompilerType(); auto type_system = GetTypeSystem(/*prefer_dynamic*/ false); - auto *symbol_file = type_system->GetSymbolFile(); - if (!symbol_file) -return CompilerType(); - auto decl_context = type_system->GetCompilerDeclContextForType(m_static_type); - if (!decl_context.IsValid()) -return CompilerType(); - TypeQuery query(decl_context, ConstString(name), -
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/89183 >From c24f35cb4f568c42da9a11c7d91cfbaf7bf2f59e Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 18 Apr 2024 07:34:45 + Subject: [PATCH] [lldb] Make SBType::FindDirectNestedType work with expression ASTs The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- lldb/include/lldb/Symbol/TypeSystem.h | 7 .../TypeSystem/Clang/TypeSystemClang.cpp | 30 .../TypeSystem/Clang/TypeSystemClang.h| 3 ++ lldb/source/Symbol/Type.cpp | 16 ++--- lldb/test/API/python_api/type/TestTypeList.py | 35 ++- 5 files changed, 61 insertions(+), 30 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector _indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..68b82e9688f12b 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,9 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector _indexes) override; + CompilerType GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..d9894ac355c6f0 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,8 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) { if (name.empty()) return CompilerType(); auto type_system = GetTypeSystem(/*prefer_dynamic*/ false); - auto *symbol_file = type_system->GetSymbolFile(); - if (!symbol_file) -return CompilerType(); - auto decl_context = type_system->GetCompilerDeclContextForType(m_static_type); - if (!decl_context.IsValid()) -return CompilerType(); - TypeQuery query(decl_context, ConstString(name), -
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
labath wrote: In terms of #68705, I think this will fix the issue where the data formatter works on `frame var foo` but not `expr -- foo`. At least that's the problem I ran into when trying to use this in my own formatter.. One thing I'd like to get feedback on is whether this should be looking into base classes. The discussion on the original PR focuses on lexically nested types, but going through base classes (following usual language rules) is another form of nesting. Both the existing and the new implementations don't do that, but I think it would be more natural to do it. Of course, then the function can definitely return more than one result... https://github.com/llvm/llvm-project/pull/89183 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBType::FindDirectNestedType work with expression ASTs (PR #89183)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/89183 The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). >From d8e6fdff0df7b1a2e9242d747831966c5db03e44 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 18 Apr 2024 07:34:45 + Subject: [PATCH] [lldb] Make SBType::FindDirectNestedType work with expression ASTs The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). --- lldb/include/lldb/Symbol/TypeSystem.h | 7 .../TypeSystem/Clang/TypeSystemClang.cpp | 30 .../TypeSystem/Clang/TypeSystemClang.h| 4 +++ lldb/source/Symbol/Type.cpp | 15 +--- lldb/test/API/python_api/type/TestTypeList.py | 35 ++- 5 files changed, 61 insertions(+), 30 deletions(-) diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index f647fcbf1636ea..3a927d313b823d 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -28,6 +28,7 @@ #include "lldb/Symbol/CompilerDeclContext.h" #include "lldb/Symbol/Type.h" #include "lldb/lldb-private.h" +#include "lldb/lldb-types.h" class PDBASTParser; @@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface, lldb::opaque_compiler_type_t type, llvm::StringRef name, bool omit_empty_base_classes, std::vector _indexes) = 0; + virtual CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { +return CompilerType(); + } + virtual bool IsTemplateType(lldb::opaque_compiler_type_t type); virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index be0ddb06f82c18..2621f682011b41 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type, return UINT32_MAX; } +CompilerType +TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) { + if (!type || name.empty()) +return CompilerType(); + + clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type)); + const clang::Type::TypeClass type_class = qual_type->getTypeClass(); + + switch (type_class) { + case clang::Type::Record: { +if (!GetCompleteType(type)) + return CompilerType(); +const clang::RecordType *record_type = +llvm::cast(qual_type.getTypePtr()); +const clang::RecordDecl *record_decl = record_type->getDecl(); + +clang::DeclarationName decl_name(().Idents.get(name)); +for (NamedDecl *decl : record_decl->lookup(decl_name)) { + if (auto *tag_decl = dyn_cast(decl)) +return GetType(getASTContext().getTagDeclType(tag_decl)); +} +break; + } + default: +break; + } + return CompilerType(); +} + bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) { if (!type) return false; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 05c303baa41640..cfe5327e14f823 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -897,6 +897,10 @@ class TypeSystemClang : public TypeSystem { bool omit_empty_base_classes, std::vector _indexes) override; + CompilerType + GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type, + llvm::StringRef name) override; + bool IsTemplateType(lldb::opaque_compiler_type_t type) override; size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type, diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp index 44a24d7178f562..5c49c4726157fb 100644 --- a/lldb/source/Symbol/Type.cpp +++ b/lldb/source/Symbol/Type.cpp @@ -1177,20 +1177,7 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef
[Lldb-commits] [lldb] 3c721b9 - Revert "[lldb] Fix evaluation of expressions with static initializers (#89063)"
Author: Pavel Labath Date: 2024-04-18T07:30:18Z New Revision: 3c721b90d363bf73b78467f6e86c879235bac1b2 URL: https://github.com/llvm/llvm-project/commit/3c721b90d363bf73b78467f6e86c879235bac1b2 DIFF: https://github.com/llvm/llvm-project/commit/3c721b90d363bf73b78467f6e86c879235bac1b2.diff LOG: Revert "[lldb] Fix evaluation of expressions with static initializers (#89063)" It breaks expression evaluation on arm, and the x86 breakage has been fixed in 6cea7c491f4c4c68aa0494a9b18f36ff40c22c81. This reverts commit 915c84b1480bb3c6d2e44ca83822d2c2304b763a. Added: Modified: lldb/source/Expression/IRExecutionUnit.cpp Removed: diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp index 7ad0e5ff22b2f6..cb9bee8733e15d 100644 --- a/lldb/source/Expression/IRExecutionUnit.cpp +++ b/lldb/source/Expression/IRExecutionUnit.cpp @@ -13,7 +13,6 @@ #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" -#include "llvm/Support/CodeGen.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/raw_ostream.h" @@ -280,13 +279,10 @@ void IRExecutionUnit::GetRunnableInfo(Status , lldb::addr_t _addr, llvm::EngineBuilder builder(std::move(m_module_up)); llvm::Triple triple(m_module->getTargetTriple()); - // PIC needed for ELF to avoid generating 32-bit relocations (which overflow - // if the object is loaded into high memory). - bool want_pic = triple.isOSBinFormatMachO() || triple.isOSBinFormatELF(); - builder.setEngineKind(llvm::EngineKind::JIT) .setErrorStr(_string) - .setRelocationModel(want_pic ? llvm::Reloc::PIC_ : llvm::Reloc::Static) + .setRelocationModel(triple.isOSBinFormatMachO() ? llvm::Reloc::PIC_ + : llvm::Reloc::Static) .setMCJITMemoryManager(std::make_unique(*this)) .setOptLevel(llvm::CodeGenOptLevel::Less); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
@@ -572,9 +572,14 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() { ModuleSpec module_spec(file, target.GetArchitecture()); if (ModuleSP module_sp = target.GetOrCreateModule(module_spec, -true /* notify */)) { +false /* notify */)) { labath wrote: Might as well change to the [official style](https://llvm.org/docs/CodingStandards.html#comment-formatting): `/*notify=*/false` https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
labath wrote: So, could the fix be as simple as passing notify=false in the first call ? https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix evaluation of expressions with static initializers (PR #89063)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/89063 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
labath wrote: > > > > why not just call ModulesDidLoad and delegate this (and possibly other > > > > binary-just-loaded) behaviors to it? > > > > > > > > > That's what I did first, but it breaks the test > > > `TestModuleLoadedNotifys.ModuleLoadedNotifysTestCase.test_launch_notifications` > > > because of extra broadcaster event. So, I moved out the part just > > > updating breakpoints. > > > > > > The point of the test is to ensure you don't get a hundred notifications, > > one for each module. Having one notification for ld.so, and 99 for the rest > > of the modules is ok. It should be fine to just update the test to match > > new reality. > > Updated to use `Target::ModulesDisLoad` and the test. Oh, so the problem is that the loaded event gets sent twice (and not that it gets sent as a separate event, as I originally thought). While it's not the end of the world (it appears the mac loaded does something similar as well), this does seem like something that we could prevent. Can you check where does the second event get sent from? Is it by any chance when we go through [this](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L460) place ? If so, it should be fairly easy to refactor that code to avoid putting the interpreter module into the event list when it is already loaded. https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix evaluation of expressions with static initializers (PR #89063)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/89063 After 281d71604f418eb952e967d9dc4b26241b7f96a, llvm generates 32-bit relocations, which overflow when we load these objects into high memory. Interestingly, setting the code model to "large" does not help here (perhaps it is the default?). I'm not completely sure that this is the right thing to do, but it doesn't seem to cause any ill effects. I'll follow up with the author of that patch about the expected behavior here. >From e9b74aae92b8d8583d3549c6ef7257fd026678f5 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 17 Apr 2024 11:58:44 + Subject: [PATCH] [lldb] Fix evaluation of expressions with static initializers After 281d71604f418eb952e967d9dc4b26241b7f96a, llvm generates 32-bit relocations, which overflow when we load these objects into high memory. Interestingly, setting the code model to "large" does not help here (perhaps it is the default?). I'm not completely sure that this is the right thing to do, but it doesn't seem to cause any ill effects. I'll follow up with the author of that patch about the expected behavior here. --- lldb/source/Expression/IRExecutionUnit.cpp | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp index cb9bee8733e15d..7ad0e5ff22b2f6 100644 --- a/lldb/source/Expression/IRExecutionUnit.cpp +++ b/lldb/source/Expression/IRExecutionUnit.cpp @@ -13,6 +13,7 @@ #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" +#include "llvm/Support/CodeGen.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/raw_ostream.h" @@ -279,10 +280,13 @@ void IRExecutionUnit::GetRunnableInfo(Status , lldb::addr_t _addr, llvm::EngineBuilder builder(std::move(m_module_up)); llvm::Triple triple(m_module->getTargetTriple()); + // PIC needed for ELF to avoid generating 32-bit relocations (which overflow + // if the object is loaded into high memory). + bool want_pic = triple.isOSBinFormatMachO() || triple.isOSBinFormatELF(); + builder.setEngineKind(llvm::EngineKind::JIT) .setErrorStr(_string) - .setRelocationModel(triple.isOSBinFormatMachO() ? llvm::Reloc::PIC_ - : llvm::Reloc::Static) + .setRelocationModel(want_pic ? llvm::Reloc::PIC_ : llvm::Reloc::Static) .setMCJITMemoryManager(std::make_unique(*this)) .setOptLevel(llvm::CodeGenOptLevel::Less); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/88494 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)
@@ -0,0 +1,48 @@ +#include "pseudo_barrier.h" +#include +#include +#include +#include +#include +#include + +pseudo_barrier_t barrier; + +constexpr size_t nthreads = 5; +volatile bool wait_for_debugger_flag = true; + +void break_here() {} + +void tfunc() { + pseudo_barrier_wait(barrier); + + break_here(); +} + +int main(int argc, char const *argv[]) { + lldb_enable_attach(); labath wrote: Yes, but it's not really a "special" OS. Most linux systems are configured like that by default, where you're only allowed to debug your own children (unless they explicitly allow that, or you're root, etc.). Check out https://www.kernel.org/doc/html/v4.15/admin-guide/LSM/Yama.html. lldb_enable_attach is our own invention, which calls the appropriate os interface to enable attaching if necessary. Currently, it's only implemented on linux. https://github.com/llvm/llvm-project/pull/88494 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)
@@ -0,0 +1,59 @@ +""" +Test that the process continues running after we detach from it. +""" + +import lldb +import time +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class DetachResumesTestCase(TestBase): +NO_DEBUG_INFO_TESTCASE = True + +def test_detach_resumes(self): +self.build() +exe = self.getBuildArtifact() + +# The inferior will use this file to let us know it is ready to be +# attached. +sync_file_path = lldbutil.append_to_process_working_directory( +self, "sync_file_%d" % (int(time.time())) +) + +# And this one to let us know it is running after we've detached from +# it. +exit_file_path = lldbutil.append_to_process_working_directory( +self, "exit_file_%d" % (int(time.time())) +) + +popen = self.spawnSubprocess( +self.getBuildArtifact(exe), [sync_file_path, exit_file_path] +) +lldbutil.wait_for_file_on_target(self, sync_file_path) + +self.runCmd("process attach -p " + str(popen.pid)) + +# Set a breakpoint at a place that will be called by multiple threads +# simultaneously. On systems (e.g. linux) where the debugger needs to +# send signals to suspend threads, these signals will race with threads +# hitting the breakpoint (and stopping on their own). +bpno = lldbutil.run_break_set_by_symbol(self, "break_here") + +# And let the inferior know it can call the function. +self.runCmd("expr -- wait_for_debugger_flag = false") + +self.runCmd("continue") + +self.expect( +"thread list", +STOPPED_DUE_TO_BREAKPOINT, +substrs=["stopped", "stop reason = breakpoint"], +) + +# Detach, the process should keep running after this, and not be stopped +# by the signals that the debugger may have used to suspend the threads. +self.runCmd("detach") + +lldbutil.wait_for_file_on_target(self, exit_file_path) labath wrote: Yes, and just in case it does not, the file name has an embedded time stamp (mainly a remnant of the time where we were not automatically cleaning up the test dir :) ) https://github.com/llvm/llvm-project/pull/88494 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
labath wrote: > > why not just call ModulesDidLoad and delegate this (and possibly other > > binary-just-loaded) behaviors to it? > > That's what I did first, but it breaks the test > `TestModuleLoadedNotifys.ModuleLoadedNotifysTestCase.test_launch_notifications` > because of extra broadcaster event. So, I moved out the part just updating > breakpoints. The point of the test is to ensure you don't get a hundred notifications, one for each module. Having one notification for ld.so, and 99 for the rest of the modules is ok. It should be fine to just update the test to match new reality. https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/88494 >From c6b2c5e58321e72155b8c45cd3591487c1cafacf Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Sat, 6 Apr 2024 17:51:12 + Subject: [PATCH] [lldb/linux] Make sure the process continues running after a detach Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all other threads in the process to force them to stop as well. If those threads stop on their own before they get a signal, this SIGSTOP will remain pending and be delivered the next time the process resumes. Normally, this is not a problem, because lldb-server will detect this stale SIGSTOP and resume the process. However, if we detach from the process while it has these SIGSTOPs pending, they will get immediately delivered, and the process will remain stopped (most likely forever). This patch fixes that by sending a SIGCONT just before detaching from the process. This signal cancels out any pending SIGSTOPs, and ensures it is able to run after we detach. It does have one somewhat unfortunate side-effect that in that the process's SIGCONT handler (if it has one) will get executed spuriously (from the process's POV). This could be _sometimes_ avoided by tracking which threads got send a SIGSTOP, and whether those threads stopped due to it. From what I could tell by observing its behavior, this is what gdb does. I have not tried to replicate that behavior here because it adds a nontrivial amount of complexity and the result is still uncertain -- we still need to send a SIGCONT (and execute the handler) when any thread stops for some other reason (and leaves our SIGSTOP hanging). Furthermore, since SIGSTOPs don't stack, it's also possible that our SIGSTOP/SIGCONT combination will cancel a genuine SIGSTOP being sent to the debugger application (by someone else), and there is nothing we can do about that. For this reason I think it's simplest and most predictible to just always send a SIGCONT when detaching, but if it turns out this is breaking something, we can consider implementing something more elaborate. One alternative I did try is to use PTRACE_INTERRUPT to suspend the threads instead of a SIGSTOP. PTRACE_INTERUPT requires using PTRACE_SEIZE to attach to the process, which also made this solution somewhat complicated, but the main problem with that approach is that PTRACE_INTERRUPT is not considered to be a signal-delivery-stop, which means it's not possible to resume it while injecting another signal to the inferior (which some of our tests expect to be able to do). This limitation could be worked around by forcing the thread into a signal delivery stop whenever we need to do this, but this additional complication is what made me think this approach is also not worthwhile. This patch should fix (at least some of) the problems with TestConcurrentVFork, but I've also added a dedicated test for checking that a process keeps running after we detach. Although the problem I'm fixing here is linux-specific, the core functinoality of not stopping after a detach should function the same way everywhere. --- .../Process/Linux/NativeProcessLinux.cpp | 4 ++ .../commands/process/detach-resumes/Makefile | 4 ++ .../detach-resumes/TestDetachResumes.py | 59 +++ .../commands/process/detach-resumes/main.cpp | 48 +++ .../concurrent_vfork/TestConcurrentVFork.py | 16 - 5 files changed, 115 insertions(+), 16 deletions(-) create mode 100644 lldb/test/API/commands/process/detach-resumes/Makefile create mode 100644 lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py create mode 100644 lldb/test/API/commands/process/detach-resumes/main.cpp diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index 5d2b4b03fe60cb..59fc8726b76739 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1089,6 +1089,10 @@ Status NativeProcessLinux::Detach() { if (GetID() == LLDB_INVALID_PROCESS_ID) return error; + // Cancel out any SIGSTOPs we may have sent while stopping the process. + // Otherwise, the process may stop as soon as we detach from it. + kill(GetID(), SIGCONT); + for (const auto : m_threads) { Status e = Detach(thread->GetID()); if (e.Fail()) diff --git a/lldb/test/API/commands/process/detach-resumes/Makefile b/lldb/test/API/commands/process/detach-resumes/Makefile new file mode 100644 index 00..c46619c6623481 --- /dev/null +++ b/lldb/test/API/commands/process/detach-resumes/Makefile @@ -0,0 +1,4 @@ +CXX_SOURCES := main.cpp +ENABLE_THREADS := YES + +include Makefile.rules diff --git a/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py new file mode 100644 index 00..57727294ddc3d3 --- /dev/null +++
[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/88494 >From 8a9837bd306377ec44efdb8a4ff25f0132496cc0 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Sat, 6 Apr 2024 17:51:12 + Subject: [PATCH] [lldb/linux] Make sure the process continues running after a detach Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all other threads in the process to force them to stop as well. If those threads stop on their own before they get a signal, this SIGSTOP will remain pending and be delivered the next time the process resumes. Normally, this is not a problem, because lldb-server will detect this stale SIGSTOP and resume the process. However, if we detach from the process while it has these SIGSTOPs pending, they will get immediately delivered, and the process will remain stopped (most likely forever). This patch fixes that by sending a SIGCONT just before detaching from the process. This signal cancels out any pending SIGSTOPs, and ensures it is able to run after we detach. It does have one somewhat unfortunate side-effect that in that the process's SIGCONT handler (if it has one) will get executed spuriously (from the process's POV). This could be _sometimes_ avoided by tracking which threads got send a SIGSTOP, and whether those threads stopped due to it. From what I could tell by observing its behavior, this is what gdb does. I have not tried to replicate that behavior here because it adds a nontrivial amount of complexity and the result is still uncertain -- we still need to send a SIGCONT (and execute the handler) when any thread stops for some other reason (and leaves our SIGSTOP hanging). Furthermore, since SIGSTOPs don't stack, it's also possible that our SIGSTOP/SIGCONT combination will cancel a genuine SIGSTOP being sent to the debugger application (by someone else), and there is nothing we can do about that. For this reason I think it's simplest and most predictible to just always send a SIGCONT when detaching, but if it turns out this is breaking something, we can consider implementing something more elaborate. One alternative I did try is to use PTRACE_INTERRUPT to suspend the threads instead of a SIGSTOP. PTRACE_INTERUPT requires using PTRACE_SEIZE to attach to the process, which also made this solution somewhat complicated, but the main problem with that approach is that PTRACE_INTERRUPT is not considered to be a signal-delivery-stop, which means it's not possible to resume it while injecting another signal to the inferior (which some of our tests expect to be able to do). This limitation could be worked around by forcing the thread into a signal delivery stop whenever we need to do this, but this additional complication is what made me think this approach is also not worthwhile. This patch should fix (at least some of) the problems with TestConcurrentVFork, but I've also added a dedicated test for checking that a process keeps running after we detach. Although the problem I'm fixing here is linux-specific, the core functinoality of not stopping after a detach should function the same way everywhere. --- .../Process/Linux/NativeProcessLinux.cpp | 4 ++ .../commands/process/detach-resumes/Makefile | 4 ++ .../detach-resumes/TestDetachResumes.py | 57 +++ .../commands/process/detach-resumes/main.cpp | 48 .../concurrent_vfork/TestConcurrentVFork.py | 16 -- 5 files changed, 113 insertions(+), 16 deletions(-) create mode 100644 lldb/test/API/commands/process/detach-resumes/Makefile create mode 100644 lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py create mode 100644 lldb/test/API/commands/process/detach-resumes/main.cpp diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index 5d2b4b03fe60cb..59fc8726b76739 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1089,6 +1089,10 @@ Status NativeProcessLinux::Detach() { if (GetID() == LLDB_INVALID_PROCESS_ID) return error; + // Cancel out any SIGSTOPs we may have sent while stopping the process. + // Otherwise, the process may stop as soon as we detach from it. + kill(GetID(), SIGCONT); + for (const auto : m_threads) { Status e = Detach(thread->GetID()); if (e.Fail()) diff --git a/lldb/test/API/commands/process/detach-resumes/Makefile b/lldb/test/API/commands/process/detach-resumes/Makefile new file mode 100644 index 00..c46619c6623481 --- /dev/null +++ b/lldb/test/API/commands/process/detach-resumes/Makefile @@ -0,0 +1,4 @@ +CXX_SOURCES := main.cpp +ENABLE_THREADS := YES + +include Makefile.rules diff --git a/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py new file mode 100644 index 00..ab2ed8a6d24c85 --- /dev/null +++
[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/88494 Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all other threads in the process to force them to stop as well. If those threads stop on their own before they get a signal, this SIGSTOP will remain pending and be delivered the next time the process resumes. Normally, this is not a problem, because lldb-server will detect this stale SIGSTOP and resume the process. However, if we detach from the process while it has these SIGSTOPs pending, they will get immediately delivered, and the process will remain stopped (most likely forever). This patch fixes that by sending a SIGCONT just before detaching from the process. This signal cancels out any pending SIGSTOPs, and ensures it is able to run after we detach. It does have one somewhat unfortunate side-effect that in that the process's SIGCONT handler (if it has one) will get executed spuriously (from the process's POV). This could be _sometimes_ avoided by tracking which threads got send a SIGSTOP, and whether those threads stopped due to it. From what I could tell by observing its behavior, this is what gdb does. I have not tried to replicate that behavior here because it adds a nontrivial amount of complexity and the result is still uncertain -- we still need to send a SIGCONT (and execute the handler) when any thread stops for some other reason (and leaves our SIGSTOP hanging). Furthermore, since SIGSTOPs don't stack, it's also possible that our SIGSTOP/SIGCONT combination will cancel a genuine SIGSTOP being sent to the debugger application (by someone else), and there is nothing we can do about that. For this reason I think it's simplest and most predictible to just always send a SIGCONT when detaching, but if it turns out this is breaking something, we can consider implementing something more elaborate. One alternative I did try is to use PTRACE_INTERRUPT to suspend the threads instead of a SIGSTOP. PTRACE_INTERUPT requires using PTRACE_SEIZE to attach to the process, which also made this solution somewhat complicated, but the main problem with that approach is that PTRACE_INTERRUPT is not considered to be a signal-delivery-stop, which means it's not possible to resume it while injecting another signal to the inferior (which some of our tests expect to be able to do). This limitation could be worked around by forcing the thread into a signal delivery stop whenever we need to do this, but this additional complication is what made me think this approach is also not worthwhile. This patch should fix (at least some of) the problems with TestConcurrentVFork, but I've also added a dedicated test for checking that a process keeps running after we detach. Although the problem I'm fixing here is linux-specific, the core functinoality of not stopping after a detach should function the same way everywhere. >From 9e1aca01da4a4aa5039c6ac1f8320f8c7d85b8e3 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Sat, 6 Apr 2024 17:51:12 + Subject: [PATCH] [lldb/linux] Make sure the process continues running after a detach Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all other threads in the process to force them to stop as well. If those threads stop on their own before they get a signal, this SIGSTOP will remain pending and be delivered the next time the process resumes. Normally, this is not a problem, because lldb-server will detect this stale SIGSTOP and resume the process. However, if we detach from the process while it has these SIGSTOPs pending, they will get immediately delivered, and the process will remain stopped (most likely forever). This patch fixes that by sending a SIGCONT just before detaching from the process. This signal cancels out any pending SIGSTOPs, and ensures it is able to run after we detach. It does have one somewhat unfortunate side-effect that in that the process's SIGCONT handler (if it has one) will get executed spuriously (from the process's POV). This could be _sometimes_ avoided by tracking which threads got send a SIGSTOP, and whether those threads stopped due to it. From what I could tell by observing its behavior, this is what gdb does. I have not tried to replicate that behavior here because it adds a nontrivial amount of complexity and the result is still uncertain -- we still need to send a SIGCONT (and execute the handler) when any thread stops for some other reason (and leaves our SIGSTOP hanging). Furthermore, since SIGSTOPs don't stack, it's also possible that our SIGSTOP/SIGCONT combination will cancel a genuine SIGSTOP being sent to the debugger application (by someone else), and there is nothing we can do about that. For this reason I think it's simplest and most predictible to just always send a SIGCONT when detaching, but if it turns out this is breaking something, we can consider implementing something more elaborate. One alternative I did try is to use
[Lldb-commits] [lldb] 78b00c1 - Revert "[lldb] Improve maintainability and readability for ValueObject methods (#75865)"
Author: Pavel Labath Date: 2024-01-24T07:12:52Z New Revision: 78b00c116be8b3b53ff13552e31eb305b11cb169 URL: https://github.com/llvm/llvm-project/commit/78b00c116be8b3b53ff13552e31eb305b11cb169 DIFF: https://github.com/llvm/llvm-project/commit/78b00c116be8b3b53ff13552e31eb305b11cb169.diff LOG: Revert "[lldb] Improve maintainability and readability for ValueObject methods (#75865)" This reverts commit d657519838e4b2310e13ec5ff52599e041860825 as it breaks two dozen tests. The breakages are related to variable path expression parsing and summary string parsing (possibly the same code). Added: Modified: lldb/source/Core/ValueObject.cpp Removed: diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index d58bf2ca763d9e2..9208047be36668d 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -1582,64 +1582,62 @@ bool ValueObject::IsUninitializedReference() { ValueObjectSP ValueObject::GetSyntheticArrayMember(size_t index, bool can_create) { - if (!IsPointerType() && !IsArrayType()) -return ValueObjectSP(); - - std::string index_str = llvm::formatv("[{0}]", index); - ConstString index_const_str(index_str); - // Check if we have already created a synthetic array member in this valid - // object. If we have we will re-use it. - if (auto existing_synthetic_child = GetSyntheticChild(index_const_str)) -return existing_synthetic_child; - - // We haven't made a synthetic array member for INDEX yet, so lets make - // one and cache it for any future reference. - ValueObject *synthetic_child = CreateChildAtIndex(0, true, index); - - if (!synthetic_child) -return ValueObjectSP(); - - // Cache the synthetic child's value because it's valid. - AddSyntheticChild(index_const_str, synthetic_child); - auto synthetic_child_sp = synthetic_child->GetSP(); - synthetic_child_sp->SetName(ConstString(index_str)); - synthetic_child_sp->m_flags.m_is_array_item_for_pointer = true; + ValueObjectSP synthetic_child_sp; + if (IsPointerType() || IsArrayType()) { +std::string index_str = llvm::formatv("[{0}]", index); +ConstString index_const_str(index_str); +// Check if we have already created a synthetic array member in this valid +// object. If we have we will re-use it. +synthetic_child_sp = GetSyntheticChild(index_const_str); +if (!synthetic_child_sp) { + ValueObject *synthetic_child; + // We haven't made a synthetic array member for INDEX yet, so lets make + // one and cache it for any future reference. + synthetic_child = CreateChildAtIndex(0, true, index); + + // Cache the value if we got one back... + if (synthetic_child) { +AddSyntheticChild(index_const_str, synthetic_child); +synthetic_child_sp = synthetic_child->GetSP(); +synthetic_child_sp->SetName(ConstString(index_str)); +synthetic_child_sp->m_flags.m_is_array_item_for_pointer = true; + } +} + } return synthetic_child_sp; } ValueObjectSP ValueObject::GetSyntheticBitFieldChild(uint32_t from, uint32_t to, bool can_create) { - if (!IsScalarType()) -return ValueObjectSP(); - - std::string index_str = llvm::formatv("[{0}-{1}]", from, to); - ConstString index_const_str(index_str); - - // Check if we have already created a synthetic array member in this valid - // object. If we have we will re-use it. - if (auto existing_synthetic_child = GetSyntheticChild(index_const_str)) -return existing_synthetic_child; - - uint32_t bit_field_size = to - from + 1; - uint32_t bit_field_offset = from; - if (GetDataExtractor().GetByteOrder() == eByteOrderBig) -bit_field_offset = -GetByteSize().value_or(0) * 8 - bit_field_size - bit_field_offset; - - // We haven't made a synthetic array member for INDEX yet, so lets make - // one and cache it for any future reference. - ValueObjectChild *synthetic_child = new ValueObjectChild( - *this, GetCompilerType(), index_const_str, GetByteSize().value_or(0), 0, - bit_field_size, bit_field_offset, false, false, eAddressTypeInvalid, 0); - - if (!synthetic_child) -return ValueObjectSP(); - - // Cache the synthetic child's value because it's valid. - AddSyntheticChild(index_const_str, synthetic_child); - auto synthetic_child_sp = synthetic_child->GetSP(); - synthetic_child_sp->SetName(ConstString(index_str)); - synthetic_child_sp->m_flags.m_is_bitfield_for_scalar = true; + ValueObjectSP synthetic_child_sp; + if (IsScalarType()) { +std::string index_str = llvm::formatv("[{0}-{1}]", from, to); +ConstString index_const_str(index_str); +// Check if we have already created a synthetic array member in this valid +// object. If we have we will re-use it. +synthetic_child_sp =
[Lldb-commits] [lldb] Ensure that the executable module is ModuleList[0] (PR #78360)
labath wrote: > Will the value of ObjectFile::GetType ever be meaningful, then? It's partially meaningful. eTypeCoreFile, eTypeObjectFile and eTypeExecutable (when it is set) should mean what they say. The problem is with eTypeSharedLibrary (and to a lesser degree with eTypeDebugInfo). Just because an object is a shared library doesn't mean that it can be executed (as the main executable or the dynamic linker). It's not that these concepts don't exist, it's just that it's not possible to determine the role of a file in isolation. I don't think that was anyone's intention, it's just ELF (unlike, say, MachO) doesn't have the flags to differentiate these. > Do you change it after running based on what the loader tells you? We don't, and I don't think we should, because that's about a role that the object plays in a specific process. In theory you could use that object in a different role in another process. > So long as ObjectFileELF::CalculateType never returns ET_EXEC, this won't be > an issue. But if some ObjectFile that's not in fact the main binary says it > has this type, not only will this change to Append not do the right thing, > Target::GetExecutableModule will also return this module. CalculateType() will return eTypeExecutable for ET_EXEC, which is used for non-position-independent executables, and I don't think you can load those as libraries (the dynamic loader will not let them. However, there is nothing stopping a user from adding that module manually (exactly like your test does) and that could mess things up a bit. Not too much, because (outside of Swift, I guess) we don't care that much about which module is the main executable, but it will confuse anyone who asks for it. To be honest, I don't particularly like this functionality of automatically shuffling module lists. I think it would be better to have separate API to set the executable module or tag an existing module as such. It's probably the only thing that can reliably work for elf, but I also think its clearer/more explicit in general. https://github.com/llvm/llvm-project/pull/78360 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Ensure that the executable module is ModuleList[0] (PR #78360)
labath wrote: The problem is that linux/elf does not really have hard line between shared libraries, (position-independent) executables and the dynamic linker. they all have `e_type = ET_DYN` in their header. It is possible to create a shared library that can also be executed (unless one is *extremely* careful it will crash very quickly, but that crash may exactly be something that one may want to observe in a debugger). It's probably also possible to create a library that will also serve as the dynamic loader. So, the "is this module an executable" question is not really well defined. This isn't an intrinsic property of the module, but rather a role that the module plays in a specific process. Once a process starts, this can be (and is) determined by the dynamic linker plugin. Before that, we need to rely on some external signal for that (if we need to know that at all). With that in mind, I'm going to disable the before-run part of the test for elf. https://github.com/llvm/llvm-project/pull/78360 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 15311d5 - [lldb] Skip TestExecutableFirst.test_executable_is_first_before_run on ELF
Author: Pavel Labath Date: 2024-01-17T10:36:44Z New Revision: 15311d5822f5fcaf53bc7cfc728ad2b477a430e4 URL: https://github.com/llvm/llvm-project/commit/15311d5822f5fcaf53bc7cfc728ad2b477a430e4 DIFF: https://github.com/llvm/llvm-project/commit/15311d5822f5fcaf53bc7cfc728ad2b477a430e4.diff LOG: [lldb] Skip TestExecutableFirst.test_executable_is_first_before_run on ELF ELF does not have a hard distinction between shared libraries (and position-independent) executables. It is possible to create a shared library that will also be executable. Added: Modified: lldb/test/API/functionalities/executable_first/TestExecutableFirst.py Removed: diff --git a/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py b/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py index 957628f695c128..4f753239f3dbb8 100644 --- a/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py +++ b/lldb/test/API/functionalities/executable_first/TestExecutableFirst.py @@ -10,6 +10,9 @@ class TestExecutableIsFirst(TestBase): NO_DEBUG_INFO_TESTCASE = True +# ELF does not have a hard distinction between shared libraries and +# (position-independent) executables +@skipIf(oslist=no_match(lldbplatformutil.getDarwinOSTriples()+["windows"])) def test_executable_is_first_before_run(self): self.build() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 14268ad - [lldb] Skip part of TestDataFormatterAdv (#72233)
Author: Pavel Labath Date: 2024-01-15T12:49:24Z New Revision: 14268ad2a2ea0b3bbe6b767d67ace1d0ae992a6d URL: https://github.com/llvm/llvm-project/commit/14268ad2a2ea0b3bbe6b767d67ace1d0ae992a6d DIFF: https://github.com/llvm/llvm-project/commit/14268ad2a2ea0b3bbe6b767d67ace1d0ae992a6d.diff LOG: [lldb] Skip part of TestDataFormatterAdv (#72233) libstdc++ data formatter simply forwards to the `const char *` formatter -- which means it suffers from the same problem/bug as that one. Added: Modified: lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py Removed: diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py b/lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py index c275904eaf2014..d296e60d6e6e99 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py @@ -6,6 +6,7 @@ import lldb from lldbsuite.test.lldbtest import * import lldbsuite.test.lldbutil as lldbutil +import re class AdvDataFormatterTestCase(TestBase): @@ -298,7 +299,11 @@ def cleanup(): self.runCmd("settings set target.max-string-summary-length 5") some_string = self.frame().FindVariable("some_string") some_string_summary = some_string.GetSummary() -self.assertEqual(some_string_summary, '"01234"...') +if (re.match(r"^std::__\w+::", some_string.GetTypeName())): + self.assertEqual(some_string_summary, '"01234"...') +else: + #libstdc++ string formatter suffers from the same problem as some_cstring below + pass some_carr = self.frame().FindVariable("some_carr") some_carr_summary = some_carr.GetSummary() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix a quirk in SBValue::GetDescription (PR #75793)
labath wrote: Fixed by https://github.com/llvm/llvm-project/pull/75908. https://github.com/llvm/llvm-project/pull/75793 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestSBValueSynthetic on windows (PR #75908)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/75908 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestSBValueSynthetic on windows (PR #75908)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/75908 >From b8dbd31b95058f8098f9ef57c540a1635a9a1fde Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 19 Dec 2023 09:37:20 +0100 Subject: [PATCH 1/2] [lldb] Fix TestSBValueSynthetic on windows We don't have a std::vector formatter on windows, so use a custom formatter in this test to avoid relying on std::vector. --- .../sbvalue_synthetic/TestSBValueSynthetic.py | 10 .../python_api/sbvalue_synthetic/formatter.py | 23 +++ .../API/python_api/sbvalue_synthetic/main.cpp | 12 ++ 3 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 lldb/test/API/python_api/sbvalue_synthetic/formatter.py diff --git a/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py b/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py index 5dcf3c1a9c6c44..62a9477533ba44 100644 --- a/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py +++ b/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py @@ -12,8 +12,10 @@ def test_str(self): lldbutil.run_to_source_breakpoint( self, "break here", lldb.SBFileSpec("main.cpp") ) +self.runCmd("command script import formatter.py") +self.runCmd("type synthetic add --python-class formatter.FooSyntheticProvider Foo") -vector = self.frame().FindVariable("vector") -has_vector = self.frame().FindVariable("has_vector") -self.expect(str(vector), exe=False, substrs=["42", "47"]) -self.expect(str(has_vector), exe=False, substrs=["42", "47"]) +formatted = self.frame().FindVariable("foo") +has_formatted = self.frame().FindVariable("has_foo") +self.expect(str(formatted), exe=False, substrs=["synth_child"]) +self.expect(str(has_formatted), exe=False, substrs=["synth_child"]) diff --git a/lldb/test/API/python_api/sbvalue_synthetic/formatter.py b/lldb/test/API/python_api/sbvalue_synthetic/formatter.py new file mode 100644 index 00..17a436c37ed0f3 --- /dev/null +++ b/lldb/test/API/python_api/sbvalue_synthetic/formatter.py @@ -0,0 +1,23 @@ +import lldb + + +class FooSyntheticProvider: +def __init__(self, valobj, dict): +target = valobj.GetTarget() +data = lldb.SBData.CreateDataFromCString(lldb.eByteOrderLittle, 8, "S") +self._child = valobj.CreateValueFromData( +"synth_child", data, target.GetBasicType(lldb.eBasicTypeChar) +) + +def num_children(self): +return 1 + +def get_child_at_index(self, index): +if index != 0: +return None +return self._child + +def get_child_index(self, name): +if name == "synth_child": +return 0 +return None diff --git a/lldb/test/API/python_api/sbvalue_synthetic/main.cpp b/lldb/test/API/python_api/sbvalue_synthetic/main.cpp index e6b6ec50f307f8..52c6474d7a1b28 100644 --- a/lldb/test/API/python_api/sbvalue_synthetic/main.cpp +++ b/lldb/test/API/python_api/sbvalue_synthetic/main.cpp @@ -1,11 +1,13 @@ -#include +struct Foo { + int real_child = 47; +}; -struct HasVector { - std::vector v; +struct HasFoo { + Foo f; }; int main() { - std::vector vector = {42, 47}; - HasVector has_vector = {vector}; + Foo foo; + HasFoo has_foo; return 0; // break here } >From beb831ccee568bc2cee79e3fbcacc07866700b32 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 19 Dec 2023 09:54:45 +0100 Subject: [PATCH 2/2] fixup! [lldb] Fix TestSBValueSynthetic on windows --- .../API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py | 4 +++- lldb/test/API/python_api/sbvalue_synthetic/formatter.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py b/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py index 62a9477533ba44..2fd1e0ce9c6a3d 100644 --- a/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py +++ b/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py @@ -13,7 +13,9 @@ def test_str(self): self, "break here", lldb.SBFileSpec("main.cpp") ) self.runCmd("command script import formatter.py") -self.runCmd("type synthetic add --python-class formatter.FooSyntheticProvider Foo") +self.runCmd( +"type synthetic add --python-class formatter.FooSyntheticProvider Foo" +) formatted = self.frame().FindVariable("foo") has_formatted = self.frame().FindVariable("has_foo") diff --git a/lldb/test/API/python_api/sbvalue_synthetic/formatter.py b/lldb/test/API/python_api/sbvalue_synthetic/formatter.py index 17a436c37ed0f3..65e65afc3ef186 100644 --- a/lldb/test/API/python_api/sbvalue_synthetic/formatter.py +++ b/lldb/test/API/python_api/sbvalue_synthetic/formatter.py @@ -6,7 +6,7 @@ def __init__(self, valobj, dict): target =
[Lldb-commits] [lldb] [lldb] Fix TestSBValueSynthetic on windows (PR #75908)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/75908 We don't have a std::vector formatter on windows, so use a custom formatter in this test to avoid relying on std::vector. >From b8dbd31b95058f8098f9ef57c540a1635a9a1fde Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 19 Dec 2023 09:37:20 +0100 Subject: [PATCH] [lldb] Fix TestSBValueSynthetic on windows We don't have a std::vector formatter on windows, so use a custom formatter in this test to avoid relying on std::vector. --- .../sbvalue_synthetic/TestSBValueSynthetic.py | 10 .../python_api/sbvalue_synthetic/formatter.py | 23 +++ .../API/python_api/sbvalue_synthetic/main.cpp | 12 ++ 3 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 lldb/test/API/python_api/sbvalue_synthetic/formatter.py diff --git a/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py b/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py index 5dcf3c1a9c6c44..62a9477533ba44 100644 --- a/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py +++ b/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py @@ -12,8 +12,10 @@ def test_str(self): lldbutil.run_to_source_breakpoint( self, "break here", lldb.SBFileSpec("main.cpp") ) +self.runCmd("command script import formatter.py") +self.runCmd("type synthetic add --python-class formatter.FooSyntheticProvider Foo") -vector = self.frame().FindVariable("vector") -has_vector = self.frame().FindVariable("has_vector") -self.expect(str(vector), exe=False, substrs=["42", "47"]) -self.expect(str(has_vector), exe=False, substrs=["42", "47"]) +formatted = self.frame().FindVariable("foo") +has_formatted = self.frame().FindVariable("has_foo") +self.expect(str(formatted), exe=False, substrs=["synth_child"]) +self.expect(str(has_formatted), exe=False, substrs=["synth_child"]) diff --git a/lldb/test/API/python_api/sbvalue_synthetic/formatter.py b/lldb/test/API/python_api/sbvalue_synthetic/formatter.py new file mode 100644 index 00..17a436c37ed0f3 --- /dev/null +++ b/lldb/test/API/python_api/sbvalue_synthetic/formatter.py @@ -0,0 +1,23 @@ +import lldb + + +class FooSyntheticProvider: +def __init__(self, valobj, dict): +target = valobj.GetTarget() +data = lldb.SBData.CreateDataFromCString(lldb.eByteOrderLittle, 8, "S") +self._child = valobj.CreateValueFromData( +"synth_child", data, target.GetBasicType(lldb.eBasicTypeChar) +) + +def num_children(self): +return 1 + +def get_child_at_index(self, index): +if index != 0: +return None +return self._child + +def get_child_index(self, name): +if name == "synth_child": +return 0 +return None diff --git a/lldb/test/API/python_api/sbvalue_synthetic/main.cpp b/lldb/test/API/python_api/sbvalue_synthetic/main.cpp index e6b6ec50f307f8..52c6474d7a1b28 100644 --- a/lldb/test/API/python_api/sbvalue_synthetic/main.cpp +++ b/lldb/test/API/python_api/sbvalue_synthetic/main.cpp @@ -1,11 +1,13 @@ -#include +struct Foo { + int real_child = 47; +}; -struct HasVector { - std::vector v; +struct HasFoo { + Foo f; }; int main() { - std::vector vector = {42, 47}; - HasVector has_vector = {vector}; + Foo foo; + HasFoo has_foo; return 0; // break here } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix a quirk in SBValue::GetDescription (PR #75793)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/75793 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix a quirk in SBValue::GetDescription (PR #75793)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/75793 The function was using the default version of ValueObject::Dump, which has a default of using the synthetic-ness of the top-level value for determining whether to print _all_ values as synthetic. This resulted in some unusual behavior, where e.g. a std::vector is stringified as synthetic if its dumped as the top level object, but in its raw form if it is a member of a struct without a pretty printer. The SBValue class already has properties which determine whether one should be looking at the synthetic view of the object (and also whether to use dynamic types), so it seems more natural to use that. >From 0c727d2c339893b439c5a17cf9ba6ae03b5cf87e Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 18 Dec 2023 14:25:42 +0100 Subject: [PATCH] [lldb] Fix a quirk in SBValue::GetDescription The function was using the default version of ValueObject::Dump, which has a default of using the synthetic-ness of the top-level value for determining whether to print _all_ values as synthetic. This resulted in some unusual behavior, where e.g. a std::vector is stringified as synthetic if its dumped as the top level object, but in its raw form if it is a member of a struct without a pretty printer. The SBValue class already has properties which determine whether one should be looking at the synthetic view of the object (and also whether to use dynamic types), so it seems more natural to use that. --- lldb/source/API/SBValue.cpp | 11 --- .../API/python_api/sbvalue_synthetic/Makefile | 3 +++ .../sbvalue_synthetic/TestSBValueSynthetic.py | 19 +++ .../API/python_api/sbvalue_synthetic/main.cpp | 11 +++ 4 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 lldb/test/API/python_api/sbvalue_synthetic/Makefile create mode 100644 lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py create mode 100644 lldb/test/API/python_api/sbvalue_synthetic/main.cpp diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp index 34d01d759ba55a..89d26a1fbe2824 100644 --- a/lldb/source/API/SBValue.cpp +++ b/lldb/source/API/SBValue.cpp @@ -24,6 +24,7 @@ #include "lldb/Core/ValueObject.h" #include "lldb/Core/ValueObjectConstResult.h" #include "lldb/DataFormatters/DataVisualization.h" +#include "lldb/DataFormatters/DumpValueObjectOptions.h" #include "lldb/Symbol/Block.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Symbol/Type.h" @@ -1209,10 +1210,14 @@ bool SBValue::GetDescription(SBStream ) { ValueLocker locker; lldb::ValueObjectSP value_sp(GetSP(locker)); - if (value_sp) -value_sp->Dump(strm); - else + if (value_sp) { +DumpValueObjectOptions options; +options.SetUseDynamicType(m_opaque_sp->GetUseDynamic()); +options.SetUseSyntheticValue(m_opaque_sp->GetUseSynthetic()); +value_sp->Dump(strm, options); + } else { strm.PutCString("No value"); + } return true; } diff --git a/lldb/test/API/python_api/sbvalue_synthetic/Makefile b/lldb/test/API/python_api/sbvalue_synthetic/Makefile new file mode 100644 index 00..8b20bcb050 --- /dev/null +++ b/lldb/test/API/python_api/sbvalue_synthetic/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py b/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py new file mode 100644 index 00..5dcf3c1a9c6c44 --- /dev/null +++ b/lldb/test/API/python_api/sbvalue_synthetic/TestSBValueSynthetic.py @@ -0,0 +1,19 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestSBValueSynthetic(TestBase): +NO_DEBUG_INFO_TESTCASE = True + +def test_str(self): +self.build() +lldbutil.run_to_source_breakpoint( +self, "break here", lldb.SBFileSpec("main.cpp") +) + +vector = self.frame().FindVariable("vector") +has_vector = self.frame().FindVariable("has_vector") +self.expect(str(vector), exe=False, substrs=["42", "47"]) +self.expect(str(has_vector), exe=False, substrs=["42", "47"]) diff --git a/lldb/test/API/python_api/sbvalue_synthetic/main.cpp b/lldb/test/API/python_api/sbvalue_synthetic/main.cpp new file mode 100644 index 00..e6b6ec50f307f8 --- /dev/null +++ b/lldb/test/API/python_api/sbvalue_synthetic/main.cpp @@ -0,0 +1,11 @@ +#include + +struct HasVector { + std::vector v; +}; + +int main() { + std::vector vector = {42, 47}; + HasVector has_vector = {vector}; + return 0; // break here +} ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d4c3c28 - [lldb] Fix Process::SyncIOHandler
Author: Pavel Labath Date: 2023-09-08T10:17:16+02:00 New Revision: d4c3c2872ff6acd75ee3e0083fa62b2a1cc5310c URL: https://github.com/llvm/llvm-project/commit/d4c3c2872ff6acd75ee3e0083fa62b2a1cc5310c DIFF: https://github.com/llvm/llvm-project/commit/d4c3c2872ff6acd75ee3e0083fa62b2a1cc5310c.diff LOG: [lldb] Fix Process::SyncIOHandler D157648 broke the function because it put the blocking wait into a critical section. This meant that, if m_iohandler_sync was not updated before entering the function, no amount of waiting would help. Fix that by restriciting the scope of the critical section to the iohandler check. Added: Modified: lldb/source/Target/Process.cpp Removed: diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 9555ff717859472..2b0774588138881 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -627,8 +627,7 @@ void Process::SyncIOHandler(uint32_t iohandler_id, const Timeout ) { // don't sync (potentially context switch) in case where there is no process // IO - std::lock_guard guard(m_process_input_reader_mutex); - if (!m_process_input_reader) + if (!ProcessIOHandlerExists()) return; auto Result = m_iohandler_sync.WaitForValueNotEqualTo(iohandler_id, timeout); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] b71ac7e - [lldb/test] Fix command-disassemble-mixed.c
Author: Pavel Labath Date: 2023-07-18T10:17:09+02:00 New Revision: b71ac7eea4c5851203432dde94241d56301a9398 URL: https://github.com/llvm/llvm-project/commit/b71ac7eea4c5851203432dde94241d56301a9398 DIFF: https://github.com/llvm/llvm-project/commit/b71ac7eea4c5851203432dde94241d56301a9398.diff LOG: [lldb/test] Fix command-disassemble-mixed.c Add it to lit.local.cfg so that it's actually run, and change it to (properly) use the %clang_host substitution. Added: Modified: lldb/test/Shell/Commands/command-disassemble-mixed.c lldb/test/Shell/Commands/lit.local.cfg Removed: diff --git a/lldb/test/Shell/Commands/command-disassemble-mixed.c b/lldb/test/Shell/Commands/command-disassemble-mixed.c index d15832cabd8402..4c4f6408d9b20e 100644 --- a/lldb/test/Shell/Commands/command-disassemble-mixed.c +++ b/lldb/test/Shell/Commands/command-disassemble-mixed.c @@ -1,6 +1,6 @@ // invalid mixed disassembly line -// RUN: %clang -g %s -o %t +// RUN: %clang_host -g %s -o %t // RUN: %lldb %t -o "dis -m -n main" -o "exit" | FileCheck %s // CHECK: int main diff --git a/lldb/test/Shell/Commands/lit.local.cfg b/lldb/test/Shell/Commands/lit.local.cfg index b83eee443fccac..0a7d1d473b23f1 100644 --- a/lldb/test/Shell/Commands/lit.local.cfg +++ b/lldb/test/Shell/Commands/lit.local.cfg @@ -1,4 +1,4 @@ -config.suffixes = ['.s', '.test', '.yaml'] +config.suffixes = ['.s', '.test', '.yaml', '.c'] # This is needed by command-target-create-resolve-exe.test config.substitutions.append(('%{PATH}', config.environment['PATH'])) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] afe8f20 - Revert "[lldb] Rate limit progress reports -- different approach [WIP-ish]"
Author: Pavel Labath Date: 2023-06-16T09:09:56+02:00 New Revision: afe8f20bb8dd1808cae542eb7ba0fc11a7886918 URL: https://github.com/llvm/llvm-project/commit/afe8f20bb8dd1808cae542eb7ba0fc11a7886918 DIFF: https://github.com/llvm/llvm-project/commit/afe8f20bb8dd1808cae542eb7ba0fc11a7886918.diff LOG: Revert "[lldb] Rate limit progress reports -- different approach [WIP-ish]" This reverts commit c30853460da7446f92bc1e516f9cbe2c5df6e136, which I pushed accidentally -- sorry. Added: Modified: lldb/include/lldb/Core/Progress.h lldb/source/Core/Progress.cpp Removed: diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h index 3bafd3d4b34ea..b2b8781a43b05 100644 --- a/lldb/include/lldb/Core/Progress.h +++ b/lldb/include/lldb/Core/Progress.h @@ -9,11 +9,9 @@ #ifndef LLDB_CORE_PROGRESS_H #define LLDB_CORE_PROGRESS_H -#include "lldb/Host/HostThread.h" #include "lldb/Utility/ConstString.h" #include "lldb/lldb-types.h" #include -#include #include #include @@ -94,26 +92,24 @@ class Progress { void Increment(uint64_t amount = 1, std::string update = {}); private: - void SendPeriodicReports(std::shared_future done); - void ReportProgress(std::string update); + void ReportProgress(std::string update = {}); static std::atomic g_id; /// The title of the progress activity. std::string m_title; + std::mutex m_mutex; /// A unique integer identifier for progress reporting. const uint64_t m_id; /// How much work ([0...m_total]) that has been completed. - std::atomic m_completed; + uint64_t m_completed; /// Total amount of work, UINT64_MAX for non deterministic progress. const uint64_t m_total; /// The optional debugger ID to report progress to. If this has no value then /// all debuggers will receive this event. std::optional m_debugger_id; - - std::mutex m_update_mutex; - std::string m_update; - - std::promise m_stop_reporting_thread; - HostThread m_reporting_thread; + /// Set to true when progress has been reported where m_completed == m_total + /// to ensure that we don't send progress updates after progress has + /// completed. + bool m_complete = false; }; } // namespace lldb_private diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp index a531b9b70437e..08be73f1470f3 100644 --- a/lldb/source/Core/Progress.cpp +++ b/lldb/source/Core/Progress.cpp @@ -9,8 +9,6 @@ #include "lldb/Core/Progress.h" #include "lldb/Core/Debugger.h" -#include "lldb/Host/ThreadLauncher.h" -#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/StreamString.h" using namespace lldb; @@ -24,67 +22,39 @@ Progress::Progress(std::string title, uint64_t total, assert(total > 0); if (debugger) m_debugger_id = debugger->GetID(); - - // Using a shared_future because std::function needs to be copyable. - if (llvm::Expected reporting_thread = - ThreadLauncher::LaunchThread( - "", - [this, future = std::shared_future( - m_stop_reporting_thread.get_future())]() { -SendPeriodicReports(future); -return lldb::thread_result_t(); - })) { -m_reporting_thread = std::move(*reporting_thread); - } else { -LLDB_LOG_ERROR(GetLog(LLDBLog::Host), reporting_thread.takeError(), - "failed to launch host thread: {}"); - } + std::lock_guard guard(m_mutex); + ReportProgress(); } Progress::~Progress() { - m_stop_reporting_thread.set_value(); - if (m_reporting_thread.IsJoinable()) { -m_reporting_thread.Join(nullptr); + // Make sure to always report progress completed when this object is + // destructed so it indicates the progress dialog/activity should go away. + std::lock_guard guard(m_mutex); + if (!m_completed) { +m_completed = m_total; +ReportProgress(); } } -void Progress::SendPeriodicReports(std::shared_future done) { - uint64_t last_completed = 0; - Debugger::ReportProgress(m_id, m_title, "", last_completed, m_total, - m_debugger_id); - - while (last_completed != m_total && - done.wait_for(std::chrono::milliseconds(100)) == - std::future_status::timeout) { -uint64_t current_completed = m_completed.load(); -if (current_completed == last_completed) - continue; - -if (current_completed == m_total || -current_completed < last_completed /*overflow*/) { - break; -} - -std::string current_update; -{ - std::lock_guard guard(m_update_mutex); - current_update = std::move(m_update); - m_update.clear(); -} -Debugger::ReportProgress(m_id, m_title, std::move(current_update), - current_completed, m_total, m_debugger_id); -last_completed = current_completed; +void Progress::Increment(uint64_t amount, std::string update) { + if
[Lldb-commits] [lldb] 244fcec - [lldb] Fix MainLoopTest for changes in D152712
Author: Pavel Labath Date: 2023-06-16T09:05:27+02:00 New Revision: 244fcecb90fa7a3fb710ca5768d3bae9af5868cc URL: https://github.com/llvm/llvm-project/commit/244fcecb90fa7a3fb710ca5768d3bae9af5868cc DIFF: https://github.com/llvm/llvm-project/commit/244fcecb90fa7a3fb710ca5768d3bae9af5868cc.diff LOG: [lldb] Fix MainLoopTest for changes in D152712 Added: Modified: lldb/unittests/Host/MainLoopTest.cpp Removed: diff --git a/lldb/unittests/Host/MainLoopTest.cpp b/lldb/unittests/Host/MainLoopTest.cpp index b1857fec3c49e..4084e90782fd5 100644 --- a/lldb/unittests/Host/MainLoopTest.cpp +++ b/lldb/unittests/Host/MainLoopTest.cpp @@ -9,6 +9,7 @@ #include "lldb/Host/MainLoop.h" #include "TestingSupport/SubsystemRAII.h" #include "lldb/Host/ConnectionFileDescriptor.h" +#include "lldb/Host/FileSystem.h" #include "lldb/Host/PseudoTerminal.h" #include "lldb/Host/common/TCPSocket.h" #include "llvm/Testing/Support/Error.h" @@ -20,7 +21,7 @@ using namespace lldb_private; namespace { class MainLoopTest : public testing::Test { public: - SubsystemRAII subsystems; + SubsystemRAII subsystems; void SetUp() override { bool child_processes_inherit = false; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] c308534 - [lldb] Rate limit progress reports -- different approach [WIP-ish]
Author: Pavel Labath Date: 2023-06-16T08:28:29+02:00 New Revision: c30853460da7446f92bc1e516f9cbe2c5df6e136 URL: https://github.com/llvm/llvm-project/commit/c30853460da7446f92bc1e516f9cbe2c5df6e136 DIFF: https://github.com/llvm/llvm-project/commit/c30853460da7446f92bc1e516f9cbe2c5df6e136.diff LOG: [lldb] Rate limit progress reports -- different approach [WIP-ish] Have the Progress class spawn a thread to periodically send progress reports. The reporting period could be made configurable, but for now I've hardcoded it to 100ms. (This is the main WIP part) It could be argued that creating a thread for progress reporting adds overhead, but I would counter that by saying "If the task is so fast that creating a thread noticably slows it down, then it really doesn't need progress reporting". For me, this speeds up DWARF indexing by about 1.5% (which is only slightly above the error bars), but I expect it will have a much bigger impact in situations where printing a single progress update takes a nontrivial amount of time. Differential Revision: https://reviews.llvm.org/D152364 Added: Modified: lldb/include/lldb/Core/Progress.h lldb/source/Core/Progress.cpp Removed: diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h index b2b8781a43b05..3bafd3d4b34ea 100644 --- a/lldb/include/lldb/Core/Progress.h +++ b/lldb/include/lldb/Core/Progress.h @@ -9,9 +9,11 @@ #ifndef LLDB_CORE_PROGRESS_H #define LLDB_CORE_PROGRESS_H +#include "lldb/Host/HostThread.h" #include "lldb/Utility/ConstString.h" #include "lldb/lldb-types.h" #include +#include #include #include @@ -92,24 +94,26 @@ class Progress { void Increment(uint64_t amount = 1, std::string update = {}); private: - void ReportProgress(std::string update = {}); + void SendPeriodicReports(std::shared_future done); + void ReportProgress(std::string update); static std::atomic g_id; /// The title of the progress activity. std::string m_title; - std::mutex m_mutex; /// A unique integer identifier for progress reporting. const uint64_t m_id; /// How much work ([0...m_total]) that has been completed. - uint64_t m_completed; + std::atomic m_completed; /// Total amount of work, UINT64_MAX for non deterministic progress. const uint64_t m_total; /// The optional debugger ID to report progress to. If this has no value then /// all debuggers will receive this event. std::optional m_debugger_id; - /// Set to true when progress has been reported where m_completed == m_total - /// to ensure that we don't send progress updates after progress has - /// completed. - bool m_complete = false; + + std::mutex m_update_mutex; + std::string m_update; + + std::promise m_stop_reporting_thread; + HostThread m_reporting_thread; }; } // namespace lldb_private diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp index 08be73f1470f3..a531b9b70437e 100644 --- a/lldb/source/Core/Progress.cpp +++ b/lldb/source/Core/Progress.cpp @@ -9,6 +9,8 @@ #include "lldb/Core/Progress.h" #include "lldb/Core/Debugger.h" +#include "lldb/Host/ThreadLauncher.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/StreamString.h" using namespace lldb; @@ -22,39 +24,67 @@ Progress::Progress(std::string title, uint64_t total, assert(total > 0); if (debugger) m_debugger_id = debugger->GetID(); - std::lock_guard guard(m_mutex); - ReportProgress(); + + // Using a shared_future because std::function needs to be copyable. + if (llvm::Expected reporting_thread = + ThreadLauncher::LaunchThread( + "", + [this, future = std::shared_future( + m_stop_reporting_thread.get_future())]() { +SendPeriodicReports(future); +return lldb::thread_result_t(); + })) { +m_reporting_thread = std::move(*reporting_thread); + } else { +LLDB_LOG_ERROR(GetLog(LLDBLog::Host), reporting_thread.takeError(), + "failed to launch host thread: {}"); + } } Progress::~Progress() { - // Make sure to always report progress completed when this object is - // destructed so it indicates the progress dialog/activity should go away. - std::lock_guard guard(m_mutex); - if (!m_completed) { -m_completed = m_total; -ReportProgress(); + m_stop_reporting_thread.set_value(); + if (m_reporting_thread.IsJoinable()) { +m_reporting_thread.Join(nullptr); } } -void Progress::Increment(uint64_t amount, std::string update) { - if (amount > 0) { -std::lock_guard guard(m_mutex); -// Watch out for unsigned overflow and make sure we don't increment too -// much and exceed m_total. -if (amount > (m_total - m_completed)) - m_completed = m_total; -else - m_completed += amount; -ReportProgress(update); +void
[Lldb-commits] [lldb] 09ba7b6 - [lldb] Add a sleep to TestObjectFileJSON
Author: Pavel Labath Date: 2023-04-14T14:37:32+02:00 New Revision: 09ba7b605327812cfcec4f3c01d7fc232dee651d URL: https://github.com/llvm/llvm-project/commit/09ba7b605327812cfcec4f3c01d7fc232dee651d DIFF: https://github.com/llvm/llvm-project/commit/09ba7b605327812cfcec4f3c01d7fc232dee651d.diff LOG: [lldb] Add a sleep to TestObjectFileJSON The test fails when the two generated files have the same timestamp (lldb uses second granularity). Added: Modified: lldb/test/API/macosx/symbols/TestObjectFileJSON.py Removed: diff --git a/lldb/test/API/macosx/symbols/TestObjectFileJSON.py b/lldb/test/API/macosx/symbols/TestObjectFileJSON.py index 04f91ec62f88b..67d9f8e3c5d02 100644 --- a/lldb/test/API/macosx/symbols/TestObjectFileJSON.py +++ b/lldb/test/API/macosx/symbols/TestObjectFileJSON.py @@ -7,6 +7,7 @@ import uuid import os import shutil +import time class TestObjectFileJSON(TestBase): @@ -80,6 +81,9 @@ def test_module(self): } ], } + +# Sleep to ensure the new file has a diff erent timestamp +time.sleep(2) self.emitJSON(data, json_object_file) module = target.AddModule(self.toModuleSpec(json_object_file)) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e289b53 - [lldb] Remove unused functions from RegisterContextLinux_x86
Author: Pavel Labath Date: 2023-04-05T20:16:48+02:00 New Revision: e289b53f4d9bffd71613d6f91747bf9bda0ae352 URL: https://github.com/llvm/llvm-project/commit/e289b53f4d9bffd71613d6f91747bf9bda0ae352 DIFF: https://github.com/llvm/llvm-project/commit/e289b53f4d9bffd71613d6f91747bf9bda0ae352.diff LOG: [lldb] Remove unused functions from RegisterContextLinux_x86 These should be overridden by individual subclasses, they ended up here because of copy-pasta. Added: Modified: lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h Removed: diff --git a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h index 663c1d9d123d5..0e1863864aa6c 100644 --- a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h +++ b/lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h @@ -19,15 +19,6 @@ class RegisterContextLinux_x86 : public RegisterInfoInterface { RegisterInfo orig_ax_info) : RegisterInfoInterface(target_arch), m_orig_ax_info(orig_ax_info) {} - static size_t GetGPRSizeStatic(); - size_t GetGPRSize() const override { return GetGPRSizeStatic(); } - - const lldb_private::RegisterInfo *GetRegisterInfo() const override; - - uint32_t GetRegisterCount() const override; - - uint32_t GetUserRegisterCount() const override; - const RegisterInfo () const { return m_orig_ax_info; } private: ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] af9e1fa - [lldb] Detach the child process when stepping over a fork
Author: Pavel Labath Date: 2023-04-05T13:25:43+02:00 New Revision: af9e1fa178433653eb3d36c42cad016449873cfc URL: https://github.com/llvm/llvm-project/commit/af9e1fa178433653eb3d36c42cad016449873cfc DIFF: https://github.com/llvm/llvm-project/commit/af9e1fa178433653eb3d36c42cad016449873cfc.diff LOG: [lldb] Detach the child process when stepping over a fork Step over thread plans were claiming to explain the fork stop reasons, which prevented the default fork logic (detaching from the child process) from kicking in. This patch changes that. Differential Revision: https://reviews.llvm.org/D141605 Added: lldb/test/API/functionalities/fork/resumes-child/Makefile lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py lldb/test/API/functionalities/fork/resumes-child/main.cpp Modified: lldb/source/Target/ThreadPlan.cpp Removed: diff --git a/lldb/source/Target/ThreadPlan.cpp b/lldb/source/Target/ThreadPlan.cpp index 9913ecb591fa7..7927fc3140145 100644 --- a/lldb/source/Target/ThreadPlan.cpp +++ b/lldb/source/Target/ThreadPlan.cpp @@ -171,6 +171,9 @@ bool ThreadPlan::IsUsuallyUnexplainedStopReason(lldb::StopReason reason) { case eStopReasonExec: case eStopReasonThreadExiting: case eStopReasonInstrumentation: + case eStopReasonFork: + case eStopReasonVFork: + case eStopReasonVForkDone: return true; default: return false; diff --git a/lldb/test/API/functionalities/fork/resumes-child/Makefile b/lldb/test/API/functionalities/fork/resumes-child/Makefile new file mode 100644 index 0..8b20bcb05 --- /dev/null +++ b/lldb/test/API/functionalities/fork/resumes-child/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py b/lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py new file mode 100644 index 0..70df4f1c51a7c --- /dev/null +++ b/lldb/test/API/functionalities/fork/resumes-child/TestForkResumesChild.py @@ -0,0 +1,22 @@ +""" +Make sure that the fork child keeps running. +""" + + + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * + + +class TestForkResumesChild(TestBase): + +NO_DEBUG_INFO_TESTCASE = True + +@skipIfWindows +def test_step_over_fork(self): +self.build() +lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp")) +self.runCmd("next") +self.expect("continue", substrs = ["exited with status = 0"]) diff --git a/lldb/test/API/functionalities/fork/resumes-child/main.cpp b/lldb/test/API/functionalities/fork/resumes-child/main.cpp new file mode 100644 index 0..2d37f323bdf6a --- /dev/null +++ b/lldb/test/API/functionalities/fork/resumes-child/main.cpp @@ -0,0 +1,29 @@ +#include +#include +#include +#include +#include +#include + +int main() { + pid_t fork_result = fork(); // break here + assert(fork_result >= 0); + if (fork_result == 0) { +// child +_exit(47); + } + // parent + // Use polling to avoid blocking if the child is not actually resumed. + auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); + std::chrono::milliseconds poll_interval{10}; + while (std::chrono::steady_clock::now() < deadline) { +int status; +pid_t waitpid_result = waitpid(fork_result, , WNOHANG); +if (waitpid_result == fork_result) + return 0; +assert(waitpid_result == 0); +std::this_thread::sleep_for(poll_interval); +poll_interval *= 2; + } + abort(); +} ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 933d3ee - [lldb] Drop RegisterInfoInterface::GetDynamicRegisterInfo
Author: Pavel Labath Date: 2023-04-05T13:25:43+02:00 New Revision: 933d3ee60007f5798319cad05b981cb265578ba0 URL: https://github.com/llvm/llvm-project/commit/933d3ee60007f5798319cad05b981cb265578ba0 DIFF: https://github.com/llvm/llvm-project/commit/933d3ee60007f5798319cad05b981cb265578ba0.diff LOG: [lldb] Drop RegisterInfoInterface::GetDynamicRegisterInfo "Dynamic register info" is a very overloaded term, and this particular instance of it was only used for passing the information about the "orig_[re]ax" pseudo-register on x86 through some generic code. Since both sides of the code are x86-specific, I have replaced this with a more direct route. Differential Revision: https://reviews.llvm.org/D147045 Added: lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86.h Modified: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.h lldb/source/Plugins/Process/Utility/RegisterContextLinux_s390x.cpp lldb/source/Plugins/Process/Utility/RegisterContextLinux_s390x.h lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.h lldb/source/Plugins/Process/Utility/RegisterInfoInterface.h Removed: diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp index 4f1d2fab50f2b..e81ef3301f1f1 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp @@ -263,17 +263,17 @@ NativeRegisterContextLinux::DetermineArchitecture(lldb::tid_t tid) { // NativeRegisterContextLinux_x86_64 members. -static RegisterInfoInterface * +static std::unique_ptr CreateRegisterInfoInterface(const ArchSpec _arch) { if (HostInfo::GetArchitecture().GetAddressByteSize() == 4) { // 32-bit hosts run with a RegisterContextLinux_i386 context. -return new RegisterContextLinux_i386(target_arch); +return std::make_unique(target_arch); } else { assert((HostInfo::GetArchitecture().GetAddressByteSize() == 8) && "Register setting path assumes this is a 64-bit host"); // X86_64 hosts know how to work with 64-bit and 32-bit EXEs using the // x86_64 register context. -return new RegisterContextLinux_x86_64(target_arch); +return std::make_unique(target_arch); } } @@ -297,7 +297,7 @@ static std::size_t GetXSTATESize() { NativeRegisterContextLinux_x86_64::NativeRegisterContextLinux_x86_64( const ArchSpec _arch, NativeThreadProtocol _thread) : NativeRegisterContextRegisterInfo( - native_thread, CreateRegisterInfoInterface(target_arch)), + native_thread, CreateRegisterInfoInterface(target_arch).release()), NativeRegisterContextLinux(native_thread), NativeRegisterContextDBReg_x86(native_thread), m_xstate_type(XStateType::Invalid), m_ymm_set(), m_mpx_set(), @@ -757,13 +757,8 @@ Status NativeRegisterContextLinux_x86_64::ReadAllRegisterValues( * **/ RegisterValue value((uint64_t)-1); - const RegisterInfo *reg_info = - GetRegisterInfoInterface().GetDynamicRegisterInfo("orig_eax"); - if (reg_info == nullptr) -reg_info = GetRegisterInfoInterface().GetDynamicRegisterInfo("orig_rax"); - - if (reg_info != nullptr) -return DoWriteRegisterValue(reg_info->byte_offset, reg_info->name, value); + const RegisterInfo = GetRegisterInfo().GetOrigAxInfo(); + return DoWriteRegisterValue(info.byte_offset, info.name, value); return error; } diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h index 43d939da84a39..40d086e0811bb 100644 --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h @@ -13,6 +13,7 @@ #include "Plugins/Process/Linux/NativeRegisterContextLinux.h" #include "Plugins/Process/Utility/NativeRegisterContextDBReg_x86.h" +#include "Plugins/Process/Utility/RegisterContextLinux_x86.h" #include "Plugins/Process/Utility/RegisterContext_x86.h" #include "Plugins/Process/Utility/lldb-x86-register-enums.h" #include @@ -130,6 +131,11 @@ class NativeRegisterContextLinux_x86_64 bool IsMPX(uint32_t reg_index) const; void UpdateXSTATEforWrite(uint32_t reg_index); + + RegisterContextLinux_x86 () const { +return static_cast( +*m_register_info_interface_up); + } }; } // namespace process_linux diff --git a/lldb/source/Plugins/Process/Utility/RegisterContextLinux_i386.cpp
[Lldb-commits] [lldb] e64cc75 - [lldb-server/linux] Use waitpid(-1) to collect inferior events
Author: Pavel Labath Date: 2023-03-30T12:48:36+02:00 New Revision: e64cc756819d567f453467bf7cc16599ad296fdd URL: https://github.com/llvm/llvm-project/commit/e64cc756819d567f453467bf7cc16599ad296fdd DIFF: https://github.com/llvm/llvm-project/commit/e64cc756819d567f453467bf7cc16599ad296fdd.diff LOG: [lldb-server/linux] Use waitpid(-1) to collect inferior events This is a follow-up to D116372, which had a rather unfortunate side effect of making the processing of a single SIGCHLD quadratic in the number of threads -- which does not matter for simple applications, but can get really bad for applications with thousands of threads. This patch fixes the problem by implementing the other possibility mentioned in the first patch -- doing waitpid(-1) centrally and then routing the events to the correct process instance. The "uncollected" threads are held in the process factory class -- which I've renamed to Manager for this purpose, as it now does more than creating processes. Differential Revision: https://reviews.llvm.org/D146977 Added: Modified: lldb/include/lldb/Host/common/NativeProcessProtocol.h lldb/source/Host/common/NativeProcessProtocol.cpp lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp lldb/source/Plugins/Process/Linux/NativeProcessLinux.h lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h lldb/tools/lldb-server/lldb-gdbserver.cpp Removed: diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h index 057c07b1536f..744699210d4b 100644 --- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h +++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h @@ -256,7 +256,7 @@ class NativeProcessProtocol { virtual Status GetFileLoadAddress(const llvm::StringRef _name, lldb::addr_t _addr) = 0; - /// Extension flag constants, returned by Factory::GetSupportedExtensions() + /// Extension flag constants, returned by Manager::GetSupportedExtensions() /// and passed to SetEnabledExtension() enum class Extension { multiprocess = (1u << 0), @@ -272,9 +272,14 @@ class NativeProcessProtocol { LLVM_MARK_AS_BITMASK_ENUM(siginfo_read) }; - class Factory { + class Manager { public: -virtual ~Factory(); +Manager(MainLoop ) : m_mainloop(mainloop) {} +Manager(const Manager &) = delete; +Manager =(const Manager &) = delete; + +virtual ~Manager(); + /// Launch a process for debugging. /// /// \param[in] launch_info @@ -294,8 +299,8 @@ class NativeProcessProtocol { /// A NativeProcessProtocol shared pointer if the operation succeeded or /// an error object if it failed. virtual llvm::Expected> -Launch(ProcessLaunchInfo _info, NativeDelegate _delegate, - MainLoop ) const = 0; +Launch(ProcessLaunchInfo _info, + NativeDelegate _delegate) = 0; /// Attach to an existing process. /// @@ -316,14 +321,16 @@ class NativeProcessProtocol { /// A NativeProcessProtocol shared pointer if the operation succeeded or /// an error object if it failed. virtual llvm::Expected> -Attach(lldb::pid_t pid, NativeDelegate _delegate, - MainLoop ) const = 0; +Attach(lldb::pid_t pid, NativeDelegate _delegate) = 0; /// Get the bitmask of extensions supported by this process plugin. /// /// \return /// A NativeProcessProtocol::Extension bitmask. virtual Extension GetSupportedExtensions() const { return {}; } + + protected: +MainLoop _mainloop; }; /// Notify tracers that the target process will resume diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp index 975b3d0f7d53..95258e1ddc5e 100644 --- a/lldb/source/Host/common/NativeProcessProtocol.cpp +++ b/lldb/source/Host/common/NativeProcessProtocol.cpp @@ -759,4 +759,4 @@ void NativeProcessProtocol::DoStopIDBumped(uint32_t /* newBumpId */) { // Default implementation does nothing. } -NativeProcessProtocol::Factory::~Factory() = default; +NativeProcessProtocol::Manager::~Manager() = default; diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp index dbaa83fd15d7..84a009be50bf 100644 ---
[Lldb-commits] [lldb] 0165b73 - [lldb] Relax expectation on TestMainThreadExit
Author: Pavel Labath Date: 2023-03-21T11:26:32+01:00 New Revision: 0165b73be37c2330ae196799cb3d93445532072b URL: https://github.com/llvm/llvm-project/commit/0165b73be37c2330ae196799cb3d93445532072b DIFF: https://github.com/llvm/llvm-project/commit/0165b73be37c2330ae196799cb3d93445532072b.diff LOG: [lldb] Relax expectation on TestMainThreadExit The exit code of the (funky) test inferior depends on the linux kernel version (changed some time between 5.15 and 6.1). Added: Modified: lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py Removed: diff --git a/lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py b/lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py index 50060dd242f8c..7a8dd80faf944 100644 --- a/lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py +++ b/lldb/test/API/functionalities/thread/main_thread_exit/TestMainThreadExit.py @@ -26,4 +26,5 @@ def test(self): self.expect_expr("call_me()", result_value="12345") self.runCmd("continue") -self.assertEquals(self.process().GetExitStatus(), 47) +# Exit code depends on the version of the linux kernel +self.assertIn(self.process().GetExitStatus(), [42, 47]) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 090205f - [lldb] Fix TestStepOverWatchpoint
Author: Pavel Labath Date: 2023-03-21T10:45:12+01:00 New Revision: 090205fb57a1ca65bec6fada32232c2e975d0c48 URL: https://github.com/llvm/llvm-project/commit/090205fb57a1ca65bec6fada32232c2e975d0c48 DIFF: https://github.com/llvm/llvm-project/commit/090205fb57a1ca65bec6fada32232c2e975d0c48.diff LOG: [lldb] Fix TestStepOverWatchpoint Added: Modified: lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py Removed: diff --git a/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py b/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py index fd70bd692a216..f88428b872c00 100644 --- a/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py +++ b/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py @@ -34,13 +34,13 @@ def get_to_start(self, bkpt_text): # stepping off from the breakpoint: bkpt.SetEnabled(False) -return (target, process, thread, read_watchpoint) +return (target, process, thread, frame, read_watchpoint) # Read-write watchpoints not supported on SystemZ @expectedFailureAll(archs=['s390x']) @add_test_categories(["basic_process"]) def test_step_over(self): -target, process, thread, wp = self.get_to_start("Set a breakpoint here") +target, process, thread, frame, wp = self.get_to_start("Set a breakpoint here") thread.StepOver() self.assertStopReason(thread.GetStopReason(), lldb.eStopReasonWatchpoint, @@ -61,7 +61,7 @@ def test_step_over(self): bugnumber="") @add_test_categories(["basic_process"]) def test_step_instruction(self): -target, process, thread, wp = self.get_to_start("Set breakpoint after call") +target, process, thread, frame, wp = self.get_to_start("Set breakpoint after call") self.step_inst_for_watchpoint(1) @@ -75,6 +75,7 @@ def test_step_instruction(self): if re.match("^mips", arch) or re.match("powerpc64le", arch): self.runCmd("watchpoint delete 1") +error = lldb.SBError() # resolve_location=True, read=False, write=True write_watchpoint = write_value.Watch(True, False, True, error) self.assertTrue(write_watchpoint, "Failed to set write watchpoint.") ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] fad6010 - [lldb/test] Update error message in debug-types-signature-loop.s
Author: Pavel Labath Date: 2023-03-01T13:51:58+01:00 New Revision: fad60105ace546deb219adb14e0eef01a2b6c9d6 URL: https://github.com/llvm/llvm-project/commit/fad60105ace546deb219adb14e0eef01a2b6c9d6 DIFF: https://github.com/llvm/llvm-project/commit/fad60105ace546deb219adb14e0eef01a2b6c9d6.diff LOG: [lldb/test] Update error message in debug-types-signature-loop.s The error message changed in D144664. Added: Modified: lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s Removed: diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s index d0d0fd5705a45..64b835353ed4e 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s @@ -4,7 +4,7 @@ # RUN: ld.lld %t.o -o %t # RUN: %lldb %t -o "target variable e" -b | FileCheck %s -# CHECK: e = +# CHECK: Error: 'Unable to determine byte size.' .type e,@object # @e .section.rodata,"a",@progbits ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d8bd179 - Clear read_fd_set if EINTR received
Author: Emre Kultursay Date: 2023-02-23T13:27:19+01:00 New Revision: d8bd179a173876a7a9ee11828b63efffe145356c URL: https://github.com/llvm/llvm-project/commit/d8bd179a173876a7a9ee11828b63efffe145356c DIFF: https://github.com/llvm/llvm-project/commit/d8bd179a173876a7a9ee11828b63efffe145356c.diff LOG: Clear read_fd_set if EINTR received Leaving bits uncleared set causes callbacks to be triggered even though there are no events to process. Starting with D131160 we have a callback that makes blocking read calls over pipe which was causing the lldb-server main loop to become unresponsive / blocked on Android. Reviewed By: labath Differential Revision: https://reviews.llvm.org/D144240 Added: Modified: lldb/source/Host/posix/MainLoopPosix.cpp Removed: diff --git a/lldb/source/Host/posix/MainLoopPosix.cpp b/lldb/source/Host/posix/MainLoopPosix.cpp index b185c3d3b7076..5b50b450433ea 100644 --- a/lldb/source/Host/posix/MainLoopPosix.cpp +++ b/lldb/source/Host/posix/MainLoopPosix.cpp @@ -156,9 +156,12 @@ Status MainLoopPosix::RunImpl::Poll() { size_t sigset_len; } extra_data = {_sigset, sizeof(kernel_sigset)}; if (syscall(__NR_pselect6, nfds, _fd_set, nullptr, nullptr, nullptr, - _data) == -1 && - errno != EINTR) -return Status(errno, eErrorTypePOSIX); + _data) == -1) { +if (errno != EINTR) + return Status(errno, eErrorTypePOSIX); +else + FD_ZERO(_fd_set); + } return Status(); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 88ac913 - [lldb] Try harder to optimize away a test variable
Author: Pavel Labath Date: 2023-02-02T11:24:47+01:00 New Revision: 88ac9138f4eb7b8c06154dd6e3f801d9882b66d7 URL: https://github.com/llvm/llvm-project/commit/88ac9138f4eb7b8c06154dd6e3f801d9882b66d7 DIFF: https://github.com/llvm/llvm-project/commit/88ac9138f4eb7b8c06154dd6e3f801d9882b66d7.diff LOG: [lldb] Try harder to optimize away a test variable The test was checking that we can print an error message when a variable is optimized away, but the optimizer got smarter (D140404) in tracking the variable's value (so that we were not able to recover its value). Using a value in an argument registers (argc) makes it more likely to be overwritten by subsequent function calls (and permanently lost). Added: Modified: lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py lldb/test/API/tools/lldb-vscode/optimized/main.cpp Removed: diff --git a/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py b/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py index 3f7ac7cdc77a..5dfc35a927f2 100644 --- a/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py +++ b/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py @@ -45,6 +45,6 @@ def test_optimized_variable(self): self.assertEqual(len(breakpoint_ids), len(lines), "expect correct number of breakpoints") self.continue_to_breakpoints(breakpoint_ids) -optimized_variable = self.vscode.get_local_variable('optimized') +optimized_variable = self.vscode.get_local_variable('argc') self.assertTrue(optimized_variable['value'].startswith(' 1 ? std::stoi(argv[1]) : 0; - - printf("argc: %d, optimized: %d\n", argc, optimized); - int result = foo(argc, 20); + printf("argc: %d\n", argc); + int result = foo(20, argv[0][0]); printf("result: %d\n", result); // breakpoint 2 return 0; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 8b4d263 - [lldb] Fix TestVSCode_completions for D141828
Author: Pavel Labath Date: 2023-01-30T13:20:30+01:00 New Revision: 8b4d263799e15e287ce1722eec5a133dd86aa06c URL: https://github.com/llvm/llvm-project/commit/8b4d263799e15e287ce1722eec5a133dd86aa06c DIFF: https://github.com/llvm/llvm-project/commit/8b4d263799e15e287ce1722eec5a133dd86aa06c.diff LOG: [lldb] Fix TestVSCode_completions for D141828 Added: Modified: lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py Removed: diff --git a/lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py b/lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py index fd215c67c940..93415d63d267 100644 --- a/lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py +++ b/lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py @@ -41,7 +41,7 @@ def test_completions(self): [ { "text": "var", -"label": "var -- vector > &", +"label": "var -- vector> &", } ], [{"text": "var1", "label": "var1 -- int &"}], @@ -66,7 +66,7 @@ def test_completions(self): [ { "text": "var", -"label": "var -- vector > &", +"label": "var -- vector> &", } ], ) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [EXTERNAL] [lldb] fbaf48b - [lldb] Remove redundant .c_str() and .get() calls
(I fixed the test failures with 071c62c5d3eda2836174c0de82f6c55b082e26fc, so this should be safe to reapply now.) On 19/12/2022 01:09, Stella Stamenova via lldb-commits wrote: I think this broke the windows lldb bot: https://lab.llvm.org/buildbot/#/builders/83/builds/27352 Can you address the failure or revert the change? I can revert it tomorrow otherwise. Thanks, -Stella -Original Message- From: lldb-commits On Behalf Of Fangrui Song via lldb-commits Sent: Saturday, December 17, 2022 5:16 PM To: lldb-commits@lists.llvm.org Subject: [EXTERNAL] [Lldb-commits] [lldb] fbaf48b - [lldb] Remove redundant .c_str() and .get() calls Author: Fangrui Song Date: 2022-12-18T01:15:25Z New Revision: fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd URL: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2Ffbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd=05%7C01%7CSTILIS%40microsoft.com%7Cbb5532e79e21420847da08dae0955e85%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638069229409870530%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=w2DRW2raXlSi3NYQL%2BeSqHZtuImPdR%2Bbi2EMqYzCZnQ%3D=0 DIFF: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2Ffbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd.diff=05%7C01%7CSTILIS%40microsoft.com%7Cbb5532e79e21420847da08dae0955e85%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638069229409870530%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=GS4cYm4kPFClslEDMeaQl5Rpb26TVVf%2Fg0mAlaVsLcQ%3D=0 LOG: [lldb] Remove redundant .c_str() and .get() calls Removing .c_str() has a semantics difference, but the use scenarios likely do not matter as we don't have NUL in the strings. Added: Modified: lldb/include/lldb/Target/Process.h lldb/source/API/SBCommandReturnObject.cpp lldb/source/API/SBExpressionOptions.cpp lldb/source/API/SBSourceManager.cpp lldb/source/Breakpoint/BreakpointResolverScripted.cpp lldb/source/Commands/CommandObjectBreakpoint.cpp lldb/source/Commands/CommandObjectExpression.cpp lldb/source/Commands/CommandObjectHelp.cpp lldb/source/Commands/CommandObjectMultiword.cpp lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Commands/CommandObjectThread.cpp lldb/source/Core/Disassembler.cpp lldb/source/Core/FormatEntity.cpp lldb/source/Core/IOHandlerCursesGUI.cpp lldb/source/Expression/REPL.cpp lldb/source/Interpreter/CommandInterpreter.cpp lldb/source/Interpreter/OptionArgParser.cpp lldb/source/Interpreter/OptionValueArch.cpp lldb/source/Interpreter/ScriptInterpreter.cpp lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp lldb/source/Plugins/Language/ObjC/Cocoa.cpp lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp lldb/source/Plugins/Platform/Android/AdbClient.cpp lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.h lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h lldb/source/Symbol/LocateSymbolFileMacOSX.cpp lldb/source/Target/LanguageRuntime.cpp lldb/source/Target/TargetList.cpp lldb/source/Target/Thread.cpp lldb/source/Target/ThreadPlanStack.cpp lldb/tools/debugserver/source/DNBTimer.h lldb/tools/lldb-vscode/BreakpointBase.cpp Removed: diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 47040137078f..8bb8b85e22fb 100644 ---
[Lldb-commits] [lldb] 071c62c - [lldb] Modernize sprintf in FormatEntity.cpp
Author: Pavel Labath Date: 2022-12-19T10:53:20+01:00 New Revision: 071c62c5d3eda2836174c0de82f6c55b082e26fc URL: https://github.com/llvm/llvm-project/commit/071c62c5d3eda2836174c0de82f6c55b082e26fc DIFF: https://github.com/llvm/llvm-project/commit/071c62c5d3eda2836174c0de82f6c55b082e26fc.diff LOG: [lldb] Modernize sprintf in FormatEntity.cpp Avoid buffer overflows with large indexes, and spurious nul characters with small ones. Added: Modified: lldb/source/Core/FormatEntity.cpp Removed: diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index c5cad95ecafe1..4f615a1106520 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -618,11 +618,8 @@ static bool DumpRegister(Stream , StackFrame *frame, RegisterKind reg_kind, static ValueObjectSP ExpandIndexedExpression(ValueObject *valobj, size_t index, bool deref_pointer) { Log *log = GetLog(LLDBLog::DataFormatters); - const char *ptr_deref_format = "[%d]"; - std::string ptr_deref_buffer(10, 0); - ::sprintf(_deref_buffer[0], ptr_deref_format, index); - LLDB_LOGF(log, "[ExpandIndexedExpression] name to deref: %s", -ptr_deref_buffer.c_str()); + std::string name_to_deref = llvm::formatv("[{0}]", index); + LLDB_LOG(log, "[ExpandIndexedExpression] name to deref: {0}", name_to_deref); ValueObject::GetValueForExpressionPathOptions options; ValueObject::ExpressionPathEndResultType final_value_type; ValueObject::ExpressionPathScanEndReason reason_to_stop; @@ -630,8 +627,7 @@ static ValueObjectSP ExpandIndexedExpression(ValueObject *valobj, size_t index, (deref_pointer ? ValueObject::eExpressionPathAftermathDereference : ValueObject::eExpressionPathAftermathNothing); ValueObjectSP item = valobj->GetValueForExpressionPath( - ptr_deref_buffer.c_str(), _to_stop, _value_type, options, - _next); + name_to_deref, _to_stop, _value_type, options, _next); if (!item) { LLDB_LOGF(log, "[ExpandIndexedExpression] ERROR: why stopping = %d," ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a06342d - [lldb] Modify TestThreadJump to work around a change in clang's debug_line generation
Author: Pavel Labath Date: 2022-12-12T09:31:35+01:00 New Revision: a06342d250ec7bee37dc93477f233e43e597aca5 URL: https://github.com/llvm/llvm-project/commit/a06342d250ec7bee37dc93477f233e43e597aca5 DIFF: https://github.com/llvm/llvm-project/commit/a06342d250ec7bee37dc93477f233e43e597aca5.diff LOG: [lldb] Modify TestThreadJump to work around a change in clang's debug_line generation After D133376, jumping to the return line in the otherfn function became ambiguous because it has two line entries associated with it. Work around that problem by changing the function. Filed PR59458 to track possible improvements in jump target disambiguation. Added: Modified: lldb/test/API/functionalities/thread/jump/other.cpp Removed: diff --git a/lldb/test/API/functionalities/thread/jump/other.cpp b/lldb/test/API/functionalities/thread/jump/other.cpp index 741f13bbd5221..c44786f721d57 100644 --- a/lldb/test/API/functionalities/thread/jump/other.cpp +++ b/lldb/test/API/functionalities/thread/jump/other.cpp @@ -1,4 +1,4 @@ -int otherfn() -{ -return 4; // other marker +int otherfn() { + int x = 4; // other marker + return x; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 6335deb - [lldb/test] Use SBPlatform info for lldbplatformutil.getPlatform()
Author: Pavel Labath Date: 2022-11-29T11:29:58+01:00 New Revision: 6335deb68b1dd417da4dff4f36fc344d8656e81a URL: https://github.com/llvm/llvm-project/commit/6335deb68b1dd417da4dff4f36fc344d8656e81a DIFF: https://github.com/llvm/llvm-project/commit/6335deb68b1dd417da4dff4f36fc344d8656e81a.diff LOG: [lldb/test] Use SBPlatform info for lldbplatformutil.getPlatform() Previously, we just used the platform name. This worked mostly OK, but it required adding special handling for any unusual (and potentially downstream) platform plugins, as evidenced by the hardcoding of the qemu-user platform. The current implementation was added in D121605/21c5bb0a636c23ec75b13681c0a6fdb03ecd9c0d, which this essentially reverts and goes back to the previous method of retrieving the platform name from the platform triple (the "OS" field). The motivation for D121605 was the ability to retrieve the process without constructing an SBDebugger object (which would be necessary in a world where SBPlatforms are managed by SBDebuggers). However, this world did not arrive (mainly due to other commitments on my part), and I now think that if we do want to go in that direction, that we should just create a dummy/empty SBDebugger object for holding the initial SBPlatform. One benefit of D121605 was the unification of getPlatform and getHostPlatform code paths, and I preserve that benefit by unifying them in the other direction -- using the host SBPlatform for getHostPlatform. Differential Revision: https://reviews.llvm.org/D138430 Added: Modified: lldb/packages/Python/lldbsuite/test/dotest.py lldb/packages/Python/lldbsuite/test/lldbplatformutil.py Removed: diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index 818e8f618afa7..db7a3450ffc4d 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -849,14 +849,14 @@ def checkDebugServerSupport(): skip_msg = "Skipping %s tests, as they are not compatible with remote testing on this platform" if lldbplatformutil.platformIsDarwin(): configuration.skip_categories.append("llgs") -if configuration.lldb_platform_name: +if lldb.remote_platform: # configuration.skip_categories.append("debugserver") if configuration.verbose: print(skip_msg%"debugserver"); else: configuration.skip_categories.append("debugserver") -if configuration.lldb_platform_name and lldbplatformutil.getPlatform() == "windows": +if lldb.remote_platform and lldbplatformutil.getPlatform() == "windows": configuration.skip_categories.append("llgs") if configuration.verbose: print(skip_msg%"lldb-server"); @@ -891,14 +891,6 @@ def run_suite(): lldb.SBDebugger.Initialize() lldb.SBDebugger.PrintStackTraceOnError() -checkLibcxxSupport() -checkLibstdcxxSupport() -checkWatchpointSupport() -checkDebugInfoSupport() -checkDebugServerSupport() -checkObjcSupport() -checkForkVForkSupport() - # Use host platform by default. lldb.remote_platform = None lldb.selected_platform = lldb.SBPlatform.GetHostPlatform() @@ -957,8 +949,16 @@ def run_suite(): # Note that it's not dotest's job to clean this directory. lldbutil.mkdir_p(configuration.test_build_dir) +checkLibcxxSupport() +checkLibstdcxxSupport() +checkWatchpointSupport() +checkDebugInfoSupport() +checkDebugServerSupport() +checkObjcSupport() +checkForkVForkSupport() + skipped_categories_list = ", ".join(configuration.skip_categories) -print("Skipping the following test categories: {}".format(skipped_categories_list)) +print("Skipping the following test categories: {}".format(configuration.skip_categories)) for testdir in configuration.testdirs: for (dirpath, dirnames, filenames) in os.walk(testdir): diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py index 3f95e8054f789..56d402418d457 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py @@ -98,21 +98,23 @@ def finalize_build_dictionary(dictionary): return dictionary +def _get_platform_os(p): +# Use the triple to determine the platform if set. +triple = p.GetTriple() +if triple: +platform = triple.split('-')[2] +if platform.startswith('freebsd'): +platform = 'freebsd' +elif platform.startswith('netbsd'): +platform = 'netbsd' +return platform + +return '' + + def getHostPlatform(): """Returns the host platform running the test suite.""" -# Attempts to return a platform name matching a target
[Lldb-commits] [lldb] b32931c - [lldb][nfc] Deindent ProcessGDBRemote::SetThreadStopInfo by two levels
Author: Pavel Labath Date: 2022-11-25T13:51:13+01:00 New Revision: b32931c5b32eb0d2cf37d688b34f8548c9674c19 URL: https://github.com/llvm/llvm-project/commit/b32931c5b32eb0d2cf37d688b34f8548c9674c19 DIFF: https://github.com/llvm/llvm-project/commit/b32931c5b32eb0d2cf37d688b34f8548c9674c19.diff LOG: [lldb][nfc] Deindent ProcessGDBRemote::SetThreadStopInfo by two levels Added: Modified: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Removed: diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index ba180cc821e2..83b496b3b978 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1607,289 +1607,276 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo( // queue_serial are valid LazyBool associated_with_dispatch_queue, addr_t dispatch_queue_t, std::string _name, QueueKind queue_kind, uint64_t queue_serial) { - ThreadSP thread_sp; - if (tid != LLDB_INVALID_THREAD_ID) { -// Scope for "locker" below -{ - // m_thread_list_real does have its own mutex, but we need to hold onto - // the mutex between the call to m_thread_list_real.FindThreadByID(...) - // and the m_thread_list_real.AddThread(...) so it doesn't change on us - std::lock_guard guard( - m_thread_list_real.GetMutex()); - thread_sp = m_thread_list_real.FindThreadByProtocolID(tid, false); - - if (!thread_sp) { -// Create the thread if we need to -thread_sp = std::make_shared(*this, tid); -m_thread_list_real.AddThread(thread_sp); - } -} - -if (thread_sp) { - ThreadGDBRemote *gdb_thread = - static_cast(thread_sp.get()); - RegisterContextSP gdb_reg_ctx_sp(gdb_thread->GetRegisterContext()); - gdb_reg_ctx_sp->InvalidateIfNeeded(true); - - auto iter = std::find(m_thread_ids.begin(), m_thread_ids.end(), tid); - if (iter != m_thread_ids.end()) { -SetThreadPc(thread_sp, iter - m_thread_ids.begin()); - } - - for (const auto : expedited_register_map) { -StringExtractor reg_value_extractor(pair.second); -WritableDataBufferSP buffer_sp(new DataBufferHeap( -reg_value_extractor.GetStringRef().size() / 2, 0)); -reg_value_extractor.GetHexBytes(buffer_sp->GetData(), '\xcc'); -uint32_t lldb_regnum = -gdb_reg_ctx_sp->ConvertRegisterKindToRegisterNumber( -eRegisterKindProcessPlugin, pair.first); -gdb_thread->PrivateSetRegisterValue(lldb_regnum, buffer_sp->GetData()); - } + if (tid == LLDB_INVALID_THREAD_ID) +return nullptr; - // AArch64 SVE specific code below calls AArch64SVEReconfigure to update - // SVE register sizes and offsets if value of VG register has changed - // since last stop. - const ArchSpec = GetTarget().GetArchitecture(); - if (arch.IsValid() && arch.GetTriple().isAArch64()) { -GDBRemoteRegisterContext *reg_ctx_sp = -static_cast( -gdb_thread->GetRegisterContext().get()); - -if (reg_ctx_sp) - reg_ctx_sp->AArch64SVEReconfigure(); - } + ThreadSP thread_sp; + // Scope for "locker" below + { +// m_thread_list_real does have its own mutex, but we need to hold onto the +// mutex between the call to m_thread_list_real.FindThreadByID(...) and the +// m_thread_list_real.AddThread(...) so it doesn't change on us +std::lock_guard guard(m_thread_list_real.GetMutex()); +thread_sp = m_thread_list_real.FindThreadByProtocolID(tid, false); + +if (!thread_sp) { + // Create the thread if we need to + thread_sp = std::make_shared(*this, tid); + m_thread_list_real.AddThread(thread_sp); +} + } - thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str()); + ThreadGDBRemote *gdb_thread = static_cast(thread_sp.get()); + RegisterContextSP gdb_reg_ctx_sp(gdb_thread->GetRegisterContext()); - gdb_thread->SetThreadDispatchQAddr(thread_dispatch_qaddr); - // Check if the GDB server was able to provide the queue name, kind and - // serial number - if (queue_vars_valid) -gdb_thread->SetQueueInfo(std::move(queue_name), queue_kind, - queue_serial, dispatch_queue_t, - associated_with_dispatch_queue); - else -gdb_thread->ClearQueueInfo(); + gdb_reg_ctx_sp->InvalidateIfNeeded(true); - gdb_thread->SetAssociatedWithLibdispatchQueue( - associated_with_dispatch_queue); + auto iter = std::find(m_thread_ids.begin(), m_thread_ids.end(), tid); + if (iter != m_thread_ids.end()) +SetThreadPc(thread_sp, iter - m_thread_ids.begin()); - if (dispatch_queue_t !=
[Lldb-commits] [lldb] 3427cb5 - [lldb] Prevent an infinite loop while reading memory regions
Author: Pavel Labath Date: 2022-11-25T11:55:41+01:00 New Revision: 3427cb5b3a55a454e5750f6318ced8a5d7c1442d URL: https://github.com/llvm/llvm-project/commit/3427cb5b3a55a454e5750f6318ced8a5d7c1442d DIFF: https://github.com/llvm/llvm-project/commit/3427cb5b3a55a454e5750f6318ced8a5d7c1442d.diff LOG: [lldb] Prevent an infinite loop while reading memory regions A malformed qMemoryRegionInfo response can easily trigger an infinite loop if regions end (base + size) wraps the address space. A particularly interesting is the case where base+size=0, which a stub could use to say that the rest of the memory space is unmapped, even though lldb expects 0xff... in this case. One could argue which behavior is more correct (technically, the current behavior does not say anything about the last byte), but unless we stop using 0xff... to mean "invalid address", that discussion is very academic. This patch truncates address ranges which wraps the address space, which handles the zero case as well as other kinds of malformed packets. Added: lldb/test/API/functionalities/gdb_remote_client/TestGdbClientMemoryRegions.py Modified: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp Removed: diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index f03fd4231c74a..347c47b078f45 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -1527,8 +1527,14 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo( if (!value.getAsInteger(16, addr_value)) region_info.GetRange().SetRangeBase(addr_value); } else if (name.equals("size")) { - if (!value.getAsInteger(16, addr_value)) + if (!value.getAsInteger(16, addr_value)) { region_info.GetRange().SetByteSize(addr_value); +if (region_info.GetRange().GetRangeEnd() < +region_info.GetRange().GetRangeBase()) { + // Range size overflowed, truncate it. + region_info.GetRange().SetRangeEnd(LLDB_INVALID_ADDRESS); +} + } } else if (name.equals("permissions") && region_info.GetRange().IsValid()) { saw_permissions = true; diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGdbClientMemoryRegions.py b/lldb/test/API/functionalities/gdb_remote_client/TestGdbClientMemoryRegions.py new file mode 100644 index 0..c6ad39bcdc72f --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGdbClientMemoryRegions.py @@ -0,0 +1,44 @@ +import lldb +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * +from lldbsuite.test.gdbclientutils import * +from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase + + +class TestGdbClientMemoryRegions(GDBRemoteTestBase): + +def test(self): +""" +Test handling of overflowing memory regions. In particular, make sure +they don't trigger an infinite loop. +""" +class MyResponder(MockGDBServerResponder): + +def qHostInfo(self): +return "ptrsize:8;endian:little;" + +def qMemoryRegionInfo(self, addr): +if addr == 0: +return "start:0;size:8000;permissions:rw;" +if addr == 0x8000: +return "start:8000;size:8000;permissions:r;" + +self.runCmd("log enable gdb-remote packets") +self.runCmd("log enable lldb temp") +self.server.responder = MyResponder() +target = self.dbg.CreateTarget('') +process = self.connect(target) + +regions = process.GetMemoryRegions() +self.assertEqual(regions.GetSize(), 2) + +region = lldb.SBMemoryRegionInfo() +self.assertTrue(regions.GetMemoryRegionAtIndex(0, region)) +self.assertEqual(region.GetRegionBase(), 0) +self.assertEqual(region.GetRegionEnd(), 0x8000) +self.assertTrue(region.IsWritable()) + +self.assertTrue(regions.GetMemoryRegionAtIndex(1, region)) +self.assertEqual(region.GetRegionBase(), 0x8000) +self.assertEqual(region.GetRegionEnd(), 0x) +self.assertFalse(region.IsWritable()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] c699a81 - [lldb/test] Remove the module cache directory in module-ownership.mm
Author: Pavel Labath Date: 2022-11-25T11:55:42+01:00 New Revision: c699a81bc98804650fdd4a85de085e257e074878 URL: https://github.com/llvm/llvm-project/commit/c699a81bc98804650fdd4a85de085e257e074878 DIFF: https://github.com/llvm/llvm-project/commit/c699a81bc98804650fdd4a85de085e257e074878.diff LOG: [lldb/test] Remove the module cache directory in module-ownership.mm The stale cache directory can cause compilation to fail when ast serialization changes. Added: Modified: lldb/test/Shell/SymbolFile/DWARF/x86/module-ownership.mm Removed: diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/module-ownership.mm b/lldb/test/Shell/SymbolFile/DWARF/x86/module-ownership.mm index 311fd34d40e83..a8dd6cf1d901a 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/module-ownership.mm +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/module-ownership.mm @@ -1,3 +1,4 @@ +// RUN: rm -rf %t.cache // RUN: %clang --target=x86_64-apple-macosx -g -gmodules -Wno-objc-root-class \ // RUN:-fmodules -fmodules-cache-path=%t.cache \ // RUN:-c -o %t.o %s -I%S/Inputs ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 42237d5 - [lldb] Fix PathMappingListTest for the Optional interface change
Author: Pavel Labath Date: 2022-11-24T11:11:05+01:00 New Revision: 42237d51abf00417f5a3164d843e245ad584f7c4 URL: https://github.com/llvm/llvm-project/commit/42237d51abf00417f5a3164d843e245ad584f7c4 DIFF: https://github.com/llvm/llvm-project/commit/42237d51abf00417f5a3164d843e245ad584f7c4.diff LOG: [lldb] Fix PathMappingListTest for the Optional interface change Added: Modified: lldb/unittests/Target/PathMappingListTest.cpp Removed: diff --git a/lldb/unittests/Target/PathMappingListTest.cpp b/lldb/unittests/Target/PathMappingListTest.cpp index cf2bc181b0796..29d83782e08fb 100644 --- a/lldb/unittests/Target/PathMappingListTest.cpp +++ b/lldb/unittests/Target/PathMappingListTest.cpp @@ -40,7 +40,8 @@ static void TestPathMappings(const PathMappingList , map.RemapPath(ConstString(match.original.GetPath()), actual_remapped)); EXPECT_EQ(FileSpec(actual_remapped.GetStringRef()), match.remapped); FileSpec unmapped_spec; -EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec).hasValue()); +EXPECT_TRUE( +map.ReverseRemapPath(match.remapped, unmapped_spec).has_value()); std::string unmapped_path = unmapped_spec.GetPath(); EXPECT_EQ(unmapped_path, orig_normalized); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 14aace3 - Revert "Add runToBinaryEntry option for lldb-vscode"
Author: Pavel Labath Date: 2022-11-23T13:26:11+01:00 New Revision: 14aace34c35da1ae5ffc4abc599cbdb36bf33a01 URL: https://github.com/llvm/llvm-project/commit/14aace34c35da1ae5ffc4abc599cbdb36bf33a01 DIFF: https://github.com/llvm/llvm-project/commit/14aace34c35da1ae5ffc4abc599cbdb36bf33a01.diff LOG: Revert "Add runToBinaryEntry option for lldb-vscode" This reverts commit f0c16f89124f2dc0630162ff9ea23934f5b2b75b because it breaks linux and mac bots. Added: Modified: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py lldb/tools/lldb-vscode/VSCode.cpp lldb/tools/lldb-vscode/VSCode.h lldb/tools/lldb-vscode/lldb-vscode.cpp lldb/tools/lldb-vscode/package.json Removed: diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py index 99baad59dc12d..a91f3b2b8feff 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py @@ -286,7 +286,7 @@ def launch(self, program=None, args=None, cwd=None, env=None, stopCommands=None, exitCommands=None, terminateCommands=None, sourcePath=None, debuggerRoot=None, sourceInitFile=False, launchCommands=None, sourceMap=None, disconnectAutomatically=True, runInTerminal=False, - expectFailure=False, postRunCommands=None, runToBinaryEntry=False): + expectFailure=False, postRunCommands=None): '''Sending launch request to vscode ''' @@ -323,8 +323,7 @@ def cleanup(): sourceMap=sourceMap, runInTerminal=runInTerminal, expectFailure=expectFailure, -postRunCommands=postRunCommands, -runToBinaryEntry=runToBinaryEntry) +postRunCommands=postRunCommands) if expectFailure: return response @@ -347,7 +346,7 @@ def build_and_launch(self, program, args=None, cwd=None, env=None, terminateCommands=None, sourcePath=None, debuggerRoot=None, sourceInitFile=False, runInTerminal=False, disconnectAutomatically=True, postRunCommands=None, - lldbVSCodeEnv=None, runToBinaryEntry=False): + lldbVSCodeEnv=None): '''Build the default Makefile target, create the VSCode debug adaptor, and launch the process. ''' @@ -360,5 +359,4 @@ def build_and_launch(self, program, args=None, cwd=None, env=None, terminateCommands, sourcePath, debuggerRoot, sourceInitFile, runInTerminal=runInTerminal, disconnectAutomatically=disconnectAutomatically, -postRunCommands=postRunCommands, -runToBinaryEntry=runToBinaryEntry) +postRunCommands=postRunCommands) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py index f5f9ae30bbd46..c2de4ad5c7d9a 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py @@ -650,7 +650,7 @@ def request_launch(self, program, args=None, cwd=None, env=None, terminateCommands=None ,sourcePath=None, debuggerRoot=None, launchCommands=None, sourceMap=None, runInTerminal=False, expectFailure=False, - postRunCommands=None, runToBinaryEntry=False): + postRunCommands=None): args_dict = { 'program': program } @@ -662,8 +662,6 @@ def request_launch(self, program, args=None, cwd=None, env=None, args_dict['env'] = env if stopOnEntry: args_dict['stopOnEntry'] = stopOnEntry -if runToBinaryEntry: -args_dict['runToBinaryEntry'] = runToBinaryEntry if disableASLR: args_dict['disableASLR'] = disableASLR if disableSTDIO: diff --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py index 8bb89085bb156..6e916d72dda7d 100644 --- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py +++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py @@ -78,29 +78,6 @@ def test_stopOnEntry(self): reason, 'breakpoint', 'verify stop isn\'t "main" breakpoint') -@skipIfWindows -@skipIfRemote -def