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
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:
https://reviews.llvm.org/D511
Author: aleksandr.urakov
Date: Wed Aug 29 00:26:11 2018
New Revision: 340901
URL: http://llvm.org/viewvc/llvm-project?rev=340901&view=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 requ
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 "r
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 UUID
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
___
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 thi
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?
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 Phab
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?
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 Mo
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 set
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 co
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() d
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
===
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 don
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
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 (arch.GetTrip
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 element
Author: teemperor
Date: Wed Aug 29 12:55:33 2018
New Revision: 340958
URL: http://llvm.org/viewvc/llvm-project?rev=340958&view=rev
Log:
Removed commented out includes [NFC]
Modified:
lldb/trunk/include/lldb/DataFormatters/ValueObjectPrinter.h
Modified: lldb/trunk/include/lldb/DataFormatters/
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: so
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 t
Author: gclayton
Date: Wed Aug 29 13:34:08 2018
New Revision: 340966
URL: http://llvm.org/viewvc/llvm-project?rev=340966&view=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 CvRecordPdb
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB340966: Don't include the Age in the UUID for
CvRecordPdb70 UUID records in minidump… (authored by gclayton, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51442?vs=163143&id=1631
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 *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_L
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: source/Breakpoint/Br
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 to
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
http://lists.llvm.org/cgi-bin/mailman/li
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 act
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
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
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
http://l
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 (minim
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
http://lists.llvm.org/cgi-bin/mai
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB340988: Don't 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:
include/lld
Author: teemperor
Date: Wed Aug 29 15:50:54 2018
New Revision: 340988
URL: http://llvm.org/viewvc/llvm-project?rev=340988&view=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
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-co
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 yo
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
Author: adrian
Date: Wed Aug 29 16:16:42 2018
New Revision: 340994
URL: http://llvm.org/viewvc/llvm-project?rev=340994&view=rev
Log:
Refactor BreakpointResolver::SetSCMatchesByLine() to make it easier to
read/understand/maintain.
As a side-effect, this should also improve the performance by avoid
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:
https://reviews.llvm
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@lists.llvm.or
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
This looks great, just a few small nits and a test request.
Comment at: include/lldb/Breakpoint/BreakpointResolverFileLine.h:71
+ uint32_t m_column; ///< This is
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
breakp
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@l
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
include/lldb/Breakpoint/BreakpointResol
Author: teemperor
Date: Wed Aug 29 17:09:21 2018
New Revision: 341003
URL: http://llvm.org/viewvc/llvm-project?rev=341003&view=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
t
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:
https://reviews.llvm.
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
http://lists.llvm.org/cgi-bin/m
Author: friss
Date: Wed Aug 29 17:37:23 2018
New Revision: 341006
URL: http://llvm.org/viewvc/llvm-project?rev=341006&view=rev
Log:
Provide a default implementation of TypeSystem::GetNumTemplateArguments
... and remove the dummy implementations from the languages that do not
support it.
Modified
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 pa
labath added a comment.
Not blocked on anything, just haven't gotten around to committing it yet. Feel
free to land it for me.
https://reviews.llvm.org/D50384
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
52 matches
Mail list logo
