[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

JDevlieghere updated this revision to Diff 512946. JDevlieghere added a comment. - Improve error reporting - Fix bug CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148062/new/ https://reviews.llvm.org/D148062 Files: lldb/include/lldb/Core/Section.h lldb/include/lldb/Symbol/ObjectFil

[Lldb-commits] [PATCH] D148063: [lldb] Parse the crashlog only once

This revision was automatically updated to reflect the committed changes. Closed by commit rG88f409194d5a: [lldb] Parse the crashlog only once (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148

[Lldb-commits] [PATCH] D148063: [lldb] Parse the crashlog only once

JDevlieghere created this revision. JDevlieghere added a reviewer: mib. Herald added a project: All. JDevlieghere requested review of this revision. Now that we can pass Python objects to the scripted process instance, we don't need to parse the crashlog twice anymore. https://reviews.llvm.org/

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

JDevlieghere created this revision. JDevlieghere added reviewers: mib, jingham. Herald added a project: All. JDevlieghere requested review of this revision. This patch adds support for creating modules from JSON object files. This is necessary for crashlogs (see c17a1f3df70b

[Lldb-commits] [PATCH] D148053: [lldb] Parse image high address from JSON crashlogs

This revision was automatically updated to reflect the committed changes. Closed by commit rGb67b7bb2f735: [lldb] Parse image high address from JSON crashlogs (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[Lldb-commits] [PATCH] D148053: [lldb] Parse image high address from JSON crashlogs

JDevlieghere created this revision. JDevlieghere added a reviewer: mib. Herald added a project: All. JDevlieghere requested review of this revision. Correctly parse the image high address from JSON crashlogs. This is invisible in the output but necessary for a follow up patch that will test the

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147833/new/ https://reviews.llvm.org/D147833 ___

[Lldb-commits] [PATCH] D147820: debugserver: move AArch64 watchpoint traps within a watchpointed region, parse ESR flags and send them to lldb

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. I didn't double check the spec so I'm assuming those bitfield indexes are correct. Everything else makes sense to me. LGTM. Comment at: lldb/tools/debugserver/so

[Lldb-commits] [PATCH] D147833: [lldb] Change return type of EventData::GetFlavor

JDevlieghere added a comment. In D147833#4253499 , @bulbazord wrote: > Yes, we definitely do that in many places. I see your point though, comparing > const c-string pointers can get pretty tricky if somebody refactors this in > the future. In that case

[Lldb-commits] [PATCH] D147748: [lldb] Implement SymbolFile::GetCompileOptions

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM with or without the DenseMap, whichever makes the most sense. Comment at: lldb/include/lldb/Symbol/SymbolFile.h:441 + /// associated with that compilation u

[Lldb-commits] [PATCH] D147801: [lldb] Add unittests for a few FileSpec methods

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147801/new/ https://reviews.llvm.org/D147801 ___

[Lldb-commits] [PATCH] D147801: [lldb] Add unittests for a few FileSpec methods

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Utility/FileSpec.h:330 /// filename has no extension, ConstString(nullptr) is returned. The dot - /// ('.') character is not returned as part of the extension + /// ('.') character is the first character in th

[Lldb-commits] [PATCH] D147753: Move "SelectMostRelevantFrame" from Thread::WillStop

JDevlieghere added a comment. Jim asked me to land this on his behalf as he's OOO. I've addressed my own comments :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147753/new/ https://reviews.llvm.org/D147753 _

[Lldb-commits] [PATCH] D147753: Move "SelectMostRelevantFrame" from Thread::WillStop

This revision was automatically updated to reflect the committed changes. Closed by commit rG730c8e160c9d: [lldb] Move "SelectMostRelevantFrame" from Thread::WillStop (authored by jingham, committed by JDevlieghere). Changed prior to commit: https://reviews.llvm.org/D147753?vs=511579&id=511768

[Lldb-commits] [PATCH] D147748: [lldb] Implement SymbolFile::ContainsCompileOption

JDevlieghere added a comment. (adding myself as a reviewer so this shows up in my review queue) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147748/new/ https://reviews.llvm.org/D147748 ___ lldb-commits

[Lldb-commits] [PATCH] D147748: [lldb] Implement SymbolFile::ContainsCompileOption

JDevlieghere added a comment. At a higher level I wonder if this is really the best interface. If you ever need all the compile options, you probably want something like `Args SymbolFile::GetCompileOptions()`. Wouldn't that be a more generic way to do the same thing here? Or do we expect that f

[Lldb-commits] [PATCH] D147753: Move "SelectMostRelevantFrame" from Thread::WillStop

JDevlieghere added inline comments. Comment at: lldb/source/Target/StackFrameList.cpp:823 +SelectMostRelevantFrame(); +m_selected_frame_set = true; + } jingham wrote: > JDevlieghere wrote: > > IIUC `SetSelectedFrame` always sets `m_selected_frame_set`

[Lldb-commits] [PATCH] D147753: Move "SelectMostRelevantFrame" from Thread::WillStop

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Target/StackFrameList.h:139-144 /// The currently selected frame. uint32_t m_selected_frame_idx; + // Records whether anyone has set the selected frame on this stack yet. + // We only let recognizers change

[Lldb-commits] [PATCH] D147736: [lldb] Add an LLDB_DEPRECATED macro similar to LLVM_DEPRECATED

This revision was automatically updated to reflect the committed changes. Closed by commit rG2279f77d2855: [lldb] Add an LLDB_DEPRECATED macro similar to LLVM_DEPRECATED (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D147736?vs=511516

[Lldb-commits] [PATCH] D147736: [lldb] Add an LLDB_DEPRECATED macro similar to LLVM_DEPRECATED

JDevlieghere updated this revision to Diff 511516. JDevlieghere added a comment. Add a comment explaining why we're undefing the macro. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147736/new/ https://reviews.llvm.org/D147736 Files: lldb/include/lldb/API/SBDefines.h lldb/include/l

[Lldb-commits] [PATCH] D147736: [lldb] Add an LLDB_DEPRECATED macro similar to LLVM_DEPRECATED

JDevlieghere added a comment. FWIW I added the current `[[deprecated]]` attribute in ea95da180900 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147736/new/ https://reviews.llvm.org/D147736 _

[Lldb-commits] [PATCH] D147736: [lldb] Add an LLDB_DEPRECATED macro similar to LLVM_DEPRECATED

JDevlieghere created this revision. JDevlieghere added reviewers: labath, DavidSpickett, jingham, bulbazord. Herald added a project: All. JDevlieghere requested review of this revision. Add an LLDB_DEPRECATED macro similar to LLVM_DEPRECATED. We cannot directly reuse the LLVM one, because it's def

[Lldb-commits] [PATCH] D147482: [lldb] Add an overload to SetModuleLoadAddress that takes an unsigned value

This revision was automatically updated to reflect the committed changes. Closed by commit rGea95da180900: [lldb] Add an overload to SetModuleLoadAddress that takes an unsigned value (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D147

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

JDevlieghere added a comment. In D147606#4247462 , @jwnhy wrote: > In D147606#4246832 , @JDevlieghere > wrote: > >> The change looks fine and regardless of whether this makes sense or even >> complies with the s

[Lldb-commits] [PATCH] D147482: [lldb] Add an overload to SetModuleLoadAddress that takes an unsigned value

JDevlieghere added a comment. Writing this made me realize we can solve this more elegantly. We can have both overloads and deprecate the one that takes an `int64_t`, mark it as deprecated and not expose it to SWIG. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147482/new/ https://rev

[Lldb-commits] [PATCH] D147482: [lldb] Add an overload to SetModuleLoadAddress that takes an unsigned value

JDevlieghere added a comment. In D147482#4248858 , @jasonmolenda wrote: > LGTM. We can detect this This is one of those cases where I'd like to mark > SBTarget::SetModuleLoadAddress as deprecated. I like that you detect this > situation in SBTarget:

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

JDevlieghere added a comment. The change looks fine and regardless of whether this makes sense or even complies with the standard, we should be resilient against it. I would like to see a test though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D147587: Fix the check in StopInfoBreakpoint for "are we running an expression"

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM modulo naming preference Comment at: lldb/include/lldb/Target/Process.h:287-292 + bool CurrentlyRunningExpression() const { +// Don't return true if we

[Lldb-commits] [PATCH] D147482: [lldb] Add an overload to SetModuleLoadAddress that takes an unsigned value

JDevlieghere created this revision. JDevlieghere added a reviewer: jasonmolenda. Herald added a project: All. JDevlieghere requested review of this revision. Currently, `SBTarget::SetModuleLoadAddress` cannot accept the large slides needed to load images in high memory. This function should alway

[Lldb-commits] [PATCH] D147292: [lldb] Add support for the DW_AT_trampoline attribute with a boolean

JDevlieghere added a comment. > Be handy to mark this abandoned when you get a chance Augusto had already abandoned it when he wrote that reply, maybe you're looking at a different patch or stale tab? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D147462: Use kernel's global variable indicating how many bits are used in addressing when loading Darwin xnu kernel

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. I would still prefer to not change the global state, but after speaking to Jason offline, that might be better tackled when we unify the current Process/Target bifurcation.

[Lldb-commits] [PATCH] D147453: [LLDB] Ensure LLDB symbols are exported in LLDB_EXPORT_ALL_SYMBOLS is provided.

JDevlieghere accepted this revision. JDevlieghere added a comment. Thanks! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147453/new/ https://reviews.llvm.org/D147453 ___ lldb-commits mailing list ll

[Lldb-commits] [PATCH] D147453: [LLDB] Ensure LLDB symbols are exported in LLDB_EXPORT_ALL_SYMBOLS is provided.

JDevlieghere added inline comments. Comment at: lldb/cmake/modules/AddLLDB.cmake:175 +CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU") + target_compile_options(${name} PRIVATE "-fvisibility=default") +endif() Rather than changing the compile options di

[Lldb-commits] [PATCH] D147462: Use kernel's global variable indicating how many bits are used in addressing when loading Darwin xnu kernel

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:1087 + if (symbol && wordsize == 8) { +if (symbol->GetByteSizeIsValid()) { + // Mark all bits as addressable so we don't strip any

[Lldb-commits] [PATCH] D147292: [lldb] Add support for the DW_AT_trampoline attribute with a boolean

JDevlieghere added inline comments. Comment at: lldb/source/Core/Module.cpp:780 + bool is_trampoline = + Target::GetGlobalProperties().GetEnableTrampolineSupport() && + sc.function && sc.function->IsGenericTrampoline(); You can hoist `targe

[Lldb-commits] [PATCH] D147292: [lldb] Add support for the DW_AT_trampoline attribute with a boolean

JDevlieghere added a comment. There seems to be overlap in the code added in this patch and D146679 . Does one supersede the other or is there a dependency? If it's the latter you should extract that into a separate patch and make it a parent revision of this a

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

JDevlieghere added a comment. The change looks good: it matches what the DWARFv5 standard says: > The DW_OP_div operation pops the top two stack values, divides the former > second entry by the former top of the stack using signed division, and pushes > the result. Can you please add a regress

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147248/new/ https://reviews.llvm.org/D147248 ___

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:227-234 void StoringDiagnosticConsumer::SetCurrentModuleProgress( -llvm::StringRef module_name) { - // Ensure the ordering of: - // 1. Completing the ex

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:228 void StoringDiagnosticConsumer::SetCurrentModuleProgress( -llvm::StringRef module_name) { - // Ensure the ordering of: - // 1. Completing the existi

[Lldb-commits] [PATCH] D147246: [lldb] Replace sprintf with snprintf (NFC)

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Ship it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147246/new/ https://reviews.llvm.org/D147246

[Lldb-commits] [PATCH] D147248: [lldb] Use one Progress event per root module build

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:228 void StoringDiagnosticConsumer::SetCurrentModuleProgress( -llvm::StringRef module_name) { - // Ensure the ordering of: - // 1. Completing the existi

[Lldb-commits] [PATCH] D147249: [lldb] Replace deprecated CFPropertyListWriteToStream (NFC)

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147249/new/ https://reviews.llvm.org/D147249 ___

[Lldb-commits] [PATCH] D147246: [lldb] Replace sprintf with snprintf (NFC)

JDevlieghere added a comment. This looks fine, but I wonder if using `llvm::formatv` wouldn't make this a whole lot easier? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147246/new/ https://reviews.llvm.org/D147246 ___

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

This revision was automatically updated to reflect the committed changes. Closed by commit rGfda53ad9374b: [lldb] Support Universal Mach-O binaries with a fat64 header (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

JDevlieghere marked 2 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp:205 +const lldb::offset_t slice_file_offset = +fat_arch.GetOffset() + file_of

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

JDevlieghere marked 2 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp:205 +const lldb::offset_t slice_file_offset = +fat_arch.GetOffset() + file_of

[Lldb-commits] [PATCH] D147012: [lldb] Support Universal Mach-O binaries with a fat64 header

JDevlieghere created this revision. JDevlieghere added reviewers: jasonmolenda, pete, aprantl. Herald added a project: All. JDevlieghere requested review of this revision. Support universal Mach-O binaries with a fat64 header. After D146879 , dsymutil can now gen

[Lldb-commits] [PATCH] D147007: [lldb] Fix double free in python bindings error handling.

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM. I think it would be worth adding that information to the top of the file to prevent similar mistakes in the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147007/new/

[Lldb-commits] [PATCH] D146779: Fix handling of backticks in CommandObjectParsed commands

JDevlieghere closed this revision. JDevlieghere added a comment. https://reviews.llvm.org/rG75ca15fcbb2e1b3671e41f971a000c6d59f5e5ae Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146779/new/ https://reviews.llvm.org/D146779 ___

[Lldb-commits] [PATCH] D146765: [lldb/crashlog] Load inlined symbol into interactive crashlog

JDevlieghere added inline comments. Comment at: lldb/examples/python/crashlog.py:1187-1192 +symbol_file = os.path.join(tmp_dir.name, module.GetUUIDString() + ".json") +with open(symbol_file, 'w') as f: +f.write(json.dumps(symbol_data)) + +ci.H

[Lldb-commits] [PATCH] D146779: Fix handling of backticks in CommandObjectParsed commands

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:1754-1760 +if (error.Success()) { + command.erase(start_backtick, end_backtick - start_ba

[Lldb-commits] [PATCH] D146679: [lldb] Add support for the DW_AT_trampoline attribute with mangled names

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:664 + llvm::StringRef m_trampoline_target; + I'd like to see a doxygen comment (1) explaining what the trampoline target is (i.e. according to the summary the mangled name of

[Lldb-commits] [PATCH] D146714: [lldb] Explicitly set libcxx paths when USE_SYSTEM_LIBCXX is provided

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:434 +ifeq "$(SDKROOT)" "" + $(error "SDKROOT should have been set") +endif Repository: r

[Lldb-commits] [PATCH] D146553: [lldb][CMake] Enforce not linking against plugin libs in core libs

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146553/new/ https://reviews.llvm.org/D146553 ___

[Lldb-commits] [PATCH] D145568: [lldb] Add dummy field to RegisterInfo for register flags use later

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM Comment at: lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp:57-68 {"r1", nullptr, 4, 0, eEncodingUint, eFormatHex, {ehframe_r1, dwarf_r1, LLDB_REGNUM_GENERIC_ARG2, LLD

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

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

[Lldb-commits] [PATCH] D145574: [lldb] Read register fields from target XML

JDevlieghere accepted this revision. JDevlieghere added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4053 + // appears once, so we don't have to handle that here. + if (attr_name == "name") { +LLDB_LOGF(log, --

[Lldb-commits] [PATCH] D145181: Revert "Remove the LINK_COMPONENTS entry from lldb-instr CMakery"

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM (see https://github.com/llvm/llvm-project/issues/60314) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145181/new/ https://revie

[Lldb-commits] [PATCH] D146473: [lldb][CMake] Enforce not letting lldbUtility link against any other lldb libs

JDevlieghere added a comment. Can we do something similar for plugins? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146473/new/ https://reviews.llvm.org/D146473 ___ lldb-commits mailing list lldb-commit

[Lldb-commits] [PATCH] D146473: [lldb][CMake] Enforce not letting lldbUtility link against any other lldb libs

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146473/new/ https://reviews.llvm.org/D146473 ___

[Lldb-commits] [PATCH] D145574: [lldb] Read register fields from target XML

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4053 + // appears once, so we don't have to handle that here. + if (attr_name == "name") { +LLDB_LOGF(log, Do you think that using a `Str

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

JDevlieghere added a subscriber: bulbazord. JDevlieghere added inline comments. Comment at: lldb/include/lldb/Core/DumpRegisterValue.h:12 +#include "Plugins/TypeSystem/Clang/TypeSystemClang.h" #include "lldb/lldb-enumerations.h" Core components (not just the C

[Lldb-commits] [PATCH] D145568: [lldb] Add dummy field to RegisterInfo for register flags use later

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp:57-68 {"r1", nullptr, 4, 0, eEncodingUint, eFormatHex, {ehframe_r1, dwarf_r1, LLDB_REGNUM_GENERIC_ARG2, LLDB_INVALID_REGNUM, Wou

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

JDevlieghere accepted this revision. JDevlieghere added a comment. LGTM modulo comments Comment at: lldb/include/lldb/Target/RegisterFlags.h:21-22 + public: +Field(llvm::StringRef name, unsigned start, unsigned end) +: m_name(name.str()), m_start(start), m_end(end)

[Lldb-commits] [PATCH] D146263: [lldb] Move UnixSignals creation into Platform plugins

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. We're only using the OS part of the triple/ArchSpec so there should be a 1:1 mapping with the platform. I don't see any issue with this. LGTM. Repository: rG LLVM Github Monorep

[Lldb-commits] [PATCH] D146337: Partially fix stepping over watchpoints on "watch triggers before" systems

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146337/new/ https://reviews.llvm.org/D146337 ___

[Lldb-commits] [PATCH] D146335: [lldb] Introduce CMake variable LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Thanks, this is great. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146335/new/ https://reviews.llvm.org/D146335 ___

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Herald added a subscriber: jplehr. Comment at: lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp:112 +symtab.AddSymbol(Symbol( +/*symID*/ 0, Mangled(symbol.name), eSymbolTypeCode,

[Lldb-commits] [PATCH] D146262: [lldb] Set the watchpoint spec for expression watchpoints

This revision was automatically updated to reflect the committed changes. Closed by commit rG2a7642977876: [lldb] Set the watchpoint spec for expression watchpoints (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[Lldb-commits] [PATCH] D146262: [lldb] Set the watchpoint spec for expression watchpoints

JDevlieghere created this revision. JDevlieghere added reviewers: jingham, mib, delcypher. Herald added a project: All. JDevlieghere requested review of this revision. When setting a variable watchpoint, the watchpoint stores the variable name in the watchpoint spec. For expression variables we s

[Lldb-commits] [PATCH] D146230: [lldb][test] Replace use of p with expression in Shell tests (NFC)

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. I probably would've gone with `expo` but as `expression` is the canonical name of the command, this makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[Lldb-commits] [PATCH] D145136: Add a Debugger interruption mechanism in parallel to the Command Interpreter interruption

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145136/new/ https://reviews.llvm.org/D145136 ___

[Lldb-commits] [PATCH] D145136: Add a Debugger interruption mechanism in parallel to the Command Interpreter interruption

JDevlieghere added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1973-1977 +HostThread &Debugger::SetIOHandlerThread(HostThread &new_thread) { + HostThread &old_host = m_io_handler_thread; + m_io_handler_thread = new_thread; + return old_host; +}

[Lldb-commits] [PATCH] D146152: Add __lldb_init_module_with_target for use when sourcing modules for a target

JDevlieghere added a comment. Thanks for adding this, Jim. This is one of those things that comes up one in a while when it confuses our users. It's great we'll be able to help them adopt this. Comment at: lldb/bindings/python/python-wrapper.swig:1031-1048 + // First call th

[Lldb-commits] [PATCH] D145136: Add a Debugger interruption mechanism in parallel to the Command Interpreter interruption

JDevlieghere added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1220-1221 +return m_interrupt_requested != 0; + } else +return GetCommandInterpreter().WasInterrupted(); +} Nit: else-after-return. Comment at: lldb/source/

[Lldb-commits] [PATCH] D146045: [LLDB] Show sub type of signals when debugging a core file

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Nice. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146045/new/ https://reviews.llvm.org/D146045

[Lldb-commits] [PATCH] D146043: [lldb] Refactor CrashReason

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

[Lldb-commits] [PATCH] D146044: [lldb] Implement CrashReason using UnixSignals

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM with Pavel's comments. Comment at: lldb/unittests/Signals/UnixSignalsTest.cpp:132-134 + EXPECT_EQ(expected, signals.GetSignalAsString(16, 3)); + EXPECT_EQ(

[Lldb-commits] [PATCH] D145294: [lldb/API] Introduce SBProcess::ForceScriptedState method

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145294/new/ https://reviews.llvm.org/D145294 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D142260: Actually report an error when `command script add` is passed a non-existent class

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM modulo the else-after-return. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1949-1950 +new StructuredPythonOb

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM if @clayborg is happy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145624/new/ https://reviews.llvm.org/D145624 _

[Lldb-commits] [PATCH] D145295: [lldb] Move ScriptedProcess private state update to implementation

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145295/new/ https://reviews.llvm.org/D145295 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

This revision was automatically updated to reflect the committed changes. Closed by commit rGcf3524a5746f: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON (authored by JDevlieghere). Changed prior to commit: https://reviews.llvm.org/D145180?vs=503575&id=503611#toc Repository: rG LLVM

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere updated this revision to Diff 503575. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145180/new/ https://reviews.llvm.org/D145180 Files: lldb/include/lldb/Symbol/Symbol.h lldb/source/Plugins/ObjectFile/CMakeLists.txt lldb/source/Plugins/ObjectFile/JSON/CMakeLists.txt

[Lldb-commits] [PATCH] D145547: When setting load addresses on darwin kernel kexts, handle case where in-memory load commands are not updated

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Some code style nits but the change itself LGTM. Comment at: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:819-821 +if (ondis

[Lldb-commits] [PATCH] D145624: [lldb] Make MemoryCache::Read more resilient

JDevlieghere added inline comments. Comment at: lldb/source/Target/Memory.cpp:131 + size_t bytes_left = dst_len; This isn't used until line 180. I'd move it down, closer to where it is being used. Comment at: lldb/source/Target/Memory.cpp

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:594 lldb::TargetSP m_dummy_target_sp; + Diagnostics::CallbackID m_diagnostics_callback_id; clayborg wrote: > I am guessing these changes are in Debugger.h and Debugger.cpp are

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere updated this revision to Diff 503543. JDevlieghere marked 21 inline comments as done. JDevlieghere added a comment. - Remove 9d311dd6a71b from patch - Use `EXPECT_THAT_EXPECTED` CHANGES SINCE LAST ACTION https:

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere updated this revision to Diff 503432. JDevlieghere added a comment. - Add unit tests for JSON deserialization - Add unit tests for converting JSONSymbol to Symbol CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145180/new/ https://reviews.llvm.org/D145180 Files: lldb/inclu

[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9d311dd6a71b: [lldb] Copy log files into diagnostic directory (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: h

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere updated this revision to Diff 503166. JDevlieghere added a comment. - Remove section field - Support raw value symbols - Update test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145180/new/ https://reviews.llvm.org/D145180 Files: lldb/include/lldb/Symbol/Symbol.h lldb

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/Symbol/Symbol.cpp:44-49 + if (!section_list) +return; + + // This should've been caught during deserialization. + if (!symbol.value && !symbol.address) +return; -

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/Symbol/Symbol.cpp:44-49 + if (!section_list) +return; + + // This should've been caught during deserialization. + if (!symbol.value && !symbol.address) +return; -

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere updated this revision to Diff 502854. JDevlieghere added a comment. Implement Greg's features. I'll add a unit test for the deserialization later today. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145180/new/ https://reviews.llvm.org/D145180 Files: lldb/include/lldb/S

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere marked 3 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/include/lldb/Symbol/Symbol.h:23 +struct JSONSymbol { + uint64_t value; + std::optional size; clayborg wrote: > clayborg wrote: > > JDevlieghere wrote: > > > clay

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere marked 3 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/include/lldb/Symbol/Symbol.h:23 +struct JSONSymbol { + uint64_t value; + std::optional size; clayborg wrote: > Do we something that says "value is an address"? O

[Lldb-commits] [PATCH] D145180: [lldb] Introduce new SymbolFileJSON and ObjectFileJSON

JDevlieghere updated this revision to Diff 502330. JDevlieghere marked 3 inline comments as done. JDevlieghere added a comment. Address Greg's feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145180/new/ https://reviews.llvm.org/D145180 Files: lldb/include/lldb/Symbol/Symbol.h

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143104/new/ https://reviews.llvm.org/D143104 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D139945: [lldb] Add scripted process launch/attach option to {, platform }process commands

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM modulo inline comments. Comment at: lldb/include/lldb/Target/Process.h:235-239 + std::string m_scripted_process_class_name; // The name of the class that wi

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