labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
The change sounds reasonable to me. I should just point out that this is the
Core <-> ObjectFile**JIT** cycle you are breaking.
Comment at: lldb/include/lldb/Core/Module.h:1
labath added a comment.
Both of the solutions sound plausible to me (extra struct vs. moving REPL to
`Command`). Maybe that's because I don't know enough about the REPL to have
formed an opinion on it.
Comment at: lldb/include/lldb/Expression/REPL.h:31
+bool TryAllThreads
labath added a comment.
In a way, this makes sense, but in a lot of other ways, it actually makes
things worse :)
My long-term goal is to be able to build lldb-server without even having clang
checked out (because it doesn't really need anything clang-related), and this
would certainly make th
Btw, r333032 gave me an idea. We already have unittests/Host/linux/*** to
contain linux-specific host tests (although one of them maybe shouldn't be
there). What would you say to moving this test to unittests/Host/macosx so
we can avoid the #ifdefs?
On Fri, 11 May 2018 at 18:57, Adrian Prantl via l
Author: labath
Date: Wed May 23 03:10:36 2018
New Revision: 333073
URL: http://llvm.org/viewvc/llvm-project?rev=333073&view=rev
Log:
ProcessLauncherPosixFork: move setgid call into the if(debug) branch
This call was originally being only made when launching for debug (as an
attempt to make sure w
Author: labath
Date: Wed May 23 03:32:05 2018
New Revision: 333074
URL: http://llvm.org/viewvc/llvm-project?rev=333074&view=rev
Log:
Fix PathMappingList tests on windows
The tests added in r332842 don't work on windows, because they do path
comparisons on strings, and on windows, the paths coming
labath created this revision.
labath added reviewers: zturner, clayborg.
Herald added a subscriber: mgorny.
For lldb-server, it is sufficient to parse only the native object file
format for its target OS (no other file can be loaded into a running
process). This moves the object file initializatio
labath created this revision.
labath added reviewers: JDevlieghere, clayborg, aprantl.
I think this makes sense for several reasons:
- better separation of concerns: DWARFUnit's job should be to provide a nice
interface to its users to access the unit contents. ManualDWARFIndex can then
use thi
JDevlieghere added a comment.
I've given this some more though as I see valid points and concerns on both
sides of the argument. I'm leaning towards running test cases in parallel
because it offloads more work to lit at negligible cost (running make and
launching the inferior are the biggest ti
JDevlieghere added a comment.
I'm going to hold off on this until we have decided what to do for
https://reviews.llvm.org/D46005. If we run the test cases as separate lit
invocations, then the test format is in a better position to report this
information.
https://reviews.llvm.org/D47062
_
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D47250
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D47253
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
zturner added a comment.
In https://reviews.llvm.org/D47235#1109219, @labath wrote:
> In a way, this makes sense, but in a lot of other ways, it actually makes
> things worse :)
>
> My long-term goal is to be able to build lldb-server without even having
> clang checked out (because it doesn't
> On May 22, 2018, at 6:57 PM, Zachary Turner wrote:
>
> Yea I don’t think this addresses the problem. We should be able to link
> against parts of lldb without a dependency on clang. Since this is about
> configuring something related to clang, it seems like it should be isolated
> to some
labath created this revision.
Herald added subscribers: JDevlieghere, eraman.
This needs to be cleaned up before we can consider submitting it.
However, I am putting it out here, so we you can test it's impact on
your setup.
PS: This does not prevent the debug-info replication for "inline" tests.
There's not really a diagram, because we don't have an exact vision of what
the final layering is going to look like (some things will need to be split
up, entirely new targets will need to be introduced, etc). Mostly it's
just built from experience based on what the primary logical function of a
zturner added a comment.
If the majority of active contributors think this is the right direction then
don't let me hold it up. I mostly just wanted to raise the concern, but if
you've evaluated the pros and cons and made a decision, I'm fine with that :)
https://reviews.llvm.org/D46005
__
labath added a comment.
Thank you for the support Jonas. I've quickly prototyped a patch
(https://reviews.llvm.org/D47265) of how would this integrate with lit. The
nice part about it is that it is very small. If you consider the inline test
refactor it even has negative LOC. It would definitel
zturner added a comment.
FWIW, I think the single biggest improvement one could make to the LLDB test
suite runtime is to compile all inferiors up front as part of the CMake step.
If you run the test suite twice every inferior is unnecessarily compiled twice.
https://reviews.llvm.org/D46005
labath added a comment.
In https://reviews.llvm.org/D47235#1109580, @zturner wrote:
> In https://reviews.llvm.org/D47235#1109219, @labath wrote:
>
> > I guess it would be nice to encapsulate this in some sort of a plugin
> > (since the setting is used from the clang expression parser plugin, I g
> On May 23, 2018, at 8:51 AM, Zachary Turner wrote:
>
> There's not really a diagram, because we don't have an exact vision of what
> the final layering is going to look like (some things will need to be split
> up, entirely new targets will need to be introduced, etc). Mostly it's just
>
SystemInitializerFull.cpp, in the
function SystemInitializerFull::Initialize().
On Wed, May 23, 2018 at 9:44 AM Adrian Prantl wrote:
>
>
> On May 23, 2018, at 8:51 AM, Zachary Turner wrote:
>
> There's not really a diagram, because we don't have an exact vision of
> what the final layering is g
JDevlieghere added a comment.
In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
> FWIW, I think the single biggest improvement one could make to the LLDB test
> suite runtime is to compile all inferiors up front as part of the CMake step.
> If you run the test suite twice every infer
The expression command had two modes before introducing the REPL. You can do:
(lldb) expr -- some C expression
or you can do:
(lldb) expr
Enter expressions, then terminate with an empty line to evaluate:
1: first C expression
2: Second C Expression
3:
The second only differs from the fi
jingham added a subscriber: zturner.
jingham added a comment.
The expression command had two modes before introducing the REPL. You can do:
(lldb) expr -- some C expression
or you can do:
(lldb) expr
Enter expressions, then terminate with an empty line to evaluate:
1: first C expression
2
aprantl updated this revision to Diff 148241.
aprantl added a comment.
Updated patch according to Zachary's suggestion.
https://reviews.llvm.org/D47235
Files:
include/lldb/Core/ModuleList.h
include/lldb/Host/HostInfoBase.h
source/API/SystemInitializerFull.cpp
source/Core/ModuleList.cpp
zturner added a comment.
In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
> In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
>
> > FWIW, I think the single biggest improvement one could make to the LLDB
> > test suite runtime is to compile all inferiors up front as par
aprantl updated this revision to Diff 148248.
aprantl added a comment.
Forgot to update unit tests.
https://reviews.llvm.org/D47235
Files:
include/lldb/API/SystemInitializerFull.h
include/lldb/Core/ModuleList.h
include/lldb/Host/HostInfoBase.h
source/API/SystemInitializerFull.cpp
sour
zturner added a comment.
Can you revert the changes to `HostInfoBase.h`? It looks like now it's only
whitespace changes.
Comment at: source/Core/ModuleList.cpp:16
#include "lldb/Host/Symbols.h"
+#include "lldb/Host/HostInfoBase.h"
#include "lldb/Interpreter/OptionValuePrope
aprantl added a comment.
In https://reviews.llvm.org/D46005#1109817, @zturner wrote:
> In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
>
> > In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
> >
> > > FWIW, I think the single biggest improvement one could make to the LL
zturner added a comment.
In https://reviews.llvm.org/D46005#1109911, @aprantl wrote:
> In https://reviews.llvm.org/D46005#1109817, @zturner wrote:
>
> > In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
> >
> > > In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
> > >
> >
aprantl updated this revision to Diff 148264.
aprantl added a comment.
Remove whitespace changes.
https://reviews.llvm.org/D47235
Files:
include/lldb/API/SystemInitializerFull.h
include/lldb/Core/ModuleList.h
source/API/SystemInitializerFull.cpp
source/Core/ModuleList.cpp
tools/lldb-t
aprantl updated this revision to Diff 148265.
aprantl added a comment.
Remove unused include.
https://reviews.llvm.org/D47235
Files:
include/lldb/API/SystemInitializerFull.h
include/lldb/Core/ModuleList.h
source/API/SystemInitializerFull.cpp
source/Core/ModuleList.cpp
tools/lldb-test/
aprantl added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
aprantl added a comment.
In https://reviews.llvm.org/D46005#1109920, @zturner wrote:
> In https://reviews.llvm.org/D46005#1109911, @aprantl wrote:
>
> > In https://reviews.llvm.org/D46005#1109817, @zturner wrote:
> >
> > > In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
> > >
> >
zturner added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
jankratochvil created this revision.
jankratochvil added reviewers: labath, clayborg.
Herald added subscribers: JDevlieghere, aprantl, mgorny.
As Pavel Labath said in https://reviews.llvm.org/D46810 this is new
`DWARFBasicDIE` to be used for `DWARFUnit::GetUnitDIEOnly()`. This patch is
only mech
jankratochvil created this revision.
jankratochvil added reviewers: labath, clayborg.
Herald added subscribers: JDevlieghere, aprantl.
As suggested by Pavel Labath in https://reviews.llvm.org/D46810
`DWARFUnit::GetUnitDIEOnly()` returning a pointer to `m_first_die` should not
permit using method
aprantl added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
zturner added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
zturner added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
xiaobai created this revision.
xiaobai added reviewers: compnerd, sas, labath, beanz, zturner.
Herald added a subscriber: mgorny.
Generating LLDB.framework when building with CMake+Ninja will copy the
lldb-private headers because public_headers contains them, even though we try
to make sure they d
zturner added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
aprantl added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
clayborg added a comment.
Marked things that don't belong in DWARFBasicDIE.
Also DWARFBasicDIE doesn't really explain what it actually is. Maybe we should
rename this DWARFUnitDIE? DWARFTopDIE? DWARFRootDIE?
Comment at: source/Plugins/SymbolFile/DWARF/DWARFBasicDIE.h:49
+
+
clayborg added a comment.
It would be good to verify all headers that are in the LLDB.framework produced
by Xcode builds are also in the LLDB.framework from cmake/ninja
https://reviews.llvm.org/D47278
___
lldb-commits mailing list
lldb-commits@list
clayborg added a comment.
Waiting for answers on renaming from patch 1/3 before commenting.
https://reviews.llvm.org/D47276
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
zturner added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
zturner added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
xiaobai added a comment.
I also think that'd be a great idea. The only way I can think to do that would
be to build LLDB.framework using both build systems and compare the two. Is
there a faster way?
https://reviews.llvm.org/D47278
___
lldb-commit
clayborg added a comment.
In https://reviews.llvm.org/D47278#1110092, @xiaobai wrote:
> I also think that'd be a great idea. The only way I can think to do that
> would be to build LLDB.framework using both build systems and compare the
> two. Is there a faster way?
That is the only guarantee
Actually, now I’m starting to wonder why can’t
ClangModulesDeclVendor.cpp just call this function in clangDriver to get
the default if the accessor returns an empty string? That sidesteps all of
this initialization funny business and lets the client just handle it.
Would that work?
On Wed, May
zturner added a comment.
Actually, now I’m starting to wonder why can’t
ClangModulesDeclVendor.cpp just call this function in clangDriver to get
the default if the accessor returns an empty string? That sidesteps all of
this initialization funny business and lets the client just handle it.
Wo
> On May 23, 2018, at 1:33 PM, Zachary Turner wrote:
>
> Actually, now I’m starting to wonder why can’t ClangModulesDeclVendor.cpp
> just call this function in clangDriver to get the default if the accessor
> returns an empty string? That sidesteps all of this initialization funny
> busi
xiaobai added a comment.
In https://reviews.llvm.org/D47278#1110104, @clayborg wrote:
> That is the only guaranteed way as new headers could come along.
> LLDB.framework doesn't have headers in installed Xcode.app bundles
In that case, adding this test could double build times since we would
clayborg added a comment.
sorry, not as a test, but just as a way to figure out if we are getting all the
needed header files when we modify this framework header file copying code.
https://reviews.llvm.org/D47278
___
lldb-commits mailing list
lldb
xiaobai added a comment.
In https://reviews.llvm.org/D47278#1110155, @clayborg wrote:
> sorry, not as a test, but just as a way to figure out if we are getting all
> the needed header files when we modify this framework header file copying
> code.
Ah, yeah. I'm in the process of trying to bui
clayborg added a comment.
In https://reviews.llvm.org/D47278#1110164, @xiaobai wrote:
> In https://reviews.llvm.org/D47278#1110155, @clayborg wrote:
>
> > sorry, not as a test, but just as a way to figure out if we are getting all
> > the needed header files when we modify this framework header
xiaobai added a comment.
In https://reviews.llvm.org/D47278#1110171, @clayborg wrote:
> In https://reviews.llvm.org/D47278#1110164, @xiaobai wrote:
>
> > In https://reviews.llvm.org/D47278#1110155, @clayborg wrote:
> >
> > > sorry, not as a test, but just as a way to figure out if we are getting
alexandreyy created this revision.
alexandreyy added a reviewer: clayborg.
Add PPC64le support information on LLDB site
https://reviews.llvm.org/D47285
Files:
www/features.html
www/index.html
www/status.html
Index: www/status.html
clayborg added a comment.
Is PPC64le only supported? no PPC64?
https://reviews.llvm.org/D47285
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Author: jyknight
Date: Wed May 23 15:04:20 2018
New Revision: 333134
URL: http://llvm.org/viewvc/llvm-project?rev=333134&view=rev
Log:
Remove spurious dependency on Process/elf-core from Process/Utility.
These checks do absolutely nothing other than cause a library layering
violation in the code.
alexandreyy added a comment.
In https://reviews.llvm.org/D47285#1110232, @clayborg wrote:
> Is PPC64le only supported? no PPC64?
Only PPC64le is supported.
https://reviews.llvm.org/D47285
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
alexandreyy added a comment.
In https://reviews.llvm.org/D47285#1110260, @alexandreyy wrote:
> In https://reviews.llvm.org/D47285#1110232, @clayborg wrote:
>
> > Is PPC64le only supported? no PPC64?
>
>
> Only PPC64le is supported.
Thanks @clayborg
@labath Could you commit this patch?
https:
xiaobai updated this revision to Diff 148305.
xiaobai added a comment.
Remove SystemInitializerFull.h from framework headers
https://reviews.llvm.org/D47278
Files:
source/API/CMakeLists.txt
Index: source/API/CMakeLists.txt
===
clayborg added a comment.
The issue is actually that SystemInitializerFull.h and
SystemInitializerFull.cpp are in the wrong directories. They belong in the
"lldb/Initialization" and "Source//Initialization". We should fix this and then
some/all of your changes won't be needed?
https://reviews
Author: adrian
Date: Wed May 23 16:33:50 2018
New Revision: 333140
URL: http://llvm.org/viewvc/llvm-project?rev=333140&view=rev
Log:
Add a --synchronous option to lldb-mi to facilitate reliable testing.
Patch by Alexander Polyakov!
Differential Revision: https://reviews.llvm.org/D47110
Modified
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333140: Add a --synchronous option to lldb-mi to facilitate
reliable testing. (authored by adrian, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47110?vs=147806&id=148318#toc
Repos
polyakov.alex created this revision.
polyakov.alex added a reviewer: aprantl.
Herald added a subscriber: llvm-commits.
The new method adds to the current target a path to search for a shared objects.
Repository:
rL LLVM
https://reviews.llvm.org/D47302
Files:
include/lldb/API/SBDebugger.h
aprantl added a comment.
The missing context here is that the lldb-mi -target-select command currently
calls `HandleCommand("target modules search-paths add", ...)`.
Is adding a new SBAPI the right approach to implementing this functionality
without going through HandleCommand? Or is HandleComma
Author: zturner
Date: Wed May 23 16:56:09 2018
New Revision: 333143
URL: http://llvm.org/viewvc/llvm-project?rev=333143&view=rev
Log:
Break dependency from Core to ObjectFileJIT.
The only reason this was here was so that Module could have a
function called CreateJITModule which created things in
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333143: Break dependency from Core to ObjectFileJIT.
(authored by zturner, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47228?vs=148100&id=
polyakov.alex added a comment.
In https://reviews.llvm.org/D47302#1110441, @aprantl wrote:
> The missing context here is that the lldb-mi -target-select command currently
> calls `HandleCommand("target modules search-paths add", ...)`.
> Is adding a new SBAPI the right approach to implementing
zturner added a comment.
In https://reviews.llvm.org/D47232#1108865, @jingham wrote:
> Perhaps a better way to handle this is to think of REPL.cpp/h as adjuncts of
> CommandObjectExpression.cpp/h - they mostly define the fancy IOHandler that
> gets pushed when you run the command in this mode.
aprantl added inline comments.
Comment at: lit/lit.cfg:61
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
It looks like "lldb-mi" may not be a valid substitution. On Darwin the command
`# RUN: %lldb_mi `
is expanded to
bin/lldb -S .../llvm/tools/lldb/
Author: zturner
Date: Wed May 23 17:11:24 2018
New Revision: 333145
URL: http://llvm.org/viewvc/llvm-project?rev=333145&view=rev
Log:
Add missing include.
Modified:
lldb/trunk/include/lldb/Core/Module.h
Modified: lldb/trunk/include/lldb/Core/Module.h
URL:
http://llvm.org/viewvc/llvm-project
aprantl added inline comments.
Comment at: lit/lit.cfg:61
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
aprantl wrote:
> It looks like "lldb-mi" may not be a valid substitution. On Darwin the command
>
> `# RUN: %lldb_mi `
>
> is expanded to
>
> bin
zturner added a comment.
FWIW `REPL` only uses 4 of the 10+ fields of
`CommandObjectExpression::Options`. If you don't like the separate struct
idea, I can just pass them directly as as independent arguments to
`REPL::SetCommandOptions`.
https://reviews.llvm.org/D47232
___
polyakov.alex added inline comments.
Comment at: lit/lit.cfg:61
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
aprantl wrote:
> aprantl wrote:
> > It looks like "lldb-mi" may not be a valid substitution. On Darwin the
> > command
> >
> > `# RUN: %lldb
polyakov.alex updated this revision to Diff 148323.
polyakov.alex added a comment.
Removed underscore in the lldb-mi variable from lit.cfg.
Repository:
rL LLVM
https://reviews.llvm.org/D46588
Files:
lit/lit.cfg
lit/tools/lldb-mi/breakpoint/break-insert.test
lit/tools/lldb-mi/breakpoint
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
Adding an SB API to set the target's search paths seems fine to me. I think in
theory the totality of LLDB functionality should be available through the SB
API's, (a) because it i
jingham added a comment.
I worry when concerns of layering which seem a little artificial to me would
make us add a shadow class for something that is already well expressed as it
is. If you pass them as arguments, and then I add another useful one and want
to use it in both the regular and th
On Wed, May 23, 2018 at 7:04 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:
> jingham added a comment.
>
> I worry when concerns of layering which seem a little artificial to me
> would make us add a shadow class for something that is already well
> expressed as it is. If you pa
Author: jyknight
Date: Wed May 23 20:42:38 2018
New Revision: 333151
URL: http://llvm.org/viewvc/llvm-project?rev=333151&view=rev
Log:
Remove unused include, and corresponding library dependency.
Modified:
lldb/trunk/source/Symbol/CMakeLists.txt
lldb/trunk/source/Symbol/ObjectFile.cpp
Mo
85 matches
Mail list logo
