[Lldb-commits] [PATCH] D122283: [lldb] Add a Stream variant that escapes null bytes

2022-03-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision. JDevlieghere added reviewers: jingham, labath. Herald added a project: All. JDevlieghere requested review of this revision. In D122025 Jim suggested that we should sanitize the input to the command return object to retain the

[Lldb-commits] [PATCH] D122025: [lldb] Remove lldbassert from CommandInterpreter::PrintCommandOutput

2022-03-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 417483. JDevlieghere added a comment. Use `StringRef::split` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122025/new/ https://reviews.llvm.org/D122025 Files: lldb/source/Interpreter/CommandInterpreter.cpp Index: lldb/source/Interpreter/Co

[Lldb-commits] [PATCH] D122193: [lldb/test] Add events listener helper function to lldbtest

2022-03-22 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 417474. mib retitled this revision from "Reland "[lldb/test] Add events listener helper class to lldbtest"" to "[lldb/test] Add events listener helper function to lldbtest". mib edited the summary of this revision. mib added a comment. Address @labath comments:

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Reid Kleckner via Phabricator via lldb-commits
rnk added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1124 + // Inlinee is the id index to the function id record that is inlined. + PdbTypeSymId func_id(inline_site.Inlinee, true); + // Look up the function decl by the id index to s

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1155 + TypeDeserializer::deserializeAs(parent_cvt, sir)); + parent = GetOrCreateNamespaceDecl(sir.String.data(), *parent); +} rnk wrote: > Can

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 417432. zequanwu marked 5 inline comments as done. zequanwu added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121967/new/ https://reviews.llvm.org/D121967 Files: lldb/source/P

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

2022-03-22 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/Target/TraceCursor.h:211-214 + /// A unique identifier for the instruction or error this cursor is + /// pointing to

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Reid Kleckner via Phabricator via lldb-commits
rnk added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1123 + + PdbTypeSymId func_id(inline_site.Inlinee, true); + if (clang::Decl *decl = TryGetDecl(func_id)) Please add a comment about what the inlinee is, and what t

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 417419. zequanwu added a comment. Fix test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121967/new/ https://reviews.llvm.org/D121967 Files: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added a comment. > I'd consider writing the live test case in c++ (with judicious use of > always_inline, noinline, etc. attributes) I think it's better to just add live test case on compiled `inline_sites.s` so the test result is not influenced by optimization change. =

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 417414. zequanwu added a comment. Use `cast` instead of `dyn_cast`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121967/new/ https://reviews.llvm.org/D121967 Files: lldb/source/Plugins/SymbolFile/NativePDB

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 417413. zequanwu marked an inline comment as done. zequanwu added a comment. - Split the inline_sites.s test into two. One for live testing on windows, another one for all platforms. - Fix wrong variable address range when there are gaps. Repository: rG

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

2022-03-22 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 accepted this revision. jj10306 added a comment. This revision is now accepted and ready to land. nice work! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122254/new/ https://reviews.llvm.org/D122254 ___

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

2022-03-22 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 417389. wallace marked 2 inline comments as done. wallace added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122254/new/ https://reviews.llvm.org/D122254 Files: lldb/include/lldb

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

2022-03-22 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision. jj10306 added a comment. This revision now requires changes to proceed. Couple minor things Comment at: lldb/include/lldb/Target/TraceInstructionDumper.h:66 private: + /// Indicate the dumper that no more data is available in the tr

[Lldb-commits] [PATCH] D122254: [trace][intelpt] Introduce instruction Ids

2022-03-22 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision. wallace added reviewers: jj10306, clayborg, zrthxn. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In order to support quick arbitrary access to instructions in the trace,

[Lldb-commits] [lldb] f549318 - [lldb] Set the TERM environment variable for the API tests

2022-03-22 Thread Jonas Devlieghere via lldb-commits
Author: Jonas Devlieghere Date: 2022-03-22T11:01:38-07:00 New Revision: f54931865de823fd4c3ab3650d8b8a65900433ff URL: https://github.com/llvm/llvm-project/commit/f54931865de823fd4c3ab3650d8b8a65900433ff DIFF: https://github.com/llvm/llvm-project/commit/f54931865de823fd4c3ab3650d8b8a65900433ff.d

[Lldb-commits] [PATCH] D122193: Reland "[lldb/test] Add events listener helper class to lldbtest"

2022-03-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D122193#3399921 , @mib wrote: > Hey @labath, thanks for the feedback! Here are some other questions ? > > In D122193#3399109 , @labath wrote: > >> I like this implementation a lot. The l

[Lldb-commits] [PATCH] D122193: Reland "[lldb/test] Add events listener helper class to lldbtest"

2022-03-22 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment. Hey @labath, thanks for the feedback! Here are some other questions ? In D122193#3399109 , @labath wrote: > I like this implementation a lot. The less threads we use, the better. I have > a couple of remarks/questions though: > > -

[Lldb-commits] [PATCH] D121425: [VFS] Add a new getVFSFromYAMLs API

2022-03-22 Thread Ben Barham via Phabricator via lldb-commits
bnbarham planned changes to this revision. bnbarham added a comment. Blocked on the dependent Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121425/new/ https://reviews.llvm.org/D121425 ___ lldb-commits m

[Lldb-commits] [lldb] 56fb745 - [lldb/test] Increase pexpect termination timeouts

2022-03-22 Thread Pavel Labath via lldb-commits
Author: Pavel Labath Date: 2022-03-22T15:14:21+01:00 New Revision: 56fb7456950d2564d16500e40c5719c954a6987a URL: https://github.com/llvm/llvm-project/commit/56fb7456950d2564d16500e40c5719c954a6987a DIFF: https://github.com/llvm/llvm-project/commit/56fb7456950d2564d16500e40c5719c954a6987a.diff

[Lldb-commits] [PATCH] D122193: Reland "[lldb/test] Add events listener helper class to lldbtest"

2022-03-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I like this implementation a lot. The less threads we use, the better. I have a couple of remarks/questions though: - given how simple the new approach is, is a dedicated test class really needed? It seems like it should be sufficient to add two utility functions to: cr

[Lldb-commits] [PATCH] D122025: [lldb] Remove lldbassert from CommandInterpreter::PrintCommandOutput

2022-03-22 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Agreed we generally don't want to have embedded nuls in the command output, but I don't think we have to go to such great lengths to enforce it. > The latter doesn't trigger this bug because i

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I don't know much about PDBs, but overall, the patch seems fine to me. In D121967#3393556 , @zequanwu wrote: > I think adding a live debugging test case is necessary. I found several bugs > when working on this via live debugging