[Lldb-commits] [PATCH] D32148: [Utility/StringLexer] Remove dead code.

2017-04-19 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Thanks! https://reviews.llvm.org/D32148 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32148: [Utility/StringLexer] Remove dead code.

2017-04-19 Thread Davide Italiano via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL300729: [Utility/StringLexer] Remove dead code. (authored by davide). Changed prior to commit: https://reviews.llvm.org/D32148?vs=95519=95788#toc Repository: rL LLVM https://reviews.llvm.org/D32148

[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning

2017-04-20 Thread Davide Italiano via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL300845: [Utility] Placate another GCC warning. (authored by davide). Changed prior to commit: https://reviews.llvm.org/D32137?vs=95485=95961#toc Repository: rL LLVM https://reviews.llvm.org/D32137

[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning

2017-04-20 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D32137#731727, @labath wrote: > Thanks for digging into this, I've learned something new today. Fixing this > with a cast seems reasonable then. Me too, apparently C++ integer promotion is less obvious than I thought ;)

[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning

2017-04-19 Thread Davide Italiano via Phabricator via lldb-commits
davide added a subscriber: nlewycky. davide added a comment. And I was wrong. @nlewycky explained on IRC: 14:23 < nlewycky> gcc is correct, in 'char + char' each char gets promoted to 'int' first then summed, then you've got an unsigned int == int comparison 14:23 < nlewycky> uint8_t and

[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning

2017-04-19 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Reference for the future (http://www.open-std.org/jtc1/sc22/open/n2356/conv.html [conv.prom1]) https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning

2017-04-19 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. I reduced it: https://gist.github.com/dcci/01c10b405041fa8d139a4f71aec102f7 https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D32148: [Utility/StringLexer] Remove dead code.

2017-04-17 Thread Davide Italiano via Phabricator via lldb-commits
davide created this revision. https://reviews.llvm.org/D32148 Files: include/lldb/Utility/StringLexer.h source/Utility/StringLexer.cpp Index: source/Utility/StringLexer.cpp === --- source/Utility/StringLexer.cpp +++

[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning

2017-04-17 Thread Davide Italiano via Phabricator via lldb-commits
davide created this revision. For reference/discussion. GCC complains about signed-vs-unsigned comparison. I'm actually surprised that `m_registers_count` is a `signed` integer, as I can hardly imagine a negative register count. I'm under the impression that we could change `m_register_count`

[Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

2017-09-04 Thread Davide Italiano via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL312501: [ABI] Rewrite RegisterIsCalleeSaved. (authored by davide). Changed prior to commit: https://reviews.llvm.org/D37420?vs=113675=113790#toc Repository: rL LLVM https://reviews.llvm.org/D37420

[Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

2017-09-02 Thread Davide Italiano via Phabricator via lldb-commits
davide created this revision. The goal of this patch is twofold: First, it removes a wrong comment (at least, not correctly describing what the function does). Then, it rewrites the function to use a stringswitch where the registers are enumerated explicitly instead of being computed

[Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

2017-09-02 Thread Davide Italiano via Phabricator via lldb-commits
davide added inline comments. Comment at: source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:1916 + std::string Name = std::string(reg_info->name); + bool IsCalleeSaved = llvm::StringSwitch(Name) +.Cases("r12", "r13", "r14", "r15", Currently this uses two

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL316393: [lldbtests] Handle errors instead of crashing. (authored by davide). Changed prior to commit: https://reviews.llvm.org/D39199?vs=119917=119965#toc Repository: rL LLVM

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I was going to propose the same :) Maybe an heads up on lldb-dev? Thanks! https://reviews.llvm.org/D39215 ___ lldb-commits mailing list

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. I think we should just use the `_COMPILER` variant and default to the clang in tree (wheter possible) https://reviews.llvm.org/D39215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Apologies, I generally do (but I forgot this time), let me update the patch. Repository: rL LLVM https://reviews.llvm.org/D39199 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide updated this revision to Diff 119917. https://reviews.llvm.org/D39199 Files: packages/Python/lldbsuite/test/dotest.py Index: packages/Python/lldbsuite/test/dotest.py === --- packages/Python/lldbsuite/test/dotest.py +++

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide added inline comments. Comment at: packages/Python/lldbsuite/test/dotest.py:52 def is_exe(fpath): +"""Returns true if fpath is an executable. clayborg wrote: > We could add a default parameter here like: > > ``` > def is_exe(fpath, fatal=False): >

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. If there are cases where `is_exe` shouldn't be fatal, then we might consider splitting the utility into two bits (exists & is_exe). In my mind, if you're checking whether something is executable you should run the check on a valid entity, and it's up to the caller

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D39199#904169, @zturner wrote: > I just wanted to make sure nobody was *already* relying on that behavior. If > we can get away with being strict, we should be strict. I agree. From a quick skim I don't see anything really relying on that,

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide added inline comments. Comment at: packages/Python/lldbsuite/test/dotest.py:56 +sys.exit(-1) return os.path.isfile(fpath) and os.access(fpath, os.X_OK) lemo wrote: > minor: can we print quotes around fpath? (this usually helps in messages to

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

2017-10-23 Thread Davide Italiano via Phabricator via lldb-commits
davide created this revision. If you pass an invalid compiler/debugger path on the cmdline to `dotest.py` this is what you get. $ python dotest.py --executable /home/davide/work/build-lldb/bin/lldb --compiler /home/davide/work/build-lldb/bin/clandasfaasdfsg Traceback (most recent call

[Lldb-commits] [PATCH] D40519: Remove some duplicated code in UUID.cpp

2017-11-27 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. LGTM https://reviews.llvm.org/D40519 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-11-29 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. This one is a little weird when written as unittest. Not the worst thing, but I agree it should use `llvm-lit`. Can you give it a shot, Pavel? If that doesn't work, we should at

[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-11-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Another very good reason for writing a `FileCheck` test rather than a unittest is that writing unittest is tedious :) In particular for new contributors, FileCheck tests are much easier to write and in this case testing the API surface doesn't seem to add much value.

[Lldb-commits] [PATCH] D40475: DWZ 12/12: DWZ test mode

2017-11-30 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. @jankratochvil I run Fedora for now on my desktop, but I don't have time and resources to devote to a buildbot. IMHO, having a fedora bot would be a great thing for two reasons: 1. I found out Fedora exposes bugs that ubuntu doesn't (e.g. in symbol resolution, due to

[Lldb-commits] [PATCH] D40636: Create a trivial lldb-test program

2017-11-30 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. This LGTM. I think we can iterate in tree from what we have. https://reviews.llvm.org/D40636 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

2017-11-27 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Yes, what Zachary said. Thanks for the cleanup. There's a decent amount of code in lldb that can be written in a very compact way but it's manually unrolled. I think in this case the code produced should be equivalent (and if not, I'd be curious to see the codegen).

[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

2017-11-27 Thread Davide Italiano via Phabricator via lldb-commits
davide added subscribers: vsk, aprantl, davide. davide added a comment. I thought about this, and it's the only patch from your patchset that I don't feel really confident about. I'm afraid it's a little risky to add more code to the objectfile reader without having proper unittesting. In

[Lldb-commits] [PATCH] D40536: Simplify UUID constructors

2017-11-27 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D40536 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D41169: ObjectFile: remove ReadSectionData/MemoryMapSectionData mutual recursion

2017-12-13 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D41169 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

2017-11-06 Thread Davide Italiano via Phabricator via lldb-commits
davide added a subscriber: zturner. davide added a comment. Not sure lld is the default? I think I always build with bfd (or gold). I'll check later today when I'm in the office. I'm not against this change per se, but adding another dependency seems annoying. cc: @zturner

[Lldb-commits] [PATCH] D41008: MainLoop: avoid infinite loop when pty slave gets closed

2017-12-08 Thread Davide Italiano via Phabricator via lldb-commits
davide added subscribers: jingham, davide. davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM. @jingham ? I'll try this change on macOS to make sure it won't break anything. https://reviews.llvm.org/D41008

[Lldb-commits] [PATCH] D40757: Disable warnings related to anonymous types

2017-12-02 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM, I stumbled upon the same issue :) An alternative would be that of naming (as you suggested), but if this is consistent, no worries. https://reviews.llvm.org/D40757

[Lldb-commits] [PATCH] D40757: Disable warnings related to anonymous types in the ObjC plugin

2017-12-07 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. LGTM (again) https://reviews.llvm.org/D40757 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

2017-12-11 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D40616#951256, @labath wrote: > @davide: Any thoughts on `.yaml` as a test file suffix? I think this is fine. https://reviews.llvm.org/D40616 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D41101: [testsuite] Remove testing failures vestiges

2017-12-11 Thread Davide Italiano via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL320444: [testsuite] Remove testing failures vestiges. (authored by davide). Changed prior to commit: https://reviews.llvm.org/D41101?vs=126478=126479#toc Repository: rL LLVM

[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Davide Italiano via Phabricator via lldb-commits
davide added subscribers: vsk, aprantl. davide added a comment. I really like this approach. I think it's going to expose a large amount of bugs, and probably facilitate the transition in case we want to move to the LLVM readers for this. @aprantl / @vsk , what do you think?

[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test

2017-12-01 Thread Davide Italiano via Phabricator via lldb-commits
davide added inline comments. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2764-2765 switch (reloc_type(rel)) { case R_386_32: case R_386_PC32: default: It's unclear to me why PC-rel and 32-bit abs rel are not

[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Thanks, I'll try this patch tomorrow. I know this may be a little off, but how hard is to write a test for this so that it doesn't regress? https://reviews.llvm.org/D39307 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Thanks for taking care of the test. About the libmath question. I thought libmath exported a symbol named `a` ,but I haven't checked the `readelf` output. This is what I saw for the symbols: (lldb) image lookup --address 0x777ec290 Address:

[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Sorry, my bad, I haven't grepped properly, there are 3 internal symbols named `a` in `libm.so`. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2017-10-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a subscriber: rsmith. davide added a comment. Richard Smith (@rsmith) owns clang and is familiar with this. https://reviews.llvm.org/D39307 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

2018-04-27 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. This is fine, I'll commit it for you today. https://reviews.llvm.org/D45628 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D45628: [LLDB] Support GNU-style compressed debug sections (.zdebug)

2018-05-11 Thread Davide Italiano via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL332162: [LLDB] Support GNU-style compressed debug sections (.zdebug) (authored by davide, committed by ). Changed prior

[Lldb-commits] [PATCH] D46529: Add support to object files for accessing the .debug_types section

2018-05-08 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. Thanks for taking the time to split this! https://reviews.llvm.org/D46529 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47014: Fix _NSCFBoolean data formatter.

2018-05-17 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D47014 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

2018-05-22 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Looks fine to me, but let's wait for Pavel. Are you back working on lldb ? :) https://reviews.llvm.org/D47232 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

2018-05-18 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Can you commit the whitespace fixes separately? Repository: rL LLVM https://reviews.llvm.org/D47062 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48114: Add dataformatter for NSDecimalNumber

2018-06-12 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I didn't check whether the representation was correct (although it looks lo). The structure of the patch looks fine to me, thanks for adding the comments :) Repository: rL LLVM

[Lldb-commits] [PATCH] D48096: Disable warnings for the generated LLDB wrapper source

2018-06-12 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Unfortunate, but this seems reasonable. lg. https://reviews.llvm.org/D48096 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D48084: [FileSpec] Delegate common operations to llvm::sys::path

2018-06-12 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. LGTM with a nit. Comment at: source/Utility/FileSpec.cpp:18-21 +#include "llvm/ADT/Triple.h" // for Triple +#include "llvm/ADT/Twine.h" // for Twine +#include "llvm/Support/ErrorOr.h" // for ErrorOr #include

[Lldb-commits] [PATCH] D48500: [DWARFASTParser] Remove special cases for `llvm-gcc`

2018-06-22 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. I think it's fair and correct supporting everything we can, but I guess `llvm-gcc` is largely dead at this point. https://reviews.llvm.org/D48500 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D48500: [DWARFASTParser] Remove special cases for `llvm-gcc`

2018-06-22 Thread Davide Italiano via Phabricator via lldb-commits
davide created this revision. davide added reviewers: friss, aprantl, clayborg, labath. Herald added a subscriber: JDevlieghere. To the best of my understanding modern compilers handle all these cases correctly. So, I think this is basically dead code. https://reviews.llvm.org/D48500 Files:

[Lldb-commits] [PATCH] D46395: Remove Process references from the Host module

2018-05-03 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. lg https://reviews.llvm.org/D46395 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

2017-10-26 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D39314#908115, @sas wrote: > In https://reviews.llvm.org/D39314#908065, @davide wrote: > > > can you please try adding a test for this one? :) > > > To be fully honest, I'm not 100% sure how I'm supposed to add tests for this. > I looked

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

2017-10-26 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. can you please try adding a test for this one? :) https://reviews.llvm.org/D39314 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-26 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D39315#908246, @sas wrote: > Same thing here, I have no idea how to go about testing something specific > like this. Given that this is Windows Phone, it's even worse than simply > Windows Desktop because the only way to test is by doing

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-26 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision. davide added a comment. This change should be unit-testable, no? https://reviews.llvm.org/D39315 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D41871: [dotest] Remove crashinfo hook

2018-01-09 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. No objections from me. Thanks. https://reviews.llvm.org/D41871 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D41824: Remove use of private header in LLDB

2018-01-08 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Opened https://bugs.llvm.org/show_bug.cgi?id=35857 Repository: rL LLVM https://reviews.llvm.org/D41824 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D41902: Remove Platform references from the Host module

2018-01-10 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. This LGTM but please wait for a second opinion. https://reviews.llvm.org/D41902 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D41902: Remove Platform references from the Host module

2018-01-10 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D41902#972619, @zturner wrote: > In https://reviews.llvm.org/D41902#972614, @clayborg wrote: > > > As long as: > > > > % lldb /path/to/Foo.app > > (lldb) r > > > > > > Still works, then I am fine with this. The resolve executable should

[Lldb-commits] [PATCH] D42182: Add LLDB_LOG_ERROR (?)

2018-01-17 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. I think this is fine, but I'll defer to Zachary. https://reviews.llvm.org/D42182 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42264: Update lldb architecture doc

2018-01-18 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Do you have commit rights or you need somebody to commit this on your behalf? https://reviews.llvm.org/D42264 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42264: Update lldb architecture doc

2018-01-18 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM, ideally this file should be autogenerated. https://reviews.llvm.org/D42264 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42281: WIP: compile the LLDB tests out-of-tree

2018-01-19 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42281#981973, @aprantl wrote: > In https://reviews.llvm.org/D42281#981969, @clayborg wrote: > > > Looks like a good start. It might be nice to validate that after "clean" > > that we have no files that are untracked in the test directory?

[Lldb-commits] [PATCH] D41702: Add SysV Abi for PPC64le

2018-01-19 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. lgtm, feel free to go ahead. https://reviews.llvm.org/D41702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

2018-01-20 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D40283#983057, @nelhage wrote: > I did a little bit of looking into performance implications today. It looks > like `DWARFASTParserClang::ParseTypeFromDWARF` is only called lazily as > symbols are needed, which alleviates some of my concerns,

[Lldb-commits] [PATCH] D42348: Prevent unaligned memory read in parseMinidumpString

2018-01-21 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. I'll be happy to review all your patch set tomorrow, Raphael. Do you mind to add me as reviewer so I don't lose track of your work? https://reviews.llvm.org/D42348 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42488: Remove ObjectFile usage from HostLinux::GetProcessInfo

2018-01-24 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. Thanks for the spring cleaning! https://reviews.llvm.org/D42488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

2018-01-25 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42563#988700, @alexshap wrote: > @davide - the test case is in the description but i can try to add it to the > test suite. yes, please. Repository: rL LLVM https://reviews.llvm.org/D42563

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

2018-01-25 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. Testcase? Repository: rL LLVM https://reviews.llvm.org/D42563 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

2018-01-17 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. I think this one is reasonable, but please wait for another opinion before committing (and write a test). Pavel touched this code very recently and added tests for this so it should

[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

2018-01-17 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42206#979566, @vsk wrote: > Why not take an approach similar to the one in > https://reviews.llvm.org/D41008? It looks like it's possible to set up a poll > loop, call signal(), and verify that the loop is still running. Yes, that was what

[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

2018-01-17 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42206#979570, @jasonmolenda wrote: > I tried sending signals to lldb-server via kill() and the signal handler > caught them, the bit of code I had printing out the return value & errno > value never got executed. The only way I was able to

[Lldb-commits] [PATCH] D42210: Re-enable logging on the child side of a fork() in lldb-server platform mode

2018-01-17 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. I took a look and I think this is a sensible change, but we should really test it. Repository: rL LLVM https://reviews.llvm.org/D42210

[Lldb-commits] [PATCH] D42215: [CMake] Make check-lldb work with LLDB_CODESIGN_IDENTITY=''

2018-01-17 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM with Adrian's suggestion applied, thanks for doing this! https://reviews.llvm.org/D42215 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

2018-01-12 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D41997#974957, @clayborg wrote: > Very cool and close. It would be nice to function correctly without asserts, > see inlined comment. Out of curiosity, why does `lldb` roll its own assertion() mechanism instead of using the standard one?

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42656#991065, @jingham wrote: > There are a whole bunch of other tests that test completion in this file that > use the exact same mechanism but don't seem to be flakey. Why is this one > test flakey? So, I take a look at this to reply to

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42656#991239, @davide wrote: > In https://reviews.llvm.org/D42656#991065, @jingham wrote: > > > There are a whole bunch of other tests that test completion in this file > > that use the exact same mechanism but don't seem to be flakey. Why

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42656#991065, @jingham wrote: > There are a whole bunch of other tests that test completion in this file that > use the exact same mechanism but don't seem to be flakey. Why is this one > test flakey? > > If for instance it's because "Fo"

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42656#991065, @jingham wrote: > There are a whole bunch of other tests that test completion in this file that > use the exact same mechanism but don't seem to be flakey. Why is this one > test flakey? > > If for instance it's because "Fo"

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

2018-01-30 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a subscriber: aprantl. davide added a comment. This revision is now accepted and ready to land. This looks good. Feel free to go ahead and commit, but please coordinate with @aprantl as he just landed his changes for the testsuite (so you might need to

[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images

2018-02-02 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42870#996899, @clayborg wrote: > Probably take a ELF file that is NetBSD and obj2yaml it. The test would run > yaml2obj on it and then test that things are recognized correctly via the SB > interfaces (check triple is correct)? The SBApi

[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images

2018-02-02 Thread Davide Italiano via Phabricator via lldb-commits
davide added a subscriber: zturner. davide added a comment. In https://reviews.llvm.org/D42870#996913, @krytarowski wrote: > Is there a working example of this? I would clone an existing code for Linux > or other supported OS and adapt it for NetBSD. > > Please note that I'm in the process of

[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images

2018-02-02 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. Please add a test case. Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

2018-01-28 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. As there are no strong objections, I'm going to check this in tomorrow PST and see how it goes. Thanks for your contribution. https://reviews.llvm.org/D40283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42620: [lldb] Silence signed <-> unsigned integer comparison warning

2018-01-28 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. LGTM. `unsigned` is probably fine as well. Do you need somebody to commit this on your behalf? https://reviews.llvm.org/D42620 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

2018-01-29 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42656#991284, @zturner wrote: > If we just need to test completion, write a lit-style test that uses > lldb-test that looks like this: > > RUN: lldb-test complete --target=%T/foo --complete_str=MyPrefix | FileCheck > %s > > CHECK:

[Lldb-commits] [PATCH] D42912: Sync PlatformNetBSD.cpp with Linux

2018-02-05 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. LGTM. I always found supercumbersome having to check `if log()` and error prone. Repository: rL LLVM https://reviews.llvm.org/D42912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D42914: Rewrite TestTargetSymbolsBuildidCase to be more focused

2018-02-05 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D42914#997974, @zturner wrote: > Yea this seems like a good candidate for an lldb-test test. Something like > this. > > RUN: yaml2obj %S/Inputs/stripped.yaml > %t.stripped.out > RUN: yaml2obj %S/Inputs/unstripped.yaml > >

[Lldb-commits] [PATCH] D43024: [test] Enable test category for inline tests.

2018-02-07 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. thanks for fixing this. Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:526 return None - + header = os.path.join(

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

2018-02-07 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In https://reviews.llvm.org/D43048#1001293, @davide wrote: > In https://reviews.llvm.org/D43048#1001287, @jingham wrote: > > > The current auto-completer tests aren't interactive - they do exactly the > > same thing your command does, but from Python. It's fine if you

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

2018-02-07 Thread Davide Italiano via Phabricator via lldb-commits
davide created this revision. davide added reviewers: aprantl, vsk, friss, labath, zturner, jingham, jasonmolenda. This is an experiment to improve out lldb testing capabilities and making them more similar to the one used in LLVM. Example: davide@Davidinos-Mac-Pro ~/w/l/b/bin> ./lldb-test

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

2018-02-07 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. You can take a look at the `test/testcases/functionalities/completion/TestCompletion.py` for the python equivalent. I find the potential FileCheck'ed version much easier to read/write/understand. I'm possibly biased having worked many years on LLVM, hence I'm asking for

[Lldb-commits] [PATCH] D43059: Add implementation for MSVC in CPlusPlusLanguage::IsCPPMangledName

2018-02-07 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Can you add a unittest for this? :) Repository: rL LLVM https://reviews.llvm.org/D43059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42914: Rewrite TestTargetSymbolsBuildidCase to be more focused

2018-02-05 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. Nice :) This looks already fine as-is, but I wonder whether we can get rid of the python boilerplate altogether? There has been quite a bit of discussion about using `lldb-test` for this sort of more focused testing, so I wonder whether you gave it a try? (just a random

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

2018-02-09 Thread Davide Italiano via Phabricator via lldb-commits
davide added inline comments. Comment at: source/Target/Target.cpp:3949 + idx); + assert(option_value); + return option_value->GetCurrentValue(); aprantl wrote: > davide wrote: > > add an assertion

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

2018-02-09 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. LGTM modulo minor. Comment at: source/Plugins/ExpressionParser/Clang/CMakeLists.txt:26 clangCodeGen +clangDriver clangEdit aprantl wrote: > I checked and this does not affects LLDB's binary

  1   2   3   4   5   6   >