[Lldb-commits] [PATCH] D49632: [WIP] Re-implement MI HandleProcessEventStateSuspended.

clayborg added a comment. So with CMICmnLLDBDebuggerHandleEvents::HandleProcessEventStateSuspended() it will report a bunch of text back through the MI interface with this each time? Why would it do that? I would assume that the MI interface would handle this programmatically? ==

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

clayborg added a comment. We do need to test the performance of this as demangling is a hot spot when we parse any object file. If it is faster, then we should just replace it and not add any options to be able to switch. Also we should compare demangled names between the two to ensure there ar

[Lldb-commits] [PATCH] D49632: [WIP] Re-implement MI HandleProcessEventStateSuspended.

clayborg added a comment. If you look at what this patch is doing, it ends sending text to stdout at the end. So it seems like this function's sole purpose is to report the process status after stop to STDOUT. Seeing as this is a machine interface (MI) to a debugger, I was wondering why it woul

[Lldb-commits] [PATCH] D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties.

clayborg added a comment. We could end up moving anything that is set by a target property into the lldb_private target property class if it isn't already there and then that would fix things. https://reviews.llvm.org/D47302 ___ lldb-commits maili

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I think doing this in the module list is not the right place. Why? Some platforms might have multiple sysroot to check. iOS for instance has a directory for each device that Xcod

[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

clayborg added inline comments. Comment at: include/lldb/API/SBTarget.h:275-276 + void AppendImageSearchPath(const char *from, const char *to); + + void AppendImageSearchPath(const char *from, const char *to, Remove this first one? Other functions were first

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg added a comment. We should never be loading the wrong shared libraries. The module spec we fill out must contain the UUID of the file are looking for. If it doesn't we have no chance of every really loading the right stuff. Repository: rL LLVM https://reviews.llvm.org/D49685 ___

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg added a comment. In https://reviews.llvm.org/D49685#1173720, @EugeneBi wrote: > In https://reviews.llvm.org/D49685#1173640, @clayborg wrote: > > > We should never be loading the wrong shared libraries. The module spec we > > fill out must contain the UUID of the file are looking for. If

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg created this revision. clayborg added reviewers: labath, zturner, markmentovai. Herald added a reviewer: javed.absar. Herald added subscribers: chrib, kristof.beyls. In this patch I add support for ARM and ARM64 break pad files. There are two flavors of ARM: Apple where FP is https://rev

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. This will help, but not fix us loading incorrect versions of the shared library. I wonder if there is anything in the core file that could help use get the build ID/UUID of each binary. I

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg added a comment. I will make update the patch with many of the suggested inline comments. Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:150 + if (m_arch.IsValid()) +return m_arch; const MinidumpSystemInfo *system_info = GetSystemInfo(); ---

[Lldb-commits] [PATCH] D49990: Use rich mangling information in Symtab::InitNameIndexes()

clayborg added inline comments. Comment at: include/lldb/Core/Mangled.h:25-34 +namespace lldb_private { +class RichManglingInfo; +class RichManglingSpec; +} namespace lldb_private { class RegularExpression; } move any forward decls to lldb-forward.h and remove

[Lldb-commits] [PATCH] D49632: [lldb-mi] Re-implement MI HandleProcessEventStateSuspended.

clayborg added a comment. I still don't get why we are printing process stopped information to STDOUT. MI is a machine interface for a IDE. The IDE should be showing the process state in the GUI. https://reviews.llvm.org/D49632 ___ lldb-commits ma

[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

clayborg added inline comments. Comment at: source/API/SBTarget.cpp:1476-1477 + } else +error.SetErrorStringWithFormat(" can't be empty " + "(SBTarget::%s) failed", __FUNCTION__); +} check if csFrom or csTo is empty and give

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg updated this revision to Diff 158321. clayborg added a comment. Herald added a subscriber: srhines. - Fixed inline comments - Moved platform setting into Target::SetArchitecture(...) instead of doing this manually in the core file code. - Added ARM64 w0-w31, d0-d31, s0-s31 and h0-h31 reg

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg added inline comments. Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:192 + +static size_t k_num_reg_infos = llvm::array_lengthof(g_reg_infos); + lemo wrote: > constexpr? will do Comment at: source/Plugins/

[Lldb-commits] [PATCH] D49963: Preliminary patch to support prompt interpolation

clayborg added a comment. This will be very cool. Biggest issues to watch out for is to make sure it doesn't return incorrect values when running for things like the thread count. If you switch to use the "m_debugger.GetCommandInterpreter().GetExecutionContext()" it should solve this by making

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg updated this revision to Diff 158371. clayborg added a comment. Removed unnecessary Xcode project changes and removed #include that wasn't needed. https://reviews.llvm.org/D49750 Files: include/lldb/Target/Target.h lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/f

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg added inline comments. Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:15 // Other libraries and framework includes +//#include "lldb/Core/Architecture.h" #include "lldb/Core/Module.h" lemo wrote: > it this set for removal? Ah yes! ==

[Lldb-commits] [PATCH] D50038: Introduce install-lldb-framework target

clayborg added a comment. Might be nice to put a blurb in the build page about this in the MacOS section? https://reviews.llvm.org/D50038 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-c

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg updated this revision to Diff 158631. clayborg added a comment. - run clang-format - fix doxygen parameter names https://reviews.llvm.org/D49750 Files: include/lldb/Target/Target.h lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/postmortem/minidump-

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

clayborg marked 2 inline comments as done. clayborg added inline comments. Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp:57 +nullptr, nullptr, nullptr, 0} + +// Zero based LLDB register numbers for this register context No need

