[Lldb-commits] [PATCH] D107470: 2/3: [llvm+lldb] Remove dead-code in DWARFListTableHeader::extract modifying DWARFDataExtractor

2021-08-12 Thread Igor Kudrin via Phabricator via lldb-commits
ikudrin added inline comments. Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFListTable.h:289-291 if (Header.length()) Data = DWARFDataExtractor(Data, getHeaderOffset() + Header.length()); + Data.setAddressSize(getAddrSize()); if `Header.Length` is z

[Lldb-commits] [PATCH] D108018: [lldb] [gdb-remote] Support QEnvironment fallback to hex-encoded

2021-08-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision. mgorny added reviewers: krytarowski, emaste, labath, jasonmolenda, JDevlieghere. mgorny requested review of this revision. Fall back to QEnvironmentHexEncoded if QEnvironment is not supported. The latter packet is an LLDB extension, while the former is universally sup

Re: [Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-12 Thread Jim Ingham via lldb-commits
Sure, thanks for the context! Actually, the posix_spawn usage is only in the darwin Host code, probably because for posix_spawn to work for debugging you need a couple of non-standard flags that were added on Darwin for us (the CoreOS folks really wanted us to use posix_spawn). So you probably

[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

2021-08-12 Thread hanbing wang via Phabricator via lldb-commits
hanbingwang updated this revision to Diff 366151. hanbingwang edited the summary of this revision. hanbingwang added a comment. Herald added a subscriber: pengfei. *TraceSessionSaver.h, TraceSessionSaver.cpp: -move BuildModulesSection(), BuildThreadsSection(), BuildProcessesSection(), WriteSessio

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-12 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment. In D105732#2942716 , @jingham wrote: > Do modern Linux's not have posix_spawn? If it exists that's a better > interface, and lets the system handle a lot of the complicated machinations > you have to do by hand if you roll it

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Do modern Linux's not have posix_spawn? If it exists that's a better interface, and lets the system handle a lot of the complicated machinations you have to do by hand if you roll it yourself out of fork and exec. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-12 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment. In D105732#2942355 , @clayborg wrote: > Sorry for the delay. This seems like it should work just fine. Where is this > actually used? I thought most clients should be using posix_spawn() as this > is the preferred launching mec

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-12 Thread Rumeet Dhindsa via Phabricator via lldb-commits
rdhindsa added a comment. fork is being called in LaunchProcess function in this file(ProcessLauncherPosixFork.cpp) itself. Thank you for the review! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105732/new/ https://reviews.llvm.org/D105732 ___

[Lldb-commits] [PATCH] D107213: Disassemble AArch64 pc-relative address calculations & symbolicate

2021-08-12 Thread Jason Molenda via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7150b562081f: Symbolicate aarch64 adrp+add pc-relative addr in disass (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SIN

[Lldb-commits] [lldb] 7150b56 - Symbolicate aarch64 adrp+add pc-relative addr in disass

2021-08-12 Thread Jason Molenda via lldb-commits
Author: Jason Molenda Date: 2021-08-12T14:44:17-07:00 New Revision: 7150b562081ffb2ec5406edfa579b16d3ec20d90 URL: https://github.com/llvm/llvm-project/commit/7150b562081ffb2ec5406edfa579b16d3ec20d90 DIFF: https://github.com/llvm/llvm-project/commit/7150b562081ffb2ec5406edfa579b16d3ec20d90.diff

[Lldb-commits] [PATCH] D107931: [lldb] [gdb-remote] Implement vRun packet

2021-08-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 366090. mgorny added a comment. Handle unsupported `qLaunchSuccess` gracefully on the client. This effectively makes it possible to launch programs via gdbserver. Enhance the client tests to verify that the process has actually 'started'. CHANGES SINCE LAST

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Sorry for the delay. This seems like it should work just fine. Where is this actually used? I thought most clients should be using posix_spawn() as this is the preferred launching mechanis

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

2021-08-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:305 + "ScriptedProcess::%s ERROR = %s", __FUNCTION__, + message.str().c_str()); +return false; Same comment as in the other revi

[Lldb-commits] [PATCH] D107931: [lldb] [gdb-remote] Implement vRun packet

2021-08-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 366084. mgorny added a comment. Correct the vRun packet to return a stop reason rather than 'OK' to match GDB behavior. Add initial client tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107931/new/ https://reviews.llvm.org/D107931 Files: ll

[Lldb-commits] [PATCH] D107470: 2/3: [llvm+lldb] Remove dead-code in DWARFListTableHeader::extract modifying DWARFDataExtractor

2021-08-12 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked an inline comment as done. jankratochvil added inline comments. Comment at: llvm/unittests/DebugInfo/DWARF/DWARFListTableTest.cpp:128 + EXPECT_EQ(Table.getAddrSize(), 8U); + Extractor.setAddressSize(Table.getAddrSize()); + Expected List = Table.findList(Ex

[Lldb-commits] [PATCH] D107470: 2/3: [llvm+lldb] Remove dead-code in DWARFListTableHeader::extract modifying DWARFDataExtractor

2021-08-12 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 366017. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107470/new/ https://reviews.llvm.org/D107470 Files: lldb/unittests/SymbolFile/DWARF/DWARFUnitTest.cpp llvm/include/llvm/DebugInfo/DWARF/DWARFListTa

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

2021-08-12 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > Only clang-tidy is upset and it is falsely flagging "missing" files This is because llvm precommit doesn't actually build lldb (due to some test failures they were/are having) so those headers are unknown to clang-tidy when it runs. You can ignore it. CHANGES

[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

2021-08-12 Thread Alf via Phabricator via lldb-commits
gAlfonso-bit added a comment. Only clang-tidy is upset and it is falsely flagging "missing" files CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107704/new/ https://reviews.llvm.org/D107704 ___ lldb-commits mailing list lldb-commits@lists.llvm

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-12 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev updated this revision to Diff 365958. OmarEmaraDev added a comment. - Separate environment field into two fields. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107869/new/ https://reviews.llvm.org/D107869 Files: lldb/include/lldb/Ta

[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

2021-08-12 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor updated this revision to Diff 365944. teemperor added a comment. - Fix double printing and formatting of the old plain text format. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107079/new/ https://reviews.llvm.org/D107079 Files: lldb/docs/design/logging.rst lldb/docs/ind

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-12 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added a comment. Adding a boolean for inheriting the environment may not be necessary, we can just add two environment fields, one for inherited and one for user specified. The inherited one will be put in advanced settings with another boolean that show or hide the field. Both wil

[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

2021-08-12 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added inline comments. Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3223 + +const Environment &target_environment = target->GetEnvironment(); +m_enviroment_field->AddEnvironmentVariables(target_environment); clayborg wrote: > Does this

[Lldb-commits] [PATCH] D107213: Disassemble AArch64 pc-relative address calculations & symbolicate

2021-08-12 Thread David Spickett via Phabricator via lldb-commits
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/D107213/new/ https://reviews.llvm.org/D107213 _

[Lldb-commits] [PATCH] D107931: [lldb] [gdb-remote] Implement vRun packet [WIP]

2021-08-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 365927. mgorny edited the summary of this revision. mgorny added a comment. Include server tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107931/new/ https://reviews.llvm.org/D107931 Files: lldb/include/lldb/Utility/StringExtractorGDBRemote.

[Lldb-commits] [PATCH] D107213: Disassemble AArch64 pc-relative address calculations & symbolicate

2021-08-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda updated this revision to Diff 365925. jasonmolenda added a comment. Address David's second review comments -- explained the unusual formatting of the main.c in the test case, change the m_adrp_insn to be an Optional instead of using 0 as a "not-set" value. I'll prob land this tomor