[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

labath added a reviewer: labath. labath added a comment. Forgive my ignorance, but what makes getenv unportable? llvm uses in a bunch of places so it can't be that bad... Is it some wide string thingy? Regardless, using the lldb function for that seems fine, but if I remember correctly, each Ge

[Lldb-commits] [PATCH] D56231: [lldb-server] Improve support on Windows

labath added a reviewer: labath. labath added a comment. Nice to see some action on the lldb-server for windows front. It looks like it should be easy to add a unit test for the File::Write bug you fixed. Can you add something like that? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION ht

[Lldb-commits] [PATCH] D56232: [lldb-server] Add remote platform capabilities for Windows

labath added reviewers: labath, jingham. labath added a comment. All of these functions seem identical to their PlatformPOSIX counterparts. Is that right? And I seem to remember already seeing a lot of code duplication between PlatformPOSIX and PlatformWindows. It sounds to me like we should cr

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

labath added a reviewer: labath. labath added inline comments. Comment at: include/lldb/Host/windows/PosixApi.h:79 +#define WNOHANG 1 +#define WUNTRACED 2 + I don't see `WUNTRACED` being used anywhere within lldb. Why did you need this? Comment

[Lldb-commits] [PATCH] D56237: Implement GetFileLoadAddress for the Windows process plugin

labath added a comment. I think there is something wrong here. Normally, it should be the job of the dynamic loader plugin to figure out where a module got loaded (and populate the target section load list). `Process::GetFileLoadAddress` is there to help it with this job. So, the fact that `Pro

[Lldb-commits] [PATCH] D56229: [PECOFF] Implementation of ObjectFilePECOFF:: GetUUID()

labath added a comment. I would definitely encourage using something better than the file checksum as UUID, if at all possible. Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:139 +uuid = +UUID::fromOptionalData(llvm::ArrayRef(Resu

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

On 03/01/2019 02:08, Zachary Turner via lldb-commits wrote: Very excited to see this.  I'm technically on vacation so I might not be able to review it immediately, but I'm looking forward to getting to it soon. I am also very excited. Unlike PDBs, I think I actually know a thing or two abou

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

labath requested review of this revision. labath added a comment. Greg, given the intended usage of these sections as demonstrated in D56173 , do you agree with representing the sections of the breakpad object file in this way? CHANGES SINCE LAST ACTION https

[Lldb-commits] [PATCH] D56129: Simplify ObjectFile::GetArchitecture

labath updated this revision to Diff 180008. labath added a comment. Add unit test for ArchSpec::operator bool CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56129/new/ https://reviews.llvm.org/D56129 Files: include/lldb/Core/Module.h include/lldb/Expression/IRExecutionUnit.h incl

[Lldb-commits] [PATCH] D56129: Simplify ObjectFile::GetArchitecture

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB350291: Simplify ObjectFile::GetArchitecture (authored by labath, committed by ). Changed prior to commit: https://reviews.llvm.org/D56129?vs=180008&id=180009#toc Repository: rLLDB LLDB CHANGES S

[Lldb-commits] [lldb] r350291 - Simplify ObjectFile::GetArchitecture

Author: labath Date: Thu Jan 3 02:37:19 2019 New Revision: 350291 URL: http://llvm.org/viewvc/llvm-project?rev=350291&view=rev Log: Simplify ObjectFile::GetArchitecture Summary: instead of returning the architecture through by-ref argument and a boolean value indicating success, we can just retu

Re: [Lldb-commits] [PATCH] D56173: Introduce SymbolFileBreakpad and use it to fill symtab

Ok, lgtm then On Wed, Jan 2, 2019 at 11:53 PM Pavel Labath wrote: > On 02/01/2019 18:32, Zachary Turner wrote: > > Just to be sure, this new test will fail without your Symtab changes > > right? I'm not in a state where I can look at code right now, and you > > say anything that symbolicates an

[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

labath updated this revision to Diff 180027. labath added a comment. Set N=0 default parameter, and remove the size specification from all usages (including the one in DWARFDebugArranges, where N was 1). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56170/new/ https://reviews.llvm.org/D

[Lldb-commits] [lldb] r350294 - Fix some -Wreorder warnings introduced in r350274

Author: labath Date: Thu Jan 3 03:31:50 2019 New Revision: 350294 URL: http://llvm.org/viewvc/llvm-project?rev=350294&view=rev Log: Fix some -Wreorder warnings introduced in r350274 Modified: lldb/trunk/include/lldb/Symbol/LineTable.h Modified: lldb/trunk/include/lldb/Symbol/LineTable.h URL

[Lldb-commits] [lldb] r350298 - PECOFF: Remove tabs introduced accidentally in r350094

Author: labath Date: Thu Jan 3 04:07:38 2019 New Revision: 350298 URL: http://llvm.org/viewvc/llvm-project?rev=350298&view=rev Log: PECOFF: Remove tabs introduced accidentally in r350094 Modified: lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Modified: lldb/trunk/source/P

[Lldb-commits] [PATCH] D56234: [lldb-server] Add unnamed pipe support to PipeWindows

labath added inline comments. Comment at: source/Host/windows/PipeWindows.cpp:28 std::atomic g_pipe_serial(0); -} +static std::string g_pipe_name_prefix = ".\\Pipe\\"; +} // namespace This would be better as llvm::StringLiteral (or just a char array) to avoi

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

clayborg added a comment. So is this done as one section per function? Or one section for contiguous functions? What about if there are only symbols? I tried to read the code but wasn't able to decipher everything clearly in my head. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55434/

[Lldb-commits] [PATCH] D55434: ObjectFileBreakpad: Implement sections

clayborg added a comment. Just read the original description again and now code makes sense. Main questions for me: is there a benefit to creating multiple sections? Can we just create one section and name it ".breakpad"? Should we not try to find a section that contains the address from the FU

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

clayborg added a comment. excited to see this starting as well! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56233/new/ https://reviews.llvm.org/D56233 ___ lldb-commits mailing list lldb-commits@lists.llvm.org htt

[Lldb-commits] [PATCH] D56237: Implement GetFileLoadAddress for the Windows process plugin

clayborg added a comment. Seems like DynamicLoaderWindowsDYLD isn't doing its job correctly here and DynamicLoaderWindowsDYLD should be fixed. We shouldn't have to go to the process at all in order to set the section load addresses correctly. Why? As you might have seen lldb-server.exe is being

[Lldb-commits] [PATCH] D56237: Implement GetFileLoadAddress for the Windows process plugin

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So it seems like DynamicLoaderWindowsDYLD is not doings its job correctly and it needs to be fixed. DynamicLoaderWindowsDYLD is responsible for setting all of the section load ad

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

krytarowski added inline comments. Comment at: include/lldb/Host/windows/PosixApi.h:78 +#define WNOHANG 1 +#define WUNTRACED 2 I think that these symbols should not be leaked here in the first place. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https

[Lldb-commits] [lldb] r350360 - TestQueues: Move the synchronisation code into the binary itself.

Author: adrian Date: Thu Jan 3 14:34:48 2019 New Revision: 350360 URL: http://llvm.org/viewvc/llvm-project?rev=350360&view=rev Log: TestQueues: Move the synchronisation code into the binary itself. Thanks to Pavel Labath for the suggestion! Modified: lldb/trunk/packages/Python/lldbsuite/tes

[Lldb-commits] [PATCH] D56208: Add file-based synchronization to flaky test

aprantl added a comment. Herald added a reviewer: serge-sans-paille. I implemented a poor man's version of that in r350360. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56208/new/ https://reviews.llvm.org/D56208 ___

[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

lemo created this revision. lemo added reviewers: labath, zturner. lemo added a project: LLDB. Herald added subscribers: jfb, abidh. If the minidump contains a saved exception record use it automatically. (This patch is cherry picked from the larger https://reviews.llvm.org/D55142) Repository:

[Lldb-commits] [PATCH] D56115: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. A testcase that triggers this situation would be nice, too. Comment at: source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp:558 + if (!threa

[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

zturner requested changes to this revision. zturner added a comment. This revision now requires changes to proceed. I don't think we can check in an executable file, we should try to compile it on the spot. We have 1-2 existing unit tests that check in an exe and we occasionally get reports tha

[Lldb-commits] [lldb] r350368 - symbols.enable-external-lookup=false on all hosts (not just OSX)

Author: jankratochvil Date: Thu Jan 3 15:11:06 2019 New Revision: 350368 URL: http://llvm.org/viewvc/llvm-project?rev=350368&view=rev Log: symbols.enable-external-lookup=false on all hosts (not just OSX) There is already in use: lit/lit-lldb-init: settings set symbols.ena

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

This revision was automatically updated to reflect the committed changes. jankratochvil marked an inline comment as done. Closed by commit rLLDB350368: symbols.enable-external-lookup=false on all hosts (not just OSX) (authored by jankratochvil, committed by ). Herald added a reviewer: serge-sans-p

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

aprantl added inline comments. Comment at: source/Core/ModuleList.cpp:72 {}, "Control the use of external tools or libraries to locate symbol files. " + "Directories listed in target.debug-file-search-paths and directory of " jankratochvil wrote:

[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

clayborg added a comment. I have a minidump generator if you need me to make any specific minidump files for you. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56293/new/ https://reviews.llvm.org/D56293 ___ lldb-c

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

jankratochvil marked an inline comment as done. jankratochvil added inline comments. Comment at: source/Core/ModuleList.cpp:72 {}, "Control the use of external tools or libraries to locate symbol files. " + "Directories listed in target.debug-file-search-paths and

[Lldb-commits] [lldb] r350375 - [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

Author: kuba.brecka Date: Thu Jan 3 16:20:52 2019 New Revision: 350375 URL: http://llvm.org/viewvc/llvm-project?rev=350375&view=rev Log: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread Differential Revision: https://reviews.llvm.org/D56115 Modified:

[Lldb-commits] [PATCH] D56115: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB350375: [lldb] Check SafeToCallFunctions before calling functions in… (authored by kuba.brecka, committed by ). Changed prior to commit: https://reviews.llvm.org/D56115?vs=179589&id=180173#toc Repos

[Lldb-commits] [PATCH] D56115: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread

kubamracek added a comment. > I recently started writing these early-exit returns as return {}; Good suggestion! > A testcase that triggers this situation would be nice, too. I think this is too tricky. If I understand correctly, an example of an unsafe-to-call thread state is when we're parke

[Lldb-commits] [lldb] r350376 - [lldb] Fix ObjCExceptionRecognizedStackFrame to populate the list of recognized arguments

Author: kuba.brecka Date: Thu Jan 3 16:25:08 2019 New Revision: 350376 URL: http://llvm.org/viewvc/llvm-project?rev=350376&view=rev Log: [lldb] Fix ObjCExceptionRecognizedStackFrame to populate the list of recognized arguments Differential Revision: https://reviews.llvm.org/D56027 Modified:

[Lldb-commits] [PATCH] D56027: [lldb] Fix ObjCExceptionRecognizedStackFrame to populate the list of recognized arguments

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB350376: [lldb] Fix ObjCExceptionRecognizedStackFrame to populate the list of recognized… (authored by kuba.brecka, committed by ). Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.

[Lldb-commits] [lldb] r350380 - RangeMap.h: merge RangeDataArray and RangeDataVector

Author: labath Date: Thu Jan 3 23:14:17 2019 New Revision: 350380 URL: http://llvm.org/viewvc/llvm-project?rev=350380&view=rev Log: RangeMap.h: merge RangeDataArray and RangeDataVector Summary: The main difference between the classes was supposed to be the fact that one is backed by llvm::SmallV

[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

This revision was automatically updated to reflect the committed changes. Closed by commit rL350380: RangeMap.h: merge RangeDataArray and RangeDataVector (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D56170?vs=180

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

labath added inline comments. Comment at: source/Core/ModuleList.cpp:72 {}, "Control the use of external tools or libraries to locate symbol files. " + "Directories listed in target.debug-file-search-paths and directory of " jankratochvil wrote: >

[Lldb-commits] [PATCH] D56293: Use the minidump exception record if present

labath added a comment. In D56293#1345790 , @zturner wrote: > Is there something about this executable that makes it impractical to compile > on the fly using the `%build` substitution? I think the impractical part comes when you need to use that compil