aprantl added a comment.
Did the old implementation come with a testcase? Perhaps I'm misunderstanding
the question, but it would probably be best to preserve the old behavior.
Comment at: tools/lldb-mi/MICmdCmdExec.cpp:515
+ lldb::SBError error;
+ if (nThreadId != UINT64_MA
aprantl accepted this revision.
aprantl added inline comments.
Comment at: lit/tools/lldb-mi/exec/exec-next.test:19
+
+-exec-next --thread 0
+# Check that exec-next can process the case of invalid thread ID.
0 feels like it might actually be a valid thread id on
zturner added a subscriber: aleksandr.urakov.
zturner added a comment.
Do you just need a pdb, or does it really need to be a vs pdb? lld can
generate high quality pdbs now. So it might be possible to use lld to link
and produce a pdb when you run the test.
Pavel’s suggestion is equally viable, y
Do you just need a pdb, or does it really need to be a vs pdb? lld can
generate high quality pdbs now. So it might be possible to use lld to link
and produce a pdb when you run the test.
Pavel’s suggestion is equally viable, you can dump a pdb to yaml and
convert it back to a pdb at test time.
Th
asmith added a comment.
In https://reviews.llvm.org/D47708#1123334, @labath wrote:
> I have reverted this because of the broken tests.
>
> However, I have to also ask: isn't there better way to test this? (one that
> does not depend on opaque checked-in binaries). On linux, I could check-in a
>
xiaobai abandoned this revision.
xiaobai added a comment.
Going to go with the direction we discussed here. This shouldn't be needed.
https://reviews.llvm.org/D47801
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi
xiaobai added a comment.
In https://reviews.llvm.org/D47801#1123933, @labath wrote:
> In https://reviews.llvm.org/D47801#1123895, @xiaobai wrote:
>
> > > - rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB
> > > or whatever)
> > > - make the liblldb -> tool dependency not co
xiaobai added inline comments.
Comment at: source/API/CMakeLists.txt:184
+ set(EXECUTABLE_NAME "LLDB")
+ set(CURRENT_PROJECT_VERSION "360.99.0")
set_target_properties(liblldb PROPERTIES
clayborg wrote:
> sas wrote:
> > clayborg wrote:
> > > Currently the app
labath added a comment.
In https://reviews.llvm.org/D47801#1123895, @xiaobai wrote:
> > - rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or
> > whatever)
> > - make the liblldb -> tool dependency not conditioned by
> > LLDB_BUILD_FRAMEWORK
> > - remove the lldb->tool dep
sas added a comment.
> One idea I had was to introduce another target for the framwork itself, e.g.
> lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would
> depend on liblldb and all the necessary tools, headers, etc, that the
> framework would need. That way liblldb can dep
polyakov.alex added a comment.
I'm a little bit confused about situation with thread id before my changes. For
example:
(gdb)
thread list
~"Process 31642 stopped\n* thread #1: tid = 31642, 0x0041eed0
bash`main, name = 'bash', stop reason = breakpoint 1.1\n"
^done
What we should
xiaobai added a comment.
> No existing INCLUDE_IN_FRAMEWORK tool links to liblldb, just it's individual
> components, which is fine. And this is probably a state that we should
> maintain for the future.
I think I agree with you here.
> So, how about we do this instead:
>
> - rename INCLUDE_I
polyakov.alex created this revision.
polyakov.alex added reviewers: aprantl, clayborg, labath, stella.stamenova.
Herald added a subscriber: ki.stfu.
Now -exec-step uses SB API instead of HandleCommand hack.
https://reviews.llvm.org/D47838
Files:
lit/tools/lldb-mi/exec/exec-step.test
tools/l
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Just one nit you can choose to fix or not.
Comment at: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:39-41
+if (!unit)
+ continue;
+if (m_units_to_avo
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Much better. Anything we can do to clearly define and simplify the indexers is
great stuff.
https://reviews.llvm.org/D47781
___
lldb-commits
labath created this revision.
labath added reviewers: JDevlieghere, clayborg.
Herald added a subscriber: aprantl.
It possible that a single module has indexed and non-indexed compile
units. In this case, we can use the fast indexed lookup for the first
ones and fall back to the manual index for th
labath updated this revision to Diff 150149.
labath added a comment.
Ok, so it turns out we **were** passing qualified names into the
GetGlobalVariables function. In the apple-tables case we were then searching
based on the basename and completely ignoring the context. This resulted in
incorrect m
polyakov.alex added inline comments.
Comment at: tools/lldb-mi/MICmdCmdExec.cpp:384
+ if (nThreadId != UINT64_MAX) {
+lldb::SBThread sbThread =
rSessionInfo.GetProcess().GetThreadByID(nThreadId);
+if (sbThread.IsValid())
We can't test this branch until
polyakov.alex updated this revision to Diff 150137.
polyakov.alex added a reviewer: labath.
polyakov.alex added a comment.
Added some checks for invalid thread id.
https://reviews.llvm.org/D47797
Files:
lit/tools/lldb-mi/exec/exec-next.test
lit/tools/lldb-mi/exec/inputs/main.c
tools/lldb-
zturner added a comment.
The problem is that Phabricator doesn't preserve binary in diffs
https://reviews.llvm.org/D47708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aleksandr.urakov added a comment.
> I think you have to specify --binary when generating the diff.
I have specified --binary when I was generating the patch, and there is encoded
binaries in the patch file, may be they was somehow cutted out during an upload
to the Phabricator... I have attache
asmith added a comment.
In https://reviews.llvm.org/D47708#1123286, @aleksandr.urakov wrote:
> Thank you!
>
> But it looks like the binary files (*.pdb and *.exe) have not been committed
> properly (they have zero size).
I've seen that issue before. I think you have to specify --binary when
g
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334088: [DWARF] Add (empty) DebugNamesDWARFIndex class and a
setting to control its use (authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews
Author: labath
Date: Wed Jun 6 04:35:23 2018
New Revision: 334088
URL: http://llvm.org/viewvc/llvm-project?rev=334088&view=rev
Log:
[DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use
Summary:
This patch adds the skeleton for implementing the DWARF v5 name index
class
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334080: [lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON
(authored by mgorny, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47812?vs=150
Author: mgorny
Date: Wed Jun 6 02:44:14 2018
New Revision: 334080
URL: http://llvm.org/viewvc/llvm-project?rev=334080&view=rev
Log:
[lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON
Skip all Python-based tests as unsupported when LLDB_DISABLE_PYTHON is
enabled. Otherwise, those tests simply
labath added a comment.
I have reverted this because of the broken tests.
However, I have to also ask: isn't there better way to test this? (one that
does not depend on opaque checked-in binaries). On linux, I could check-in a .s
file which has the line table exactly as I want it and then have
Author: labath
Date: Wed Jun 6 02:16:00 2018
New Revision: 334076
URL: http://llvm.org/viewvc/llvm-project?rev=334076&view=rev
Log:
Revert "PDB support of function-level linking and splitted functions"
This reverts commit r334030 because it adds a broken test.
Removed:
lldb/trunk/unittests
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Makes sense to me.
https://reviews.llvm.org/D47812
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
mgorny created this revision.
mgorny added reviewers: JDevlieghere, labath.
Skip all Python-based tests as unsupported when LLDB_DISABLE_PYTHON is
enabled. Otherwise, those tests simply fail being unable to import lldb
module.
https://reviews.llvm.org/D47812
Files:
lit/CMakeLists.txt
lit/S
aleksandr.urakov added a comment.
Thank you!
But it looks like the binary files (*.pdb and *.exe) have not been committed
properly (they have zero size).
https://reviews.llvm.org/D47708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
htt
labath added inline comments.
Comment at: tools/lldb-mi/MICmdCmdExec.cpp:384
if (nThreadId != UINT64_MAX)
-strCmd += CMIUtilString::Format(" %llu", nThreadId);
- rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-
labath added a comment.
In https://reviews.llvm.org/D47801#1123051, @sas wrote:
> Don't we risk creating circular dependencies with this? What happens when a
> tool depends on liblldb? you'll have theTool depend on liblldb and liblldb
> depend on theTool.
The same thought has occurred to me,
33 matches
Mail list logo
