[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

labath added a comment. In D86667#2243781 , @mib wrote: > In D86667#2242566 , @jingham wrote: > >> Do people really call command-line shells interpreters? I would have >> thought --shell would be a better name. l

[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

jasonmolenda added a comment. Hi, thanks so much for showing all the unwind plans, sorry for dropping off the thread for a day. I'm seeing something I very much don't expect, and it may be important. The unwind information for __restore_rt is marked as a trap handler in both the eh_frame unwi

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

jasonmolenda added a comment. In D86667#2243878 , @labath wrote: > In D86667#2243781 , @mib wrote: > >> In D86667#2242566 , @jingham wrote: >> >>> Do people really call comma

[Lldb-commits] [PATCH] D86745: [lldb/test] Use @skipIfWindows for PExpectTest

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks like it could work, though only barely. `@skipIfWindows` checks for target system, but we would really need this to check the host OS (for remote testing, although I don't know if a

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

labath added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759 + QualType FromTy = ArrayFrom->getElementType(); + QualType ToTy = ArrayTo->getElementType(); + + FromRecordDecl = FromTy->getAsRecordDecl(); + ToRecordDecl = ToT

[Lldb-commits] [PATCH] D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners

labath added a comment. This is going to derail this patch somewhat (sorry), but since this is an important long-standing open issue, I think we should discuss it in more detail. The part that's bothering me about this for quite some time is... Why do we even have this DoOnRemoval stuff in the

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

labath added a comment. In D85705#2243536 , @wallace wrote: > - I'm still using StructuredData for the parsing. There's a chance that once > we release this feature users will want support for other formats besides > JSON, so for now I prefer to ask for

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

labath added a comment. I think it would be appropriate to discuss the design of this feature on lldb-dev before going over the individual patches. One of the fundamental aspects of this patchset that I think should not be overlooked is that it essentially adds a new level of structure ( a "Tar

[Lldb-commits] [lldb] 1f9595e - [lldb] Reduce intentation in SymbolFileDWARF::ParseVariableDIE

Author: Pavel Labath Date: 2020-08-28T11:44:03+02:00 New Revision: 1f9595ede48d694b8506d22fd71f7ce13e00dafa URL: https://github.com/llvm/llvm-project/commit/1f9595ede48d694b8506d22fd71f7ce13e00dafa DIFF: https://github.com/llvm/llvm-project/commit/1f9595ede48d694b8506d22fd71f7ce13e00dafa.diff

[Lldb-commits] [lldb] 9b50546 - [lldb/Utility] Polish the Scalar class

Author: Pavel Labath Date: 2020-08-28T11:51:25+02:00 New Revision: 9b50546b0b4087ec2ee8479d68e70b89c7956aaa URL: https://github.com/llvm/llvm-project/commit/9b50546b0b4087ec2ee8479d68e70b89c7956aaa DIFF: https://github.com/llvm/llvm-project/commit/9b50546b0b4087ec2ee8479d68e70b89c7956aaa.diff

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759 + QualType FromTy = ArrayFrom->getElementType(); + QualType ToTy = ArrayTo->getElementType(); + + FromRecordDecl = FromTy->getAsRecordDecl(); + ToRecordDecl = To

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

labath added a comment. I'm going to respond to the rest of your (very insightful) comment later. So far, I'm just responding to this: >> This isn't exactly layout related, but there is the question of covariant >> methods. If a method is covariant, then its return type must be complete. > >

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

mib marked 3 inline comments as done. mib added inline comments. Comment at: lldb/include/lldb/Target/Platform.h:643 + *command_output, // Pass nullptr if you don't want the command output + const Timeout &timeout, bool run_in_shell = true); labat

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

mib updated this revision to Diff 288593. mib edited the summary of this revision. mib added a comment. - Changed CommandObject option from `-i|--interpreter` to `-s|--shell`. - Updated test program build settings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

labath added inline comments. Comment at: lldb/include/lldb/Target/Platform.h:643 + *command_output, // Pass nullptr if you don't want the command output + const Timeout &timeout, bool run_in_shell = true); mib wrote: > labath wrote: > > The run_i

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

mib updated this revision to Diff 288597. mib edited the summary of this revision. mib added a comment. Removed `bool run_in_shell` argument from `Platform` classes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86667/new/ https://reviews.llvm.org/

[Lldb-commits] [PATCH] D86792: [lldb] Improve test failure reporting for expect()

DavidSpickett created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. DavidSpickett requested review of this revision. Herald added a subscriber: JDevlieghere. This updates the errors reported by expect() to something like: ''' Ran command: "help" Got output:

[Lldb-commits] [PATCH] D86792: [lldb] Improve test failure reporting for expect()

DavidSpickett added reviewers: teemperor, labath. DavidSpickett added a comment. I realise the test is a bit meta so if there might be a better place to put it. It does at least run successfully from the API test dir. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

wallace added a comment. You changed my mind. I'll move to Support/JSON.h Comment at: lldb/test/API/commands/trace/intelpt-trace/trace.json:11 + }, + "triple": "x86_64-*-linux", + "processes": [ labath wrote: > What if one of the processes is 64-bit and the

[Lldb-commits] [PATCH] D86792: [lldb] Improve test failure reporting for expect()

DavidSpickett added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:2460 +matched = output.startswith(startstr) +log_lines.append("{} start string: \"{}\" ({})".format( +expecting_str, startstr, found_str(ma

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

mib updated this revision to Diff 288636. mib added a comment. Replaced all the `interpreter` occurrences with `shell` in the code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86667/new/ https://reviews.llvm.org/D86667 Files: lldb/bindings/int

[Lldb-commits] [PATCH] D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners

jingham added a comment. In D86652#2244043 , @labath wrote: > This is going to derail this patch somewhat (sorry), but since this is an > important long-standing open issue, I think we should discuss it in more > detail. > > The part that's bothering me

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

gkistanova added a comment. Hi Mateusz, Yes, this looks like a separate patch on top of this revert. By the way, this hasn't been merged yet. Are we waiting for anything? Harmen, do you need a help committing this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

gkistanova added a comment. Fair enough. Reverted. https://github.com/llvm/llvm-project/commit/cdcb9ab10e53ff08293915af3cd897c42112bcc5 Thanks, everyone! Petr, please feel free to send me a patch you want me to check for you. Make sure I could apply it on top of the trunk. Repository: rG LL

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

phosek added a comment. I have suggested an alternative in https://reviews.llvm.org/D85820#2237357 to try and fix this forward and I was waiting for @haampie's response before landing this. I can implement that today if you're OK fixing this forward? I think it'd be easier to do that than rever

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

haampie added a comment. @phosek it would be great to get that patch in, but the truth is it is difficult testing across multiple platforms for me; I can only test on Linux and macOS, not Windows (GNU). Since that patch would touch predefined, platform-dependent cmake variables, I would want to

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

gkistanova added a comment. Hi Petr, I don't have a strong opinion on the exact way of fixing the problem. But we should do this as soon as possible, as keeping the bots red for that long is not Ok. Please send me your patch and I will check it on lld-perf-testsuite bot. If you do not have th

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcdcb9ab10e53: Revert "Use find_library for ncurses" (authored by haampie, committed by gkistanova). Changed prior to commit: https://reviews.llvm.

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

clayborg added a comment. In D86670#2244092 , @labath wrote: > I think it would be appropriate to discuss the design of this feature on > lldb-dev before going over the individual patches. One of the fundamental > aspects of this patchset that I think sh

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

wallace marked an inline comment as not done. wallace added a comment. I agree with that Greg said. > So it would simplify things right now if we say that "trace dump" dumps the > trace data for the currently selected target right now. That will map well > with the stepping commands that will s

[Lldb-commits] [lldb] e5e05ec - [lldb/test] Use @skipIfWindows for PExpectTest

Author: Jordan Rupprecht Date: 2020-08-28T11:41:07-07:00 New Revision: e5e05ecf65aba836802154279efbc8cbce6fe2ea URL: https://github.com/llvm/llvm-project/commit/e5e05ecf65aba836802154279efbc8cbce6fe2ea DIFF: https://github.com/llvm/llvm-project/commit/e5e05ecf65aba836802154279efbc8cbce6fe2ea.di

[Lldb-commits] [PATCH] D86745: [lldb/test] Use @skipIfWindows for PExpectTest

This revision was automatically updated to reflect the committed changes. Closed by commit rGe5e05ecf65ab: [lldb/test] Use @skipIfWindows for PExpectTest (authored by rupprecht). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86745/new/ https://revie

[Lldb-commits] [lldb] cdc1816 - [lldb] Fix typo in disassemble_options_line description

Author: Jonas Devlieghere Date: 2020-08-28T11:41:59-07:00 New Revision: cdc18163cd1449c3a1c20e65a4d95a35ba3f6c23 URL: https://github.com/llvm/llvm-project/commit/cdc18163cd1449c3a1c20e65a4d95a35ba3f6c23 DIFF: https://github.com/llvm/llvm-project/commit/cdc18163cd1449c3a1c20e65a4d95a35ba3f6c23.d

[Lldb-commits] [PATCH] D86752: [lldb/test] Use shorter test case names in TestStandardUnwind

This revision was automatically updated to reflect the committed changes. Closed by commit rG8bd895cac0cd: [lldb/test] Use shorter test case names in TestStandardUnwind (authored by rupprecht). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86752/new/

[Lldb-commits] [lldb] 8bd895c - [lldb/test] Use shorter test case names in TestStandardUnwind

Author: Jordan Rupprecht Date: 2020-08-28T11:49:50-07:00 New Revision: 8bd895cac0cd4eaf76b9bb296a995e5ee485205b URL: https://github.com/llvm/llvm-project/commit/8bd895cac0cd4eaf76b9bb296a995e5ee485205b DIFF: https://github.com/llvm/llvm-project/commit/8bd895cac0cd4eaf76b9bb296a995e5ee485205b.di

[Lldb-commits] [PATCH] D86817: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

JDevlieghere created this revision. JDevlieghere added reviewers: rupprecht, labath, mgorny. Herald added a subscriber: danielkiss. JDevlieghere requested review of this revision. This patch removes the very confusing LLDB_LIB_DIR and LLDB_IMPLIB_DIR environment variables. They are confusing beca

[Lldb-commits] [lldb] 031554e - Reland "[test] Exit with an error if no tests are run."

Author: Jordan Rupprecht Date: 2020-08-28T14:27:37-07:00 New Revision: 031554ed46c8540e8efdf5dde4c10ddd156ac1fa URL: https://github.com/llvm/llvm-project/commit/031554ed46c8540e8efdf5dde4c10ddd156ac1fa DIFF: https://github.com/llvm/llvm-project/commit/031554ed46c8540e8efdf5dde4c10ddd156ac1fa.di

[Lldb-commits] [PATCH] D86817: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

JDevlieghere updated this revision to Diff 288708. JDevlieghere added a comment. - Fix LLDB.framework path - Don't try to guess the LLDB.framework location CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86817/new/ https://reviews.llvm.org/D86817 Files: lldb/packages/Python/lldbsuite/t

[Lldb-commits] [PATCH] D86818: [lldb/Host] Add missing proc states

rupprecht created this revision. Herald added subscribers: lldb-commits, danielkiss. Herald added a project: LLDB. rupprecht requested review of this revision. Herald added a subscriber: JDevlieghere. The /proc//status parsing is missing a few cases: - Idle - Parked - Dead If we encounter an unk

[Lldb-commits] [PATCH] D86821: [lldb] Make the majority of the lit configuration values optional for the API tests

JDevlieghere created this revision. JDevlieghere added reviewers: rupprecht, labath. Herald added subscribers: danielkiss, teemperor. JDevlieghere requested review of this revision. LIT uses a model where the test suite is configurable trough a `lit.site.cfg` file. Most of the time we use the `li

[Lldb-commits] [lldb] 141c847 - [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

Author: Jonas Devlieghere Date: 2020-08-28T15:45:54-07:00 New Revision: 141c8475b693e245388cf7a4ac9ec17303988700 URL: https://github.com/llvm/llvm-project/commit/141c8475b693e245388cf7a4ac9ec17303988700 DIFF: https://github.com/llvm/llvm-project/commit/141c8475b693e245388cf7a4ac9ec17303988700.d

[Lldb-commits] [PATCH] D86817: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

This revision was automatically updated to reflect the committed changes. Closed by commit rG141c8475b693: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[Lldb-commits] [lldb] 55e7d91 - [lldb] Dervice dotest.py path from config.lldb_src_root (NFC)

Author: Jonas Devlieghere Date: 2020-08-28T15:45:54-07:00 New Revision: 55e7d91072e865d36953e91a7b2c8bfc219464d6 URL: https://github.com/llvm/llvm-project/commit/55e7d91072e865d36953e91a7b2c8bfc219464d6 DIFF: https://github.com/llvm/llvm-project/commit/55e7d91072e865d36953e91a7b2c8bfc219464d6.d

[Lldb-commits] [PATCH] D86825: [lldb/Gui] zero-initialize children_stop_id

rupprecht created this revision. Herald added subscribers: lldb-commits, danielkiss. Herald added a project: LLDB. rupprecht requested review of this revision. Herald added a subscriber: JDevlieghere. This is currently causing msan warnings in the API tests when run under msan, e.g. `commands/gui

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

wallace updated this revision to Diff 288739. wallace added a comment. Herald added subscribers: llvm-commits, danielkiss, hiraditya. Herald added a project: LLVM. Addressed all comments. - Wwitched to using llvm::json instead of StructuredData. Fortunately, the logic is pretty much the same. -

[Lldb-commits] [PATCH] D86821: [lldb] Make the majority of the lit configuration values optional for the API tests

This revision was automatically updated to reflect the committed changes. Closed by commit rG3f2fb0132f7b: [lldb] Make the lit configuration values optional for the API tests (authored by JDevlieghere). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D86821?vs=28

[Lldb-commits] [lldb] 3f2fb01 - [lldb] Make the lit configuration values optional for the API tests

Author: Jonas Devlieghere Date: 2020-08-28T18:08:22-07:00 New Revision: 3f2fb0132f7b09e1309e8f7e0b5ba8ea471b17e7 URL: https://github.com/llvm/llvm-project/commit/3f2fb0132f7b09e1309e8f7e0b5ba8ea471b17e7 DIFF: https://github.com/llvm/llvm-project/commit/3f2fb0132f7b09e1309e8f7e0b5ba8ea471b17e7.d

[Lldb-commits] [lldb] 2965e9b - [lldb] Hoist --framework argument out of LLDB_TEST_COMMON_ARGS (NFC)

Author: Jonas Devlieghere Date: 2020-08-28T18:15:33-07:00 New Revision: 2965e9bd5edb079746a668794865be37f6f4d3d8 URL: https://github.com/llvm/llvm-project/commit/2965e9bd5edb079746a668794865be37f6f4d3d8 DIFF: https://github.com/llvm/llvm-project/commit/2965e9bd5edb079746a668794865be37f6f4d3d8.d

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

wallace updated this revision to Diff 288747. wallace added a comment. fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85705/new/ https://reviews.llvm.org/D85705 Files: lldb/include/lldb/Core/PluginManager.h lldb/include/lldb/Target/Tra

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

wallace updated this revision to Diff 288751. wallace added a comment. Herald added subscribers: dang, danielkiss. Addressed comments - Now the "dump" command specifically refers to the currently selected thread. That will be aligned with the future stepping commands we will be adding. - The for

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

clayborg added a comment. A few simple changes from inlined comments. Other than that looks good. Should get another OK from others. Also not sure if the JSON stuff in llvm needs to be done separately? Comment at: lldb/include/lldb/Target/Trace.h:115 + /// implementation,

[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

jasonmolenda added a comment. Herald added a subscriber: danielkiss. Hi, could you try diff --git a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp index 36e7b90cad2..66eb86fe1c0 100644 - a/lldb/sou