[Lldb-commits] [PATCH] D51104: [PDB] Resolve a symbol context block info correctly

2018-08-29 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment. Thanks! Repository: rLLDB LLDB https://reviews.llvm.org/D51104 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51104: [PDB] Resolve a symbol context block info correctly

2018-08-29 Thread Aleksandr Urakov via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL340901: [PDB] Resolve a symbol context block info correctly (authored by aleksandr.urakov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [lldb] r340901 - [PDB] Resolve a symbol context block info correctly

2018-08-29 Thread Aleksandr Urakov via lldb-commits
Author: aleksandr.urakov Date: Wed Aug 29 00:26:11 2018 New Revision: 340901 URL: http://llvm.org/viewvc/llvm-project?rev=340901=rev Log: [PDB] Resolve a symbol context block info correctly Summary: This patch allows to resolve a symbol context block info even if a function info was not

Re: [Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Zachary Turner via lldb-commits
For PE/COFF files, the Age is also in the executable and Guid+Age actually constitute a 20-byte UUID. Is this not the case on Apple? What object file format are you dealing with? On Wed, Aug 29, 2018 at 10:11 AM Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg created

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a subscriber: clayborg. zturner added a comment. For PE/COFF files, the Age is also in the executable and Guid+Age actually constitute a 20-byte UUID. Is this not the case on Apple? What object file format are you dealing with? https://reviews.llvm.org/D51442

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo added a subscriber: zturner. lemo added a comment. I'm curious too: where did the PDB70 age create matching problems? On a related note, I just noticed that ObjectFilePECOFF::GetUUID() doesn't have a real implementation (just returns false). How do we extract module UUID for PE/COFF files?

Re: [Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Leonard Mosescu via lldb-commits
I'm curious too: where did the PDB70 age create matching problems? On a related note, I just noticed that ObjectFilePECOFF::GetUUID() doesn't have a real implementation (just returns false). How do we extract module UUID for PE/COFF files? On Wed, Aug 29, 2018 at 10:28 AM, Zachary Turner via

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

2018-08-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik created this revision. shafik added reviewers: jingham, aprantl, davide. lldb::StateType is an enum without an explicit underlying type. According to [dcl.enum]p8 http://eel.is/c++draft/dcl.enum#8 and [expr.static.cast]p10 http://eel.is/c++draft/expr.static.cast#10 we are limited to

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In https://reviews.llvm.org/D51442#1217894, @lemo wrote: > I'm curious too: where did the PDB70 age create matching problems? For breakpad ARM and ARM64 minidumps that are for Apple vendor triples. > On a related note, I just noticed that ObjectFilePECOFF::GetUUID()

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg updated this revision to Diff 163143. clayborg added a comment. Make 16 byte UUIDs Apple specific. https://reviews.llvm.org/D51442 Files: source/Plugins/Process/minidump/MinidumpParser.cpp Index: source/Plugins/Process/minidump/MinidumpParser.cpp

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In https://reviews.llvm.org/D51442#1217829, @zturner wrote: > For PE/COFF files, the Age is also in the executable and Guid+Age actually > constitute a 20-byte UUID. Is this not the case on Apple? What object file > format are you dealing with? Breakpad files that

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-08-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Looks fine. A bit of cleanup might be nice on the TypeSystem.h to provide default implementations that just return 0 for some of these that every type system currently is required to override for no reason (all template related type system calls seem to have default

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg created this revision. clayborg added reviewers: zturner, labath, dvlahovski, lemo. The CvRecordPdb70 structure looks like: struct CvRecordPdb70 { uint8_t Uuid[16]; llvm::support::ulittle32_t Age; // char PDBFileName[]; }; We are currently including the "Age" in the

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-08-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:284 + ((bool)pack_name == (bool)packed_args) && + (!packed_args || !packed_args->args.empty()); } Is this saying that an empty parameter pack is invalid?

Re: [Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Zachary Turner via lldb-commits
We probably don't. Windows debug info in LLDB is currently based off of DIA SDK, which handles everything for us (but obviously only works on Windows). That's one of the things that would need to be fixed to debug Windows minidumps on Linux (I assume). On Wed, Aug 29, 2018 at 10:47 AM Leonard

[Lldb-commits] [lldb] r340966 - Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files for Apple vendors.

2018-08-29 Thread Greg Clayton via lldb-commits
Author: gclayton Date: Wed Aug 29 13:34:08 2018 New Revision: 340966 URL: http://llvm.org/viewvc/llvm-project?rev=340966=rev Log: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files for Apple vendors. The CvRecordPdb70 structure looks like: struct CvRecordPdb70 {

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB340966: Dont include the Age in the UUID for CvRecordPdb70 UUID records in minidump… (authored by gclayton, committed by ). Changed prior to commit:

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Other than that, this looks fine to me. https://reviews.llvm.org/D51445 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51442: Don't include the Age in the UUID for CvRecordPdb70 UUID records in minidump files

2018-08-29 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo accepted this revision. lemo added a comment. This revision is now accepted and ready to land. Looks good (with one inline request for a comment) Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:85 + auto arch = GetArchitecture(); + if

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision. aprantl added a reviewer: jingham. Herald added a subscriber: mgrang. Refactor BreakpointResolver::SetSCMatchesByLine() to make it easier to read/understand/maintain. As a side-effect, this should also improve the performance by avoiding lots ofcostly vector

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: scripts/Python/python-typemaps.swig:89 + PyErr_SetString(PyExc_ValueError, "Not a valid StateType value"); + return nullptr ; +} nice! There's an extra space before the `;` Comment at:

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: include/lldb/lldb-enumerations.h:60 ///< or threads get the chance to run. + kNumStateType }; I think we usually do something like eLastsState = eStateSuspended. This avoid having to add the case

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment. Very nice! Do we have a way of verifying the performance gain? Comment at: source/Breakpoint/BreakpointResolver.cpp:183 llvm::StringRef log_ident) { - Log

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I wonder if it is worth asserting if a process ever returns a state of kNumStateType? I don't think it is necessary in things like StateIsStoppedState, somebody might have found their way here from the SB API's and we want to be forgiving there. But wherever we've

[Lldb-commits] [lldb] r340958 - Removed commented out includes [NFC]

2018-08-29 Thread Raphael Isemann via lldb-commits
Author: teemperor Date: Wed Aug 29 12:55:33 2018 New Revision: 340958 URL: http://llvm.org/viewvc/llvm-project?rev=340958=rev Log: Removed commented out includes [NFC] Modified: lldb/trunk/include/lldb/DataFormatters/ValueObjectPrinter.h Modified:

[Lldb-commits] [PATCH] D48465: Added initial code completion support for the `expr` command

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Yes, this is fine. Thanks for working on this! https://reviews.llvm.org/D48465 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl marked an inline comment as done. aprantl added a comment. FYI: I tried to benchmark this using `break set -A -p begin` and similar things, but in all my experiments the variation between test runs was much larger than any difference with or without my patch. The filtering of the

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I'm not much surprised by that, but thanks for the experiments. As you say, that wasn't the primary purpose of this patch. Repository: rL LLVM https://reviews.llvm.org/D51453 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-08-29 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:284 + ((bool)pack_name == (bool)packed_args) && + (!packed_args || !packed_args->args.empty()); } shafik wrote: > Is this saying that an empty parameter

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Except for the nit about the comment, this looks fine. https://reviews.llvm.org/D51453 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This is fine by me. https://reviews.llvm.org/D50912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] r340994 - Refactor BreakpointResolver::SetSCMatchesByLine() to make it easier to

2018-08-29 Thread Adrian Prantl via lldb-commits
Author: adrian Date: Wed Aug 29 16:16:42 2018 New Revision: 340994 URL: http://llvm.org/viewvc/llvm-project?rev=340994=rev Log: Refactor BreakpointResolver::SetSCMatchesByLine() to make it easier to read/understand/maintain. As a side-effect, this should also improve the performance by avoiding

[Lldb-commits] [lldb] r341006 - Provide a default implementation of TypeSystem::GetNumTemplateArguments

2018-08-29 Thread Frederic Riss via lldb-commits
Author: friss Date: Wed Aug 29 17:37:23 2018 New Revision: 341006 URL: http://llvm.org/viewvc/llvm-project?rev=341006=rev Log: Provide a default implementation of TypeSystem::GetNumTemplateArguments ... and remove the dummy implementations from the languages that do not support it. Modified:

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl updated this revision to Diff 163198. aprantl marked 2 inline comments as done. aprantl added a comment. Address review feedback. https://reviews.llvm.org/D51453 Files: include/lldb/Breakpoint/BreakpointResolver.h source/Breakpoint/BreakpointResolver.cpp Index:

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl marked an inline comment as done. aprantl added a comment. I don't have any numbers to back up the performance claim. My primary motivation for this patch was to prepare the code for adding extra functionality; the performance gain was only side-effect. One way to measure it would be

[Lldb-commits] [PATCH] D50384: Move Predicate.h from Host to Utility

2018-08-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment. Is this blocked by something? If you're busy @labath I can commit this and watch the bots. https://reviews.llvm.org/D50384 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. By name breakpoints don't use this function so a function regex wouldn't show the change. But the source regex does use this filter, so something like: (lldb) break set -p ; on a large source file might do it. That's a pretty silly thing to do, however. The only

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment. lldb-bench now has a benchmark called `br-by-regex` that should profile this code. It should run tonight, so if merge this tomorrow or later we should be able to see the performance impact of this patch. https://reviews.llvm.org/D51453

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Oh, yeah, gotta remember to check the box... https://reviews.llvm.org/D51453 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

2018-08-29 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB340988: Dont cancel the current IOHandler when we push a handler for an utility… (authored by teemperor, committed by ). Repository: rLLDB LLDB https://reviews.llvm.org/D50912 Files:

[Lldb-commits] [lldb] r340988 - Don't cancel the current IOHandler when we push a handler for an utility function run.

2018-08-29 Thread Raphael Isemann via lldb-commits
Author: teemperor Date: Wed Aug 29 15:50:54 2018 New Revision: 340988 URL: http://llvm.org/viewvc/llvm-project?rev=340988=rev Log: Don't cancel the current IOHandler when we push a handler for an utility function run. Summary: D48465 is currently blocked by the fact that tab-completing the

[Lldb-commits] [PATCH] D51466: Move the column marking functionality to the Highlighter framework

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thank you, this looks great! Repository: rLLDB LLDB https://reviews.llvm.org/D51466 ___ lldb-commits mailing list

[Lldb-commits] [lldb] r341003 - Move the column marking functionality to the Highlighter framework

2018-08-29 Thread Raphael Isemann via lldb-commits
Author: teemperor Date: Wed Aug 29 17:09:21 2018 New Revision: 341003 URL: http://llvm.org/viewvc/llvm-project?rev=341003=rev Log: Move the column marking functionality to the Highlighter framework Summary: The syntax highlighting feature so far is mutually exclusive with the lldb feature that

[Lldb-commits] [PATCH] D51466: Move the column marking functionality to the Highlighter framework

2018-08-29 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL341003: Move the column marking functionality to the Highlighter framework (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D51461: Support setting a breakpoint by FileSpec+Line+Column in the SBAPI.

2018-08-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That's great, thanks! https://reviews.llvm.org/D51461 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D51461: Support setting a breakpoint bile FileSpec+Line+Column in the SBAPI.

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision. aprantl added a reviewer: jingham. Herald added subscribers: abidh, mgrang. Support setting a breakpoint bile FileSpec+Line+Column in the SBAPI. This patch extends the SBAPI to allow for setting a breakpoint not only at a specific line, but also at a specific

[Lldb-commits] [PATCH] D48465: Added initial code completion support for the `expr` command

2018-08-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment. Friendly ping on this (as the blockers for this patch are now all committed). https://reviews.llvm.org/D48465 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D51466: Move the column marking functionality to the Highlighter framework

2018-08-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor created this revision. teemperor added a reviewer: aprantl. Herald added a subscriber: lldb-commits. The syntax highlighting feature so far is mutually exclusive with the lldb feature that marks the current column in the line by underlining it via an ANSI color code. Meaning that if

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

2018-08-29 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL340994: Refactor BreakpointResolver::SetSCMatchesByLine() to make it easier to (authored by adrian, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D51461: Support setting a breakpoint by FileSpec+Line+Column in the SBAPI.

2018-08-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl updated this revision to Diff 163231. aprantl marked 4 inline comments as done. aprantl added a comment. Address review feedback. https://reviews.llvm.org/D51461 Files: include/lldb/API/SBTarget.h include/lldb/Breakpoint/BreakpointResolver.h