[Lldb-commits] [PATCH] D63591: DWARFDebugLoc: Make parsing and error reporting more robust

clayborg added a comment. DataExtractor is a copy of the one from LLDB from a while back and changes have been made to adapt it to llvm. DataExtractor was designed so that you can have one of them (like for .debug_info or any other DWARF section) and use this same extractor from multiple

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1719 +// artificial. return GetVariable() && GetVariable()->IsArtificial(); } jingham wrote: > clayborg wrote: > > Things brings the questions: do we really need to filter these

[Lldb-commits] [PATCH] D62894: DWARF: Share line tables of type units

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Still want to resolve getting files from a DWARFUnit a bit better. See inlined comment. Comment at:

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

clayborg added a comment. I don't see much I would change here as long as this works and gets tested by the generic GDB remote protocol testing? Any others have comments? Comment at: source/Plugins/Process/Windows/Common/DebuggerThread.cpp:350 +

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

clayborg created this revision. clayborg added reviewers: labath, aprantl, jingham. Herald added a subscriber: arphaman. Improve manual indexing performance when indexing non objective C code. One question I have is all Darwin compilers currently support the apple DWARF indexes, so do we even

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

clayborg marked an inline comment as done. clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:255 + bool is_objc_method = false; + if (check_objc) { +ObjCLanguage::MethodName objc_method(name, true);

[Lldb-commits] [PATCH] D63171: Don't try to parse ObjC method if CU isn't ObjC

clayborg added a comment. In D63171#1539049 , @aprantl wrote: > I suppose one could compile Objective-C code on Linux using GCC. Will GCC not set the language to ObjC or ObjC++? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63171/new/

[Lldb-commits] [PATCH] D61732: FuncUnwinders: Add a new "SymbolFile" unwind plan

clayborg added a comment. Looks good except the inline comment about all the unwind plans we have now. Would love to simplify this so that EH frame, compact unwind, ARM unwind, breakpad unwind all come from the symbol file or object files. Also be nice to have a register resolver that works

[Lldb-commits] [PATCH] D61733: Breakpad: Generate unwind plans from STACK CFI records

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:378 + // Unwind rules are of the form + // register1: expression1 register2: expression2 ... + // We assume none of the tokens in expression end with a colon.

[Lldb-commits] [PATCH] D61611: [JITLoaderGDB] Set eTypeJIT for objects read from JIT descriptors

clayborg accepted this revision. clayborg added a comment. lgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61611/new/ https://reviews.llvm.org/D61611 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D61733: Breakpad: Generate unwind plans from STACK CFI records

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Thanks for the clarification! LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61733/new/ https://reviews.llvm.org/D61733 ___

[Lldb-commits] [PATCH] D61908: DWARF: Add ability to reference debug info coming from multiple sections

clayborg added a comment. See inline comments. Nice patch overall though, real close. Comment at: source/Plugins/SymbolFile/DWARF/DIERef.cpp:18 if (form_value.IsValid()) { const DWARFUnit *dwarf_cu = form_value.GetCompileUnit(); if (dwarf_cu) { We

[Lldb-commits] [PATCH] D62246: DWARF: Implement DW_AT_signature lookup for type unit support

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just fix the return when we have a DW_AT_signature and this is good to go. Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:267 +} +

[Lldb-commits] [PATCH] D62316: DWARFContext: Make loading of sections thread-safe

clayborg added a comment. In D62316#1513894 , @labath wrote: > Two other options I see are: > > - initialize the sections immediately after creating the dwarf context. The > main advantage of that would that it alings to code more with llvm (which >

[Lldb-commits] [PATCH] D62302: DWARF: Fix address range support in mixed 4+5 scenario

clayborg added a comment. Nice patch. See inlined comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:435-448 + llvm::Expected expected_ranges = + (form_value.Form() == DW_FORM_rnglistx) + ?

[Lldb-commits] [PATCH] D62316: DWARFContext: Make loading of sections thread-safe

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/SymbolFile/DWARF/DWARFContext.h:25 + struct SectionData { +llvm::once_flag flag; +DWARFDataExtractor data; is

[Lldb-commits] [PATCH] D62395: WIP: Dont vend lldb_private::CompileUnits for DWARFTypeUnits

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:318-319 case DW_AT_decl_file: -decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex( -form_value.Unsigned()));

[Lldb-commits] [PATCH] D61783: [DWARF] Use sequential integers for the IDs of the SymbolFileDWOs

clayborg added a comment. Might want the person that did DWO support to chime in here if that wasn't you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61783/new/ https://reviews.llvm.org/D61783 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

clayborg added inline comments. Comment at: include/lldb/Symbol/FuncUnwinders.h:117 + class LazyPlan { +lldb::UnwindPlanSP m_plan_sp; + labath wrote: > clayborg wrote: > > maybe use: > > > > ``` > > llvm::Optional m_plan_sp; > > ``` > > > > Then just

[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

clayborg added inline comments. Comment at: include/lldb/Symbol/FuncUnwinders.h:117 + class LazyPlan { +lldb::UnwindPlanSP m_plan_sp; + clayborg wrote: > labath wrote: > > clayborg wrote: > > > maybe use: > > > > > > ``` > > > llvm::Optional m_plan_sp; > >

[Lldb-commits] [PATCH] D61779: FuncUnwinders: General clean up and optimization

clayborg added inline comments. Comment at: include/lldb/Symbol/FuncUnwinders.h:117 + class LazyPlan { +lldb::UnwindPlanSP m_plan_sp; + maybe use: ``` llvm::Optional m_plan_sp; ``` Then just check if it has no value, and if so compute and set it either to

[Lldb-commits] [PATCH] D61853: [FuncUnwinders] Use "symbol file" unwind plans for unwinding

clayborg added inline comments. Comment at: source/Symbol/FuncUnwinders.cpp:63-64 return plan_sp; if (UnwindPlanSP plan_sp = GetDebugFrameUnwindPlan(target)) return plan_sp; if (UnwindPlanSP plan_sp = GetCompactUnwindUnwindPlan(target)) We should

[Lldb-commits] [PATCH] D62211: Simplify `GetName`+`AppendTypeName` by `DWARFDIE`

clayborg added a comment. In D62211#1510903 , @clayborg wrote: > We really shouldn't have any DWARFDIE references inside > DWARFDebugInfoEntry.h. I noticed there are a lot now which is wrong. I take this back. Many uses are good inside of

[Lldb-commits] [PATCH] D62221: [lldb-server][LLGS] Support 'g' packets

clayborg added a comment. Using 'g' packets seems fine to me and we might be able to just enable it. We will need to ensure that this doesn't regress stepping times so we will need to get people to try this out on their systems first. Most of our targets expedite enough registers in the stop

[Lldb-commits] [PATCH] D62211: Simplify `GetName`+`AppendTypeName` by `DWARFDIE`

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. The whole goal of the DWARFDebugInfoEntry::GetName() and DWARFDebugInfoEntry::AppendTypeName() we might be needing to dump the name of a DIE in DWARF that was not parsed yet.

[Lldb-commits] [PATCH] D62211: Simplify `GetName`+`AppendTypeName` by `DWARFDIE`

clayborg added a comment. We really shouldn't have any DWARFDIE references inside DWARFDebugInfoEntry.h. I noticed there are a lot now which is wrong. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62211/new/ https://reviews.llvm.org/D62211

[Lldb-commits] [PATCH] D62008: DWARF: Introduce DWARFTypeUnit class

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Ok, as long as we get things in soon, some functionality is better than none. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62008/new/ https://reviews.llvm.org/D62008

[Lldb-commits] [PATCH] D62073: DWARF: Introduce DWARFUnitHeader class

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Fine if we get things in quick! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62073/new/ https://reviews.llvm.org/D62073 ___

[Lldb-commits] [PATCH] D61311: PostfixExpression: Use signed integers in IntegerNode

clayborg added inline comments. Comment at: include/lldb/Symbol/PostfixExpression.h:97 private: - uint32_t m_value; + int64_t m_value; }; Do we want to try and use lldb_private::Scalar here? Then this could handle signed/unsigned ints, floats and anything

[Lldb-commits] [PATCH] D59015: [lldb-mi] Include full path in the -data-disassemble response

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. LGTM. If there are any other " std::unique_ptr" instances you would like to fix after this patch, feel free to submit more fixes without need for review. Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D61587: [lldb] Added support for dwarf expressions DW_OP_call2/DW_OP_call4

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. A few nits and waiting for the test case. Comment at: source/Expression/DWARFExpression.cpp:2701-2704 + auto ref_die = dwarf_cu->GetDIE(die_ref_offset); +

[Lldb-commits] [PATCH] D61648: [DWARF] Centralize user_id <-> DWARFDIE conversions

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:137 - SymbolFileDWARF *GetDWARFForUID(lldb::user_id_t uid); - - DWARFDIE - GetDIEFromUID(lldb::user_id_t uid); + DWARFDIE GetDIEFromUID(lldb::user_id_t uid); + lldb::user_id_t

[Lldb-commits] [PATCH] D61659: Fix bug in ArchSpec::MergeFrom

clayborg created this revision. clayborg added reviewers: labath, zturner, jingham. Previous ArchSpec tests didn't catch this bug since we never tested just the OS being out of date. Fixed the bug and covered this with a test that would catch this. This was found when trying to load a core

[Lldb-commits] [PATCH] D61292: Include inlined functions when figuring out a contiguous address range

clayborg added a comment. Jim: this is a modified version of the patch we had talked about a while ago with needed fixes for making sure the inline function call site is the same as the starting file and line. We have iterated on this locally already, so I am good with this patch. Let us know

[Lldb-commits] [PATCH] D61423: MinidumpYAML: add support for the ThreadList stream

clayborg added a comment. lgtm Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61423/new/ https://reviews.llvm.org/D61423 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D61543: Initialization: move InstructionEmulation to full initialization

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. lgtm Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61543/new/ https://reviews.llvm.org/D61543 ___

[Lldb-commits] [PATCH] D61587: [lldb] Added support for dwarf expressions DW_OP_call2/DW_OP_call4

clayborg added a comment. Need to add a test for this as well. And to cover all cases, you will need to test in a multi-compile unit example so we can ensure the relative CU offset of the call2 or call4 works in compile units that aren't the first one. Repository: rLLDB LLDB CHANGES SINCE

[Lldb-commits] [PATCH] D61578: [Driver] Add command line option to allow loading local lldbinit file

clayborg added a comment. Just wanted to verify that we can put a: settings set target.load-cwd-lldbinit true in our ~/.lldbinit file and it will then load the local init file in the current working directory? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D61587: [lldb] Added support for dwarf expressions DW_OP_call2/DW_OP_call4

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: include/lldb/Expression/DWARFExpression.h:307 const lldb::RegisterKind reg_set, const Value

[Lldb-commits] [PATCH] D61292: Include inlined functions when figuring out a contiguous address range

clayborg added inline comments. Comment at: lldb/include/lldb/Symbol/Declaration.h:113 + /// Compares the specification from \a rhs. If the file specifications are + /// equal, then continue to compare the line. + /// How about just returning a bool: ``` bool

[Lldb-commits] [PATCH] D61183: PostfixExpression: Introduce CFANode

clayborg added a comment. FYI: for all DWARF expressions found in EH frame stuff, they expect the CFA address to be pushed onto the stack prior to executing the expression. Do we just want to follow suit with the these expressions and avoid the extra needed node? CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D59015: [lldb-mi] Include full path in the -data-disassemble response

clayborg added inline comments. Comment at: lldb/tools/lldb-mi/MICmdCmdData.cpp:419 + // Get a full path to the file. + std::unique_ptr pPathBuffer(new char[PATH_MAX]); + lineEntry.GetFileSpec().GetPath(pPathBuffer.get(), PATH_MAX); anton.kolesov

[Lldb-commits] [PATCH] D63802: Handle nested register definition xml files from the remote serial protocol stub

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. few nits, but looks good. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4584-4585 }); +} +else { + XMLNode feature_node =

[Lldb-commits] [PATCH] D61233: Refactor ObjectFile::GetSDKVersion

clayborg added a comment. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61233/new/ https://reviews.llvm.org/D61233 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; aprantl wrote: > jingham wrote: > > clayborg

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added a comment. Jim: do you have any other solutions that could make this work? Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false;

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; davide wrote: > clayborg wrote: > > Still

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added inline comments. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetName())) + return false; Still seems weird that any language can white

[Lldb-commits] [PATCH] D64373: Don't use PyInt on Python 3

clayborg added a comment. ok, so do you want to abandon this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64373/new/ https://reviews.llvm.org/D64373 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

clayborg added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h:103 lldb_private::LazyBool m_associated_with_libdispatch_queue; + bool m_use_g_packet; Not a big deal, but might be easier to store this in

[Lldb-commits] [PATCH] D64373: Don't use PyInt on Python 3

clayborg added a comment. never mind, I missed that you had! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64373/new/ https://reviews.llvm.org/D64373 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D64535: Add convenience methods to convert LLDB to LLVM data structures.

clayborg added a comment. Just wondering if we want to cache the llvm::DWARFContext in the LLDB DWARFContext. See inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:69 + + std::unique_ptr GetAsLLVM() const; }; Should we cache

