[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function

ki.stfu added a comment. In https://reviews.llvm.org/D29256#662167, @krytarowski wrote: > @ki.stfu do you still agree with this patch? Please follow my comments and then we can go ahead. Comment at: tools/lldb-mi/MIDriver.cpp:512

[Lldb-commits] [lldb] r293742 - When I added the use of the new compression framework (present in

Author: jmolenda Date: Tue Jan 31 22:23:15 2017 New Revision: 293742 URL: http://llvm.org/viewvc/llvm-project?rev=293742=rev Log: When I added the use of the new compression framework (present in Mac OS X 10.11, El Capitan, released c. Oct 2015) I conditionalized the use of the framework on "if

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

Are you interested in seeing the followup patches for review (moving classes from Core -> Utility etc), or does it sound reasonable just based on my description? On Tue, Jan 31, 2017 at 6:05 PM Jim Ingham wrote: > BTW, not to exclude the positive because it doesn't need

[Lldb-commits] [PATCH] D28758: Fix typos and update "GDB To LLDB Command Map" to be a bit more clear

saagarjha added a comment. Sorry if this is a stupid question, but it's been a while and it doesn't appear that anything has happened with this patch; what should I do with it? Should I be sending this patch to lldb-commits to be merged? Or will this all happen automatically? (Please bear

[Lldb-commits] [PATCH] D29347: Transform ProcessLauncherLinux to ProcessLauncherPosixFork

emaste added a comment. In https://reviews.llvm.org/D29347#662503, @labath wrote: > @emaste: I'd suggest switching freebsd over to this process launcher as well. > I think it's the last user of the ProcessLauncherPosix, and it would enable > us to get rid of it. Indeed. I won't be able to

[Lldb-commits] [PATCH] D29347: Transform ProcessLauncherLinux to ProcessLauncherPosixFork

krytarowski updated this revision to Diff 86546. krytarowski added a comment. virtual -> override Repository: rL LLVM https://reviews.llvm.org/D29347 Files: include/lldb/Host/linux/ProcessLauncherLinux.h include/lldb/Host/posix/ProcessLauncherPosixFork.h source/Host/CMakeLists.txt

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

BTW, not to exclude the positive because it doesn't need discussing: all the rest of the changes you were proposing seem appropriate and good to me. Thanks for taking the trouble to clean this up. Jim > On Jan 31, 2017, at 5:45 PM, Zachary Turner wrote: > > Yea, there

[Lldb-commits] [PATCH] D29347: Transform ProcessLauncherLinux to ProcessLauncherPosixFork

labath added a comment. @emaste: I'd suggest switching freebsd over to this process launcher as well. I think it's the last user of the ProcessLauncherPosix, and it would enable us to get rid of it. Repository: rL LLVM https://reviews.llvm.org/D29347

[Lldb-commits] [PATCH] D29347: Transform ProcessLauncherLinux to ProcessLauncherPosixFork

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Perfect. Thank you. Comment at: include/lldb/Host/posix/ProcessLauncherPosixFork.h:19 +public: + virtual HostProcess LaunchProcess(const ProcessLaunchInfo _info, +

[Lldb-commits] [PATCH] D29347: Transform ProcessLauncherLinux to ProcessLauncherPosixFork

krytarowski updated this revision to Diff 86536. krytarowski added a comment. Fix typo. Repository: rL LLVM https://reviews.llvm.org/D29347 Files: include/lldb/Host/linux/ProcessLauncherLinux.h include/lldb/Host/posix/ProcessLauncherPosixFork.h source/Host/CMakeLists.txt

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

Yea, there may be value in both. If i ever tried to do this I'd probably take the approach of converting all the obvious cases first that should enforce checking and then see what's left. Then we could use llvm:Error and lldb::Status, or something On Tue, Jan 31, 2017 at 5:39 PM Jim Ingham

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

Yeah, I have no idea how you'd make sure that the SBError returned to Python in a Python SBValue was checked before it went out of scope, and I'm not sure it's our business to be enforcing that... So we're going to have to do something special for those errors. We also use the pattern where

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

llvm::Error only enforces checking at the point that it goes out of scope. So the example you mention should be supported, as long as you propagate the value all the way up and don't destroy it. There's also ways to explicitly ignore an Error (similar to casting to void to make the compiler not

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

I think we discussed this before, but we need an informational error object. IIRC, the llvm::Error has to be checked. But for instance if you ask a value object to evaluate itself, but you've moved to a section of code where the variable has no location, then evaluating that value will result

[Lldb-commits] [PATCH] D29347: Add ProcessLauncherNetBSD to spawn a tracee

krytarowski updated this revision to Diff 86524. krytarowski added a comment. Herald added subscribers: srhines, danalbert. Transform ProcessLauncherLinux to ProcessLauncherPosixFork and attach to Linux and NetBSD. Repository: rL LLVM https://reviews.llvm.org/D29347 Files:

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

My vote is to err on the side of leaving stuff in Utility that is general (like SharingPtr) but only used by one client, rather than moving it next to its current only client. After all, you are likely to go dumpster diving in Utility when you need a tool, but you're less likely to look

Re: [Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

> On Jan 31, 2017, at 3:59 PM, Zachary Turner via Phabricator via lldb-commits > wrote: > > zturner created this revision. > Herald added a subscriber: mgorny. > > The goal here is to make `lldbUtility` a library which depends on no other > libraries. It seems

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

zturner added a comment. In https://reviews.llvm.org/D29359#662417, @labath wrote: > Looks reasonable. To make sure things stay this way, we should make sure that > the Utility unit test has only this module specified as a dependency (I guess > after @beanz is done with digging through that).

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

zturner added inline comments. Comment at: lldb/source/Target/ThreadList.cpp:387 if (log) - log->Printf("ThreadList::%s thread 0x%4.4" PRIx64 - ": voted %s, but lost out because result was %s", - __FUNCTION__,

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

labath added a comment. Looks reasonable. To make sure things stay this way, we should make sure that the Utility unit test has only this module specified as a dependency (I guess after @beanz is done with digging through that). Comment at:

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

zturner added a comment. > Move Error from Core -> Utility Also, I almost forgot. Long term: Delete and use `llvm::Error` https://reviews.llvm.org/D29359 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

zturner created this revision. Herald added a subscriber: mgorny. The goal here is to make `lldbUtility` a library which depends on no other libraries. It seems like this library already exists in spirit as a place to house very low level code which is commonly used by all parts of LLDB, so it

[Lldb-commits] [PATCH] D29352: [CMake] Final dependency cleanup patch!

labath added a comment. The main code seems to be fine now, but linking of unit tests (check-lldb-unit target) fails now. You'll probably need to explicitly set their dependencies as well before this can be removed. https://reviews.llvm.org/D29352

[Lldb-commits] [PATCH] D29352: [CMake] Final dependency cleanup patch!

beanz updated this revision to Diff 86509. beanz added a comment. Fixing the issue labath reported in the review. Generally speaking we shouldn't be configuring or compiling plugins that can't be used. https://reviews.llvm.org/D29352 Files: cmake/LLDBDependencies.cmake

[Lldb-commits] [PATCH] D29352: [CMake] Final dependency cleanup patch!

labath added a comment. When linking liblldb.so: /usr/bin/ld.gold: error: cannot find -llldbPluginDynamicLoaderDarwinKernel https://reviews.llvm.org/D29352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D29352: [CMake] Final dependency cleanup patch!

labath added a comment. I'll give this a spin on linux now. https://reviews.llvm.org/D29352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections

labath added a comment. Sorry, I have been a bit busy. I've committed this in now. Thank you for the patch. Repository: rL LLVM https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections

This revision was automatically updated to reflect the committed changes. Closed by commit rL293714: Open ELF core dumps with more than 64K sections (authored by labath). Changed prior to commit: https://reviews.llvm.org/D29095?vs=86148=86507#toc Repository: rL LLVM

[Lldb-commits] [lldb] r293714 - Open ELF core dumps with more than 64K sections

Author: labath Date: Tue Jan 31 17:09:46 2017 New Revision: 293714 URL: http://llvm.org/viewvc/llvm-project?rev=293714=rev Log: Open ELF core dumps with more than 64K sections Summary: Problem: There are three filelds in the ELF header - e_phnum, e_shnum, and e_shstrndx - that could be bigger

[Lldb-commits] [PATCH] D29352: [CMake] Final dependency cleanup patch!

beanz created this revision. Herald added subscribers: jgosnell, mgorny, srhines, danalbert. This patch removes the over-specified dependencies from LLDBDependencies and instead relies on the dependencies as expressed in each library and tool. This also removes the library looping in favor of

[Lldb-commits] [lldb] r293701 - [CMake] [4/4] Update a batch of plugins

Author: cbieneman Date: Tue Jan 31 16:31:38 2017 New Revision: 293701 URL: http://llvm.org/viewvc/llvm-project?rev=293701=rev Log: [CMake] [4/4] Update a batch of plugins This is extending the updates from r293696 to more LLDB plugins. Modified:

[Lldb-commits] [lldb] r293700 - [CMake] [3/4] Update a batch of plugins

Author: cbieneman Date: Tue Jan 31 16:29:11 2017 New Revision: 293700 URL: http://llvm.org/viewvc/llvm-project?rev=293700=rev Log: [CMake] [3/4] Update a batch of plugins This is extending the updates from r293696 to more LLDB plugins. Modified:

[Lldb-commits] [lldb] r293699 - [CMake] [2/4] Update a batch of plugins

Author: cbieneman Date: Tue Jan 31 16:23:49 2017 New Revision: 293699 URL: http://llvm.org/viewvc/llvm-project?rev=293699=rev Log: [CMake] [2/4] Update a batch of plugins This is extending the updates from r293696 to more LLDB plugins. Modified:

[Lldb-commits] [lldb] r293698 - [CMake] [1/4] Update a batch of plugins

Author: cbieneman Date: Tue Jan 31 16:21:19 2017 New Revision: 293698 URL: http://llvm.org/viewvc/llvm-project?rev=293698=rev Log: [CMake] [1/4] Update a batch of plugins This is extending the updates from r293696 to more LLDB plugins. Modified:

[Lldb-commits] [lldb] r293696 - [CMake] Add explicit dependencies to plugins

Author: cbieneman Date: Tue Jan 31 16:12:59 2017 New Revision: 293696 URL: http://llvm.org/viewvc/llvm-project?rev=293696=rev Log: [CMake] Add explicit dependencies to plugins Summary: This patch does two things. First it updates all the ABI plugins with accurate dependencies, and second it

[Lldb-commits] [PATCH] D29266: Synchronize PlatformNetBSD with Linux

krytarowski added a comment. In https://reviews.llvm.org/D29266#661839, @labath wrote: > After looking at this closer, I don't think we will even need a separate > class to achieve deduplication. The code is so heavily duplicated (see > comments on the first three non-boilerplate functions, I

[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function

krytarowski added a comment. @ki.stfu do you still agree with this patch? Repository: rL LLVM https://reviews.llvm.org/D29256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D29348: [CMake] Add explicit dependencies to plugins

beanz created this revision. Herald added subscribers: jgosnell, mgorny, nemanjai. This patch does two things. First it updates all the ABI plugins with accurate dependencies, and second it adds a tracking mechanism for add_lldb_library to denote plugin libraries, allowing us to build up a list

[Lldb-commits] [PATCH] D29347: Add ProcessLauncherNetBSD to spawn a tracee

krytarowski added a comment. In https://reviews.llvm.org/D29347#662145, @labath wrote: > This is also copying a lot of code only to make tiny changes to it. Please > put the Linux process launcher into the posix folder (I suggest naming it > ProcessLauncherPosixFork). Ifdef out the

[Lldb-commits] [PATCH] D29347: Add ProcessLauncherNetBSD to spawn a tracee

labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. This is also copying a lot of code only to make tiny changes to it. Please put the Linux process launcher into the posix folder (I suggest naming it ProcessLauncherPosixFork). Ifdef

[Lldb-commits] [PATCH] D29347: Add ProcessLauncherNetBSD to spawn a tracee

krytarowski added a comment. This patch was already open in a discussion with @labath, I'm pushing it here to point the appropriate solution during review. Repository: rL LLVM https://reviews.llvm.org/D29347 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D29347: Add ProcessLauncherNetBSD to spawn a tracee

krytarowski created this revision. krytarowski added a project: LLDB. Herald added a subscriber: mgorny. Base this code on Linux. Sponsored by Repository: rL LLVM https://reviews.llvm.org/D29347 Files: include/lldb/Host/netbsd/ProcessLauncherNetBSD.h source/Host/CMakeLists.txt

[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function

tnn added a comment. Attaching full error message as requested. Still reproducable on NetBSD's head-LLVM snapshot. F3050737: lldb-build.txt Repository: rL LLVM https://reviews.llvm.org/D29256 ___ lldb-commits

[Lldb-commits] [lldb] r293690 - [CMake] Partial revert of r293686

Author: cbieneman Date: Tue Jan 31 15:12:52 2017 New Revision: 293690 URL: http://llvm.org/viewvc/llvm-project?rev=293690=rev Log: [CMake] Partial revert of r293686 This change reverts the lldb-server part of r293686, which is having trouble on Linux bots. I'm not sure if I can make lldb-server

[Lldb-commits] [lldb] r293687 - [CMake] Fix broken NetBSD bots

Author: cbieneman Date: Tue Jan 31 14:52:41 2017 New Revision: 293687 URL: http://llvm.org/viewvc/llvm-project?rev=293687=rev Log: [CMake] Fix broken NetBSD bots http://lab.llvm.org:8011/builders/lldb-amd64-ninja-netbsd7/builds/4558/steps/cmake%20local/logs/stdio Modified:

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

This revision was automatically updated to reflect the committed changes. Closed by commit rL293686: [CMake] Add accurate dependency specifications (authored by cbieneman). Changed prior to commit: https://reviews.llvm.org/D29333?vs=86461=86481#toc Repository: rL LLVM

[Lldb-commits] [lldb] r293686 - [CMake] Add accurate dependency specifications

Author: cbieneman Date: Tue Jan 31 14:43:05 2017 New Revision: 293686 URL: http://llvm.org/viewvc/llvm-project?rev=293686=rev Log: [CMake] Add accurate dependency specifications Summary: This patch adds accurate dependency specifications to the mail LLDB libraries and tools. In all cases

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

beanz added a comment. @labath, we'll have to see if CMake's handling is good enough. What CMake actually does is repeat the full chain of the cycle, so it would be something like adding `-lX -lY -lX -lY` in your example. This doesn't work for certain pathological cases, but CMake actually has

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

labath added a comment. In https://reviews.llvm.org/D29333#661994, @beanz wrote: > In https://reviews.llvm.org/D29333#661979, @labath wrote: > > > I was thinking about that as well. I am not sure if it will work if we > > actually need multiple iterations of the loop to get all the dependencies

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

beanz added a comment. In https://reviews.llvm.org/D29333#661979, @labath wrote: > I was thinking about that as well. I am not sure if it will work if we > actually need multiple iterations of the loop to get all the dependencies > converging, but it may be worth trying out. The way it is

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

beanz added a comment. @zturner, absolutely! Thanks! I expect this patch and the next one to be pretty safe because I'm not actually removing the existing dependency specifications, just adding new explicit ones. https://reviews.llvm.org/D29333

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. ok, lgtm with the caveat that if it breaks anything on Windows we might have to revert until we figure it out. https://reviews.llvm.org/D29333

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

beanz added a comment. Thankfully CMake will not complain about circular dependencies in static archive targets. If it did, we'd really be in trouble because the LLDB dependencies graph has *lots* of circular dependencies. Actually, I think CMake even handles them properly on Linux, which

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

zturner added a comment. As long as you considered the tradeoffs then and you like this better, that's fine then. Is CMake going to complain about circular dependencies? For example, Breakpoint depends on Core and Core depends on Breakpoint. https://reviews.llvm.org/D29333

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

beanz added a comment. @zturner That mechanism does work, however I generally find that it is cleaner to pass named parameters into CMake functions which provides a bit of a self-documenting API, as opposed to relying on known named variables. The named-parameter was introduced for LLD, and I

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

zturner added a comment. Any reason this doesn't use the same strategy as LLVM? I.e. at the top of the `CMakeLists.txt` file, write something like `set(LLVM_LINK_COMPONENTS Foo Bar Baz)`, then just call `add_lldb_library` etc. https://reviews.llvm.org/D29333

[Lldb-commits] [PATCH] D29333: [CMake] Add accurate dependency specifications

labath added a comment. I like where this is going. I am not sure about a couple of details though. Please see comments. Comment at: source/Breakpoint/CMakeLists.txt:32 +lldbPluginObjCLanguage +LLVMSupport ) Shouldn't this be: `LINK_COMPONENTS

[Lldb-commits] [PATCH] D29266: Synchronize PlatformNetBSD with Linux

labath added a comment. After looking at this closer, I don't think we will even need a separate class to achieve deduplication. The code is so heavily duplicated (see comments on the first three non-boilerplate functions, I did not look at the rest yet, but I expect the situation to be the

[Lldb-commits] [PATCH] D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables.

This revision was automatically updated to reflect the committed changes. Closed by commit rL293660: Add a command to access and manipulate the Intel(R) MPX Boundary Tables. (authored by valentinagiusti). Changed prior to commit: https://reviews.llvm.org/D29078?vs=85906=86453#toc Repository:

[Lldb-commits] [lldb] r293660 - Add a command to access and manipulate the Intel(R) MPX Boundary Tables.

Author: valentinagiusti Date: Tue Jan 31 12:02:54 2017 New Revision: 293660 URL: http://llvm.org/viewvc/llvm-project?rev=293660=rev Log: Add a command to access and manipulate the Intel(R) MPX Boundary Tables. Summary: The Boundary Table Entries are stored in the application memory and allow to

[Lldb-commits] [lldb] r293647 - [CMake] Add LINK_LIBS and LINK_COMPONENTS options

Author: cbieneman Date: Tue Jan 31 10:59:46 2017 New Revision: 293647 URL: http://llvm.org/viewvc/llvm-project?rev=293647=rev Log: [CMake] Add LINK_LIBS and LINK_COMPONENTS options This patch adds CMake options to add_lldb_library and add_lldb_executable for specifying LLVM components and

[Lldb-commits] [lldb] r293646 - NFC. Remove unused header include.

Author: cbieneman Date: Tue Jan 31 10:48:20 2017 New Revision: 293646 URL: http://llvm.org/viewvc/llvm-project?rev=293646=rev Log: NFC. Remove unused header include. Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp Modified:

[Lldb-commits] [PATCH] D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables.

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. If we can add a comment around "bndcfgu" where we use an SBData since it is a vector register and SBValue::GetValueAsUnsigned(...) won't work because it is a vector that would be nice for

[Lldb-commits] [PATCH] D29266: Synchronize PlatformNetBSD with Linux

krytarowski added a comment. Diff diff -u source/Plugins/Platform/Linux/PlatformLinux.cpp source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp Mostly everything is the same for now. --- source/Plugins/Platform/Linux/PlatformLinux.cpp 2016-12-19 17:48:18.156356172 +0100 +++

[Lldb-commits] [lldb] r293625 - Add NetBSD support in Host::GetCurrentThreadID

Author: kamil Date: Tue Jan 31 07:38:42 2017 New Revision: 293625 URL: http://llvm.org/viewvc/llvm-project?rev=293625=rev Log: Add NetBSD support in Host::GetCurrentThreadID Summary: To retrieve the native thread ID there must be called _lwp_self(). Sponsored by Reviewers: joerg, clayborg,