[Lldb-commits] [PATCH] D50274: Misc module/dwarf logging improvements

clayborg added a comment. LGTM after Pavel's inline comments are addressed. Repository: rLLDB LLDB https://reviews.llvm.org/D50274 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commi

[Lldb-commits] [PATCH] D50336: Add support for ARM and ARM64 breakpad generated minidump files (version 2).

clayborg created this revision. clayborg added reviewers: labath, zturner, markmentovai, javed.absar. Herald added subscribers: chrib, kristof.beyls. In this patch I add support for ARM and ARM64 break pad files. There are two flavors of ARM: Apple where FP is https://reviews.llvm.org/source/open

[Lldb-commits] [PATCH] D50336: Add support for ARM and ARM64 breakpad generated minidump files (version 2).

clayborg updated this revision to Diff 159324. clayborg added a comment. Herald added a subscriber: mgorny. Added CMakeList.txt changes, tested on linux, and removed unused "log" variable. https://reviews.llvm.org/D50336 Files: include/lldb/Target/Target.h lldb.xcodeproj/project.pbxproj

[Lldb-commits] [PATCH] D50336: Add support for ARM and ARM64 breakpad generated minidump files (version 2).

clayborg added a comment. Fixed offsetof issues with: $ svn commit Sendingsource/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp Sendingsource/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp Transmitting file data ..done Committing transaction... Committed revi

[Lldb-commits] [PATCH] D50336: Add support for ARM and ARM64 breakpad generated minidump files (version 2).

clayborg added a comment. Passing patches between linux and mac the offsetof fixes got lost. When binary files are involved, patches are trickier to pass between to machines. Repository: rL LLVM https://reviews.llvm.org/D50336 ___ lldb-commits m

[Lldb-commits] [PATCH] D50336: Add support for ARM and ARM64 breakpad generated minidump files (version 2).

clayborg added a comment. More offsetof issues: $ svn commit Sendingsource/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp Transmitting file data .done Committing transaction... Committed revision 339034. Repository: rL LLVM https://reviews.llvm.org/D50336 ___

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg added a comment. In https://reviews.llvm.org/D50365#1191059, @lemo wrote: > Really cool! Are you planning to add some documentation for it? (set up > instructions, etc...) Yes. I will add a README.txt for this patch and also a python script that will create an VSCode extension. htt

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg added a comment. In https://reviews.llvm.org/D50365#1192447, @zturner wrote: > To elaborate, if you install the C/C++ plugin, and you go to Debug -> > Configurations, and you go down to the MICmdMode property, it is by default > set to "gdb". But you can change this to "lldb" and it w

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg added a comment. In https://reviews.llvm.org/D50365#1191944, @labath wrote: > I am not sure I'll have the resources to see this review through, so I'd > prefer to leave this to someone else. > > The thoughts I have had so far are: > > - the patch is very big and probably runs afoul of t

[Lldb-commits] [PATCH] D50473: [Demangle] Add another test for ItaniumPartialDemangler