[Lldb-commits] [PATCH] D64159: [Core] Generalize ValueObject::MaybeCalculateCompleteType

clayborg added a comment. The main issue I have with this is the name of the function we are adding to LanguageRuntime. See inlined comments. Comment at: include/lldb/Target/ObjCLanguageRuntime.h:253 + CompilerType CalculateCompleteType(CompilerType base_type) override; +

[Lldb-commits] [PATCH] D63914: Make the expression parser work for missing weak symbols

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. A few nits, but nothing structural. Fix those and this will be good to go. Comment at: include/lldb/Symbol/Symbol.h:257

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

clayborg accepted this revision. clayborg added inline comments. Comment at: lldb/lit/SymbolFile/sizeless-symbol.test:5 + +image lookup --address 1 +# CHECK: Summary: sizeless-symbol.test.tmp.o`sizeful Symbols are ordered in the same order as they appear in the

[Lldb-commits] [PATCH] D64194: [lldb] Fix crash due to dollar character in variable names.

clayborg added a comment. Do we really expect users to actually write code where variables start with '$'? We will need to make some clear rules of the lookup order if so. I would suggest the following order - look for real variables by exact name match first - look for expression global

[Lldb-commits] [PATCH] D61000: Kill modify-python-lldb.py

clayborg added a comment. Can you post a diff of things before and after? Hard to tell what we are opting out of just by seeing this patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61000/new/ https://reviews.llvm.org/D61000 ___

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

