[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

ruiu accepted this revision. ruiu added a comment. This revision is now accepted and ready to land. LGTM I'd write a comment explaining why we are handling debug sections in a special way, but that can be done later. Please submit. Thank you for doing this! CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D60496: [lldb-server] Use std::thread/mutex for all platforms

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. LGTM, with some inline comments about additional c++11 goodies we can use to clean up this file further. (Also, it might be good to mention in the patch title that this is about modifying the

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for lldb-server on Windows

asmith marked 2 inline comments as done. asmith added inline comments. Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:1 //===-- main.cpp *- C++ -*-===// // labath wrote: > Could you (as a s

[Lldb-commits] [PATCH] D60496: [lldb-server] Use std::thread/mutex for all platforms

asmith created this revision. asmith added reviewers: zturner, labath, jfb, Hui. Herald added subscribers: lldb-commits, dexonsmith. Herald added a project: LLDB. Some cleanup suggested when bringing up lldb-server on Windows. Thanks to Hui Huang for the patch. Repository: rLLDB LLDB https:/

[Lldb-commits] [lldb] r358044 - [lldb-server] Introduce Socket::Initialize and Terminate to simply WSASocket setup

Author: asmith Date: Tue Apr 9 21:57:18 2019 New Revision: 358044 URL: http://llvm.org/viewvc/llvm-project?rev=358044&view=rev Log: [lldb-server] Introduce Socket::Initialize and Terminate to simply WSASocket setup Reviewers: zturner, labath Reviewed By: labath Subscribers: lldb-commits Tags

[Lldb-commits] [PATCH] D60440: [lldb-server] Introduce Socket::Initialize and Terminate to simply WSASocket setup

asmith updated this revision to Diff 194443. asmith added a comment. Addressed the review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60440/new/ https://reviews.llvm.org/D60440 Files: include/lldb/Host/Socket.h source/Host/common/Socket.cpp source/Initialization/System

[Lldb-commits] [PATCH] D60458: [zorg] Add lldb-x86_64-debian builder

gkistanova accepted this revision. gkistanova added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60458/new/ https://reviews.llvm.org/D60458 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cg

[Lldb-commits] [PATCH] D59015: [lldb-mi] Include full path in the -data-disassemble response

clayborg added inline comments. Comment at: lldb/tools/lldb-mi/MICmdCmdData.cpp:420 + std::unique_ptr pPathBuffer(new char[PATH_MAX]); + lineEntry.GetFileSpec().GetPath(pPathBuffer.get(), PATH_MAX); There is a variant of FileSpec::GetPath() that retur

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

shafik added inline comments. Comment at: lldb/include/lldb/Symbol/ClangASTImporter.h:247 +struct StdModuleScope { + StdModuleHandler m_handler; aprantl wrote: > Can you add a `///` one-liner for this class? This definitely needs a description, it took

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

jingham added a comment. In D60468#1460212 , @friss wrote: > In D60468#1460065 , @labath wrote: > > > No opinion on the patch, but what is the reason for having settings that > > are shared between multiple Debugge

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for lldb-server on Windows

asmith marked 4 inline comments as done. asmith added inline comments. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h:294 + + lldb_private::UUID m_uuid; }; labath wrote: > Move UUID handling into a separate patch. The UUID change is being discu

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for lldb-server on Windows

asmith marked 4 inline comments as done. asmith added inline comments. Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:16 + +#include "IDebugDelegate.h" +#include "ProcessDebugger.h" labath wrote: > It looks like IDebugDelegate.h is missi

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

friss added inline comments. Comment at: source/Expression/ExpressionSourceCode.cpp:181 + +if (var_name == ConstString("this") && wrapping_language == lldb::eLanguageTypeC_plus_plus ) continue; This should also trigger for eLanguageTypeObjC_plus_plus

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

shafik marked an inline comment as done. shafik added inline comments. Comment at: source/Expression/ExpressionSourceCode.cpp:172 + +// We can check for .block_descriptor w/o checking for langauge since this +// is not a valid identifier in either C or C++. --

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

shafik marked 3 inline comments as done. shafik added a comment. @friss I believe I have addressed your comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59960/new/ https://reviews.llvm.org/D59960 ___ lldb-commits mailing list lldb-com

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

shafik updated this revision to Diff 194377. shafik added a comment. Addressing comments: - Now applies to all languages not just C++ - When adding locals be more selective on filtering i.e. only filter self and _cmd for Objective C etc... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D59015: [lldb-mi] Include full path in the -data-disassemble response

anton.kolesov updated this revision to Diff 194376. anton.kolesov added a comment. Replaced static local variable (as can be found in CMICmnLLDBDebugSessionInfo::GetFrameInfo) with a unique_ptr to the char array (as done in CMICmnLLDBDebuggerHandleEvents::MiHelpGetModuleInfo). Repository: rL

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

friss added a comment. In D60468#1460065 , @labath wrote: > No opinion on the patch, but what is the reason for having settings that are > shared between multiple Debugger instances? My expectation was that the > debugger objects are completely independe

[Lldb-commits] [PATCH] D60325: [lldb] [Process/NetBSD] Fix wrongly mapping mm* registers

mgorny updated this revision to Diff 194363. mgorny added a comment. Updated 'REQUIRES'. I've looked through other LLVM tests, and noticed there's already 'native' concept (when host==target), and some 'target*' features for systems. I've chosen to combine the former with new 'target-x86'. I'll

Re: [Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

The Darwin failure reproduces for me when I just add Darwin to the supported list of platforms in the import-std-module tests and run them on macOS. The dwo failure can also be reproduced by just whitelisting it in the tests. This will cause the tests to fail for me on Arch Linux. I didn’t test if

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

jingham added a comment. In D60468#1460065 , @labath wrote: > No opinion on the patch, but what is the reason for having settings that are > shared between multiple Debugger instances? My expectation was that the > debugger objects are completely indepen

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

aprantl added a comment. Can you send me a mail with instructions to reproduce the failure? I'd like to take a look. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59537/new/ https://reviews.llvm.org/D59537 ___ lldb-commits mailing list lld

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

labath added a comment. No opinion on the patch, but what is the reason for having settings that are shared between multiple Debugger instances? My expectation was that the debugger objects are completely independent, and I would be surprised if the value of some setting changed from under me b

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

teemperor marked 2 inline comments as done. teemperor added a comment. Thanks for the review! Comment at: lldb/include/lldb/Symbol/ClangASTImporter.h:240 public: Minion(ClangASTImporter &master, clang::ASTContext *target_ctx, clang::ASTContext *source_ctx) -

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/ClangASTImporter.h:240 public: Minion(ClangASTImporter &master, clang::ASTContext *target_ctx, clang::ASTContext *source_ctx) Bonus points for coming up with a more descriptiv

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

friss added a comment. In D60468#1460013 , @clayborg wrote: > Almost seems like we can build the mutex into the base class OptionValue as > we need general threaded protection for every setting. They any function that > gets or sets the value should be a

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

clayborg added a comment. Almost seems like we can build the mutex into the base class OptionValue as we need general threaded protection for every setting. They any function that gets or sets the value should be able to protect itself using the base mutex CHANGES SINCE LAST ACTION https://r

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

friss created this revision. friss added a reviewer: clayborg. Before a Debugger gets a Target, target settings are routed to a global set of settings. Even without this, some part of the LLDB which exist independently of the Debugger object (the Module cache, the Symbol vendors, ...) access direc

[Lldb-commits] [PATCH] D60458: [zorg] Add lldb-x86_64-debian builder

stella.stamenova accepted this revision. stella.stamenova added a comment. This revision is now accepted and ready to land. yay! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60458/new/ https://reviews.llvm.org/D60458 ___ lldb-commits maili

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

JDevlieghere added a comment. In D55718#1453084 , @tatyana-krasnukha wrote: > Hi, currently we have a private build server that executes the test-suite on > ARC. There are failures for now, mostly due to unimplemented features for ARC > like expressions

[Lldb-commits] [PATCH] D60405: MinidumpYAML: Add support for ModuleList stream

clayborg added inline comments. Comment at: lib/ObjectYAML/MinidumpYAML.cpp:91 + size_t BlobAllocator::allocateString(StringRef Str) { SmallVector WStr; labath wrote: > clayborg wrote: > > Might be nice to unique the strings in here? > Yeah, that would be pos

[Lldb-commits] [PATCH] D60458: [zorg] Add lldb-x86_64-debian builder

labath created this revision. labath added reviewers: jankratochvil, stella.stamenova, zturner, gkistanova. Initially, the builder will be silent, because I expect noise from flaky tests. Once those have been dealt with, I'd like to switch on breakage notifications. https://reviews.llvm.org/D604

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

rocallahan marked an inline comment as done. rocallahan added inline comments. Comment at: lld/ELF/MarkLive.cpp:190-191 + // case we still need to mark the file. + if (S && !IsLSDA && Sec->File) +if (isa(Sec) || isa(Sec)) + Sec->getFile()->HasLiveCodeOrData = true; ---

[Lldb-commits] [PATCH] D60400: Remove unneeded #ifdef SWIGs

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB357984: Remove unneeded #ifdef SWIGs (authored by labath, committed by ). Herald added a subscriber: abidh. Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D60400?vs=1

[Lldb-commits] [PATCH] D60443: Fix doxygen warning about function argument

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB357983: Fix doxygen warning about function argument (authored by JDevlieghere, committed by ). Herald added a subscriber: abidh. Changed prior to commit: https://reviews.llvm.org/D60443?vs=194265&id=

[Lldb-commits] [lldb] r357984 - Remove unneeded #ifdef SWIGs

Author: labath Date: Tue Apr 9 02:03:43 2019 New Revision: 357984 URL: http://llvm.org/viewvc/llvm-project?rev=357984&view=rev Log: Remove unneeded #ifdef SWIGs Summary: Some of these were present in files which should never be read by swig (and we also had one in the interface file, which is on

[Lldb-commits] [lldb] r357983 - Fix doxygen warning about function argument

Author: jdevlieghere Date: Tue Apr 9 02:03:30 2019 New Revision: 357983 URL: http://llvm.org/viewvc/llvm-project?rev=357983&view=rev Log: Fix doxygen warning about function argument This fixes the following doxygen warning when building the lldb-cpp-doc target. This commit fixes: SBStructured

[Lldb-commits] [PATCH] D60443: Fix doxygen warning about function argument

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60443/new/ https://reviews.llvm.org/D60443 ___

[Lldb-commits] [PATCH] D60443: Fix doxygen warning about function argument

kkleine created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. kkleine added a reviewer: JDevlieghere. This fixes the following doxygen warning when building the lldb-cpp-doc target: This commit fixes: llvm-project/lldb/include/lldb/API/SBStructuredData.h

[Lldb-commits] [PATCH] D60442: Fix doxygen warnings for lldb-cpp-doc target

JDevlieghere added a comment. In D60442#1459378 , @kkleine wrote: > I think I don't have commit access (yet) and would like if someone can commit > this for me. It was my first commit to the LLDB project and I wanted to get > familiar with phabricator an

[Lldb-commits] [PATCH] D60442: Add missing space between \btrue

kkleine updated this revision to Diff 194263. kkleine added a comment. - Fix doxygen warning about function argument Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60442/new/ https://reviews.llvm.org/D60442 Files: lldb/include/lldb/API/SBStructur

[Lldb-commits] [PATCH] D60442: Add missing space between \btrue

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB357980: Add missing space between \btrue (authored by JDevlieghere, committed by ). Herald added a subscriber: abidh. Changed prior to commit: https://reviews.llvm.org/D60442?vs=194263&id=194264#toc

[Lldb-commits] [lldb] r357980 - Add missing space between \btrue

Author: jdevlieghere Date: Tue Apr 9 01:34:07 2019 New Revision: 357980 URL: http://llvm.org/viewvc/llvm-project?rev=357980&view=rev Log: Add missing space between \btrue There was a space missing in some the documentation for lldb::BreakpointsWriteToFile. This fixes the following doxygen error

[Lldb-commits] [lldb] r357977 - Minidump: use string parsing functionality from llvm

Author: labath Date: Tue Apr 9 01:28:27 2019 New Revision: 357977 URL: http://llvm.org/viewvc/llvm-project?rev=357977&view=rev Log: Minidump: use string parsing functionality from llvm Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp lldb/trunk/source/Plugins/Proce

[Lldb-commits] [PATCH] D60442: Add missing space between \btrue

kkleine added a comment. In D60442#1459367 , @JDevlieghere wrote: > Please let me know if you don't have commit access (yet) and need someone to > commit this for you. I think I don't have commit access (yet) and would like if someone can commit this

[Lldb-commits] [PATCH] D60442: Add missing space between \btrue

JDevlieghere added a comment. Please let me know if you don't have commit access (yet) and need someone to commit this for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60442/new/ https://reviews.llvm.org/D60442 ___

[Lldb-commits] [PATCH] D60442: Add missing space between \btrue

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60442/new/ https://reviews.llvm.org/D60442

[Lldb-commits] [PATCH] D60268: Breakpad: Parse Stack CFI records

This revision was automatically updated to reflect the committed changes. Closed by commit rL357975: Breakpad: Parse Stack CFI records (authored by labath, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D6

[Lldb-commits] [lldb] r357975 - Breakpad: Parse Stack CFI records

Author: labath Date: Tue Apr 9 01:05:11 2019 New Revision: 357975 URL: http://llvm.org/viewvc/llvm-project?rev=357975&view=rev Log: Breakpad: Parse Stack CFI records Summary: This patch adds support for parsing STACK CFI records from breakpad files. The expressions specifying the values of regis

[Lldb-commits] [PATCH] D60405: MinidumpYAML: Add support for ModuleList stream

labath marked an inline comment as done. labath added inline comments. Comment at: lib/ObjectYAML/MinidumpYAML.cpp:91 + size_t BlobAllocator::allocateString(StringRef Str) { SmallVector WStr; clayborg wrote: > Might be nice to unique the strings in here? Yeah

[Lldb-commits] [PATCH] D60442: Add missing space between \btrue

kkleine updated this revision to Diff 194259. kkleine added a comment. Updated summary to reflect what doxygen error was addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60442/new/ https://reviews.llvm.org/D60442 Files: lldb/include/lldb

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

ruiu added inline comments. Comment at: lld/ELF/MarkLive.cpp:190-191 + // case we still need to mark the file. + if (S && !IsLSDA && Sec->File) +if (isa(Sec) || isa(Sec)) + Sec->getFile()->HasLiveCodeOrData = true; rocallahan wrote: > ruiu wrote: > > r

[Lldb-commits] [PATCH] D60442: Add missing space between \btrue

kkleine created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. There was a space missing in some the documentation for lldb::BreakpointsWriteToFile. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D60442 Files: lldb/include/lldb/API/SBTa

[Lldb-commits] [PATCH] D60325: [lldb] [Process/NetBSD] Fix wrongly mapping mm* registers

labath added a subscriber: zturner. labath added inline comments. Comment at: lldb/lit/Register/x86-mm-xmm-read.test:1 +# REQUIRES: x86 +# RUN: %clang %p/Inputs/x86-mm-xmm-read.s -o %t mgorny wrote: > labath wrote: > > Does this mean "the host is an x86 system",

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

rocallahan updated this revision to Diff 194255. rocallahan edited the summary of this revision. rocallahan added a comment. The results are basically unchanged with the section type checks removed, so I've just gone ahead and done that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D547

[Lldb-commits] [PATCH] D60440: [lldb-server] Introduce Socket::Initialize and Terminate to simply WSASocket setup

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thank you for doing this. We probably should have done something like this a long time ago. I have a couple of inline comments, but they're all very trivial. Feel free to commit, if you agree