clayborg added a comment. BTW: warning I saw during recent build: [98/790] Building CXX object tools/lldb/source/Core/CMakeFiles/lldbCore.dir/RichManglingContext.cpp.o /home/gclayton/local/src/llvm/svn/llvm/tools/lldb/source/Core/RichManglingContext.cpp: In member function ‘bool lldb_priva

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg updated this revision to Diff 160466. clayborg added a comment. Herald added subscribers: jfb, srhines. - Use the LLVM JSON parser - Split lldb-vscode.cpp into smaller files - Fix function names - ran clang format on everything https://reviews.llvm.org/D50365 Files: lldb.xcodeproj/pr

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg added a comment. This should be in great shape now. Can anyone find time for this? https://reviews.llvm.org/D50365 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg added inline comments. Comment at: tools/lldb-vscode/BreakpointBase.cpp:21 + return fail_value; +} +} // namespace Sure thing Comment at: tools/lldb-vscode/ExceptionBreakpoint.cpp:22 + +void ExceptionBreakpoint::ClearBreakpoint() { +

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg marked 23 inline comments as done. clayborg added inline comments. Comment at: tools/lldb-vscode/SourceBreakpoint.h:24 + // Set this breakpoint in LLDB as a new breakpoint + void SetBreakpoint(const char *source_path); +}; zturner wrote: > clayborg wro

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg marked 13 inline comments as done. clayborg added inline comments. Comment at: tools/lldb-vscode/JSONUtils.cpp:472 +char path[PATH_MAX] = ""; +file.GetPath(path, sizeof(path)); +if (path[0]) { This is a SBFileSpec. We don't allow any STL to b

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg added a subscriber: davide. clayborg added a comment. Inline comments really help if you don't mind. https://reviews.llvm.org/D50365 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/ll

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg marked 2 inline comments as done. clayborg added inline comments. Comment at: tools/lldb-vscode/lldb-vscode.cpp:2646 +g_vsc.out = fdopen(socket_fd, "w"); +if (g_vsc.in == nullptr || g_vsc.out == nullptr) { + if (g_vsc.log) The mu

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

clayborg added a comment. You can always run a.out through obj2yaml and check that in. There is test suite support for using a yaml file that converts it to a binary and debugs it functionalities/gdb_remote_client/gdbclientutils.py 429:def createTarget(self, yaml_path): 431:Cre

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

clayborg added inline comments. Comment at: source/Target/Process.cpp:4692-4693 +// the IOHandler for Editline). +bool cancel_top_handler = m_mod_id.IsRunningUtilityFunction(); +GetTarget().GetDebugger().PushIOHandler(io_handler_sp, cancel_top_handler); return tr

[Lldb-commits] [PATCH] D50481: Fix broken builtin functions in the expression command

clayborg added a comment. Changed look fine. Wondering if we want to be adding pexpect style tests though. Those tests tend to be flaky. This could easily be done with SB API calls instead of using pexpect. Repository: rLLDB LLDB https://reviews.llvm.org/D50481 __

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

clayborg added a comment. In https://reviews.llvm.org/D50912#1209994, @jingham wrote: > What would happen to any output that the process produced while running the > utility function if we did this. I believe it would still be fetched on next stop. Just not real time. Do you think anyone runn

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

clayborg added inline comments. Comment at: include/lldb/Target/Process.h:479 +else + m_running_utility_function--; + } Might want this to be: ``` else if (m_running_utility_function > 0) --m_running_utility_function; ``` Just to make sure we don't

[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Dynamic loaders are needed for loading breakpad minidumps that are for MacOSX and iOS processes. They should also be needed for loading any minidumps that have stack traces. Re

[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

clayborg added a comment. No dynamic loader plug-ins should be affecting the module list during the plug-in loading/selection, if that is happening, that is a bug and it should be fixed. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 __

[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

clayborg added a comment. Dynamic loaders will fill out the section load list in the target that saids "__TEXT" from "/tmp/a.out" was loaded at address 0x120200. So yes they are needed. If the process minidump is manually setting the section load list itself, then there really is no need fo

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

clayborg added a comment. Sounds good. I am ok with this as long as Jim Ingham is. https://reviews.llvm.org/D50912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

clayborg added a comment. So this might actually work. Take a look around and see what else the dynamic loader is used for and make sure that they won't be needed for anything else. If not, this fix should work, but we should document it. Repository: rLLDB LLDB https://reviews.llvm.org/D511

[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. I am fine with this change then. Probably best to get the OK from Zach as well. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___

[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

clayborg added a comment. In https://reviews.llvm.org/D51176#1211433, @jingham wrote: > The other option would be to move the code that populates the section load > list into the mini dump dynamic loader. That has the benefit of keeping this > consistent with the other process plugins, but OTO

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

clayborg added a comment. In https://reviews.llvm.org/D32167#1212783, @jankratochvil wrote: > Just a ping, that `ConcatenatingDataExtractor` is still not coded, even as > some draft patch? Just checking to prevent duplicating some existing work. > FYI providing a rebase with few simple conflict

[Lldb-commits] [PATCH] D51208: [DWARF] Fix dwarf5-index-is-used.cpp

clayborg added a comment. In https://reviews.llvm.org/D51208#1214743, @dblaikie wrote: > >> But if LLDB has different performance characteristics, or the default > >> should be different for other reasons - I'm fine with that. I think I left > >> it on for Apple so as not to mess with their stu

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

clayborg added a comment. Looks fine. A bit of cleanup might be nice on the TypeSystem.h to provide default implementations that just return 0 for some of these that every type system currently is required to override for no reason (all template related type system calls seem to have default "r

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

clayborg created this revision. clayborg added reviewers: zturner, labath, dvlahovski, lemo. The CvRecordPdb70 structure looks like: struct CvRecordPdb70 { uint8_t Uuid[16]; llvm::support::ulittle32_t Age; // char PDBFileName[]; }; We are currently including the "Age" in the UUID

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

clayborg added a comment. In https://reviews.llvm.org/D51442#1217829, @zturner wrote: > For PE/COFF files, the Age is also in the executable and Guid+Age actually > constitute a 20-byte UUID. Is this not the case on Apple? What object file > format are you dealing with? Breakpad files that co

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

clayborg added a comment. In https://reviews.llvm.org/D51442#1217894, @lemo wrote: > I'm curious too: where did the PDB70 age create matching problems? For breakpad ARM and ARM64 minidumps that are for Apple vendor triples. > On a related note, I just noticed that ObjectFilePECOFF::GetUUID() d

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

clayborg updated this revision to Diff 163143. clayborg added a comment. Make 16 byte UUIDs Apple specific. https://reviews.llvm.org/D51442 Files: source/Plugins/Process/minidump/MinidumpParser.cpp Index: source/Plugins/Process/minidump/MinidumpParser.cpp ===

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good to me. Jim should take a look as well. https://reviews.llvm.org/D51387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org h

[Lldb-commits] [PATCH] D51569: Hold GIL while allocating memory for PythonString.

clayborg added a comment. Not sure about this one. IIUC we now wouldn't take the GIL for these functions now and hope that the str() function doesn't do something that would require thread safety? Repository: rLLDB LLDB https://reviews.llvm.org/D51569

[Lldb-commits] [PATCH] D51557: Replace uses of LazyBool with LazyBool template

clayborg added a comment. Very nice. LGTM https://reviews.llvm.org/D51557 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48704: [LLDB] Fix for "Bug 37950: ExecutionContext::GetByteOrder() always returns endian::InlHostByteOrder()"

clayborg added a comment. Should be easy to write unit test for ExecutionContext.cpp. See FileSpecTest.cpp for how to implement a unit test for a .cpp file. Repository: rLLDB LLDB https://reviews.llvm.org/D48704 ___ lldb-commits mailing list lld

[Lldb-commits] [PATCH] D51600: [NFC] Fixed enum constant in boolean context error

clayborg added a comment. If you insert the text: Differential Revision: https://reviews.llvm.org/D51600 Into your commit it will automatically show the SVN revision that was used for the commit. Can you attach the SVN revision to this as a comment so we can track this? Repository: rLLDB

[Lldb-commits] [PATCH] D51569: Hold GIL while allocating memory for PythonString.

clayborg added a comment. What kind of scenario causes this crash to happen? Seems like a hack? Repository: rLLDB LLDB https://reviews.llvm.org/D51569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[Lldb-commits] [PATCH] D51604: Terminate debugger if an assert was hit

clayborg added a comment. Yes abort is better as exit will cause the global C++ destructor chain to be called. I was always thinking lldbassert() was aborting, but seeing as this is not the case this could cause problems if you enable it as it will be a change. Watch the buildbots carefully.

[Lldb-commits] [PATCH] D51604: Terminate debugger if an assert was hit

clayborg added a comment. In https://reviews.llvm.org/D51604#1223451, @xbolva00 wrote: > Thanks for review, commited as https://reviews.llvm.org/rL341387 If you include the text: Differential Revision: https://reviews.llvm.org/D51604 It will automatically close this and put in a comment wit

[Lldb-commits] [PATCH] D51661: Print column info in backtraces et al. if available

clayborg added a comment. We shouldn't have the colon character be part of the ${line.column} formatting itself. See inlined comments. Comment at: source/Core/Debugger.cpp:123 +#define FILE_AND_LINE \ + "{ at ${line.fi

[Lldb-commits] [PATCH] D51661: Print column info in backtraces et al. if available

clayborg added a comment. See my inlined comments about returning true and false correctly for the column and the correction to the format string Comment at: source/Core/Debugger.cpp:123 +#define FILE_AND_LINE \ + "{ a

[Lldb-commits] [PATCH] D51661: Print column info in backtraces et al. if available

clayborg added inline comments. Comment at: source/Core/FormatEntity.cpp:1826 +} +// Column info is optional, so this always succeeds. +return true; Remove this comment Repository: rLLDB LLDB https://reviews.llvm.org/D51661 ___

[Lldb-commits] [PATCH] D51730: [DWARFExpression] Read literars as unsigned values.

clayborg added a comment. Can we add a test for this? Repository: rLLDB LLDB https://reviews.llvm.org/D51730 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

clayborg added a comment. See inlined comments. Cool stuff. Comment at: include/lldb/API/SBBreakpoint.h:26-27 + SBBreakpoint(const lldb::BreakpointSP &bp_sp); + ~SBBreakpoint(); Why does this need to be public? Comment at: include/lldb/

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

clayborg added a comment. I would rather not change the argument to be required if we can help it. That might break existing scripts or command files. I think if -d is specified without a value it should default to "no". Can we make the value optional? Repository: rLLDB LLDB https://reviews

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

clayborg added a comment. Or add a new option that is --dependents that takes the enum and leave the --no-dependents there for compatability Repository: rLLDB LLDB https://reviews.llvm.org/D51934 ___ lldb-commits mailing list lldb-commits@lists.

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Nice patch! A few minor fixes, see inlined comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:78 +uint32_t DWARFCompileUnit::GetDWARF5

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

clayborg added a comment. Only issue now is documentation and why are we requiring addresses to be in compile unit. See inlined comments. Comment at: include/lldb/API/SBTarget.h:667 + lldb::SBBreakpoint BreakpointCreateFromScript( + const char *class_name, + SBStruc

[Lldb-commits] [PATCH] D51966: Do not create new terminals when launching process on Windows by default

clayborg added a comment. We should obey the --tty option here and set CREATE_NEW_CONSOLE if it is set no? Repository: rLLDB LLDB https://reviews.llvm.org/D51966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

clayborg added inline comments. Comment at: source/Commands/CommandObjectTarget.cpp:143-151 +static OptionEnumValueElement g_dependents_enumaration[4] = { +{eLoadDependentsDefault, "default", + "Only load dependents when the target is an executables."}, +{eLoadDepende

[Lldb-commits] [PATCH] D51966: Do not create new terminals when launching process on Windows by default

clayborg added a comment. The other option is to only remove this flag if "--no-stdio" is set. The best solution IMHO is: - find a way to get stdio from a process and feed to to LLDB so it shows up when neither --tty nor --no-stdio is set - when --tty is specified, set the CREATE_NEW_CONSOLE bi

[Lldb-commits] [PATCH] D51966: Do not create new terminals when launching process on Windows by default

clayborg added a comment. In https://reviews.llvm.org/D51966#1232057, @xbolva00 wrote: > So basically the hardest problem is to "find a way to get stdio from a > process and feed to to LLDB so it shows up when neither --tty nor --no-stdio > is set" .. yes. The easier solution is to just creat

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

clayborg requested changes to this revision. clayborg added a comment. Patch looks good. A few fixes mentioned in inlined comments. And I am unsure how the test suite will run with various compilers if they don't support the -gdwarf-5 flag. We might need to run obj2yaml on a binary and then yaml

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

clayborg added a comment. In https://reviews.llvm.org/D51935#1232203, @grimar wrote: > In https://reviews.llvm.org/D51935#1232195, @clayborg wrote: > > > Patch looks good. A few fixes mentioned in inlined comments. And I am > > unsure how the test suite will run with various compilers if they do

[Lldb-commits] [PATCH] D51730: [DWARFExpression] Read literars as unsigned values.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. The test really should encode actual DWARF that contains a DW_OP_litXXX opcode (using obj2yaml/yaml2obj or llvm-mc) as compiling the code for the test won't always produce the needed DWARF

[Lldb-commits] [PATCH] D51578: DWARFConcatenatingDataExtractor for D32167

clayborg added a comment. Overall I am ok with minimal regression in speed if a few percent is all that it is costing us. I am generally ok with this patch. A few questions below. Is there a reason we need DWARFConcatenatingDataExtractor? Can we just put all functionality into DWARFDataExtracto

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Thanks for doing all requested changes! Looks great. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp:442 +ReadDescriptors(debug_line_data, offset_p

[Lldb-commits] [PATCH] D52111: Make eSearchDepthFunction work for scripted resolvers

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Very nice. You got it right. Repository: rLLDB LLDB https://reviews.llvm.org/D52111 ___ lldb-commits mailing list lldb-commits@lists.llvm.

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

clayborg added a comment. See inlined comments. Comment at: www/python-reference.html:324 + +Another use of the Python API's in lldb is to create a custom breakpoint resolver. This + allows you to implement your own logic to search the space

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

clayborg added inline comments. Comment at: www/python-reference.html:325 +Another use of the Python API's in lldb is to create a custom breakpoint resolver. This facility + was added in r51967. + Is that SVN r

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Thanks for doing all the updates. Pretty clear and concise now. Repository: rLLDB LLDB https://reviews.llvm.org/D52065 ___ lldb-commits ma

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: include/lldb/Target/StackFrame.h:506 + lldb::ValueObjectSP FindVariable(const char *name); + davide wrote: > Also, I think y

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

clayborg added a comment. I am fine with this, Jim or Jason should ok this too just to be sure https://reviews.llvm.org/D51934 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Just a documentation suggestion, but looks good. Comment at: include/lldb/Target/StackFrame.h:508 + /// Attempt to reconstruct the ValueObject for a variable with a give

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

clayborg accepted this revision. clayborg added a comment. Looks good https://reviews.llvm.org/D52247 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

clayborg added a comment. In https://reviews.llvm.org/D50365#1241149, @lemo wrote: > Hi Greg, looking at request_evaluate() I noticed that it will evaluate the > string as a lldb command if prefixed by ` . > > This is a great feature (it allows building REPL consoles on top of DAP), but > I'm c

[Lldb-commits] [PATCH] D48623: Move architecture-specific address adjustment to architecture plugins.

clayborg added a comment. So I would suggest not removing any functions from lldb_private::Target, just change the old ones to call through the arch plug-ins if there is one. Then many changes in this diff just go away and we still get the clean implementation where things are delegated to the

[Lldb-commits] [PATCH] D39545: Support scoped enums in the DWARF AST parser

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D39545 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[Lldb-commits] [PATCH] D39681: Implement core dump debugging for PPC64le

clayborg added inline comments. Comment at: source/Plugins/Process/elf-core/ThreadElfCore.h:183 lldb_private::DataExtractor m_vregset_data; + lldb_private::DataExtractor m_vsregset_data; /* For PPC VSX registers. */ labath wrote: > gpregset and fpregset sou

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

clayborg accepted this revision. clayborg added a comment. Do we want to add an option to our build system to try LLD where it is supported? Doesn't need to be part of this patch, but it would be great to be able to try it out on ELF based systems. https://reviews.llvm.org/D39689 __

[Lldb-commits] [PATCH] D39692: Disable tests in lang/c/shared_lib on Windows

clayborg added a comment. Are we planning on getting shared libraries working on windows? Or this is just an expression parser with shared libraries bug? Be nice to file a bug and mention it if it is something we are planning on fixing. https://reviews.llvm.org/D39692 __

[Lldb-commits] [PATCH] D39733: Simplify NativeProcessProtocol::GetArchitecture/GetByteOrder

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Looks fine. Seems like you should use your a const reference in a few places and this will be good to go? Comment at: source/Plugins/Process/Linux/NativeRegis

[Lldb-commits] [PATCH] D39733: Simplify NativeProcessProtocol::GetArchitecture/GetByteOrder

clayborg added a comment. If it isn't expensive to copy, then we should probably just return by value. https://reviews.llvm.org/D39733 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-comm

<    1   2   3   4   5   6   7   8   9   10   >