clayborg added a comment. In D61090#1477709 , @davide wrote: > @clayborg I would also like to point out that some of us fair amount of time > cleaning `LLDB_DISABLE_PYTHON` leaking all over the codebase, and this is > basically the last occurrence, so

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

clayborg added a comment. People use this to populate the extra system path in python scripts when "import lldb" fails and throws an exception as you can run "lldb -P" to get the python path for the lldb module for the current lldb in your path. Repository: rLLDB LLDB CHANGES SINCE LAST

[Lldb-commits] [PATCH] D61101: Hide stderr output from lldb-argdumper

clayborg added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61101/new/ https://reviews.llvm.org/D61101 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D61000: Kill modify-python-lldb.py

clayborg accepted this revision. clayborg added a comment. Thanks for the diff! LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61000/new/ https://reviews.llvm.org/D61000 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Fine to fix the current issue. We should think about building this into the base class eventually. We can always have multi-threaded access, so we will always need to protect modifying

[Lldb-commits] [PATCH] D61064: Object/Minidump: Add support for the ThreadList stream

clayborg added a comment. LGTM, but since in llvm someone else should ok too. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61064/new/ https://reviews.llvm.org/D61064 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D60405: MinidumpYAML: Add support for ModuleList stream

clayborg added inline comments. Comment at: lib/ObjectYAML/MinidumpYAML.cpp:91 + size_t BlobAllocator::allocateString(StringRef Str) { SmallVector WStr; labath wrote: > clayborg wrote: > > Might be nice to unique the strings in here? > Yeah, that would be

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

clayborg added a comment. Almost seems like we can build the mutex into the base class OptionValue as we need general threaded protection for every setting. They any function that gets or sets the value should be able to protect itself using the base mutex CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

clayborg added a comment. I agree with Pavel: no new SB APIs needed as this call is where it is supposed to be on SBHostOS. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61090/new/ https://reviews.llvm.org/D61090 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

clayborg added a comment. I agree with all that was said above. My reason for leaving it in SBHostOS is due to SBCommandInterpreter not being the right place, since we don't have SBScriptInprereter in the API, and we might as well leave it in SBHostOS until we are going to do it right by

[Lldb-commits] [PATCH] D60968: WIP, RFC: Add an example of how LLDB python plug-in could look like

clayborg added a comment. Not sure what functionality this really adds? It seems to just wrap the OS plug-in python class in a different way? Comment at: lldb/examples/python/lldb-os-plugin/arch/arc.py:15 +# Find the location of registers description file. +_interp =

[Lldb-commits] [PATCH] D59015: [lldb-mi] Include full path in the -data-disassemble response

clayborg added inline comments. Comment at: lldb/tools/lldb-mi/MICmdCmdData.cpp:419 + // Get a full path to the file. + std::unique_ptr pPathBuffer(new char[PATH_MAX]); + lineEntry.GetFileSpec().GetPath(pPathBuffer.get(), PATH_MAX); anton.kolesov

[Lldb-commits] [PATCH] D60948: yamlify TestMiniDumpUUID binaries

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-zero-uuids.yaml:5 +Processor Arch: AMD64 +Platform ID:

[Lldb-commits] [PATCH] D62570: Use LLVM's debug line parser in LLDB

clayborg added a comment. So in the actual line table parsing code it seems like we are creating a line table in LLVM format, complete with sequences and a bunch of LLVM data structures, and then copying the results over into LLDB and then throwing away the LLVM line table. It would be nicer

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added a comment. Jim, you ok with doing the symbol context scope refactoring as a separate diff? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63240/new/ https://reviews.llvm.org/D63240 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

clayborg added a comment. In D63540#1556989 , @labath wrote: > In D63540#1550858 , @clayborg wrote: > > > So I am fine with symbols having zero size being in the symbol table. I > > would be fine not changing

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

clayborg added a comment. In D63240#1562527 , @jingham wrote: > This looks good to me. I wonder if the SymbolContextScope -> Language > calculation that you do in IsRuntimeSupportValue should really be done in > SymbolContextScope? If that's going to

[Lldb-commits] [PATCH] D62570: Use LLVM's debug line parser in LLDB

clayborg added a comment. Thanks for running the perf tests, that will be great to keep handy somewhere. Might be good to check this test in somewhere. I still think that making changes to the llvm line parser might be a good idea where we can get a callback when a row is to be pushed with the

[Lldb-commits] [PATCH] D62570: Use LLVM's debug line parser in LLDB

clayborg added a comment. I agree with Adrian, this isn't something that needs to hold up this patch, but I do think it is worth doing soon in LLVM. The line table parsing code just needs to take a std::function (or llvm equivalent) that gets called like a lambda when a row is to be pushed.

[Lldb-commits] [PATCH] D65435: SymbolVendor: Introduce Module::GetSymbolFile

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just add a few null checks and this will be good to go. Comment at: source/Commands/CommandObjectTarget.cpp:2240 Module *m =

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

clayborg added a comment. IMHO: we should keep this command and expand its abilities to help report stepping issues, expression issues and more. Why? Getting good bug reports from users is quite hard. Allowing them to type "bugreport step --step-in" or "bugreport step --step-over" would be

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

clayborg added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:595-624 +bool RegisterContextLLDB::TryResolveSymbolContextAndAddressRange( +lldb_private::Address pc, lldb_private::SymbolContext _ctx, +lldb_private::AddressRange

[Lldb-commits] [PATCH] D65435: SymbolVendor: Introduce Module::GetSymbolFile

clayborg added a comment. Alternatively we can switch to using a reference for: SymbolFile ::GetSymbolFile(bool can_create Stream *feedback_strm); As I believe we always fall back to SymbolFileSymtab don't we? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65435/new/

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I will let Jason comment on the unwind specifics since this is his area. I caught a few other things that need to be cleaned up. Comment at:

[Lldb-commits] [PATCH] D65647: Fix line table resolution near the end of a section

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I vote to not add any API calls with allow_section_end and just take care of this in source/Symbol/LineTable.cpp. All of my inline comments will reflect this notion.

[Lldb-commits] [PATCH] D65611: [Driver] Expand the target in the driver.

clayborg added a comment. Also be careful if the user uses a symlink to not resolve the link. If a user tries to debug clang++: $ ls -AFlG /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang* -rwxr-xr-x 1 root wheel 8156 Jul 12 21:48

[Lldb-commits] [PATCH] D61090: [SBHostOS] Remove getting the python script interpreter path

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. We can't change functionality in our API. Fine to add something to SBDebugger, but we can't just disable a call that used to work in SBHostOS. Comment at:

[Lldb-commits] [PATCH] D65691: Various build fixes for lldb on MinGW

clayborg added a comment. In D65691#1613395 , @mstorsjo wrote: > As context (I haven't followed lldb almost at all), what's the general status > of lldb on windows - generally working, or still a work in progress? Does it > work with dwarf debug info in

[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

clayborg added inline comments. Comment at: source/Target/StopInfo.cpp:550 thread_sp->ResetStopInfo(); +thread_sp->SetStopInfo(thread_sp->GetStopInfo()); } Can you clarify what is going on here? Does this force a recalculation of the

[Lldb-commits] [PATCH] D66174: [Utility] Reimplement RegularExpression on top of llvm::Regex

clayborg added a comment. Looks fine to me. A few nits. Comment at: lldb/include/lldb/Utility/RegularExpression.h:98 + /// The compiled regular expression. + mutable llvm::Regex m_regex; }; Why does this need to be mutable? Comment at:

[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

clayborg added inline comments. Comment at: source/Target/StopInfo.cpp:550 thread_sp->ResetStopInfo(); +thread_sp->SetStopInfo(thread_sp->GetStopInfo()); } jingham wrote: > clayborg wrote: > > Can you clarify what is going on here? Does

[Lldb-commits] [PATCH] D66250: [JIT][Unwinder] Add Trampoline ObjectFile and UnwindPlan support for FCB

clayborg added a comment. Why are we not just using ObjectFileJIT? I am guessing these breakpoint expressions create one of these by compiling the breakpoint expression and JIT'ing it just like any other expression. If this is the case, then why do we need to create a ObjectFileTrampoline?

[Lldb-commits] [PATCH] D66357: Fix GetDIEForDeclContext so it only returns entries matching the provided context

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Needs a test, but looks good. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66357/new/ https://reviews.llvm.org/D66357

[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Better. Might be good to put a comment inside CalculatePublicStopInfo() regarding why the "SetStopInfo(GetStopInfo());" is needed. Seems like you could just remove the code from the sight

[Lldb-commits] [PATCH] D66102: [Symbol] Decouple clang from CompilerType

clayborg added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66102/new/ https://reviews.llvm.org/D66102 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D66357: Fix GetDIEForDeclContext so it only returns entries matching the provided context

clayborg added a comment. Pavel, any comments on the testing code? LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66357/new/ https://reviews.llvm.org/D66357 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D66345: [lldb][NFC] Allow for-range iterating over StringList

clayborg added a comment. The other thing to think about is the SB layer must be thread safe. StringList is not thread safe right now, but it probably should be. We might need to keep the internal version of StringList and make sure it is thread safe. That will ensure that our buildbots always

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/lldb-enumerations.h:170 // etc... + eFormatUnicode8, eFormatUnicode16, add to the end,

[Lldb-commits] [PATCH] D66357: Fix GetDIEForDeclContext so it only returns entries matching the provided context

clayborg added a comment. In D66357#1634175 , @guiandrade wrote: > In D66357#1633461 , @clayborg wrote: > > > Needs a test, but looks good. > > > Sure, I agree. Though, it's not total clear to me how we could do

[Lldb-commits] [PATCH] D66654: Prevent D66398 `TestDataFormatterStdList`-like regressions in the future

clayborg added a comment. What about having the first one that matched being the one that is used and avoid errors all together? We do this with the regex alias stuff already. With regular expressions people are sure to make ones that conflict with each other. Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D66863: [lldb] Unify target checking in CommandObject

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just a few string reflows and fix a typo Comment at: lldb/source/Commands/CommandObjectBreakpointCommand.cpp:593 + : CommandObjectParsed(interpreter,

[Lldb-commits] [PATCH] D66934: [ARM64] Simplify RegisterInfos_arm64.h with macro based RegisterInfo array

clayborg added a comment. Just a few nits around DEFINE_GPR64 and how the "alt" names are specified. See inline comments. Comment at: source/Plugins/Process/Utility/RegisterInfos_arm64.h:492 +// Defines a 64-bit general purpose register +#define DEFINE_GPR64(reg, alt,

[Lldb-commits] [PATCH] D67018: [lldb][NFC] Add basic test for GUI command

clayborg added a comment. It is great to have tests. The main issue I would see is some of the text might not render correctly if the screen size is too small. Can we control the size (char width and height) of the screen in pexpect tests? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION

<    7   8   9   10   11   12   13   14   15   16   >