[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

JDevlieghere closed this revision. JDevlieghere added a comment. 13dbc16b4d82 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149472/new/ https://reviews.llvm.org/D149472 __

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

JDevlieghere reopened this revision. JDevlieghere added a comment. That's my bad: I put the wrong differential URL in my commit (13dbc16b4d82b9adc98c0919873b2b59ccc46afd ) Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

jingham added a comment. I think something went wrong here. This was a patch for OptionValueDictionary, but it seems to have become a patch for OpenInExternalEditor? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149482/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

This revision was automatically updated to reflect the committed changes. Closed by commit rG5679379cc7df: Refactor and generalize AArch64 watchpoint support in debugserver (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149

[Lldb-commits] [lldb] 5679379 - Refactor and generalize AArch64 watchpoint support in debugserver

Author: Jason Molenda Date: 2023-04-28T18:24:38-07:00 New Revision: 5679379cc7df198a73c137e691346088338f URL: https://github.com/llvm/llvm-project/commit/5679379cc7df198a73c137e691346088338f DIFF: https://github.com/llvm/llvm-project/commit/5679379cc7df198a73c137e691346088338f.diff

[Lldb-commits] [lldb] 13dbc16 - [lldb] Refactor host::OpenFileInExternalEditor

Author: Jonas Devlieghere Date: 2023-04-28T18:12:41-07:00 New Revision: 13dbc16b4d82b9adc98c0919873b2b59ccc46afd URL: https://github.com/llvm/llvm-project/commit/13dbc16b4d82b9adc98c0919873b2b59ccc46afd DIFF: https://github.com/llvm/llvm-project/commit/13dbc16b4d82b9adc98c0919873b2b59ccc46afd.d

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG13dbc16b4d82: [lldb] Refactor host::OpenFileInExternalEditor (authored by JDevlieghere). Changed prior to commit: https

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Thanks. This LGTM. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:856 + // user_size == 16 -> aligned_size == 16 + aligned_size = 1

[Lldb-commits] [PATCH] D149503: Remove i386 and armv7 native support from debugserver

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/D149503/new/ https://reviews.llvm.org/D149503 _

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

JDevlieghere added a comment. +1 on making the output deterministic with a sort Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149482/new/ https://reviews.llvm.org/D149482 ___ lldb-commits mailing list ll

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

jasonmolenda updated this revision to Diff 518101. jasonmolenda marked an inline comment as done. jasonmolenda added a comment. Update patch to incorporate Jonas' feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149040/new/ https://reviews.l

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

jasonmolenda marked 7 inline comments as done. jasonmolenda added inline comments. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:833-835 +std::vector +DNBArchMachARM64::AlignRequestedWatchpoint(nub_addr_t user_addr, +

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord added a comment. In D149262#4306884 , @kuilpd wrote: > In D149262#4306820 , @bulbazord > wrote: > >> Sorry I should have brought this up earlier but I've noticed you don't have >> any tests with this c

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

kuilpd added a comment. In D149262#4306820 , @bulbazord wrote: > Sorry I should have brought this up earlier but I've noticed you don't have > any tests with this change. Is it possible you could add something there? > Something to make sure that these

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

bulbazord added inline comments. Comment at: lldb/source/Core/Disassembler.cpp:846 +// Note: The reason we are making the data_type a uint64_t when value is +// uint32_t is that there is no eTypeUInt32 enum value. +if (llvm::StringRef(value) == "uint32_t"

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord added a comment. Sorry I should have brought this up earlier but I've noticed you don't have any tests with this change. Is it possible you could add something there? Something to make sure that these settings work as expected. Sorry for the churn btw, I really appreciate your patienc

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

kuilpd added inline comments. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:340-348 +Process *process_sp; +ABISP abi_sp; +if ((process_sp = exe_ctx.GetProcessPtr()) && +(abi_sp = process_sp->GetABI())) { + stack_frame_size

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

jingham added a comment. No reason for these strings to be in the ConstString pool, so that part is fine. But if we're going to use this to store things like the env-vars, having no ordering guarantees when we dump them seems likely to be a bit annoying. If the order of element output in `sett

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord added inline comments. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:340-348 +Process *process_sp; +ABISP abi_sp; +if ((process_sp = exe_ctx.GetProcessPtr()) && +(abi_sp = process_sp->GetABI())) { + stack_frame_si

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

kuilpd added inline comments. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:38 +using namespace lldb; using namespace lldb_private; bulbazord wrote: > What in this file uses something from the lldb namespace now? ABI class Commen

[Lldb-commits] [PATCH] D149482: [lldb] Change ObjectValueDictionary to use a StringMap

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. llvm has a structure for maps where the key's type

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

bulbazord requested changes to this revision. bulbazord added a comment. This revision now requires changes to proceed. Mostly looks good to me, just a small thing about an implementation detail. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:38 +using namespace ll

[Lldb-commits] [PATCH] D149477: [lldb/crashlog] Fix json module loading issue

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdc275fd03254: [lldb/crashlog] Fix JSON ObjectFile module loading issue (authored by mib). Changed prior to commit: https://reviews.llvm.org/D14947

[Lldb-commits] [lldb] dc275fd - [lldb/crashlog] Fix JSON ObjectFile module loading issue

Author: Med Ismail Bennani Date: 2023-04-28T12:51:01-07:00 New Revision: dc275fd03254d67d29cc70a5a0569acf24d2280d URL: https://github.com/llvm/llvm-project/commit/dc275fd03254d67d29cc70a5a0569acf24d2280d DIFF: https://github.com/llvm/llvm-project/commit/dc275fd03254d67d29cc70a5a0569acf24d2280d.

[Lldb-commits] [PATCH] D149477: [lldb/crashlog] Fix json module loading issue

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/D149477/new/ https://reviews.llvm.org/D149477 ___

[Lldb-commits] [PATCH] D149477: [lldb/crashlog] Fix json module loading issue

mib created this revision. mib added a reviewer: JDevlieghere. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. In 27f27d15f6c9 , we added a

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

bulbazord accepted this revision. bulbazord added a comment. LGTM! Thanks for cleaning this up. Comment at: lldb/source/Target/Thread.cpp:1762-1763 + frame_sc.line_entry.file, frame_sc.line_entry.line)) { +LLDB_LOG_ERROR(GetLog(LLDBLog::Host), std::

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

kuilpd updated this revision to Diff 518010. kuilpd marked 4 inline comments as done. kuilpd added a comment. Removed unused variable arch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149262/new/ https://reviews.llvm.org/D149262 Files: lldb/include/lldb/Target/ABI.h lldb/include/

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

asl added inline comments. Comment at: lldb/source/Expression/IRMemoryMap.cpp:181 size_t alloc_size = back->second.m_size; auto arch = target_sp->GetArchitecture().GetTriple().getArch(); +uint64_t align = target_sp->GetExprAllocAlign(); is arch unus

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

JDevlieghere updated this revision to Diff 518007. JDevlieghere marked 10 inline comments as done. JDevlieghere added a comment. - Return an `llvm::Erorr` and percolate errors up - Don't fall back to the default editor if the one specified can't be found - Limit roles to editors only CHANGES SIN

[Lldb-commits] [PATCH] D149262: [lldb] Add settings for expression evaluation memory allocations.

kuilpd updated this revision to Diff 518003. kuilpd added a comment. Made an ABI method that returns a stack frame size for the target. Removed the condition for alignment. The default value doesn't matter too much even for a 16-bit target, no need to add another ABI method just for this. The p

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

jingham added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:387-388 + static std::once_flag g_once_flag; + std::call_once(g_once_flag, [&]() { +if (const char *external_editor = ::getenv("LLDB_EXTERNAL_EDITOR")) { + LLDB_LOG(log, "Looking for exte

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

bulbazord added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337 + + LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no); + JDevlieghere wrote: > bulbazord wrote: > > nit: Move log line below checking if the file_path i

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

jingham added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:411-412 kLSLaunchDefaults | kLSLaunchDontAddToRecents | kLSLaunchDontSwitch; - - char *external_editor = ::getenv("LLDB_EXTERNAL_EDITOR"); - - if (external_editor) { -LLDB_LOGF(log, "Lo

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

mib added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:420 + error = ::LSOpenURLsWithRole( + /*inURLs=*/cf_array.get(), /*inRole=*/kLSRolesAll, + /*inAEParam=*/&file_and_line_desc, I guess we could narrow down the `inRole` argume

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

mib added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:343-344 + CFCString file_cfstr(file_path.c_str(), kCFStringEncodingUTF8); + CFCReleaser file_URL(::CFURLCreateWithFileSystemPath( + NULL, file_cfstr.get(), kCFURLPOSIXPathStyle, false)); + --

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

JDevlieghere marked 5 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:335 + + const std::string file_path = file_spec.GetPath(); + mib wrote: > I guess this doesn't need to be a `const std::string`,

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

JDevlieghere marked 3 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337 + + LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no); + bulbazord wrote: > nit: Move log line below c

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. LGTM with some comments Comment at: lldb/source/Host/macosx/objcxx/Host.mm:335 + + const std::string file_path = file_spec.GetPath(); + I guess this doesn't need

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

bulbazord added a comment. LGTM overall. A few nits but overall an excellent cleanup! :) Comment at: lldb/source/Host/macosx/objcxx/Host.mm:337 + + LLDB_LOG(log, "Sending {0}:{1} to external editor", file_path, line_no); + nit: Move log line below checking if

[Lldb-commits] [PATCH] D149472: [lldb] Refactor host::OpenFileInExternalEditor

JDevlieghere created this revision. JDevlieghere added reviewers: aprantl, bulbazord, mib. Herald added a project: All. JDevlieghere requested review of this revision. This patch refactors the macOS implementation of `OpenFileInExternalEditor`: - Fix `AppleEvent` memory leak - Fix caching of `LLD

[Lldb-commits] [PATCH] D149397: Host: generalise `GetXcodeSDKPath`

This revision was automatically updated to reflect the committed changes. Closed by commit rGade3c6a6a88e: Host: generalise `GetXcodeSDKPath` (authored by compnerd). Changed prior to commit: https://reviews.llvm.org/D149397?vs=517764&id=517959#toc Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [lldb] ade3c6a - Host: generalise `GetXcodeSDKPath`

Author: Saleem Abdulrasool Date: 2023-04-28T09:30:59-07:00 New Revision: ade3c6a6a88ed3a9b06c076406f196da9d3cc1b9 URL: https://github.com/llvm/llvm-project/commit/ade3c6a6a88ed3a9b06c076406f196da9d3cc1b9 DIFF: https://github.com/llvm/llvm-project/commit/ade3c6a6a88ed3a9b06c076406f196da9d3cc1b9.

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

jasonmolenda added a comment. Thanks for all the helpful comments, I'll update the patch. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:833-835 +std::vector +DNBArchMachARM64::AlignRequestedWatchpoint(nub_addr_t user_addr, +

[Lldb-commits] [PATCH] D147831: [lldb-vscode] Implement RestartRequest

wallace added inline comments. Comment at: lldb/source/API/SBProcess.cpp:772 - return (event.GetBroadcasterClass() == SBProcess::GetBroadcasterClass()) && - !EventIsStructuredDataEvent(event); + return Process::ProcessEventData::GetEventDataFromEvent(event.get()) != +

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

tschuett added inline comments. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:856 + // user_size == 16 -> aligned_size == 16 + aligned_size = 1ULL << (addr_bit_size - __builtin_clzll(aligned_size - 1)); + JDevlieghere wrote: > Beau

[Lldb-commits] [PATCH] D146058: [lldb][gnustep] Add basic test and infrastructure for GNUstep ObjC runtime

labath added a comment. In D146058#4224001 , @sgraenitz wrote: > In D146058#4223575 , @labath wrote: > >> What kind of setup is necessary to make that work? If it's not too >> complicated, I think we could make s

[Lldb-commits] [PATCH] D147831: [lldb-vscode] Implement RestartRequest

labath added a comment. Looks ok-ish, though I don't feel entirely comfortable approving lldb-vscode changes. @wallace, do you have any thoughts on this? Comment at: lldb/tools/lldb-vscode/VSCode.h:152 bool is_attach; + // The process event thread normally responds to proc

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2663 +// the actual range check below. +if (addend < 0 && static_cast(std::abs(addend)) > value)

[Lldb-commits] [PATCH] D149214: [lldb] Speed up DebugAbbrev parsing

labath added a comment. I guess the most efficient (performance- and memory-wise) approach would be to have a global (well, scoped to a DWARFUnit or something) array of DWARFAttribute objects, and have the individual abbreviations just store pointers/indexes to that. Repository: rG LLVM Git

[Lldb-commits] [PATCH] D148662: [lldb] Make the libcxx unique_ptr prettyprinter support custom deleters.

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:52 +ValueObject &pair) { + ValueObjectSP value = pair.GetChildAtIndex(0, true)->GetChildMemberWithName(

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

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