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
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
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
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
beanz added a comment.
Ok. I think this patch is fully ready to go now. I added the missing
dependencies in the ObjectFileELF tests in https://reviews.llvm.org/rL294372.
Thank you @labath for all your patience and help testing this patch. I've
tested it on Darwin and FreeBSD in the current
beanz updated this revision to Diff 87265.
beanz added a comment.
Rebasing patch
https://reviews.llvm.org/D29352
Files:
cmake/LLDBDependencies.cmake
cmake/modules/AddLLDB.cmake
source/API/CMakeLists.txt
source/Initialization/CMakeLists.txt
source/Plugins/Process/CMakeLists.txt
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
beanz added a comment.
Ah! I forgot to update the unittest directory. I'll get a patch in today that
updates the unittests following the pattern from the libraries.
https://reviews.llvm.org/D29352
___
lldb-commits mailing list
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
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
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
beanz added a comment.
Looks fine to me.
https://reviews.llvm.org/D23977
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
beanz added a comment.
I think I fixed this in https://reviews.llvm.org/rL290934.
https://reviews.llvm.org/D28155
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
beanz added a comment.
@jingham I put the unit tests at the top because they depend on LLDB's Host
library (at least the current tests do). I'm attempting to write tests which
cover the socket communication between LLDB and debugserver by sending data
between the two using each side of the
beanz updated this revision to Diff 93165.
beanz added a comment.
Fleshed out the unit test logic to be more meaningful.
https://reviews.llvm.org/D31357
Files:
tools/debugserver/source/CMakeLists.txt
tools/debugserver/source/MacOSX/CMakeLists.txt
unittests/CMakeLists.txt
beanz created this revision.
Herald added a subscriber: mgorny.
This patch refactors the CMake build system's support for building debugserver
to allow us to build the majority of debugserver's sources into the
debugserverCommon library which can then be reused by unit tests.
The first unit
beanz added a comment.
Xcode is pretty magic to me. I don't know how to do much of anything in it
other than build. I think the right solution would be to take most of the
source files out of the debugserver target and generate a static archive from
them, then have debugserver and the
beanz added a comment.
Please revert your patch. It is *not* the right solution and is masking
underlying problems.
ExecutionEngine headers directly reference symbols from RuntimeDyld, so we
should enforce a requirement that anyone using ExeuctionEngine also link
RuntimeDyld. This works today
beanz added a comment.
This is definitely not the right fix. Please revert.
ExecutionEngine's LLVMBuild.txt file explicitly lists that RuntimeDyld is a
required library of ExecutionEngine (as well as a few others). LLVM's CMake
should be connecting that as an explicit dependency, and for some
beanz created this revision.
Herald added a subscriber: emaste.
This patch adds IPv6 support to LLDB/Host's TCP socket implementation.
Supporting IPv6 involved a few significant changes to the implementation of the
socket layers, and I have performed some significant code cleanup along the way.
beanz updated this revision to Diff 94526.
beanz added a comment.
Fixing variable naming conventions
https://reviews.llvm.org/D31357
Files:
tools/debugserver/source/CMakeLists.txt
tools/debugserver/source/MacOSX/CMakeLists.txt
unittests/CMakeLists.txt
beanz created this revision.
This patch adds a new wrapper for getaddrinfo which returns a std::vector of
SocketAddresses. While this patch doesn't add any uses of the new function, I
have two separable patches that are dependent on this, so I put it in its own
patch.
beanz updated this revision to Diff 94925.
beanz added a comment.
Actually uploading the updates this time...
https://reviews.llvm.org/D31969
Files:
cmake/modules/LLDBConfig.cmake
include/lldb/Host/Config.h
include/lldb/Host/Config.h.cmake
include/lldb/Host/android/Config.h
beanz created this revision.
Herald added subscribers: mgorny, srhines, emaste.
This patch removes the hand maintained config files in favor of auto-generating
the config file. We will still need to maintain the defines for the Xcode
builds on Mac, but all CMake builds use the generated header
beanz updated this revision to Diff 94924.
beanz added a comment.
- Fixing installation to make sure CMake always installs the generated Config.h
- Removing LLDB_CONFIG_FCNTL_GETPATH_SUPPORTED becasue we really don't need it
https://reviews.llvm.org/D31969
Files:
beanz updated this revision to Diff 94928.
beanz added a comment.
Updating the hard coded Config.h
https://reviews.llvm.org/D31969
Files:
cmake/modules/LLDBConfig.cmake
include/lldb/Host/Config.h
include/lldb/Host/Config.h.cmake
include/lldb/Host/android/Config.h
beanz added a comment.
@labath, I could adapt this into the `MainLoop` class, but I would actually
want to change how that class hierarchy is implemented. Regardless of the event
handling/polling model you use much of the code is identical between the
classes. I'd rather get rid of
beanz updated this revision to Diff 94374.
beanz added a comment.
Some cleanup to the test case:
- Auto-select a port, which will make this more reliable
- Open listening sockets before the fork() so that the sockets are ready for
connection (this avoids a race)
- Put test logic into reusable
beanz added a reviewer: jingham.
beanz added a comment.
Adding Jim as a reviewer.
Jim, with the added comment about debugserver being Darwin-only, are you happy
with this patch?
https://reviews.llvm.org/D31357
___
lldb-commits mailing list
beanz added a comment.
My intention in this patch is not in any way to adversely impact the Xcode
project, which is the supported and documented way to build LLDB on OS X
(http://lldb.llvm.org/build.html#BuildingLldbOnMacOSX).
The goal of this patch is to support configuration time
beanz updated this revision to Diff 95024.
beanz added a comment.
Fixing up the include guard as per feedback from zturner, and fixing up an
install logic error that I spoted.
https://reviews.llvm.org/D31969
Files:
cmake/modules/LLDBConfig.cmake
include/lldb/Host/Config.h
beanz added a reviewer: clayborg.
beanz added a comment.
Adding Greg to the reviewers.
https://reviews.llvm.org/D31824
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
beanz added a reviewer: clayborg.
beanz added a comment.
Adding Greg to the reviewers list.
https://reviews.llvm.org/D31823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
beanz added inline comments.
Comment at: include/lldb/Host/SocketAddress.h:44-45
+
//
+ static std::vector GetAddressInfo(const char *hostname,
+ const
beanz updated this revision to Diff 95355.
beanz added a comment.
Herald added a subscriber: mgorny.
Updating to use MainLoop class, and refactor MainLoop class to operate on
Windows.
I've added error cases to the MainLoop class for functionality that is not
supported. Specifically non-socket
beanz updated this revision to Diff 95453.
beanz added a comment.
Removing code I accidentally left in that was from debugging, and moving some
duplicated code that @labath spotted out of the ifdef.
https://reviews.llvm.org/D31823
Files:
cmake/modules/LLDBConfig.cmake
beanz added a comment.
Can you please move this check into `HandleLLVMOptions.cmake`? By putting it
into a module that is vended as part of LLVM's packaging then LLVM subprojects
can have consistent settings when building out-of-tree.
This would enable @zturner's request to remove the
beanz updated this revision to Diff 95448.
beanz added a comment.
Updating patches to reflect feedback from zturner.
https://reviews.llvm.org/D31823
Files:
cmake/modules/LLDBConfig.cmake
include/lldb/Host/Config.h
include/lldb/Host/Config.h.cmake
include/lldb/Host/MainLoop.h
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.
This is awesome! Thanks!
https://reviews.llvm.org/D32357
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
beanz updated this revision to Diff 91640.
beanz added a comment.
Removing some extra changes that accidentally came along for the ride in my
initial upload.
https://reviews.llvm.org/D30918
Files:
tools/debugserver/debugserver.xcodeproj/project.pbxproj
beanz updated this revision to Diff 91643.
beanz added a comment.
Updates based on feedback from Jason and Zachary.
https://reviews.llvm.org/D30918
Files:
tools/debugserver/debugserver.xcodeproj/project.pbxproj
tools/debugserver/source/MacOSX/CMakeLists.txt
beanz created this revision.
Herald added a subscriber: mgorny.
The first Sandybridge iMacs with AVX support shipped in Spring 2011 with Snow
Leopard as their OS. Unfortunately due to a kernel bug debugging AVX code was
not really possible until 10.7.4.
The old code here checked the kernel
beanz added a subscriber: lhames.
beanz added a comment.
@mgorny, because of differences in linker semantics between Darwin and ELF, I
can't reproduce the failure you have locally. I think that the patch below
works around it in a more-portable way.
I've engaged with @lhames about an
beanz added a comment.
I will fix up the naming conventions. Switching back and forth between LLVM and
LLDB conventions has done a number on my brain.
The test cases only abort on the child process side of the fork calls. This
results in printing a stack trace, but it also will get captured as
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.
This looks good to me.
https://reviews.llvm.org/D32125
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.
This looks like a nice improvement. There are some formatting inconsistencies,
can you run clang-format?
https://reviews.llvm.org/D32600
___
beanz added a comment.
@mgorny I want to apologize here but I need to ask you not to commit this until
@chapuni and I can come to an agreement between this solution and
https://reviews.llvm.org/D36211. He ping'd me on IRC yesterday after I left for
the day, I'll try and get in touch with him
beanz added a comment.
In https://reviews.llvm.org/D34322#783532, @joerg wrote:
> My suggestion would be to just use the YAML writer for now and leave a
> comment to make it clear that this is really JSON only.
Our YAML parser can parse JSON because YAML is a superset of JSON, but I don't
beanz added a comment.
I get your perspective, but holding up this relatively small patch that fixes a
bug in existing code on an architectural disagreement seems like excessive bike
shedding. If we assume that JSON is required for the use case would you have
Kuba write a full JSON parser in
beanz accepted this revision.
beanz added a comment.
One small comment below. In general I agree with the thoughts here, and I think
that this is a huge step forward for testing the debug server components.
I also agree with Zachary in principal that it would be nice to come up with
lit-based
beanz added inline comments.
Comment at: unittests/tools/CMakeLists.txt:1
+if(UNIX AND NOT APPLE)
+ add_subdirectory(lldb-server)
labath wrote:
> beanz wrote:
> > labath wrote:
> > > This is not what I meant. The only targets (at least until we have
> > >
beanz added inline comments.
Comment at: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py:1418
"include")),
-'LD_EXTRAS': "-L%s -llldb" % lib_dir}
+'LD_EXTRAS': "-L%s/../lib -llldb
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.
This LGTM.
https://reviews.llvm.org/D36598
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
beanz added a comment.
Why not just add entitlement support to ‘llvm_codesign’? Or feed through extra
arguments?
https://reviews.llvm.org/D54352
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
beanz added inline comments.
Comment at: CMakeLists.txt:403
+set(LLVM_CODESIGNING_IDENTITY "" CACHE STRING
+ "Sign executables and dylibs with the given identity or skip if empty
(Darwin Only)")
+
sgraenitz wrote:
> Identity should be a string right?
Yep,
beanz added a comment.
I can certainly foresee other LLVM-based projects needing entitlements, so I am
very much in favor of adding `ENTITLEMENTS` and `FORCE` options onto
`llvm_codesign`.
https://reviews.llvm.org/D54352
___
lldb-commits mailing
beanz added inline comments.
Comment at: cmake/modules/AddLLVM.cmake:795
- llvm_codesign(${name})
+ llvm_codesign(TARGET ${name} ENTITLEMENTS ${ARG_ENTITLEMENTS} FORCE)
endmacro(add_llvm_executable name)
sgraenitz wrote:
> Would we want to pass `FORCE` to
beanz added a comment.
Sorry for being behind on this, but I don't think this is the right solution to
the problem.
I don't think we should ever pass `--force`. To avoid duplicate code signing
when using the Xcode generator we should set the
`XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY` target
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D54443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56606/new/
https://reviews.llvm.org/D56606
___
lldb-commits
beanz added a comment.
I really don't think this is the right solution.
`$` can be passed in as source files in the unit
test target, which should work just fine on CMake 3.4.x.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63544/new/
beanz added a comment.
It is a bit gross that Python does an @rpath install name, that is generally
against convention. I can see adding an option to add rpaths to liblldb making
sense. I certainly agree LLDB shouldn't be in the business of packaging
transitive dependencies.
In a broader
beanz added a comment.
@labath sorry for not replying on-list. I've actually been discussing offline
with @compnerd. If `llvm-config` is printing an absolute path, I'm concerned
about how that will interact with binary package distributions for the llvm-dev
packages. Not sure if @tstellar has
beanz added a comment.
I conferred off-list with @compnerd. He and I talked through my concerns, and I
believe this patch is fine as-is, so LGTM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70764/new/
https://reviews.llvm.org/D70764
beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.
I think this is good, and it has been sitting a while.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70764/new/
https://reviews.llvm.org/D70764
beanz added a comment.
`LLVM_LIBRARY_DIR` and `LLVM_LIBRARY_DIRS` aren’t intended to serve the same
purpose. The `S` in `DIRS` is intentional to convey that it is a list of
directories, not necessarily a single value, and it would definitely break many
uses of `LLVM_LIBRARY_DIR` if it was a
beanz accepted this revision.
beanz added a comment.
LGTM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117639/new/
https://reviews.llvm.org/D117639
___
lldb-commits mailing list
beanz added inline comments.
Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
float groupshared [[]] i = 12;
aaron.ballman wrote:
> philnik wrote:
> > Should this also get an extension warning/should
beanz added inline comments.
Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
float groupshared [[]] i = 12;
philnik wrote:
> beanz wrote:
> > aaron.ballman wrote:
> > > philnik wrote:
> > > > Should this
69 matches
Mail list logo
