[Lldb-commits] [lldb] r316532 - Allow ObjectFilePECOFF to initialize with ARM binaries.

2017-10-24 Thread Stephane Sezer via lldb-commits
Author: sas Date: Tue Oct 24 16:40:59 2017 New Revision: 316532 URL: http://llvm.org/viewvc/llvm-project?rev=316532=rev Log: Allow ObjectFilePECOFF to initialize with ARM binaries. Summary: This is required to start debugging WinPhone ARM targets. Reviewers: compnerd, zturner, omjavaid

[Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.

2017-10-24 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL316532: Allow ObjectFilePECOFF to initialize with ARM binaries. (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D19604 Files:

[Lldb-commits] [lldb] r316529 - [ExpressionParser] Garbage-collect dead code. NFCI.

2017-10-24 Thread Davide Italiano via lldb-commits
Author: davide Date: Tue Oct 24 16:29:01 2017 New Revision: 316529 URL: http://llvm.org/viewvc/llvm-project?rev=316529=rev Log: [ExpressionParser] Garbage-collect dead code. NFCI. Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp Modified:

[Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.

2017-10-24 Thread Stephane Sezer via Phabricator via lldb-commits
sas updated this revision to Diff 120146. sas added a comment. Herald added a subscriber: kristof.beyls. Rebase. https://reviews.llvm.org/D19604 Files: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

[Lldb-commits] [lldb] r316527 - Remove some unused function calls from ClangUserExpression.cpp

2017-10-24 Thread Stephane Sezer via lldb-commits
Author: sas Date: Tue Oct 24 16:01:33 2017 New Revision: 316527 URL: http://llvm.org/viewvc/llvm-project?rev=316527=rev Log: Remove some unused function calls from ClangUserExpression.cpp Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp Modified:

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Zachary Turner via Phabricator via lldb-commits
zturner accepted this revision. zturner added a comment. I will ping them for some numbers and more details of their test setup. Regardless, I didn't mean to derail the code review. But, I really really want to reach a point where we can stop falling back on the "we need to be safe even in

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment. > There was a talk at cppcon a few weeks ago from someone at backtrace.io who > had some insightful metrics on debugger performance memory consumption, and > LLDB had ~2x the memory consumption of GDB. I haven't seen the paper, but my guess is that this is on

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: source/Plugins/Architecture/Arm/ArchitectureArm.cpp:23 + return ConstString("arm"); +} + zturner wrote: > clayborg wrote: > > zturner wrote: > > > clayborg wrote: > > > > One time at startup. No threads contending

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments. Comment at: source/Plugins/Architecture/Arm/ArchitectureArm.cpp:23 + return ConstString("arm"); +} + clayborg wrote: > zturner wrote: > > clayborg wrote: > > > One time at startup. No threads contending yet. Asking for plug-in by

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: source/Core/PluginManager.cpp:282 + ConstString name; + std::string description; + PluginManager::ArchitectureCreateInstance create_callback; zturner wrote: > clayborg wrote: > > We need "std::string" since it owns

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments. Comment at: source/Plugins/Architecture/Arm/ArchitectureArm.cpp:23 + return ConstString("arm"); +} + clayborg wrote: > One time at startup. No threads contending yet. Asking for plug-in by name is > made fast for later. I would

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments. Comment at: source/Core/PluginManager.cpp:281 +struct ArchitectureInstance { + ConstString name; + std::string description; zturner wrote: > labath wrote: > > clayborg wrote: > > > zturner wrote: > > > > Why do we need a

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: include/lldb/Target/Target.h:1217 +const ArchSpec () const { return m_spec; } +Architecture *GetPlugin() const { return m_plugin_up.get(); } + zturner wrote: > Can this return a reference instead of a pointer?

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments. Comment at: source/Core/PluginManager.cpp:281-282 +struct ArchitectureInstance { + ConstString name; + std::string description; + PluginManager::ArchitectureCreateInstance create_callback; labath wrote: > zturner wrote: > > Why

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: include/lldb/Target/Target.h:1217 +const ArchSpec () const { return m_spec; } +Architecture *GetPlugin() const { return m_plugin_up.get(); } + zturner wrote: > Can this return a reference instead of a pointer? It

Re: [Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Greg Clayton via lldb-commits
Looks fine. We can start with this. I was thinking it would be nice to lazily populate m_plugin_up, but then we would need to add a bit to see if we already tried to look for it, so the current approach will work fine. > On Oct 24, 2017, at 1:22 PM, Pavel Labath via Phabricator >

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. Looks fine. We can iterate on this. https://reviews.llvm.org/D31172 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-10-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I'm back now, and I'd like to try to push this patch to completion. After re-reading the discussion, I got the impression we have mostly reached a consensus here. A small issue remained about how to guarantee that the Architecture plugin and the ArchSpec object are in

[Lldb-commits] [PATCH] D39246: Fix LLVM_LINK_LLVM_DYLIB build (pr35053)

2017-10-24 Thread Sylvestre Ledru via Phabricator via lldb-commits
sylvestre.ledru added a comment. This fixed the issue, thanks! https://reviews.llvm.org/D39246 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

2017-10-24 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 120102. labath added a comment. This removes the LLDB_TEST_CLANG and LLDB_TEST_COMPILER settings. I've decided to keep the separate C and CXX variables because: a) that's consistent with other cmake settings b) in case of gcc, it's not easy to figure out the

[Lldb-commits] [PATCH] D39246: Fix LLVM_LINK_LLVM_DYLIB build (pr35053)

2017-10-24 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. Herald added a subscriber: mgorny. r316368 broke this build when it introduced a reference to a pthread function to the Utility module. This caused cmake to generate an incorrect link line (wrong order of libs) because it did not see the dependency from Utility to

Re: [Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Davide Italiano via lldb-commits
Reported https://bugs.llvm.org/show_bug.cgi?id=35061 so I don't forget. On Tue, Oct 24, 2017 at 9:35 AM, Zachary Turner wrote: > Actually there's fewer, I think `test/testcases` is a symlink. But there's > more than one, for sure. We should standardize on the one in

Re: [Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Zachary Turner via lldb-commits
Actually there's fewer, I think `test/testcases` is a symlink. But there's more than one, for sure. We should standardize on the one in lldbutil.py On Tue, Oct 24, 2017 at 9:33 AM Davide Italiano wrote: > Fun fact, there are 13 implementations in tree of is_exe (and

Re: [Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Davide Italiano via lldb-commits
Fun fact, there are 13 implementations in tree of is_exe (and probably which). Maybe we should try replacing all them with the one from lit? Or is there some hidden dependency I'm missing? [davide@cupiditate lldb]$ grep -R 'def is_exe' * packages/Python/lldbsuite/test/dotest.py:def is_exe(fpath):

Re: [Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Davide Italiano via lldb-commits
On Tue, Oct 24, 2017 at 9:25 AM, Pavel Labath wrote: > The breaking build is this one: < > http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/14775> > > The blame email was sent (I know because I got it, as I also had a > commit in the same build). Is it

Re: [Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Pavel Labath via lldb-commits
The breaking build is this one: < http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/14775> The blame email was sent (I know because I got it, as I also had a commit in the same build). Is it possible you overlooked it? ``` def is_exe(fpath): if not os.path.exists(fpath):

Re: [Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

2017-10-24 Thread Pavel Labath via lldb-commits
On 24 October 2017 at 08:58, Ted Woodward via Phabricator wrote: > ted added a comment. > > We build lldb, clang and tools for Hexagon only, and call them hexagon-lldb, > hexagon-clang, etc. The test infrastructure is smart enough to pick up > hexagon-lldb-mi if we

Re: [Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Davide Italiano via lldb-commits
On Tue, Oct 24, 2017 at 9:12 AM, Zachary Turner via lldb-commits wrote: > I think there's something like lit.util.which(), or a similar function in > lldb test utilities. Seems like we could solve this by writing the > function: > > ``` > def is_exe(fpath): >if

Re: [Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Zachary Turner via lldb-commits
I think there's something like lit.util.which(), or a similar function in lldb test utilities. Seems like we could solve this by writing the function: ``` def is_exe(fpath): if not os.path.exists(fpath): fpath = lit.util.which(fpath) if not (fpath and os.path.exists(fpath)):

Re: [Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Davide Italiano via lldb-commits
(and thanks for reverting, BTW :) On Tue, Oct 24, 2017 at 9:10 AM, Davide Italiano wrote: > HI, I'm a little curious of what bot this breaks. > FWIW, I don't think the bot should rely on this behaviour, and more > importantly, I haven't received any failmail. > > Thanks, >

[Lldb-commits] [PATCH] D39199: [lldbtests] Handle errors instead of crashing

2017-10-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Davide, I reverted this because it breaks the case when you just specify a filename as a compiler (with the expectation that we will look it up in the path). I think this is a good thing to have, and our buildbot was using that behavior. I like the direction this

[Lldb-commits] [lldb] r316451 - Revert "[lldbtests] Handle errors instead of crashing."

2017-10-24 Thread Pavel Labath via lldb-commits
Author: labath Date: Tue Oct 24 09:07:50 2017 New Revision: 316451 URL: http://llvm.org/viewvc/llvm-project?rev=316451=rev Log: Revert "[lldbtests] Handle errors instead of crashing." The commit breaks the case where you specify just a filename to the compiler. Previously, it would look up the

[Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

2017-10-24 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. In https://reviews.llvm.org/D39215#905259, @ted wrote: > We build lldb, clang and tools for Hexagon only, and call them hexagon-lldb, > hexagon-clang, etc. The test infrastructure is smart enough to pick up > hexagon-lldb-mi if we tell it to run with hexagon-lldb using

[Lldb-commits] [PATCH] D39215: Default to using in-tree clang for building test executables

2017-10-24 Thread Ted Woodward via Phabricator via lldb-commits
ted added a comment. We build lldb, clang and tools for Hexagon only, and call them hexagon-lldb, hexagon-clang, etc. The test infrastructure is smart enough to pick up hexagon-lldb-mi if we tell it to run with hexagon-lldb using --executable; will it be smart enough to run an in-tree