>
> After some internal discussion, it seems that the situation with the
> all-zero UUIDs is as follows:
> - breakpad symbol files do not attach a special meaning to a zero UUID -
> if a file does not have a build-id, the dump_syms tool will use a hash of
> the first page of the text section (or so
Author: lemo
Date: Mon Jan 7 09:55:42 2019
New Revision: 350546
URL: http://llvm.org/viewvc/llvm-project?rev=350546&view=rev
Log:
Use the minidump exception record if present
If the minidump contains a saved exception record use it automatically.
Differential Revision: https://reviews.llvm.org/
Great! I can see how we can put this to good use.
In the meantime, I'd like to submit this change as is - the included input
files are intended to be reused for future test cases as well (they are
extracted from my larger change to add support for the native PDB reader +
minidumps).
On Fri, Jan 4
Sounds very useful. Are you planning to add it to the LLDB repository?
On Fri, Jan 4, 2019 at 10:56 AM Greg Clayton wrote:
>
>
> On Jan 4, 2019, at 9:45 AM, Leonard Mosescu wrote:
>
> I have a minidump generator if you need me to make any specific minidump
>> files for you.
>>
>
> Maybe not in
>
> ouldn’t we have lldb generate the mini dump itself as the first step of
> the test?
>
How would this work cross-platform?
On Fri, Jan 4, 2019 at 8:48 AM Zachary Turner wrote:
> For those kinds of cases, we could use obj2yaml and check in yaml right?
> Fwiw I tried to round-trip an exe throu
>
> I have a minidump generator if you need me to make any specific minidump
> files for you.
>
Maybe not in this case, but it seems an interesting idea. What are the
capabilities of this generator tool?
On Thu, Jan 3, 2019 at 3:49 PM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote
Thanks Zach. Don't get me wrong, I have no problem tweaking it as long as
necessary assuming we all agree on the plan: we could implement a
ObjectFilePDB and a PDB SymbolVendor first, then the contentious PDB lookup
detail goes away.
My intention was to enable other developers to start consuming P
>
> I think we can fix that by changing the line to:
>
> if (!object_file || object_file->GetFileSpec() == symbol_fspec) {
> }
>
The problem is that SymbolFileNativePDB returns the module object file as
it's own object file. Are you suggesting we should track the module object
file separate from S
What's the consensus?
Personally I think that, even considering the potential issue that Paval
pointed out, the "target symbols add ..." is the most conservative approach
in terms of introducing new behavior. I'm fine with the current directory
lookup as well (the original change) since it's consi
I ended up implementing the support for "target symbols add" since it's
something we needed anyway. This allowed the removal of the contentious
implicit search in the current directory.
I tried to verify this behavior, but it seems like it should already work
> out of the box? So we're on the sa
>
> I guess I don't see why we need a temporary solution at all. If we can
> have logic that can be rolled into the SymbolVendor when we get it, and
> makes sense there, and is also simple, why not go with it? Failing that,
> doesn't the `target symbols add` solution also work fine?
>
I just che
>
> But here, we're talking about a situation where there is no EXE, only a
> minidump. If there is a minidump and no EXE then neither WinDbg nor VS
> will search the minidump folder for the PDB.
Indeed, this is key part.
> In my mind, the algorithm could be something like: ...
>
I'm not a b
>
> The Windowsy thing to do is what Zach said: Check the directory that
> contains the .dmp for the .pdb. It's the first place the VS debugger looks
> when opening a minidump. It's less sensitive to the user's environment.
> (Making the user change the current working directory for this could b
I think as combination of explicit symbol search path + something similar
to Microsoft's symsrv would be a good "real" solution (and yes, that would
be packaged as a SymbolVendor, outside SymbolFilePDB)
For short term, I don't see a clearly superior alternative to searching the
current directory.
>
> But if the minidump and PDBs are in the same directory, then wouldn't my
> proposed solution also work (while also being a permanent solution)?
>
If we're looking in the same directory as the binary file (which is how I
read your suggestion) then it would not be found in this case, since the
b
>
> We talked about this offline, but bringing the discussion back here. Can
> you describe the use case that this is addressing? As you mention, this is
> a temporary hack until we have proper symbol searching logic, but proper
> symbol searching logic will do more than just look up symbols in a
Thanks Pavel and Greg.
It sounds to me like it would be better to have a separate command
> (let's call it "target modules replace" for now) for adding an "object
> file" to a "placeholder" module, instead of repurposing "target symbols
> add" to do that.
Yes, that would be my preference as well
I can see how this works for the PDB, no-module-binary case. What about the
PDB & module-binary case?
Maybe I missed the rationale, but I think that modeling the general case:
module and symbols are separate files, is a better foundation for the
particular case where the symbols are embedded in th
> How large is the PDB file here?
100Kb
On Mon, Dec 10, 2018 at 11:48 AM Zachary Turner via Phabricator <
revi...@reviews.llvm.org> wrote:
> zturner added a comment.
>
> How large is the PDB file here?
>
>
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D55142/new/
>
> https://reviews.ll
> BTW: check my changes in: https://reviews.llvm.org/D55522
> It will be interesting to you since it parses the linux maps info if it
is available in breakpad generated minidump files. This will give us enough
info to create correct sections for object files when we have no ELF file
or no symbol fi
Hi Aleksandr, yes, no objections to this patch.
I was responding to Pavel's comments, which I also assume are
forward-looking as well, not strictly related to this patch.
On Thu, Nov 29, 2018 at 12:08 PM Aleksandr Urakov <
aleksandr.ura...@jetbrains.com> wrote:
> Yes, I agree that the current mo
Great observations Pavel! I think it's really important to have
orthogonal/composable abstractions here: the symbols should be decoupled
from the container format IMO.
You know more about the ObjectFile than me so I can't say if
ObjectFileBreakpad is the best interface, but here are my initial
obs
Thanks Greg, this is what I had in mind.
I have some ideas which involve this kind of debugger console. We'll likely
start with a basic version built on top of evaluate `cmd and once we get
around to building a full console I'll ping you.
On Sat, Sep 22, 2018 at 8:50 AM, Greg Clayton wrote:
>
>
Great. Do you think that having an abstracted stream I/O tunneled through
DAP would lose anything compared to the direct file handle? I can see a few
problems using a native platform file handle:
1. It's specific to the platform (with subtle differences between platforms)
2. It assumes a local set
>
> The solution I would love to see is to have the initialize packet return
> something via the DAP that says "I have a command line interpreter, please
> send a packet with a file handle (slave path to slave side of pseudo
> terminal maybe)". VS Code and Nuclide both emulated tty already, so we
>
Hi Greg, looking at request_evaluate() I noticed that it will evaluate the
string as a lldb command if prefixed by ` .
This is a great feature (it allows building REPL consoles on top of DAP),
but I'm curious how you picked up this convention? For example I believe
that the gdb DAP uses -exec 'com
Just curious, what prompted this change? (compiler diagnostic? forcing
value initialization in the member initializer list is harmless in this
case)
Also, if we want to do this kind of cleanup, m_compiler initialization is
also redundant.
On Thu, Aug 30, 2018 at 8:39 AM, Adrian Prantl via lldb-co
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
Author: lemo
Date: Thu Aug 23 14:34:33 2018
New Revision: 340578
URL: http://llvm.org/viewvc/llvm-project?rev=340578&view=rev
Log:
Restrict the set of plugins used for ProcessMinidump
1. The dynamic loaders should not be needed for loading minidumps
and they may create problems (ex. the macOS loa
It's an interesting idea, thanks! I don't object moving code around if
there's a strong case for it, but I'd like to keep the fix small and simple
for now, but it's worth considering if the current minidump loading path
will need more flexibility.
On Thu, Aug 23, 2018 at 1:27 PM, Greg Clayton via
>
> The LLDB MI plugin didn't work very well as was quite flaky when I tested
> it a while back.
>
Just curious, what was the flaky part, the debug adapter or the LLDB MI
interface?
On Wed, Aug 8, 2018 at 8:40 AM, Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:
> clayborg added a
Author: lemo
Date: Tue Aug 7 11:00:30 2018
New Revision: 339161
URL: http://llvm.org/viewvc/llvm-project?rev=339161&view=rev
Log:
Misc module/dwarf logging improvements
This change improves the logging for the lldb.module category to note a few
interesting cases:
1. Local object file found, bu
Really cool! Are you planning to add some documentation for it? (set up
instructions, etc...)
On Tue, Aug 7, 2018 at 9:24 AM, Greg Clayton via Phabricator via
lldb-commits wrote:
> clayborg updated this revision to Diff 159526.
> clayborg added a comment.
>
> Removed dead code from python files.
Author: lemo
Date: Fri Aug 3 19:15:26 2018
New Revision: 338949
URL: http://llvm.org/viewvc/llvm-project?rev=338949&view=rev
Log:
Fix a bug in VMRange
I noticed a suspicious failure:
[ RUN ] VMRange.CollectionContains
llvm/src/tools/lldb/unittests/Utility/VMRangeTest.cpp:146: Failure
Value of:
Thanks Zach. I can't find llvm::contains() though, any pointers to it?
On Fri, Aug 3, 2018 at 5:58 PM, Zachary Turner via Phabricator via
lldb-commits wrote:
> zturner added a subscriber: lemo.
> zturner added a comment.
>
> I think we have llvm::contains() which would allow you to make this
> s
The new test cases did catch a real bug:
[ RUN ] VMRange.CollectionContains
llvm/src/tools/lldb/unittests/Utility/VMRangeTest.cpp:146: Failure
Value of: VMRange::ContainsRange(collection, VMRange(0x100, 0x104))
Actual: false
Expected: true
class RangeInRangeUnaryPredicate {
public:
RangeI
FYI, Breakpad & Crashpad will start generating the Microsoft flavor of ARM
minidumps soon.
On Wed, Jul 25, 2018 at 9:44 AM, Leonard Mosescu via Phabricator <
revi...@reviews.llvm.org> wrote:
> lemo added inline comments.
>
>
>
> Comment at: source/Plugins/Process/minidump/Registe
>
> I never *ran LLDB tests*, not sure where they are and what they are.
I hope you're planning to look into this before submitting the change :)
On Fri, Jul 27, 2018 at 11:28 AM, Eugene Birukov via Phabricator <
revi...@reviews.llvm.org> wrote:
> EugeneBi added a comment.
>
> Hmm... I never t
>
> The problem is that shared libraries differ on these machines and
> LLDB either fails to load some libraries *or loads wrong ones*.
>
Not finding the modules is not surprising but the latter (loading the wrong
modules) is a bit concerning. Do you know why the module build-id is not
used when s
Great timing! ARM support would be most welcome. Are you planning to
support the Breakpad flavor or ARM minidumps or the Microsoft one? (Mark
Mentovai just reminded me that the ARM support was added independently and
some of the structures are different)
Regarding the invalid minidumps, I used my
That sounds reasonable to me. I'll make a note to revisit this (I don't the
the cycles to do it right away but I'm planning a few more changes in the
area soon).
On Mon, Jul 16, 2018 at 10:36 AM, Pavel Labath wrote:
> Ok, I see what you mean now.
>
> Looking at the other core file plugins (elf,
The problem is not returning an error from Minidump::Create() - if that was
the case this could easily be improved indeed. The two-phase initialization
is a consequence of the LLDB plugin lookup:
1. Target::CreateProcess() calls Process::FindPlugin()
2. ProcessMinidump::CreateInstance() then has t
Author: lemo
Date: Thu Jul 12 10:27:18 2018
New Revision: 336918
URL: http://llvm.org/viewvc/llvm-project?rev=336918&view=rev
Log:
Restructure the minidump loading path and add early & explicit consistency
checks
Corrupted minidumps was leading to unpredictable behavior.
This change adds explic
>
> I'm not sure if there is a suitable place for that function. This is
> needed in "ObjectFileMachO" and two dynamic loader plugins.
Then your factory idea may be the next best thing. While we're at it, maybe
we can remove the UUID::SetBytes() from the public interface, and make the
UUID an im
>
> However, during parsing you need to know the meaning of a "...0" UUID.
> In a MachO file (at least based on the comments in the code) this value is
> used to denote the fact that the object file has no UUID. For elf, a
> "000..0" build-id is a perfectly valid identifier (and the lack of a
>
The intention in the scope of this change is just to check that the new
overload is exposed correctly through the Python API.
In general, guaranteeing specific error codes (messages?) is likely
impractical:
1. It's not always easy to do the proper checks (ex. for 'file-not-found'
you'd actually g
ix, I also wanted the actual error for external
diagnostic purposes (for example it's valuable to know what was the actual
problem: corrupted, truncated or unsupported minidump, etc.)
On Mon, Jun 11, 2018 at 5:47 PM, Jim Ingham wrote:
>
>
> > On Jun 11, 2018, at 2:19 PM, Leon
ix, I also wanted the actual error for external
diagnostic purposes (for example it's valuable to know what was the actual
problem: corrupted, truncated or unsupported minidump, etc.)
On Mon, Jun 11, 2018 at 5:47 PM, Jim Ingham wrote:
>
>
> > On Jun 11, 2018, at 2:19 PM, Leon
>
> Ah I see. That's because the last argument is a C++ default argument. It
> looks like the convention in this file is that the error argument should be
> the last non-defaulted argument.
I think it should be:
void StepOver(lldb::RunMode stop_other_threads, lldb::SBError &error); //
no default
Author: lemo
Date: Mon Jun 11 14:19:26 2018
New Revision: 334439
URL: http://llvm.org/viewvc/llvm-project?rev=334439&view=rev
Log:
Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails
There was no way to find out what's wrong if SBProcess SBTarget::LoadCore(const
char
>
> remove space before (
>
I'd be happy to make the change, but looking at the rest of the file it
seems that almost everything uses the opposite convention: Foo (...).
So, to make sure I'm making the right change, is this how it should look?
lldb::SBProcess
LoadCore(const char *core_fi
>
> For this particular case, I don't think having a split function is
> particularly interesting.
I agree that conceptually all disjoint ranges are the same. Until they are
not: I haven't looked closely at the code changes but it's easy to
introduce assumptions about the layout hierarchy
(range-
So what's your take on this particular case? As far as I can see (please
correct me if I'm wrong), the LIT-only tests:
1. Don't cover the case where a function is split into disjoint regions,
right?
2. Also don't cover the cross-targeting case (ex. the native PDB reader
hosted on Linux)
3. A bug i
I agree, checked in binaries are not always pretty. But some coverage
depends checked in binaries (or at very least is dramatically harder to get
the same thing from source)
Are we saying that sacrificing coverage to keep tests smaller should be the
default trade off?
On Fri, Jun 8, 2018 at 10:0
Doesn't the LIT based test drop the split-function case (originally
produced with PGO)?
Sorry for being late to the party, but it seems beneficial to have both LIT
*and* checked in binaries since in general they are complementary: checking
against freshly built binaries only covers a matching set
If anyone's working on this I'd suggest adding a test case for the "split
code" case as well (where even a single function is split into multiple
ranges). MSVC with PGO should help produce hot/cold cold split repros.
On Thu, May 31, 2018 at 10:24 AM, Greg Clayton via lldb-commits <
lldb-commits@li
Author: lemo
Date: Wed May 2 13:06:17 2018
New Revision: 331394
URL: http://llvm.org/viewvc/llvm-project?rev=331394&view=rev
Log:
Use the UUID from the minidump's CodeView Record for placeholder modules
This change adds support for two types of Minidump CodeView records:
PDB70 (reference:
http
Hi Erik, the review is still marked as requiring changes. Once that is
sorted out I'd be happy to submit this on your behalf (what is the base SVN
revision for the latest patch?)
Davide Italiano, is all the CR feedback addressed in the latest revision?
On Tue, Apr 24, 2018 at 1:38 PM, Erik Weland
The mix of backward and forward slashes doesn't impact my current project
but it would be nice to have a consistent path syntax (both within a single
path and also cross platforms).
> Leonard, is it reasonable to assume that all paths in the minidumps will be
> absolute (and thus resolving is poi
>
> It looks like nobody except me is worried about the
> module-without-an-object-file situation, so I guess we can try this out and
> see how it goes.
>
Sorry Pavel, I meant to respond to this: most of the code seems to
explicitly handle this case (module-without-object-file), I just had to fix
Author: lemo
Date: Wed Apr 18 16:10:46 2018
New Revision: 330302
URL: http://llvm.org/viewvc/llvm-project?rev=330302&view=rev
Log:
Improve LLDB's handling of non-local minidumps
Normally, LLDB is creating a high-fidelity representation of a live
process, including a list of modules and sections,
Greg/Pavel, does the latest revision look good to you? Thanks!
On Wed, Apr 18, 2018 at 10:31 AM, Leonard Mosescu via Phabricator <
revi...@reviews.llvm.org> wrote:
> lemo marked an inline comment as done.
> lemo added a comment.
>
> In https://reviews.llvm.org/D45700#1070491, @amccarth wrote:
>
>
Author: lemo
Date: Thu Oct 5 16:41:28 2017
New Revision: 315037
URL: http://llvm.org/viewvc/llvm-project?rev=315037&view=rev
Log:
Implement interactive command interruption
The core of this change is the new CommandInterpreter::m_command_state,
which models the state transitions for interactive
Author: lemo
Date: Wed Oct 4 13:23:56 2017
New Revision: 314929
URL: http://llvm.org/viewvc/llvm-project?rev=314929&view=rev
Log:
LLDB cmake fix: define LLDB_CONFIGURATION_xxx based on the build type
Neither LLDB_CONFIGURATION_DEBUG nor LLDB_CONFIGURATION_RELEASE were ever set
in the CMake LLDB
Identical, no - note the checks are inverted. Definitely uglier, but it's
defaulting to LLDB_CONFIGURATION_DEBUG if CMAKE_BUILD_TYPE is not defined,
right?
With the current defaulting rule I pointed out both versions should be
equivalent. If anything I prefer the 1st version since I like to avoid
Author: lemo
Date: Tue Oct 3 15:30:02 2017
New Revision: 314856
URL: http://llvm.org/viewvc/llvm-project?rev=314856&view=rev
Log:
updating svn:ignore
Modified:
lldb/trunk/ (props changed)
Propchange: lldb/trunk/
-
Yes, using line endings as split points is somewhat arbitrary, anything
that's a reasonable compromise between interruption responsiveness and low
interrupt polling overhead would do. I feel that the lines are somewhat
nicer since arbitrary splitting may lead to confusion and/or formatting
ugliness
>
> I don't quite understand the comment about signals adding indeterminacy.
> No signal delivery is required to test this part. The lldb driver has a
> sigint handler that calls SBDebugger::DispatchInputInterrupt. But since
> you aren't testing whether SIGINT actually calls DispatchInputInterrup
Any pointers on the steps required to make changes to the SB APIs? Is it
documented anywhere? Thanks!
On Tue, Sep 19, 2017 at 11:58 AM, Jim Ingham wrote:
>
> > On Sep 19, 2017, at 11:25 AM, Leonard Mosescu
> wrote:
> >
> > These are all great suggestions, thanks everyone!
> >
> > > We should ha
Thanks for the context.
On Tue, Sep 19, 2017 at 11:58 AM, Jim Ingham wrote:
>
> > On Sep 19, 2017, at 11:25 AM, Leonard Mosescu
> wrote:
> >
> > These are all great suggestions, thanks everyone!
> >
> > > We should have a test. The test would need to use pexpect or something
> similar. If anyon
So, how about I look into exposing WasInterrupted() through SB APIs in a
follow up change?
On Tue, Sep 19, 2017 at 11:36 AM, Jim Ingham wrote:
> Xcode does, I don't know about other UI's.
>
> Jim
>
> > On Sep 19, 2017, at 11:35 AM, Leonard Mosescu
> wrote:
> >
> > I agree Jim. I'd like like to
I agree Jim. I'd like like to build thing incrementally - checking in the
current change as is does not preclude adding the SB APIs while it does
provide the foundation.
I think that going after the scenarios you mention is a significant
increase in scope. Are people even hooking up DispatchInterr
This looks beautiful indeed. The problem is that it doesn't quite work with
the current MemoryBuffer and the line_iterator : for one thing there's no
way to construct a MemoryBuffer from a StringRef, or to use the
line_iterator directly with a StringRef.
On Tue, Sep 19, 2017 at 10:39 AM, Zachary
These are all great suggestions, thanks everyone!
> We should have a test. The test would need to use pexpect or something
similar. If anyone has any pointer on how to make a test for this, please
chime in. I was thinking just a pexpect based test.
I love tests but what exactly do we want to test
Hi Greg, are you referring to the manual line splitting vs. using
StringRef::split()? This is how it's documented:
/// Split into two substrings around the first occurrence of a separator
/// character.
///
/// If \p Separator is in the string, then the result is a pair (LHS, RHS)
/// such that (*
Looking at line_iterator, it seems to be designed to work with
MemoryBuffer, which in turn seems specialized for dealing with file content
(so while it may be possible to force init a MemoryBuffer from a StringRef
it seems a bit awkward to me). Also, line_iterator has extra stuff which we
don't nee
It's a good question - here's a two part answer:
1. The actual printing of the output is the longest blocking in many cases
(as mentioned in the description: the actual data gathering for "target
module dump symtab" can take 1-2sec, but printing it can take 20min. For
quick experiment, try dis -p
Yes, this is the culprit indeed, thanks Ed for reporting it!
The fix is trivial and I can take care of it on Monday, but if anyone is
blocked by it we should revert the change.
On Sat, Sep 16, 2017 at 5:21 AM, Ed Maste via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> On 13 September 2017
How would that fix look? If the intention is to ignore nested SIGINT then
doing it directly in the handler seems cleaner and safer (re. safety, note
that m_input_reader_stack.GetMutex() is a std::*recursive_*mutex so
depending on it in signal handlers is a big no-no. The recursive_mutex
looks like
>
>
> Process already has "Error Process::WillResume()" for this very reason.
> Just have the Windows mini dump core file plug-in override it. The code in
> Process::Resume already checks this:
> Status Process::PrivateResume() {
> Status error(WillResume());
> // Tell the process it is about t
+Mark Mentovai
On Mon, Sep 11, 2017 at 12:24 PM, Leonard Mosescu
wrote:
> Thanks Jim, I'll give option #1 a try and upload a new patch.
>
> think your divisions into options is too simplistic. lldb is built of
>> many different subsystems, and a lot of "inconsistent state" can be
>> confined t
Thanks Jim, I'll give option #1 a try and upload a new patch.
think your divisions into options is too simplistic. lldb is built of
> many different subsystems, and a lot of "inconsistent state" can be
> confined to one subsystem or other. So for instance, if one CU's debug is
> unrecoverably d
I think everyone is in agreement that input should be sanitized, and
ill-formed input should not trigger an abrupt termination if there's a way
to recover (which is not always feasible).
The conversation started around fail-fast for inconsistent and
unrecoverable state, where I think the math is v
83 matches
Mail list logo