[Lldb-commits] [PATCH] D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command.

clayborg added inline comments. Comment at: tools/lldb-mi/MICmdCmdExec.cpp:248 } - } else { -// ToDo: Re-evaluate if this is required when application near finished as -// this is parsing LLDB error message -// which seems a hack and is code brittle -const

[Lldb-commits] [PATCH] D48272: Replace HostInfo::GetLLDBPath with specific functions

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. My only question is do we not require people to only fill in the directory portion of the FileSpec anymore for these functions? I am fine with way since hopefully

[Lldb-commits] [PATCH] D47992: [lldb-mi] Correct error processing in exec-next command.

clayborg added a comment. In https://reviews.llvm.org/D47992#1134120, @apolyakov wrote: > If you look at `bool CMICmdCmdExecContinue::Execute()`, you'll see that there > are cases in which we need a flexible way to finish MI command(specific > action in error case for example). We have a few

[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

clayborg added a comment. Indeed!!! So happy to see this work getting done. Great stuff. Will be great to not have to maintain the Xcode project as some point in the near future. https://reviews.llvm.org/D48060 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

clayborg accepted this revision. clayborg added a comment. Looks good to me as long as Xcode still produces the same Info.plist in its LLDB.framework after your changes. https://reviews.llvm.org/D47792 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D48060: Introduce lldb-framework CMake target and centralize its logic

clayborg resigned from this revision. clayborg added a comment. I'll defer to the cmake experts. I hope we can produce a LLDB.framework that is identical to the Xcode produced version, or at least as close. https://reviews.llvm.org/D48060 ___

[Lldb-commits] [PATCH] D48084: [FileSpec] Delegate common operations to llvm::sys::path

clayborg added inline comments. Comment at: source/Utility/FileSpec.cpp:593 + llvm::SmallString<64> current_path; + GetPath(current_path, false); + return ConstString(llvm::sys::path::extension(current_path, m_style)); We won't need the full path in order to

[Lldb-commits] [PATCH] D47991: Improve SBThread's stepping API using SBError parameter.

clayborg added a comment. In https://reviews.llvm.org/D47991#1131181, @apolyakov wrote: > In https://reviews.llvm.org/D47991#1131043, @clayborg wrote: > > > Looks good. Just a question about including the commented out default > > arguments > > > Don't you think it increases readability of this

[Lldb-commits] [PATCH] D48177: Suppress SIGSEGV on Android when the program will recover

clayborg created this revision. clayborg added a reviewer: labath. Herald added a subscriber: srhines. SIGSEGV signals are sent to Android processes when a NULL dereference happens in Java code that is being run as native code in dex, odex and oat files. In this patch I modified the Platforms

[Lldb-commits] [PATCH] D24960: Remove unreachable from apis in posix terminal compat windows and return 0

clayborg added a comment. shouldn't these be returning -1 for some of these? Zero usually means no error. You would need to check each unix function and return the right error code for failure. Repository: rL LLVM https://reviews.llvm.org/D24960

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

clayborg added inline comments. Comment at: source/API/SBHostOS.cpp:48 + case ePathTypePythonDir: +fspec = ScriptInterpreterPython::GetPythonDir(); +break; Why is this here instead of inside of "HostInfo::GetLLDBPath(path_type, fspec);"? Does this mean

[Lldb-commits] [PATCH] D48212: Revert "[lldb-mi] Add overload method for setting an error"

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. I didn't remember we already did this in SBError... Sorry about that. https://reviews.llvm.org/D48212 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

clayborg added a comment. In https://reviews.llvm.org/D48215#1133695, @zturner wrote: > The internal api has no guarantees as to its stability. What do you mean by this? If we have an internal API that claims to give out paths, it should give out paths. What am I missing here?

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

clayborg added a comment. In https://reviews.llvm.org/D48215#1133725, @labath wrote: > (pressed return too soon) > > Would that address your reservations about this idea? yeah, just don't like the idea of HostInfo::GetLLDBPath() being non functional. https://reviews.llvm.org/D48215

[Lldb-commits] [PATCH] D48215: Remove dependency from Host to python

clayborg added a comment. It might be nice to start with the removal of HostInfo::GetLLDBPath(...) first and fixing up all call sites that use that API to use the new internally sanctioned versions. Then submit this patch? https://reviews.llvm.org/D48215

[Lldb-commits] [PATCH] D48049: Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Remove extra spaces before ( and good to go. Comment at: scripts/interface/SBTarget.i:282 lldb::SBProcess -LoadCore(const char *core_file); - +LoadCore

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222 +return true; + return GetReferencedDIE(DW_AT_specification) + .GetParent() labath wrote: > clayborg wrote: > > I can never remember when a

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

clayborg added a comment. So my previous code suggestion might not work as we would want to recurse through the specification or abstract origin chain. https://reviews.llvm.org/D47470 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

clayborg added inline comments. Comment at: source/Utility/FileSpec.cpp:821 } + //-- revert whitespace change. https://reviews.llvm.org/D47495 ___

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Remove the argument since we don't need it. Just assume we keep ".". Comment at: include/lldb/Utility/FileSpec.h:535 - void RemoveLastPathComponent(); +

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D47495 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D40470: Protect DWARFCompileUnit::m_die_array by a new mutex

clayborg added a comment. I think we should expose the llvm::sys::RWMutex and lets clients like BuildAddressRangeTable and SymbolFileDWARF::Index use it and get rid of m_die_array_usecount all together. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:181 + //

[Lldb-commits] [PATCH] D47481: Initialize FunctionCaller::m_struct_valid

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. We need a clang warning that tells us when POD types are not initialized! I am sure we have other similar issues... https://reviews.llvm.org/D47481

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

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

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

clayborg added a comment. After seeing this, it might be nice to put the recursion that follows the DW_AT_specification and DW_AT_abstract_origin and avoids infinite recursion into a DWARFDIE function that takes a callback: typedef bool (*DIECallback)(DWARFDIE ); void

[Lldb-commits] [PATCH] D47147: DWARFIndex: Reduce duplication in the GetFunctions methods

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Some if statements simplifications if you want to, but looks good. Comment at: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:193-195 + if

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:122 +PropertyDefinition g_experimental_properties[] = { +{"use-debug-names", OptionValue::eTypeBoolean, true, 0, nullptr, nullptr, + "Use .debug_names index section."},

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

clayborg accepted this revision. clayborg added a comment. I like this new setting much better. Looks good. https://reviews.llvm.org/D47629 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Check the context as noted in inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:70 + continue; + +Append(entry,

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

clayborg added a comment. Feel free to check into it and modify the contract for GetGlobalVariables and possibly rename "name" to "basename" to be clear what the argument is. Then modify the headerdoc to clearly state what we are looking for if the filtering is going on higher up

[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

clayborg added inline comments. Comment at: source/API/CMakeLists.txt:184 + set(EXECUTABLE_NAME "LLDB") + set(CURRENT_PROJECT_VERSION "360.99.0") set_target_properties(liblldb PROPERTIES Currently the apple generic versioning is used and the value for this

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Much better. Anything we can do to clearly define and simplify the indexers is great stuff. https://reviews.llvm.org/D47781 ___

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:153 +return true; + bool looking_for_methods = name_type_mask & eFunctionNameTypeMethod; + return looking_for_methods == die.IsMethod(); move up to line 150 and

[Lldb-commits] [PATCH] D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. So nice to get rid of these HandleCommand hacks! Repository: rL LLVM https://reviews.llvm.org/D47415 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D46810: 3/3: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Only question is if we remove the extra empty scope as noted in inline comments. Looks good. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:86 // Keep a

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

clayborg accepted this revision. clayborg added a comment. I can't think of a better name, looks good. Thanks for taking the time, this will be great. https://reviews.llvm.org/D47470 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D48802: [lldb-mi] Re-implement symbol-list-lines command.

clayborg added a comment. I am ok with the content of this. I will let Adrian give the final OK due to lit stuff. https://reviews.llvm.org/D48802 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D48802: [lldb-mi] Re-implement symbol-list-lines command.

clayborg added a comment. I am ok with the content of this. I will let Adrian give the final OK due to lit stuff. https://reviews.llvm.org/D48802 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D47992: [lldb-mi] Clean up and update a few MI commands.

clayborg added inline comments. Comment at: tools/lldb-mi/MICmdCmdExec.cpp:139 + const CMIUtilString (CMIDriver::Instance().GetErrorDescription()); + this->SetError(CMIUtilString::Format( + MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE), any reason

[Lldb-commits] [PATCH] D48520: [lldb-mi] Re-implement a few MI commands.

clayborg added a comment. LGTM. Ok when Adrian is happy with testing changes. https://reviews.llvm.org/D48520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48596: [SymbolFile] Implement GetCompleteObjCClass for .debug_names

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. The function is looking for the complete objective C type. The code needs to be modified to return the complete type only if one is found, else just one of the other incomplete

[Lldb-commits] [PATCH] D48596: [SymbolFile] Implement GetCompleteObjCClass for .debug_names

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

[Lldb-commits] [PATCH] D48633: UUID: Add support for arbitrary-sized module IDs

clayborg added inline comments. Comment at: source/Interpreter/OptionValueUUID.cpp:82 + llvm::SmallVector uuid_bytes; + UUID::DecodeUUIDBytesFromString(s, uuid_bytes); for (size_t i = 0; i < num_modules; ++i) { Probably should have a return

[Lldb-commits] [PATCH] D48659: Allow specifying an exit code for the 'quit' command

clayborg requested changes to this revision. clayborg added a comment. This change seems fine to me. It allows each debugger to have a different exit status and each debugger can grab one. Seems like this should belong in lldb_private::CommandInterpreter and lldb::SBCommandInterpreter instead

[Lldb-commits] [PATCH] D48658: Fix and simplify lldb.command decorator

clayborg added a comment. Can we add a test for this to ensure all functionality that we want? I recently made changes to the cmdtemplate.py. Maybe we can integrate some more functionality that auto registers a class somehow? https://reviews.llvm.org/D48658

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

clayborg added a comment. We should allow 4 and 8 byte UUIDs as pointed out by inlined comments. This means we should probably modify the UUID dumper to handle those cases as well. Comment at: include/lldb/Utility/UUID.h:109 + + uint32_t m_num_uuid_bytes = 0; // Should be 0,

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Patch is good. Feel free to remove the DataExtractor::DumpUUID() in a separate NFC commit or just remove it in this patch Comment at:

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

clayborg added a comment. Factory methods make things much clearer. https://reviews.llvm.org/D48479 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

clayborg added a comment. Would love to remove the "accept_zeroes" argument everywhere. Too much matching happens in LLDB and we can't have multiple shared libraries claiming zeros as their UUID Comment at: include/lldb/Utility/UUID.h:54 + void SetBytes(const void

[Lldb-commits] [PATCH] D48463: Prevent dead locking when calling PrintAsync

clayborg added inline comments. Comment at: source/Core/Debugger.cpp:988-1004 + bool should_forward = false; + { +// We check if any user requested to delay output to a later time +// (which is designated by m_delayed_output_counter being not 0). +std::lock_guard

[Lldb-commits] [PATCH] D48520: [lldb-mi] Re-implement a few MI commands.

clayborg added a comment. Just waiting for Adrian to indicate he is happy with testing stuff. LGTM https://reviews.llvm.org/D48520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

clayborg added inline comments. Comment at: source/Commands/CommandObjectCommands.cpp:1012 void IOHandlerActivated(IOHandler _handler) override { +if (!io_handler.GetIsInteractive() && !io_handler.GetIsRealTerminal()) + // Don't print instructions during batch

[Lldb-commits] [PATCH] D48801: Add new API to SBTarget and SBModule classes.

clayborg added a comment. needs a test, but looks good. https://reviews.llvm.org/D48801 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4221 if (variable_list_sp.get() == NULL) { variable_list_sp.reset(new VariableList()); } So do we cache now somewhere

[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Sounds good. Looks fine. https://reviews.llvm.org/D46220 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D40470: DWZ 03/07: Protect DWARFCompileUnit::m_die_array by a new mutex

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Much better. https://reviews.llvm.org/D40470 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D46128: Fix expression parser to not accept any type whose basename matches for a type that must exist at root level

clayborg created this revision. clayborg added reviewers: jingham, davide, aprantl, labath, lldb-commits. This patch fixes an issue where we weren't looking for exact matches in the expression parser and also fixed the type lookup logic in the Module.cpp. Tests added to make sure we don't

[Lldb-commits] [PATCH] D40470: DWZ 03/07: Protect DWARFCompileUnit::m_die_array by a new mutex

clayborg added a subscriber: jankratochvil. clayborg added a comment. Good catch Repository: rL LLVM https://reviews.llvm.org/D40470 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D40470: DWZ 03/07: Protect DWARFCompileUnit::m_die_array by a new mutex

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:335 m_die_array.swap(tmp_array); -if (keep_compile_unit_die) +

[Lldb-commits] [PATCH] D40470: DWZ 03/07: Protect DWARFCompileUnit::m_die_array by a new mutex

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:56 +return 0; + std::shared_ptr m_die_array_size_atomic_set(nullptr, + [&](void*){

[Lldb-commits] [PATCH] D46395: Remove Process references from the Host module

clayborg added inline comments. Comment at: source/Host/macosx/Host.mm:1501 +bool monitoring = launch_info.MonitorProcess(); +(void)monitoring; +assert(monitoring); Do we not have a macro for silencing unused variables? Comment at:

[Lldb-commits] [PATCH] D46362: DWARFExpression: Convert file addresses to load addresses early on

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. great fix. Just resolve the file address using the module function and this is good to go. Comment at: source/Core/Value.cpp:683-685 + ObjectFile *objfile =

[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. Ping. I would like to get this in. As for testing, the full matrix should be run on .debug_types as it is the main form of type uniquing that is defined by the DWARF spec. Any formats that are fully accepted and written into the DWARF spec should be fully tested to

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Wow. I didn't realize windows support didn't do any of this. Looks good. Zach will want to ok this as he is one of the main windows contributors. https://reviews.llvm.org/D39314

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close. Thanks for making the changes. Just a few nits. Comment at: utils/clangdiag.py:66 +def setDiagBreakpoint(frame, bp_loc, dict): +id =

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:651-659 + ArchSpec header_arch; + GetArchitecture(header_arch); +

[Lldb-commits] [PATCH] D19603: Fix entry point lookup for ObjectFilePECOFF

clayborg added a comment. Does this need some ARM support where we strip bit zero in case the entry point is Thumb? https://reviews.llvm.org/D19603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

clayborg added inline comments. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:284 + switch (watch_flags) { + case write_mode: +rw_mode = PPC_BREAKPOINT_TRIGGER_WRITE; We should use the lldb::WatchpointKind from

[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

clayborg added a comment. Please do convert to python. Just know that you can use "lldb -P" to get the python path that is needed in order to do "import lldb" in the python script. So you can try doing a "import lldb", and if that fails, catch the exception, run "lldb -P", add that path to the

[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

clayborg added a comment. If you want to run the script from the command line, then it is necessary. If it is run from within LLDB it will just work. I like to have my LLDB python scripts work both ways. This might be better implemented as a new command that gets installed and can be used

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

clayborg added a comment. Testing would include a test that dynamically loads a shared library and sets a breakpoint it in. We have these tests, but I am guessing they are disabled on windows probably because they might use "dlopen(...)" which is probably not available on windows? I know we

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg accepted this revision. clayborg added inline comments. Comment at: utils/clangdiag.py:117 +disable(exe_ctx) +else: +getDiagtool(exe_ctx.GetTarget(), args.path[0]) hintonda wrote: > clayborg wrote: > > should probably verify that

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. ok, just move the ARM detection out of the loop and use a THUMB_ADDRESS_BIT_MASK for ARM symbols and this is good to go. https://reviews.llvm.org/D39315

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

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

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

clayborg added a comment. The test suite can be run remotely if ds2 supports the "lldb-server platform" packets. I'll be happy to help you get this going Stephane. Ping me internally if interested. https://reviews.llvm.org/D39315 ___ lldb-commits

[Lldb-commits] [PATCH] D36347: New lldb python module for adding diagnostic breakpoints

clayborg added a comment. Each lldb.SBValue has accessors for the stuff in an execution context: `` lldb::SBTarget GetTarget(); lldb::SBProcess GetProcess(); lldb::SBThread GetThread(); lldb::SBFrame GetFrame(); You could keep a global map of process ID to diagtool if you want?

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks fine. Thanks for doing the changes. https://reviews.llvm.org/D38897 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D41745: Handle O reply packets during qRcmd

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D41745#970362, @owenpshaw wrote: > Updated patch with new function name suggested by @clayborg. Added unit test > and changed to llvm::function_ref as

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

clayborg added a comment. I like the overall direction this patch is taking, just a few fixes. https://reviews.llvm.org/D41584 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D41909: Fix deadlock in dwarf logging

clayborg added a comment. In https://reviews.llvm.org/D41909#973299, @tberghammer wrote: > Why do we need to lock the Module mutex in SymbolVendor::FindFunctions? I > think a better option would be to remove that lock and if it is needed then > lock it just for the calls where it necessary.

[Lldb-commits] [PATCH] D41533: Advanced guessing of rendezvous breakpoint

clayborg added a comment. Looks good. Repository: rL LLVM https://reviews.llvm.org/D41533 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

clayborg added a comment. Sounds like finding a clang expert to clarify what Jim last asked for is the way forward. Do a source control "blame" command and see who worked on the code in the area of clang and maybe add them as reviewers so they can comment? I agree with Jim that this sounds

[Lldb-commits] [PATCH] D41909: Fix deadlock in dwarf logging

clayborg added a comment. Tough to call on this one. Yes the function is dumping simple stuff, but it is using m_arch, m_file and m_objname. It could cause crashes in multi-threaded environments. Is the deadlock caused by an A/B lock issue? https://reviews.llvm.org/D41909

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Fix comment spacing as mentioned in inline comments and this is good to go. Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:98-101 + // Since we need

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

clayborg added a comment. And I do agree with Jim that we don't want to have to mangle the typename to see if it matches, that is too much work. https://reviews.llvm.org/D40283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

clayborg added a comment. Added Adrian Prantl as a reviewer in case he has any input. Adrian: is there any way that a DIE is marked up with an extra attribute when the asm name is explicitly set? It would be great to know this from the DWARF so we don't have to end up setting the ASM name for

[Lldb-commits] [PATCH] D41533: Advanced guessing of rendezvous breakpoint

clayborg added a comment. Looks fine. Set the breakpoint using the list of names and delete the breakpoint if you get no locations and this will be good to go. Comment at: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:365-368 +static const char

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

clayborg added inline comments. Comment at: include/lldb/Target/Process.h:1916 + //-- + virtual bool BeginWriteMemoryBatch() { return true; } + owenpshaw wrote: > clayborg wrote: > > labath wrote:

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very nice overall. See inlined comments. Big issues are: - GDBRemoteCommunication::WriteEscapedBinary() is not needed as StreamGDBRemote::PutEscapedBytes() does this already -

[Lldb-commits] [PATCH] D41702: Add SysV Abi for PPC64le

clayborg added a comment. Looks nice. Only nit is we probably don't need the m_endian member variable. See inlined comment. Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.h:114 + + lldb::ByteOrder m_endian; }; Most other code uses "m_byte_order" as

[Lldb-commits] [PATCH] D42281: WIP: compile the LLDB tests out-of-tree

clayborg added a comment. Looks like a good start. It might be nice to validate that after "clean" that we have no files that are untracked in the test directory? This could help us to verify that we don't leave artifacts around. Comment at:

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

clayborg added a comment. This will need a test, preferable covering all of the cases: - breakpoint right at start with extra data after - breakpoint right at start with no extra data after - breakpoint in middle of ranges with data before and after - breakpoint at end of range with no data

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

clayborg added a comment. I am not sure we can say for sure that a breakpoint intersected with the write here? I would rather just have an error saying something like "only %u of %u bytes in section %s were written". Extra credit for checking if there are overlapping breakpoints and appending

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

clayborg added a comment. And yes, if the write memory can only write fewer byte than requested, it won't be an error if at least some bytes were written https://reviews.llvm.org/D39969 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42488: Remove ObjectFile usage from HostLinux::GetProcessInfo

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks fine to me. https://reviews.llvm.org/D42488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic

clayborg added a comment. Herald added subscribers: llvm-commits, hintonda. This code is making debugging of large C++ apps so slow it is unusable... Repository: rL LLVM https://reviews.llvm.org/D31451 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D31451: New C++ function name parsing logic

clayborg added a comment. This is part of the problem, but not the entire thing.. We had a mangled name:

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DIERef.cpp:67-68 + lldb_private::ArchSpec arch; + if (dwarf->GetObjectFile()->GetArchitecture(arch) && + arch.GetTriple().isOSBinFormatELF()) { +// For SymbolFileDWARFDwo/SymbolFileDWARFDwoDwp

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

clayborg added a comment. My objection to the BeginWriteMemoryBatch and EndWriteMemoryBatch is users must know to emit these calls when they really just want to call process.WriteMemory() and not worry about how it is done. Much like writing to read only code when setting breakpoints, we don't

[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior

clayborg added a comment. Looks fine to me. Wait for Pavel to give it the OK since he did more of the lldb-server testing stuff. https://reviews.llvm.org/D42195 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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