[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] [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] [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] 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

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

labath added a comment. In D56293#1345790 , @zturner wrote: > 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 that people

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

labath added a comment. I was under the impression I've convinced you of this direction, but your questions make it sound like you're going back to the "standalone" breakpad file idea (which I am not fond of). I'll try to explain again what I'm doing here. This is going to be somewhat repetitiv

[Lldb-commits] [PATCH] D56132: Symtab: Remove one copy of symbol size computation code

This revision was automatically updated to reflect the committed changes. Closed by commit rL350384: Symtab: Remove one copy of symbol size computation code (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://rev

[Lldb-commits] [PATCH] D56315: Add a verbose mode to "image dump line-table" and use it to write a .debug_line test

labath created this revision. labath added reviewers: clayborg, zturner. Herald added a subscriber: JDevlieghere. The motivation for this is being able to write tests for the upcoming breakpad line table parser, but this could be useful for testing the low-level workings of any line table format.

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

labath updated this revision to Diff 180240. labath added a comment. A couple of minor tweaks to the patch: - log in case we were unable to fetch the base address of the object file - extend the test case to cover ICF-ed symbols (marked with m) - move the yaml representation of an empty ELF file

[Lldb-commits] [PATCH] D55761: lldb-test ir-memory-map: Use IntervalMap::contains

labath closed this revision. labath added a comment. Landed in r350087. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55761/new/ https://reviews.llvm.org/D55761 ___ lldb-commits mailing list lldb-commits@lists.llvm.or

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

labath requested changes to this revision. labath added inline comments. This revision now requires changes to proceed. Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:1138 + // Double quote the string. + ::snprintf(arg_cstr, sizeof(arg_cstr), "

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

labath accepted this revision. labath added a comment. Looks fine to me (but please wait for an ack from Zachary). Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:313 - for (lldb::tid_t tid = 0; tid < num_threads; ++tid) { + for (size_t t_index = 0; t_index

[Lldb-commits] [PATCH] D56174: ProcessLaunchInfo: remove Debugger reference

This revision was automatically updated to reflect the committed changes. Closed by commit rL350510: ProcessLaunchInfo: remove Debugger reference (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.o

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

labath removed a reviewer: labath. labath added a comment. Removing the self-accept (oops). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55434/new/ https://reviews.llvm.org/D55434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org h

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

labath accepted this revision. labath added a comment. Thank you for the review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55434/new/ https://reviews.llvm.org/D55434 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL350511: ObjectFileBreakpad: Implement sections (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55434?vs=177209&id=180450#

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

labath added inline comments. Comment at: include/lldb/Host/windows/PosixApi.h:78-79 +#define WNOHANG 1 +#define WUNTRACED 2 + zturner wrote: > krytarowski wrote: > > I think that these symbols should not be leaked here in the first place. > In general we shoul

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

labath added a comment. In D56232#1348514 , @zturner wrote: > Is that even necessary? If a platform is not remote aware, `IsHost()` will > always just return `true` by definition. So we could put all of this in the > existing `Platform` base class. I

[Lldb-commits] [PATCH] D56400: [CMake] Some cleanup around test preparations

labath added a reviewer: stella.stamenova. labath added a comment. This looks fine to me. I recommend including Stella in changes relating to this, as I recall some subtleties here wrt. multi-config (e.g., MSVC) generators. Comment at: CMakeLists.txt:43 if(LLDB_INCLUDE_TESTS

[Lldb-commits] [PATCH] D56418: Change lldb-test to use ParseAllDebugSymbols instead of ParseDeclsForContext

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. The lldb-test change itself looks fine. I don't feel like I know the full impact of the proposed follow-up changes (but they sounds reasonable). Comment at: lldb/lit/SymbolF

[Lldb-commits] [PATCH] D56196: ProcessLaunchInfo: Remove Target reference

This revision was automatically updated to reflect the committed changes. Closed by commit rL350617: ProcessLaunchInfo: Remove Target reference (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D55998: ELF: create "container" sections from PT_LOAD segments

labath added a comment. Greg, what do you think about the current version of this patch. Joerg, I propose to check the patch in in the current form, as this solves existing issues in the lldb's ELF representation and improves the overall consistency of lldb's representation of object files. I'm

[Lldb-commits] [PATCH] D56443: [CMake] In standalone builds, LLVM_BINARY_DIR should point to LLVM's binary directory

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Makes sense to me. Comment at: CMakeLists.txt:117 # For now check that the include directory exists. -set(cxx_dir "${LLVM_DIR}/../../../include/c++") +

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

labath added a comment. > It is not that applicable for the windows process launcher to determine which > entry in the args needs to be quoted unless given very specific flag or > option. Why not? Given the argv parsing rules described here https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added a comment. My main question would be whether it is possible to specify all the lldb-mi commands up-front, which is kinda required for this approach. I was against this for the lldb-server tests, because there we often deal with values which are not sufficiently deterministic (for e

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added a comment. In https://reviews.llvm.org/D46588#1093267, @aprantl wrote: > Can you give an example of what you mean by "specifying the lldb-mi commands > up-front"? it's not immediately obvious to me how that would look like. In your example `lldb-mi-demo.input` is static text, righ

[Lldb-commits] [PATCH] D46669: Retrieve the deployment target when retrieving an object file's triple

labath added a comment. The test you copied this from predates `lldb-test` (and it tests some behavior quirks which are not nicely expressible in text output). We should have nicer ways of testing things like this now. It should be sufficient to add a print line which displays the triple in the

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

labath added a comment. The patch looks much better now, but I think we still need to discuss the data extractor sliding issue, as right now that's very hacky. https://reviews.llvm.org/D32167 ___ lldb-commits mailing list lldb-commits@lists.llvm.or

[Lldb-commits] [PATCH] D46687: Remove custom path manipulation functions from FileSpec

labath created this revision. labath added reviewers: zturner, clayborg. now that llvm supports host-agnostic path manipulation functions (and most of their kinks have been ironed out), we can remove our copies of the path parsing functions in favour of the llvm ones. This should be NFC except fo

[Lldb-commits] [PATCH] D46687: Remove custom path manipulation functions from FileSpec

labath added inline comments. Comment at: source/Utility/FileSpec.cpp:277-281 + auto style = LLVMPathSyntax(syntax); + m_filename.SetString(llvm::sys::path::filename(resolved, style)); + llvm::StringRef dir = llvm::sys::path::parent_path(resolved, style); + if (!dir.empty())

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

labath added a comment. The patch looks quite nice, though I can't say I know much about how find_package modules are supposed to be written. I won't click accept yet to see if anyone with more cmake knowledge steps in to review. In the mean time, I have two nitty comments.

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

labath added a comment. The thing here is that I don't see how can be done in a non-hacky way in DWARFDataExtractor while DWARFDataExtractor remains a (subclass of) DataExtractor. And it is a hack because as soon as you slide the extractor, a number of its functions become booby-trapped (explod

[Lldb-commits] [PATCH] D46687: Remove custom path manipulation functions from FileSpec

This revision was automatically updated to reflect the committed changes. Closed by commit rL332088: Remove custom path manipulation functions from FileSpec (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46687 Files:

[Lldb-commits] [PATCH] D46753: FileSpec: Remove PathSyntax enum and use llvm version instead

labath created this revision. labath added reviewers: zturner, clayborg. The llvm version of the enum has the same enumerators, with stlightly different names, so this is mostly just a search&replace exercise. One concrete benefit of this is that we can remove the function for converting between t

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

labath added a comment. I **think** I understand what is going on here, but I can't say for certain. Could you elaborate on the implementation details of this somewhere. It would be good to keep a note of this for future maintainers. My understanding of this is: - if nothing has been parsed, m

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

labath added inline comments. Comment at: cmake/modules/FindLibEdit.cmake:11-14 +# libedit_FOUND - true if libedit was found +# libedit_INCLUDE_DIRS - include search path +# libedit_LIBRARIES - libraries to link +# libedit_VERSION - version number -

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added a comment. Preserving the dot if it is the only path component is perfectly reasonable -- "" is not a valid path and we shouldn't convert a valid path into an invalid one. However, I think this should be done on the llvm side, in the `remote_dots` function and not as a workaround o

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

labath added a comment. In https://reviews.llvm.org/D46810#1097740, @jankratochvil wrote: > In https://reviews.llvm.org/D46810#1097503, @labath wrote: > > > (If that is true, then I think this is workable, but there are still some > > details which need to be ironed out; e.g., `m_first_die.GetFi

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

labath added a comment. Sounds like a plan. https://reviews.llvm.org/D32167 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46753: FileSpec: Remove PathSyntax enum and use llvm version instead

labath added a comment. We don't export the style right now. At some point we may need to teach SBFileSpec about path styles, but at that point I'd just do the translation at the SB level, and let everyone else use the llvm enum. https://reviews.llvm.org/D46753 _

[Lldb-commits] [PATCH] D46753: FileSpec: Remove PathSyntax enum and use llvm version instead

This revision was automatically updated to reflect the committed changes. Closed by commit rL332247: FileSpec: Remove PathSyntax enum and use llvm version instead (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46753

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL332250: Remove Process references from the Host module (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46395 Files: lldb/t

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added inline comments. Comment at: lit/Breakpoint/Inputs/break-insert.input:1-3 +-break-insert breakpoint +-file-exec-and-symbols a.out +-exec-run Based on my experiments, lldb-mi seems to ignore lines starting with `#`. If that's true then we could put t

[Lldb-commits] [PATCH] D46810: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer

labath added a comment. Trying to be smart while being lazy and multithreaded is going to make the code complicated (possibility for bugs) and/or introduce a lot of locking overhead. A lot simpler solution is to let the caller decide if it want's the full CU or just the root DIE, and then make

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added a comment. Adding a test for that in lldb is fine. If we're relying on some behavior we want to make sure it doesn't change without us noticing. However, I don't think we should make any special allowments for people using lldb with older versions of llvm. It sends the wrong messag

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + I'm not familiar with this directive. Are you sure that this actually does anything? Repository: rL LLVM https:

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + polyakov.alex wrote: > labath wrote: > > I'm not familiar with this directive. Are you sure that this actually does

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:10 +-file-exec-and-symbols a.out +# CHECK-AFTER: ^done + polyakov.alex wrote: > labath wrote: > > polyakov.alex wrote: > > > labath wrote: > > > > I'm not familiar with this

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added a comment. In https://reviews.llvm.org/D46783#1099561, @clayborg wrote: > So the function in llvm is called llvm::sys::path::remove_dots(...) and it is > removing the dots. Not sure it is correct to be changing a function that says > "remove_dots" to not remove dots and actually re

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added a comment. (Also, the function already does not remove leading `..` components, so there is precedent already for not removing stuff when it causes the path to become not equivalent.) https://reviews.llvm.org/D46783 ___ lldb-commits m

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added a comment. If those are the only tests that fail, then I'd still go for it, as I still firmly believe this is the correct behavior for such a function. Such a low level test does not have to mean that someone really cares about this, it could be more of a case of documenting existi

[Lldb-commits] [PATCH] D46888: [DWARF] Have HashedNameToDIE store a DataExtractor by value

labath created this revision. labath added reviewers: clayborg, JDevlieghere. Herald added a subscriber: aprantl. The DataExtractors are cheap to copy so there is no reason to store them by reference. Also, in my upcoming indexing refactor I am planning to remove the apple tables data extractor me

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

labath created this revision. labath added reviewers: clayborg, JDevlieghere. Herald added subscribers: aprantl, mgorny. This places the `if(m_using_apple_tables)` branches inside the SymbolFileDWARF class behind an abstract DWARFIndex class. The class currently has two implementations: - AppleIn

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added a comment. Thank you. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14 +# CHECK-AFTER: ^running +# CHECK-AFTER: *stopped,reason="breakpoint-hit" + aprantl wrote: > polyakov.alex wrote: > > aprantl wrote: > > > CHECK-AFTER is not recognized by

[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms

labath created this revision. labath added reviewers: jasonmolenda, aprantl, clayborg. Herald added subscribers: kristof.beyls, mgorny. Herald added a reviewer: javed.absar. Before this patch we were unable to write cross-platform MachO tests because the parsing code did not compile on other platf

[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms

labath added inline comments. Comment at: source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp:1037 SetError(set, Write, -1); -return KERN_INVALID_ARGUMENT; +return -1; } aprantl wrote: > Could we keep this as a local constant? > perhaps w

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added inline comments. Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:14 +# CHECK-AFTER: ^running +# CHECK-AFTER: *stopped,reason="breakpoint-hit" + aprantl wrote: > labath wrote: > > aprantl wrote: > > > polyakov.alex wrote: > > > > aprantl wr

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

labath added a comment. In https://reviews.llvm.org/D46588#1102363, @aprantl wrote: > > The advantage of the second one is that we will have the ability to > > inject commands which depend on the results of previous commands (something > > that I think we will need, sooner or later). > > That

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. It would still be interesting to align remove_dots with the "standard practice", but it looks like that would end up being a big project. I think we can live with tweak on our side in this cas

[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms

labath added a comment. In https://reviews.llvm.org/D46934#1101963, @aprantl wrote: > Does that mean we can now also remove the #ifdef __APPLE__ from the > objectfile unit tests? Which ones do you mean? I wasn't aware we had any. The thing I know of, which would be interesting to enable is t

[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms

labath updated this revision to Diff 147286. labath added a comment. This is the version with new symbolic constants. I put them in a new file, as I couldn't think of a better place for them. I didn't want to put them in a too generic place as I don't think we should encourage their use (we shoul

[Lldb-commits] [PATCH] D46888: [DWARF] Have HashedNameToDIE store a DataExtractor by value

This revision was automatically updated to reflect the committed changes. Closed by commit rL332596: [DWARF] Have HashedNameToDIE store a DataExtractor by value (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46888 Fi

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

labath added a comment. Thank you for the review. I'll have the updated diff shortly. In the mean time, here are my responses. Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:40 + +std::unique_ptr AppleIndex::Create( +Module &module, DWARFDataExtractor apple_nam

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

labath updated this revision to Diff 147326. labath added a comment. - s/AppleIndex/AppleDWARFIndex - move things to separate files https://reviews.llvm.org/D46889 Files: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h source/Plugins/S

[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms

labath updated this revision to Diff 147341. labath added a comment. - Use #ifndef approach for handling the constants. https://reviews.llvm.org/D46934 Files: lit/Modules/lc_version_min.yaml source/Initialization/CMakeLists.txt source/Initialization/SystemInitializerCommon.cpp source/Pl

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added subscribers: clayborg, davide, jingham, zturner. labath added a comment. This has broken the unit tests. Looks like a bad merge that did not take into account the refactoring in r332088. Repository: rL LLVM https://reviews.llvm.org/D46783 __

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added a comment. Sorry, I was wrong about the cause, but the bots are red nonetheless. My bet is it's the last `{"", "."},` test, which is not working because of an early return in SetFile. TBH, I am not sure we would want that to work anyway, as we too treat an empty filespec specially

[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.

labath added a comment. http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/23447 https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commi

[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes

labath added a comment. Although it may not seem that way from the number of comments, the change looks good to me. The main thing is the moving of the test file, as that will fail in the cmake build. And it also looks like some code can be simplified if my assumption about not converting "" to

[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms

This revision was automatically updated to reflect the committed changes. Closed by commit rL332702: Make ObjectFileMachO work on non-darwin platforms (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46934 Files: lld

[Lldb-commits] [PATCH] D47064: Add some apple-tables lookup tests

labath created this revision. labath added reviewers: JDevlieghere, clayborg. Herald added subscribers: ioeric, ilya-biryukov, aprantl. Now that we are able to parse MachO files everywhere, we can write some cross-platform tests for handling of apple accelerator tables. This reruns the same lookup

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

This revision was automatically updated to reflect the committed changes. Closed by commit rL332719: [DWARF] Extract indexing code into a separate class hierarchy (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D468

[Lldb-commits] [PATCH] D47064: Add some apple-tables lookup tests

labath added inline comments. Comment at: lit/SymbolFile/DWARF/find-basic-function.cpp:17 // RUN: FileCheck --check-prefix=EMPTY %s +// +// RUN: clang %s -g -c -o %t --target=x86_64-apple-macosx JDevlieghere wrote: > `%t.o`? I prefer %t, as that makes the RUN

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

labath added a comment. In https://reviews.llvm.org/D46889#1104882, @aemerson wrote: > In https://reviews.llvm.org/D46889#1104823, @aprantl wrote: > > > Thanks for jumping on this Amara — I just wanted to point out that we > > ususally don't revert lldb changes that only break the lldb-xcode bot

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

labath added a comment. Looks fine to me, just make the path computation windows-compatible. Comment at: packages/Python/lldbsuite/test/dotest.py:386 +if is_exe(lldbtest_config.lldbExec): +lldbtest_config.dotestWrapper = "{}-dotest".format( + os

[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. looks good. I don't know if anyone else has an opinion on how should we treat "" for mapping purposes, but treating it as "." seems fine to me. Comment at: unittests/Utility

[Lldb-commits] [PATCH] D47064: Add some apple-tables lookup tests

This revision was automatically updated to reflect the committed changes. Closed by commit rL332831: Add some apple-tables lookup tests (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47064 Files: lldb/trunk/lit/Sym

[Lldb-commits] [PATCH] D47133: Enable ProcessMachCore plugin on non-apple platforms

labath created this revision. labath added reviewers: JDevlieghere, jasonmolenda. Herald added a subscriber: mgorny. The plugin already builds fine on other platforms (linux, at least). All that was necessary was to revitalize the hack in PlatformDarwinKernel (not a very pretty hack, but it gets u

[Lldb-commits] [PATCH] D47147: DWARFIndex: Reduce duplication in the GetFunctions methods

labath created this revision. labath added reviewers: JDevlieghere, clayborg. Herald added a subscriber: aprantl. This extracts the common bits out of the two implementations back into the SymbolFileDWARF class. The goal was to make the function as similar as possible to the other DWARFIndex metho

[Lldb-commits] [PATCH] D46726: build: use cmake to find the libedit content

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Ok, let's give this a try. https://reviews.llvm.org/D46726 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[Lldb-commits] [PATCH] D47147: DWARFIndex: Reduce duplication in the GetFunctions methods

labath added a comment. Changing the code to filter based on the dwarf information instead of the going through CompilerDeclContexts sounds like a good idea. I've been wondering why we are doing it this way -- the explanation I gave to myself was that this would allow the individual language pl

[Lldb-commits] [PATCH] D47133: Enable ProcessMachCore plugin on non-apple platforms

This revision was automatically updated to reflect the committed changes. Closed by commit rL332997: Enable ProcessMachCore plugin on non-apple platforms (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47133 Files:

[Lldb-commits] [PATCH] D47228: Break dependency from Core to ObjectFileJIT

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. The change sounds reasonable to me. I should just point out that this is the Core <-> ObjectFile**JIT** cycle you are breaking. Comment at: lldb/include/lldb/Core/Module.h:1

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

labath added a comment. Both of the solutions sound plausible to me (extra struct vs. moving REPL to `Command`). Maybe that's because I don't know enough about the REPL to have formed an opinion on it. Comment at: lldb/include/lldb/Expression/REPL.h:31 +bool TryAllThreads

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

labath added a comment. In a way, this makes sense, but in a lot of other ways, it actually makes things worse :) My long-term goal is to be able to build lldb-server without even having clang checked out (because it doesn't really need anything clang-related), and this would certainly make th

[Lldb-commits] [PATCH] D47250: Move ObjectFile initialization out of SystemInitializerCommon

labath created this revision. labath added reviewers: zturner, clayborg. Herald added a subscriber: mgorny. For lldb-server, it is sufficient to parse only the native object file format for its target OS (no other file can be loaded into a running process). This moves the object file initializatio

[Lldb-commits] [PATCH] D47253: DWARF: Move indexing code from DWARFUnit to ManualDWARFIndex

labath created this revision. labath added reviewers: JDevlieghere, clayborg, aprantl. I think this makes sense for several reasons: - better separation of concerns: DWARFUnit's job should be to provide a nice interface to its users to access the unit contents. ManualDWARFIndex can then use thi

[Lldb-commits] [PATCH] D47265: WIP: lit: Run each test separately

labath created this revision. Herald added subscribers: JDevlieghere, eraman. This needs to be cleaned up before we can consider submitting it. However, I am putting it out here, so we you can test it's impact on your setup. PS: This does not prevent the debug-info replication for "inline" tests.

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

labath added a comment. Thank you for the support Jonas. I've quickly prototyped a patch (https://reviews.llvm.org/D47265) of how would this integrate with lit. The nice part about it is that it is very small. If you consider the inline test refactor it even has negative LOC. It would definitel

[Lldb-commits] [PATCH] D47235: Move ModuleList's dependency on clangDriver into Host

labath added a comment. In https://reviews.llvm.org/D47235#1109580, @zturner wrote: > In https://reviews.llvm.org/D47235#1109219, @labath wrote: > > > I guess it would be nice to encapsulate this in some sort of a plugin > > (since the setting is used from the clang expression parser plugin, I g

[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

labath added a reviewer: aprantl. labath added a comment. I don't think a name like `DWARFUnitDIE` is a good one bacause it would make a weird `is-a` relationship (a DWARFDIE represetning a DW_TAG_variable is certainly **not** a "unit DIE" yet you could assign it to a `DWARFUnitDIE&`). We coul

[Lldb-commits] [PATCH] D47278: Remove lldb-private headers when building LLDB.framework with CMake

labath added a comment. From a layering perspective, it makes sense for SystemInitializerFull to live in the outermost layer, as it's the thing which makes sure liblldb pulls in all required components. Since it is only included from files in `source/API` (which is as it should be), maybe we co

[Lldb-commits] [PATCH] D47275: 1/3: DWARFDIE split out to DWARFBasicDIE

labath added a comment. Looks like we missed each other, but all I said about DWARFUnitDIE applies to DWARFFirstDIE as well. I doesn't have to be called "basic" die, but the reason I proposed that name is that it does not sound weird when you say that any die "is a" basic die. Other possibility

[Lldb-commits] [PATCH] D47285: Add PPC64le support information

This revision was automatically updated to reflect the committed changes. Closed by commit rL333173: Add PPC64le support information (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47285 Files: lldb/trunk/www/featur

[Lldb-commits] [PATCH] D47253: DWARF: Move indexing code from DWARFUnit to ManualDWARFIndex

This revision was automatically updated to reflect the committed changes. Closed by commit rL333178: DWARF: Move indexing code from DWARFUnit to ManualDWARFIndex (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47253 F

<    1   2   3   4   5   6   7   8   9   10   >