[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

aleksandr.urakov added a comment. This doesn't look like the cause, the test fails for me without this patch... Here is my tests output for PDB folder: -- Testing: 10 tests, 8 threads -- FAIL: lldb :: SymbolFile/PDB/enums-layout.test (1 of 10) FAIL: lldb :: SymbolFile/PDB/typedefs.test (2

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

JDevlieghere added a comment. Looks good to me, modulo the inline test (or the current comments addressed). Thanks Shafik! https://reviews.llvm.org/D49271 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

labath added a comment. Could you also add a test case for this? I think it should be possible to test this via the gdb-client (`test/testcases/functionalities/gdb_remote_client/`) test suite. If I understood the previous comments correctly, you'll need to mock a server that sends a `thread-pcs

[Lldb-commits] [PATCH] D49368: Complete user-defined types from PDB symbol files

aleksandr.urakov created this revision. aleksandr.urakov added reviewers: asmith, zturner, labath, clayborg. Herald added a subscriber: JDevlieghere. This patch adds the implementation of an UDT completion from PDB symbol files. For now it supports different UDT kinds (union, struct and class), s

[Lldb-commits] [PATCH] D49307: Fix some crashes and deadlocks in FormatAnsiTerminalCodes

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks straight-forward enough. Comment at: unittests/Utility/AnsiTerminalTest.cpp:18 + std::string format = ansi::FormatAnsiTerminalCodes(""); + EXPECT_STREQ("", forma

Re: [Lldb-commits] [lldb] r336991 - Add abbreviated name for Debugger::EventHandlerThread.

On Fri, 13 Jul 2018 at 18:36, Jim Ingham via lldb-commits wrote: > > There's code in the ThreadHandler to handle systems with short thread names. > If that isn't producing readable names, we should fix it there. A better > algorithm might be to drop the leading "lldb" and then instead of trunc

[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support

labath added a comment. We're also trying to avoid adding new clang-specific code to the debugger core. I think it would make more sense if the (clang-based) c++ highlighter was provided by some plugin. I see a couple of options: - the c++ language plugin: I think this is the most low-level plu

[Lldb-commits] [PATCH] D49377: Move pretty stack trace printer into driver.

JDevlieghere created this revision. JDevlieghere added reviewers: jingham, labath, zturner. Herald added a reviewer: jfb. We used to have a pretty stack trace printer in SystemInitializerCommon. This was disabled on Apple because we didn't want the library to be setting signal handlers, as this

[Lldb-commits] [lldb] r337173 - Fix TestDataFormatterUnordered for older libc++ versions

Author: labath Date: Mon Jul 16 07:37:58 2018 New Revision: 337173 URL: http://llvm.org/viewvc/llvm-project?rev=337173&view=rev Log: Fix TestDataFormatterUnordered for older libc++ versions clang recently started diagnosing "exception specification in declaration does not match previous declarati

[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server

labath added a comment. I think this is fine (though the meaning of SKIP_LLDB_SERVER is subtly different than SKIP_DEBUGSERVER), but while looking at this I got an idea for a possible improvement. How do you currently convince lldb to use ds2 instead of lldb-server? Do you just set the LLDB_DE

[Lldb-commits] [PATCH] D49377: Move pretty stack trace printer into driver.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I think this makes perfect sense. Could you also add the same thing to the other binaries in the tools subfolder? https://reviews.llvm.org/D49377 __

[Lldb-commits] [PATCH] D49377: Move pretty stack trace printer into driver.

JDevlieghere updated this revision to Diff 155684. JDevlieghere added a comment. Herald added a subscriber: ki.stfu. I've added it to the tools that made sense to me. Let me know if I missed something obvious. https://reviews.llvm.org/D49377 Files: source/Initialization/SystemInitializerComm

[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID

labath added a comment. Could you please add a test for the new behavior as well? https://reviews.llvm.org/D48865 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49038: [CMake] Give lldb tools functional install targets when building LLDB.framework

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Makes sense to me. https://reviews.llvm.org/D49038 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

labath added a comment. I don't agree with the two-stage initialization of the MinidumpParser class being introduced here. We deliberately introduced the `Create` static function to avoid this. If this `Initialize` function in checking invariants which are assumed to be hold by other parser met

[Lldb-commits] [lldb] r337188 - Fix typo in find-basic-function test

Author: labath Date: Mon Jul 16 09:18:52 2018 New Revision: 337188 URL: http://llvm.org/viewvc/llvm-project?rev=337188&view=rev Log: Fix typo in find-basic-function test Wrong FileCheck header meant that we were not matching what we should. This allows us to get rid of the -allow-deprecated-dag-

[Lldb-commits] [PATCH] D49307: Fix some crashes and deadlocks in FormatAnsiTerminalCodes

teemperor updated this revision to Diff 155706. teemperor added a comment. - Removed temp string variables. https://reviews.llvm.org/D49307 Files: include/lldb/Utility/AnsiTerminal.h unittests/Utility/AnsiTerminalTest.cpp unittests/Utility/CMakeLists.txt Index: unittests/Utility/CMakeLi

[Lldb-commits] [lldb] r337189 - Fix some crashes and deadlocks in FormatAnsiTerminalCodes

Author: teemperor Date: Mon Jul 16 09:38:30 2018 New Revision: 337189 URL: http://llvm.org/viewvc/llvm-project?rev=337189&view=rev Log: Fix some crashes and deadlocks in FormatAnsiTerminalCodes Summary: This patch fixes a few problems with the FormatAnsiTerminalCodes function: * It does an infin

[Lldb-commits] [PATCH] D49307: Fix some crashes and deadlocks in FormatAnsiTerminalCodes

This revision was automatically updated to reflect the committed changes. Closed by commit rL337189: Fix some crashes and deadlocks in FormatAnsiTerminalCodes (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D4930

[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support

teemperor added a comment. @zturner We can migrate the existing AnsiTerminal.h to reuse the LLVM coloring backend. This way we fix also the code that already uses this convenient interface. @labath I think we can add to the Language class the option to add its related syntax highlighting suppo

[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

stella.stamenova added a comment. I'll spend some time looking into this today, but with commit 0fa537f42f1af238c74bf41998dc1af31195839a variables.test passes. Then with commit d9899ad86e0a9b05781015cacced1438fcf70343, the test fails. There are clearly a couple of other commits in that range, b

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

The problem is not returning an error from Minidump::Create() - if that was the case this could easily be improved indeed. The two-phase initialization is a consequence of the LLDB plugin lookup: 1. Target::CreateProcess() calls Process::FindPlugin() 2. ProcessMinidump::CreateInstance() then has t

[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

lemo added subscribers: amccarth, bgianfo, labath, penryu. lemo added a comment. The problem is not returning an error from Minidump::Create() - if that was the case this could easily be improved indeed. The two-phase initialization is a consequence of the LLDB plugin lookup: 1. Target::CreatePro

[Lldb-commits] [PATCH] D49309: No longer pass a StringRef to the Python API

dblaikie added subscribers: teemperor, dblaikie. dblaikie added a comment. If the implementation of a function requires a string - it should probably accept string (not a StringRef) as a parameter - to avoid back-and-forth conversions in cases where the caller already has a string to use. Repos

Re: [Lldb-commits] [PATCH] D49309: No longer pass a StringRef to the Python API

If the implementation of a function requires a string - it should probably accept string (not a StringRef) as a parameter - to avoid back-and-forth conversions in cases where the caller already has a string to use. On Fri, Jul 13, 2018 at 12:43 PM Stella Stamenova via Phabricator via llvm-commits

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

Ok, I see what you mean now. Looking at the other core file plugins (elf, mach-o), it looks like they do only very basic verification before the accept the file. The pretty much just check the magic numbers, which would be more-or-less equivalent to our `MinidumpHeader::Parse` call. As this does n

Re: [Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

Fwiw I’ve seen cases where tests have passed even though they shouldn’t have — the functionality being tested was broken. The one that comes to mind was where we were doing a backtrace and then checking that it matched the regex “main\(argc=3” to make sure the local variable argc had the correct va

[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

zturner added a comment. Fwiw I’ve seen cases where tests have passed even though they shouldn’t have — the functionality being tested was broken. The one that comes to mind was where we were doing a backtrace and then checking that it matched the regex “main\(argc=3” to make sure the local variab

[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server

xiaobai added a comment. In https://reviews.llvm.org/D49282#1163517, @labath wrote: > I think this is fine (though the meaning of SKIP_LLDB_SERVER is subtly > different than SKIP_DEBUGSERVER), but while looking at this I got an idea for > a possible improvement. How is it subtly different? Ad

[Lldb-commits] [lldb] r337202 - [CMake] Give lldb tools functional install targets when building LLDB.framework

Author: xiaobai Date: Mon Jul 16 12:19:56 2018 New Revision: 337202 URL: http://llvm.org/viewvc/llvm-project?rev=337202&view=rev Log: [CMake] Give lldb tools functional install targets when building LLDB.framework Summary: This change makes the install targets for lldb tools functional when build

[Lldb-commits] [PATCH] D49038: [CMake] Give lldb tools functional install targets when building LLDB.framework

This revision was automatically updated to reflect the committed changes. Closed by commit rL337202: [CMake] Give lldb tools functional install targets when building LLDB.framework (authored by xiaobai, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://review

[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server

labath added a comment. In https://reviews.llvm.org/D49282#1163853, @xiaobai wrote: > In https://reviews.llvm.org/D49282#1163517, @labath wrote: > > > I think this is fine (though the meaning of SKIP_LLDB_SERVER is subtly > > different than SKIP_DEBUGSERVER), but while looking at this I got an i

[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

stella.stamenova added a comment. The CHECK-SAME expression on line 10 can no longer find the expected string in the output. This is due to an extra `location = DW_OP_addr(000140004114) ,` in the output between the two expected strings `CHECK-SAME: scope = global, external`, so it looks lik

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

That sounds reasonable to me. I'll make a note to revisit this (I don't the the cycles to do it right away but I'm planning a few more changes in the area soon). On Mon, Jul 16, 2018 at 10:36 AM, Pavel Labath wrote: > Ok, I see what you mean now. > > Looking at the other core file plugins (elf,

[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server

xiaobai added a comment. In https://reviews.llvm.org/D49282#1164050, @labath wrote: > In https://reviews.llvm.org/D49282#1163853, @xiaobai wrote: > > > In https://reviews.llvm.org/D49282#1163517, @labath wrote: > > > > > I think this is fine (though the meaning of SKIP_LLDB_SERVER is subtly > >

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik updated this revision to Diff 155758. shafik added a comment. Refactoring test to use lldbinline https://reviews.llvm.org/D49271 Files: lldb.xcodeproj/project.pbxproj packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile package

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

shafik marked 5 inline comments as done and 3 inline comments as done. shafik added a comment. @jingham @davide I believe I have addressed all your comments https://reviews.llvm.org/D49271 ___ lldb-commits mailing list lldb-commits@lists.llvm.org ht

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

jasonmolenda added a subscriber: ramana-nvr. jasonmolenda added a comment. That's a good point Pavel. I tried to write one (below) but I never saw what the original failure mode was. Venkata, can you help to make a test case that fails before the patch and works after? Or explain what bug was

Re: [Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

That's a good point Pavel. I tried to write one (below) but I never saw what the original failure mode was. Venkata, can you help to make a test case that fails before the patch and works after? Or explain what bug was being fixed exactly? I could see that the code was wrong from reading it,

[Lldb-commits] [PATCH] D49406: Invert dependency between lldb-framework and lldb-suite

xiaobai created this revision. xiaobai added reviewers: sas, labath. Herald added a subscriber: mgorny. Currently, if you build lldb-framework the entire framework doesn't actually build. In order to build the entire framework, you need to actually build lldb-suite. This abstraction doesn't feel q

[Lldb-commits] [PATCH] D49406: Invert dependency between lldb-framework and lldb-suite

xiaobai updated this revision to Diff 155790. xiaobai added a comment. Accidentally merged the contents of two commits into one. Removing the contents of one of the commits from this one. https://reviews.llvm.org/D49406 Files: CMakeLists.txt cmake/modules/LLDBFramework.cmake source/API/C

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

asmith created this revision. asmith added reviewers: lldb-commits, aleksandr.urakov, rnk, zturner. Herald added a subscriber: llvm-commits. This is an alternative to https://reviews.llvm.org/D49368 Repository: rL LLVM https://reviews.llvm.org/D49410 Files: lit/SymbolFile/PDB/class-layout.

[Lldb-commits] [PATCH] D49411: Move from StringRef to std::string in the ScriptInterpreter API

teemperor created this revision. teemperor added a reviewer: dblaikie. After https://reviews.llvm.org/D49309 it became clear that we always need a null-terminated string (for the Python API), so we might as well change the API to accept an std::string& instead of taking a StringRef and then alway

[Lldb-commits] [PATCH] D49411: Move from StringRef to std::string in the ScriptInterpreter API

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Seems good to me - though I haven't looked too closely/don't know this code terribly well (so you're welcome to wait if you know someone else more knowledgable might take a look too - or i