[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

Mordante added a comment. In D159127#4626311 , @kastiglione wrote: > Thanks for doing this! > > Question to all: Should the summary string include the unit? lldb doesn't > always show the type, so it could help comprehension if the unit is included. >

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

JDevlieghere added a comment. Similar to the LLVM patch this depends on, this patch is a WIP to get some early feedback. This definitely needs a test. Most of the patch deals with plumbing the value of `Debugger::GetUseColor` to the `Disassembler`. CHANGES SINCE LAST ACTION https://reviews.l

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

JDevlieghere created this revision. JDevlieghere added reviewers: jasonmolenda, DavidSpickett, clayborg. Herald added subscribers: jrtc27, sdardis. Herald added a project: All. JDevlieghere requested review of this revision. Add support for syntax highlighting assembly. The current patch uses a d

[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. LGTM with @kastiglione's comments addressed. Comment at: lldb/source/Core/Section.cpp:153 + case eSectionTypeSwiftModules: +return "swift-modules"; }

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda added a comment. Oh, and I did change the LC_NOTE name that I'm adding to "process metadata" and specified that it *may* contain a `threads` key, instead of *shall* contain a `threads` key. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda updated this revision to Diff 554541. jasonmolenda marked an inline comment as done. jasonmolenda added a comment. Address feedback from Alex and Jonas. Most significantly, add a `ObjectFileMachO::FindLC_NOTEByName` method which all the LC_NOTE readers in ObjectFileMachO call. One

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda marked 6 inline comments as done. jasonmolenda added a comment. Thanks for the feedback Alex & Jonas. Jonas' comment that we have a lot of different methods manually stepping over load commands to find their LC_NOTEs was something I'd been meaning to deal with some day, but hadn't

[Lldb-commits] [PATCH] D159150: [lldb][NFCI] Replace bespoke iterator check with std::next

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, fdeazeve. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The primary goal of this change is to change `if (pair

[Lldb-commits] [PATCH] D152870: [lldb][NFCI] Remove StructuredData::Array::GetItemAtIndexAsString overloads with ConstString

This revision was automatically updated to reflect the committed changes. Closed by commit rG5d49c9ce6588: [lldb][NFCI] Remove StructuredData::Array::GetItemAtIndexAsString overloads… (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[Lldb-commits] [lldb] 5d49c9c - [lldb][NFCI] Remove StructuredData::Array::GetItemAtIndexAsString overloads with ConstString

Author: Alex Langford Date: 2023-08-29T16:02:26-07:00 New Revision: 5d49c9ce65881467295fd5ef2022b60d6601e82f URL: https://github.com/llvm/llvm-project/commit/5d49c9ce65881467295fd5ef2022b60d6601e82f DIFF: https://github.com/llvm/llvm-project/commit/5d49c9ce65881467295fd5ef2022b60d6601e82f.diff

[Lldb-commits] [PATCH] D158583: Fix shared library loading when users define duplicate _r_debug structure.

clayborg added a comment. Anyone have any objections? Super easy to repro this bug with the test program. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158583/new/ https://reviews.llvm.org/D158583 ___ ll

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

jasonmolenda added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:107 +static size_t word_size = 4U; +static size_t reg_size = word_size; + ted wrote: > DavidSpickett wrote: > > What's the reason to do this this way? It seems like ad

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

ted updated this revision to Diff 554505. ted added a comment. Updated with David Spickett's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159101/new/ https://reviews.llvm.org/D159101 Files: lldb/source/Plugins/ABI/CMakeLists.txt lldb

[Lldb-commits] [PATCH] D159011: [lldb][NFCI] Remove use of ConstString from UnixSignals

bulbazord updated this revision to Diff 554504. bulbazord added a comment. Protect StringSet with a mutex Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159011/new/ https://reviews.llvm.org/D159011 Files: lldb/include/lldb/Target/UnixSignals.h

[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

kastiglione added inline comments. Comment at: lldb/source/Core/Section.cpp:153 + case eSectionTypeSwiftModules: +return "swift-modules"; } I wonder if this should be "swiftmodules". I have never seen it spelled with a hyphen. Repository: rG LLVM Git

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

ted marked 13 inline comments as done. ted added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:107 +static size_t word_size = 4U; +static size_t reg_size = word_size; + DavidSpickett wrote: > What's the reason to do this this way? I

[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()

wallace added inline comments. Comment at: lldb/source/Expression/REPL.cpp:535 + if (current_line_idx < current_lines.GetSize()) { +for (uint32_t i = 0; i < current_line_idx; ++i) { + const char *line_cstr = current_lines.GetStringAtIndex(i); aprantl wr

[Lldb-commits] [PATCH] D159142: [lldb] Add support for recognizing swift ast sections in object files

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, aprantl, fdeazeve, kastiglione, augusto2112. Herald added a subscriber: emaste. Herald added a project: All. bulbazord requested review of this revision. Herald added subscribers: lldb-commits, MaskRay. Herald added a projec

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

kastiglione added a comment. Thanks for doing this! Question to all: Should the summary string include the unit? lldb doesn't always show the type, so it could help comprehension if the unit is included. For example `60s` instead of `60`. Comment at: lldb/source/Plugins/Lang

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

ted added a comment. In D159101#4625539 , @DavidSpickett wrote: > Also could you provide a list of things you have tested with qemu. So I can > get an idea of how sure we can be any of this works / be a test plan, albeit > manual for anyone updating th

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

aprantl added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:996 + TypeSummaryImplSP(new StringSummaryFormat( + eTypeOptionHideChildren | eTypeOptionHideValue, "${var.__rep_}"))); } Nice! ===

[Lldb-commits] [PATCH] D152870: [lldb][NFCI] Remove StructuredData::Array::GetItemAtIndexAsString overloads with ConstString

fdeazeve accepted this revision. fdeazeve 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/D152870/new/ https://reviews.llvm.org/D152870 __

[Lldb-commits] [PATCH] D158833: [lldb] Move ScriptInterpreter Interfaces to subdirectory (NFC)

mib added a comment. In D158833#4625775 , @bulbazord wrote: > Re-organizing the paths seems okay to me, especially since this is going to > grow further. I think the header guards are going to need some adjustment > though. TBH, I didn't even know they

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

ted added reviewers: jasonmolenda, labath, asb, lewis-revill, compnerd, simoncook, jrtc27, JDevlieghere, aprantl, sven, kasper81, tzb99. ted added a comment. Adding commenters from https://reviews.llvm.org/D62732 as reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[Lldb-commits] [PATCH] D158971: [lldb][NFC] Put disassembler test classes and methods in anonymous namespace

RamNalamothu added a comment. In D158971#4624001 , @DavidSpickett wrote: > And this is being done for what reason? This is a follow-up on https://reviews.llvm.org/D156086#inline-1518306 comment. I see that most of the lldb unit tests follow this style.

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

ted added a subscriber: labath. ted added a comment. With this change, I'm able to debug RISC-V 32 and 64 bit applications using Linux user space QEMU, using @labath 's QemuUser platform. Simple expression parsing with the IR Interpreter is working. Complex expression parsing with JIT is not te

[Lldb-commits] [PATCH] D159011: [lldb][NFCI] Remove use of ConstString from UnixSignals

bulbazord added a comment. In D159011#4625915 , @fdeazeve wrote: >> I decided to make it a global object to guarantee its lifetime independent >> of any `PlatformRemoteGDBServer` instance. I don't mind wrapping it in a >> concurrent structure to guarant

[Lldb-commits] [PATCH] D159127: [lldb][libc++] Adds chrono data formatters.

Mordante created this revision. Mordante added reviewers: Michael137, aprantl. Herald added a project: All. Mordante requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This adds the data formatters for chrono duration typedefs. Repository:

[Lldb-commits] [PATCH] D159011: [lldb][NFCI] Remove use of ConstString from UnixSignals

fdeazeve added a comment. > I decided to make it a global object to guarantee its lifetime independent of > any `PlatformRemoteGDBServer` instance. I don't mind wrapping it in a > concurrent structure to guarantee thread safety though. At that point we would pretty much be re-implementing the n

[Lldb-commits] [PATCH] D159011: [lldb][NFCI] Remove use of ConstString from UnixSignals

bulbazord added a comment. In D159011#4624495 , @fdeazeve wrote: > I am slightly wary of making this a global set without any kind of thread > safe mechanism, as I (naively, not really knowing how this class is used) > would expect us to be able to inst

[Lldb-commits] [PATCH] D158833: [lldb] Move ScriptInterpreter Interfaces to subdirectory (NFC)

bulbazord added a comment. Re-organizing the paths seems okay to me, especially since this is going to grow further. I think the header guards are going to need some adjustment though. Comment at: lldb/include/lldb/Interpreter/Interfaces/ScriptedProcessInterface.h:9 -#ifnd

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcf1a3d93581f: Implement the monolithic CI pipeline in the monorepo (authored by ldionne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

ldionne updated this revision to Diff 554420. ldionne marked an inline comment as done. ldionne added a comment. Address review comments. I did some testing using GH PRs and this should work, but some tweaks might be necessary. After discussing with Mikhail, I'll merge this now and we can make t

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

DavidSpickett added a comment. I understand now. If we take `WriteTLS` for example, that ends with `m_tls_is_valid = false;`. So you're suggesting instead we have a `WriteTLS` whose source is not the usual TLS buffer, but the set of saved register data instead. I'll look into it. Repository:

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

DavidSpickett added a comment. Also could you provide a list of things you have tested with qemu. So I can get an idea of how sure we can be any of this works / be a test plan, albeit manual for anyone updating this code in future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added a comment. From what little I know about the ABI this looks ok. Consider adding a riscv expert to check those bits for you. Testing wise riscv is in a strange place without a working lldb-server or a buildbot that could run such

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

omjavaid added a comment. > I don't know about expensive literally but I did wonder if we could rely on > the NativeRegisterContextLinux_arm64 buffers not being modified while the > expression runs. Then do what I think you mean which is just flush them all > to the process. > > I'll need to wo

[Lldb-commits] [lldb] f2f5d6f - [lldb] Fix test failures introduced by 8e0a087571a3

Author: Med Ismail Bennani Date: 2023-08-29T16:52:30+01:00 New Revision: f2f5d6fb8d53bc4bd93a3d4e110134ed017b636f URL: https://github.com/llvm/llvm-project/commit/f2f5d6fb8d53bc4bd93a3d4e110134ed017b636f DIFF: https://github.com/llvm/llvm-project/commit/f2f5d6fb8d53bc4bd93a3d4e110134ed017b636f.

[Lldb-commits] [lldb] 8e0a087 - [lldb] Fix build failures introduced in f0731d5b61ba (NFCI)

Author: Med Ismail Bennani Date: 2023-08-29T16:34:56+01:00 New Revision: 8e0a087571a31057bb98939e3ada73227bed83c7 URL: https://github.com/llvm/llvm-project/commit/8e0a087571a31057bb98939e3ada73227bed83c7 DIFF: https://github.com/llvm/llvm-project/commit/8e0a087571a31057bb98939e3ada73227bed83c7.

[Lldb-commits] [PATCH] D158958: [LLDB][REPL] Change the default tab size

aprantl added a comment. > @aprantl, how should I proceed with the swift branch? Should I create a new > branch in https://github.com/apple/swift and share it here so it's available > for whoever does the merge? Actually, I can just cherry-pick it myself, let's not worry about this. Repositor

[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()

aprantl added a comment. Is this covered by any tests? Comment at: lldb/source/Core/IOHandler.cpp:512 +StringList IOHandlerEditline::GetCurrentLines() const { +#if LLDB_ENABLE_LIBEDIT + if (m_editline_up) I think this would benefit from a comment explaining th

[Lldb-commits] [PATCH] D159017: [lldb/docs] Silence warnings when generating website

This revision was automatically updated to reflect the committed changes. Closed by commit rGf0731d5b61ba: Re-land "[lldb/docs] Silence warnings when generating website" (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159017/new/ ht

[Lldb-commits] [lldb] f0731d5 - Re-land "[lldb/docs] Silence warnings when generating website"

Author: Med Ismail Bennani Date: 2023-08-29T16:24:33+01:00 New Revision: f0731d5b61ba798e6d5a63a92d9228010e5a3b50 URL: https://github.com/llvm/llvm-project/commit/f0731d5b61ba798e6d5a63a92d9228010e5a3b50 DIFF: https://github.com/llvm/llvm-project/commit/f0731d5b61ba798e6d5a63a92d9228010e5a3b50.

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

DavidSpickett added a comment. > Is there a way we can optimise this may be write register directly from > source buffer. Though now I wonder if when you say source here you mean the member buffers in the register context, or literally the `src` pointer we use in the code. Repository: rG LL

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

DavidSpickett added a comment. > I have a feeling that expressions are becoming an expensive operation with > amount of data we need to move back and forth between various buffers. Is > there a way we can optimise this may be write register directly from source > buffer. I don't know about exp

[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

omjavaid added a comment. I have a feeling that expressions are becoming an expensive operation with amount of data we need to move back and forth between various buffers. Is there a way we can optimise this may be write register directly from source buffer. Also how much minimum data we need t

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

ted created this revision. Herald added subscribers: luke, sunshaoce, VincentWu, vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, jo

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

ldionne marked an inline comment as done. ldionne added inline comments. Comment at: .ci/generate-buildkite-pipeline-premerge:163 + +if [[ ! ${SPECIFIC_PIPELINE_AVAILABLE} -eq 1 ]]; then + # Figure out which projects need to be built on each platform goncharov w

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

emaste added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp:115-116 +// TODO: If we need to check unknow OS triple like armv7-unknown-unknown? +// TODO: I don't know if kFreeBSD is a type of FreeBSD and should

[Lldb-commits] [lldb] 18f1c1a - Revert "[lldb/docs] Silence warnings when generating website"

Author: Med Ismail Bennani Date: 2023-08-29T13:57:35+01:00 New Revision: 18f1c1ace7a6099f3e8e56cf4b81aa0f64a7dd23 URL: https://github.com/llvm/llvm-project/commit/18f1c1ace7a6099f3e8e56cf4b81aa0f64a7dd23 DIFF: https://github.com/llvm/llvm-project/commit/18f1c1ace7a6099f3e8e56cf4b81aa0f64a7dd23.

[Lldb-commits] [lldb] 498b59e - [lldb/docs] Silence warnings when generating website

Author: Med Ismail Bennani Date: 2023-08-29T13:45:30+01:00 New Revision: 498b59e01133bed16c72dd939a9daa8ae1f359e4 URL: https://github.com/llvm/llvm-project/commit/498b59e01133bed16c72dd939a9daa8ae1f359e4 DIFF: https://github.com/llvm/llvm-project/commit/498b59e01133bed16c72dd939a9daa8ae1f359e4.

[Lldb-commits] [PATCH] D159017: [lldb/docs] Silence warnings when generating website

mib added inline comments. Comment at: lldb/docs/conf.py:46 autodoc_default_options = { "special-members": "__int__, __len__, __hex__, __oct__, __iter__", } @JDevlieghere AFAIK, if we want to silence the warnings, we should disable generating special memb

[Lldb-commits] [PATCH] D159011: [lldb][NFCI] Remove use of ConstString from UnixSignals

fdeazeve added a comment. I am slightly wary of making this a global set without any kind of thread safe mechanism, as I (naively, not really knowing how this class is used) would expect us to be able to instantiate multiple servers and have them be accessed concurrently. As such, it makes more

[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

omjavaid added a comment. This looks fine but instead of adding a new test cant we add MTE control register tests to already available MTE tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157488/new/ https://reviews.llvm.org/D157488 _

[Lldb-commits] [PATCH] D157000: [lldb][AArch64] Check SIMD save/restore in SVE SIMD test

omjavaid accepted this revision. omjavaid added inline comments. This revision is now accepted and ready to land. Comment at: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c:43 +void write_simd_regs_expr() { +#define WRITE_SIMD(NUM)

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

aokblast updated this revision to Diff 554237. aokblast added a comment. Add subdirectory for DynamicLoader Plugin directory Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159076/new/ https://reviews.llvm.org/D159076 Files: lldb/source/Plugins/Dy

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

aokblast updated this revision to Diff 554236. aokblast added a comment. Herald added a subscriber: MaskRay. Add relocatable file support for kernel module Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159076/new/ https://reviews.llvm.org/D159076

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

aokblast updated this revision to Diff 554235. aokblast added a comment. Change DynamicLoader plugin for ProcessFreeBSDKernel Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159076/new/ https://reviews.llvm.org/D159076 Files: lldb/source/Plugins/D

[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

aokblast created this revision. Herald added a subscriber: emaste. Herald added a project: All. aokblast requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D159076 Files: lldb/

[Lldb-commits] [PATCH] D159011: [lldb][NFCI] Remove use of ConstString from UnixSignals

DavidSpickett accepted this revision. DavidSpickett 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/D159011/new/ https://reviews.llvm.org/D159011

[Lldb-commits] [PATCH] D158971: [lldb][NFC] Put disassembler test classes and methods in anonymous namespace

DavidSpickett added a comment. And this is being done for what reason? I seem to remember something about static and anonymous namespace in the developer guide, please cite that if so. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158971/new/ htt

[Lldb-commits] [PATCH] D158863: Implement the monolithic CI pipeline in the monorepo

goncharov added inline comments. Comment at: .ci/generate-buildkite-pipeline-premerge:163 + +if [[ ! ${SPECIFIC_PIPELINE_AVAILABLE} -eq 1 ]]; then + # Figure out which projects need to be built on each platform ldionne wrote: > goncharov wrote: > > do I understa