[Lldb-commits] [PATCH] D13617: Fix ref-counting of Python objects

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rGf8b22f8fea18: Fix ref counting of Python objects. (authored by zturner). Herald added a project: LLDB. Changed prior

[Lldb-commits] [PATCH] D17492: Case sensitive path compare on Windows breaks breakpoints

This revision was automatically updated to reflect the committed changes. Closed by commit rG47c03462f52a: Some fixes for case insensitive paths on Windows. (authored by zturner). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D17492?vs=48871=223585#toc

[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef

This revision was automatically updated to reflect the committed changes. Closed by commit rG182b4652e542: [StringRef] Add enable-if to StringLiteral. (authored by zturner). Herald added subscribers: llvm-commits, dexonsmith. Herald added a project: LLVM. Changed prior to commit:

[Lldb-commits] [PATCH] D55574: Remove else statements after returns

zturner added inline comments. Comment at: source/Commands/CommandObjectTarget.cpp:2569 + } + StreamString strm; + module_spec.GetUUID().Dump(); aprantl wrote: > Can you manually double-check this one? For large complex cases like

[Lldb-commits] [PATCH] D67168: [Windows] Add support of watchpoints to `ProcessWindows`

zturner added inline comments. Comment at: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp:82 -bool RegisterContextWindows::ClearHardwareBreakpoint(uint32_t hw_idx) { - return false; -} + if (!size || size > 8 || size & (size - 1)) +return false;

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

zturner added inline comments. Comment at: lldb/tools/driver/Driver.cpp:492-496 #ifdef _WIN32 - _close(fds[WRITE]); - fds[WRITE] = -1; + _close(fd); #else - close(fds[WRITE]); - fds[WRITE] = -1; + close(fd); #endif labath wrote:

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

zturner added a comment. In D59911#1446942 , @jingham wrote: > In D59911#1446745 , @davide wrote: > > > My (maybe unpopolar) opinion on the subject is that "soft assertions" are a > > way to cleanse your

[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

zturner added a comment. In D59854#1443906 , @jingham wrote: > In D59854#1443904 , @JDevlieghere > wrote: > > > In D59854#1443891 , @jingham wrote: > > > > > It isn't safe

[Lldb-commits] [PATCH] D59651: Move DebugRanges() function from SymbolFileDWARF to DWARFContext

zturner created this revision. zturner added reviewers: JDevlieghere, clayborg, labath, aprantl. This should be the last non-dwo dependent piece to get over to `DWARFContext`. After this there will need to be some careful refactoring to get all knowledge of DWO-ness into the `DWARFContext`,

[Lldb-commits] [PATCH] D59611: Move the rest of the section loading over to DWARFContext

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rL356682: Move the rest of the sections over to DWARFContext. (authored by zturner, committed by ). Herald added a

[Lldb-commits] [PATCH] D59611: Move the rest of the section loading over to DWARFContext

zturner added a comment. In D59611#1437044 , @clayborg wrote: > Use reference to DWARFContext instead of pointers? Apply to entire patch and > good to go Done. Since you conditionally LGTM'ed it based on that and I made that change I'll go ahead and

[Lldb-commits] [PATCH] D59611: Move the rest of the section loading over to DWARFContext

zturner created this revision. zturner added reviewers: clayborg, JDevlieghere, labath, aprantl. Herald added a subscriber: jdoerfert. This gets all of the remaining sections except those that are DWO-dependent. That will be a bit harder to do since we'll somehow have to get knowledge of

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

This revision was automatically updated to reflect the committed changes. Closed by commit rL356612: Introduce DWARFContext. (authored by zturner, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

zturner marked an inline comment as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:552 + data.Clear(); + m_obj_file->ReadSectionData(section_sp.get(), data); } clayborg wrote: > clayborg wrote: > >

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

zturner marked 2 inline comments as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:50 + +const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() { + return LoadOrGetSection(m_module, m_dwarf_data,

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

zturner updated this revision to Diff 191510. zturner marked an inline comment as done. zturner added a comment. Updated based on suggestions, including removal of the `m_dwarf_data` member variable which holds the contents of the `__DWARF` segment on MachO. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

zturner marked 5 inline comments as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:17 +static const DWARFDataExtractor *LoadOrGetSection( +Module , const llvm::Optional _dwarf_data, +SectionType section_type,

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

zturner updated this revision to Diff 191421. zturner added a comment. I went ahead and just removed the const `get` version entirely, while changing the other function name to `getOrLoad` as you suggested. I have another patch while I'll upload after this one that converts all of the rest of

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

zturner marked an inline comment as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:30 + + const DWARFDataExtractor *maybeLoadArangesData(); + JDevlieghere wrote: > Why do we need these two methods? I presume

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

zturner created this revision. zturner added reviewers: JDevlieghere, aprantl, clayborg. Herald added subscribers: jdoerfert, mgorny. LLVM's DWARF parsing library has a class called `DWARFContext` which holds all of the various DWARF data sections and lots of other information. LLDB's on the

[Lldb-commits] [PATCH] D59276: Delete dead code

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB356490: Delete dead code. (authored by zturner, committed by ). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D59276?vs=190350=191356#toc Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D59291: [Object] Add basic minidump support

zturner accepted this revision. zturner added a comment. This LGTM, but let's give it a day to see if anyone else chimes in with comments. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59291/new/ https://reviews.llvm.org/D59291

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB356469: Remove a couple of log statements. (authored by zturner, committed by ). Herald added a subscriber: abidh. Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

zturner marked 4 inline comments as done. zturner added a comment. In all cases, I think the question worth asking is not "could it be used for X", but rather "how often is it actually used for X". Otherwise, it's just technical debt IMO. There's a lot of things that are possible in theory,

[Lldb-commits] [PATCH] D59498: [DWARF] Remove a couple of log statements

zturner created this revision. zturner added reviewers: JDevlieghere, aprantl. Herald added a subscriber: jdoerfert. I don't find these specific log statements particularly high value, as they appear more just like simple function tracing calls (i.e. they log low level implementation details,

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

zturner added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59430/new/ https://reviews.llvm.org/D59430 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59276: Delete dead code

zturner added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59276/new/ https://reviews.llvm.org/D59276 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

zturner added a comment. "MinidumpNew" is a little bit vague. What's "new" about it? Is there a way we could come up with a better name? As an aside, since there's no interactivity in these tests, they seem like a good candidate for lit/Minidump, with 1 file for each test. Something like:

[Lldb-commits] [PATCH] D59427: [lldb] [API] Split SBRegistry into smaller files

zturner added a comment. Or better yet, make it a static method on each SB class. E.g. `SBTarget::InitializeReproducerRegistry();` etc, one for each class. Random thought, but this all seems like a very high maintenance strategy, having to manually update this registry when methods are added

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

zturner marked 3 inline comments as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:218 + assert(offset < cu->GetNextCompileUnitOffset() && + debug_info_data.ValidOffset(offset)); aprantl

[Lldb-commits] [PATCH] D59430: Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected

zturner created this revision. zturner added reviewers: clayborg, aprantl, labath. Herald added a subscriber: jdoerfert. This hits the next batch of classes and adds better error handling and recovery to them. After this, the only remaining case is the line table stuff, at which point we

[Lldb-commits] [PATCH] D59381: Change CompileUnit and ARanges interfaces to propagate errors

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rL356278: Return Error and Expected from more DWARF interfaces. (authored by zturner, committed by ). Herald added a

[Lldb-commits] [PATCH] D59400: [lldb-vscode] Fix dangling pointer in request_evaluate.

zturner added a comment. Another option would be to just assign it to a `std::string`, but this is fine too. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59400/new/ https://reviews.llvm.org/D59400 ___

[Lldb-commits] [PATCH] D59381: Change CompileUnit and ARanges interfaces to propagate errors

zturner marked 2 inline comments as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:25 + lldb::offset_t *offset_ptr) { + assert(debug_info.ValidOffset(*offset_ptr)); + clayborg

[Lldb-commits] [PATCH] D59381: Change CompileUnit and ARanges interfaces to propagate errors

zturner created this revision. zturner added reviewers: clayborg, labath, aprantl. Herald added a subscriber: jdoerfert. This is a continuation of D59370 for compile units and aranges. https://reviews.llvm.org/D59381 Files:

[Lldb-commits] [PATCH] D59370: Return llvm::Error and llvm::Expected from some DWARF parsing functions

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB356190: Return llvm::Error and llvm::Expected from DWARF parsing code. (authored by zturner, committed by ). Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D59291: [Object] Add basic minidump support

zturner added inline comments. Comment at: include/llvm/Object/Minidump.h:77 + ArrayRef Streams, + std::unordered_map StreamMap) + : Binary(ID_Minidump, Source), Header(Header), Streams(Streams), jhenderson wrote: > Are you

[Lldb-commits] [PATCH] D59370: Return llvm::Error and llvm::Expected from some DWARF parsing functions

zturner updated this revision to Diff 190654. zturner added a comment. Fix half-written comment and add comments to other extraction function declarations. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59370/new/ https://reviews.llvm.org/D59370 Files:

[Lldb-commits] [PATCH] D59370: Return llvm::Error and llvm::Expected from some DWARF parsing functions

zturner created this revision. zturner added reviewers: labath, clayborg, aprantl, probinson. Herald added a subscriber: jdoerfert. The goal here is to improve our error handling and error recovery while parsing DWARF, while at the same time getting us closer to being able to merge LLDB's DWARF

[Lldb-commits] [PATCH] D59276: Delete dead code

zturner added a comment. In D59276#1426972 , @aprantl wrote: > One one hand this seems fine to remove, but on the other hand — won't these > functions come in handy to compare and debug differences between the LLDB and > LLVM DWARF parsers, while we are

[Lldb-commits] [PATCH] D59276: Delete dead code

zturner created this revision. zturner added reviewers: clayborg, aprantl. Herald added subscribers: jdoerfert, mgorny. Most of these are Dump functions that are never called, but there is one instance of entire unused classes (DWARFDebugMacinfo and DWARFDebugMacinfoEntry) which are also

[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB355975: Remove support for DWARF64. (authored by zturner, committed by ). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D59235?vs=190173=190326#toc Repository:

[Lldb-commits] [PATCH] D59165: Remove DWARFDIECollection

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB355974: Remove DWARFDIECollection. (authored by zturner, committed by ). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D59165?vs=189961=190325#toc Repository:

[Lldb-commits] [PATCH] D59164: [SymbolFileDWARF] Move ElaboratingDIEIterator into implementation file

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB355973: Move ElaboratingDIEIterator into implementation file. (authored by zturner, committed by ). Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

zturner marked an inline comment as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp:170 case DW_FORM_sec_offset: assert(m_cu); + m_value.value.uval = data.GetMaxU64(offset_ptr, 4);

[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

zturner marked an inline comment as done. zturner added a comment. It seems like we're mostly in agreement on the direction of the patch, so if there's no outstanding comments I'll submit at the end of the day. Comment at:

[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

zturner added a comment. In D59235#1425436 , @clayborg wrote: > My main concern with the LLVM DWARF parser is all of the asserts in the code. > If you attempt to use a DWARFDIE without first checking it for validity, it > will crash on you instead of

[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

zturner added a subscriber: jankratochvil. zturner added a comment. In D59235#1425417 , @clayborg wrote: > What part of parsing DWARF64 wasn't working? I think the SPARC compiler was > the only one producing DWARF64. This patch originated from a thread

[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

zturner created this revision. zturner added reviewers: clayborg, jasonmolenda, aprantl. Herald added a subscriber: jdoerfert. LLVM doesn't produce DWARF64, and neither does GCC. LLDB's support for DWARF64 is only partial, and if enabled appears to also not work. It also appears to have no

[Lldb-commits] [PATCH] D59217: Fix/unify SBType comparison

zturner added a comment. SBType storing a pair of CompilerType and TypeSP seems like a very confusing interface and like it will be very easy to misuse or violate assumptions (perhaps even assumptions that nobody knows exists). Why exactly is this necessary? As far as I can tell, `SBType`

[Lldb-commits] [PATCH] D59200: Fix a crasher in StackFrame::GetValueForVariableExpressionPath()

zturner added inline comments. Comment at: source/Target/StackFrame.cpp:644-645 if (error.Fail()) { error.SetErrorStringWithFormatv( "Failed to dereference sythetic value: %s", deref_error); return ValueObjectSP();

[Lldb-commits] [PATCH] D59165: Remove DWARFDIECollection

zturner created this revision. zturner added reviewers: clayborg, aprantl. Herald added subscribers: jdoerfert, mgorny. This is a very thin wrapper over a std::vector and does not seem to provide any real value over just using a container directly. https://reviews.llvm.org/D59165 Files:

[Lldb-commits] [PATCH] D59164: [SymbolFileDWARF] Move ElaboratingDIEIterator into implementation file

zturner created this revision. zturner added a reviewer: clayborg. This is not used outside of the private implementation of the class, so hiding in the implementation file is a nice way of simplifying the external interface. https://reviews.llvm.org/D59164 Files:

[Lldb-commits] [PATCH] D59158: Break cycle lldb/Commands [3->] lldb/Expression [1->] lldb/Commands

zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. Ahh yea, sorry. Got confused for a second :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59158/new/ https://reviews.llvm.org/D59158

[Lldb-commits] [PATCH] D59158: Break cycle lldb/Commands [3->] lldb/Expression [1->] lldb/Commands

zturner added a comment. Interesting, I had looked at fixing this one once before but I didn't realize we had a class named `EvaluateExpressionOptions` that contained just the right set of information, and it felt gross to invent a new one just for this purpose (It's a good thing I didn't too,

[Lldb-commits] [PATCH] D59158: Break cycle lldb/Commands [3->] lldb/Expression [1->] lldb/Commands

zturner added a comment. I think maybe part of the problem is that this patch looks like actually 2 things. 1) A move of the include files from `lldb/source/Commands` to `lldb/Include/lldb/Commands`, and 2) The dependency changes. So it makes it hard to see what changes are actually needed

[Lldb-commits] [PATCH] D59158: Break cycle lldb/Commands [3->] lldb/Expression [1->] lldb/Commands

zturner added inline comments. Comment at: lldb/include/lldb/Commands/CommandObjectBreakpoint.h:36 - static void VerifyBreakpointOrLocationIDs(Args , Target *target, -CommandReturnObject , I think the clang-format

[Lldb-commits] [PATCH] D59114: [lldb-vscode] Don't hang indefinitely on invalid program

This revision was automatically updated to reflect the committed changes. Closed by commit rL355656: [lldb-vscode] Report an error if an invalid program is specified. (authored by zturner, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[Lldb-commits] [PATCH] D59114: [lldb-vscode] Don't hang indefinitely on invalid program

zturner created this revision. zturner added a reviewer: clayborg. When you configure your launch settings, if you pass a path to an invalid program, we would previously hand indefinitely. This is because a combination of two bugs working together. The first is that we did not attempt to

[Lldb-commits] [PATCH] D59104: [lldb-vscode] Make server mode work on Windows

This revision was automatically updated to reflect the committed changes. Closed by commit rL355637: [lldb-vscode] Support running in server mode on Windows. (authored by zturner, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D59104: [lldb-vscode] Make server mode work on Windows

zturner created this revision. zturner added a reviewer: clayborg. Herald added a subscriber: mgorny. For historical reasons, Windows unfortunately doesn't support using sockets in standard system calls like read/write, which means that they also can't be buffered with a `FILE*`. This violates

[Lldb-commits] [PATCH] D59043: [lldb-vscode] Correctly propagate errors back to VS Code

zturner created this revision. zturner added a reviewer: clayborg. I tried installing this, and I couldn't get it working. VS Code would launch the adapter and then it would get stuck waiting for something to happen. Eventually, I tracked this down to the fact that it couldn't locate

[Lldb-commits] [PATCH] D58985: Fix core files for 32 bit architectures that are supported in ProcessELFCore.cpp

zturner added inline comments. Comment at: source/Plugins/Process/elf-core/ThreadElfCore.cpp:276 +else + return sizeof(ELFLinuxPrStatus) - 10 * 4; } subtracting 40 from the header size seems a bit magical to those not in the know (such as myself).

[Lldb-commits] [PATCH] D58971: Move MemoryRegionInfo into the Utility module

zturner added a comment. One idea for a new module name could be `AbstractProcess`, but I can't think of anything else better at the moment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58971/new/ https://reviews.llvm.org/D58971 ___

[Lldb-commits] [PATCH] D58972: Introduce the "Formats" module and move LinuxProcMaps parser to it

zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. Great idea, this is a nice analogue to LLVM/BinaryFormat. But, "Formats" is a little generic. What about FileFormats or something? CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D58976: Introduce core2yaml tool

zturner added a comment. In D58976#1418621 , @clayborg wrote: > Strong ownership is needed for this class IMHO because it hands out pointers > to native data I disagree here, see my previous comment. Binaries grow large very quickly, and if we always

[Lldb-commits] [PATCH] D58842: Move ProcessInstanceInfo and similar to Utility

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 rL355342: Move ProcessInfo from Host to Utility. (authored by zturner, committed by ). Herald added a project: LLVM. Herald

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

zturner added a comment. Yea it would be nice if we could remove all of the `LLDB_CONFIGURATION_xxx` macros and just use either the LLVM ones or standard ones such as NDEBUG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838

[Lldb-commits] [PATCH] D58792: Add "operator bool" to SB APIs

zturner added a comment. Out of curiosity, are there known, specific examples of users who rely on the exact mangling not changing? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58792/new/ https://reviews.llvm.org/D58792 ___ lldb-commits

[Lldb-commits] [PATCH] D58842: Move ProcessInstanceInfo and similar to Utility

zturner added a comment. In D58842#1415526 , @labath wrote: > Hmm.. I think I've thought of (or remembered, because I have been thinking > about Utility too) a reason why it might be better to keep these in Host. > ProcessLaunchInfo cannot be trivially

[Lldb-commits] [PATCH] D58842: Move ProcessInstanceInfo and similar to Utility

zturner created this revision. zturner added reviewers: jingham, labath, JDevlieghere. Herald added subscribers: jdoerfert, mgorny, emaste. There are set of classes in Target that describe the parameters of a process - e.g. it's PID, name, user id, and similar. However, since it is a bare

[Lldb-commits] [PATCH] D58833: Fix embedded Python initialization according to changes in version 3.7

zturner added reviewers: davide, stella.stamenova. zturner added a comment. Davide ran into this issue recently and tried to fix it, so I'm adding him here as a reviewer. Also Stella is interested in Python 3 support as well, so adding her also. Repository: rLLDB LLDB CHANGES SINCE LAST

[Lldb-commits] [PATCH] D58730: Rename Symbols.cpp from Host to Symbols/LocateSymbolFile.{h, cpp}

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB355032: Move Host/Symbols.cpp to Symbols/LocateSymbolFile.cpp (authored by zturner, committed by ). Herald added subscribers: abidh, krytarowski. Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D58730: Rename Symbols.cpp from Host to Symbols/LocateSymbolFile.{h, cpp}

zturner created this revision. zturner added reviewers: labath, JDevlieghere, jingham. Herald added subscribers: jdoerfert, MaskRay, arichardson, mgorny, emaste. Herald added a reviewer: espindola. After this change, there is only 1 remaining dependency in Host to Target. While this change might

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

zturner added a comment. In D58654#1411084 , @labath wrote: > BTW, what's the reason that you have to have this split now? I understand its > attractiveness from a Zen perspective, but it seems to me that at the end of > the day, it doesn't have that

[Lldb-commits] [PATCH] D58350: Insert random blocks of python code with swig instead of modify-python-lldb.py

zturner accepted this revision. zturner added a comment. Also lgtm. Don't call the commit message "Insert random blocks of python code" though. Hopefully the code we're inserting is not actually random :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58350/new/

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

zturner added a comment. > Since we are stepping over we can safely ignore these calls since they will > return to the next instruction What if the call throws an exception? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58678/new/

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

zturner added a comment. In D58654#1410817 , @JDevlieghere wrote: > I'm not a fan of introducing another library for this. Without a clear > timeline of "fixing" Host it's basically as temporary as anything else in > software development. I also think

[Lldb-commits] [PATCH] D58167: Refactor user/group name resolving code

zturner added inline comments. Comment at: include/lldb/Host/UserIDResolver.h:9 + +#ifndef LLDB_HOST_USERIDRESOLVER_H +#define LLDB_HOST_USERIDRESOLVER_H I wonder if this class should actually be in Host. While some specific implementation of it might be

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

zturner updated this revision to Diff 188393. zturner added a comment. Moved to `System` instead of `Utility` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58654/new/ https://reviews.llvm.org/D58654 Files: lldb/cmake/XcodeHeaderGenerator/CMakeLists.txt

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

zturner added a comment. In D58654#1410080 , @jingham wrote: > There aren't many more platforms (OpenVMS?) that are likely to show up and > need support for lldb, so maybe this is making too much of the matter. But > having there be a well defined set

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

zturner added a comment. In D58654#1409974 , @jingham wrote: > Utility is supposed to be a bunch of stand-alone utility files & headers that > we gather together for convenience's sake. > > Host is where we put all the code that is specific to one or

[Lldb-commits] [PATCH] D58654: Move Config.h from Host to Utility

zturner created this revision. zturner added reviewers: labath, jgorbe, JDevlieghere, davide. Herald added subscribers: mgorny, emaste. `Host/Config.h` is where we have platform specific preprocessor defines that are configured at CMake time. Then, we can include this file `lldb/Host/Config.h`

[Lldb-commits] [PATCH] D57780: Don't include UnixSignals.h from Host

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB354168: Dont include UnixSignals.h from Host. (authored by zturner, committed by ). Herald added subscribers: abidh, arichardson, emaste. Herald added a reviewer: espindola. Herald added a project:

[Lldb-commits] [PATCH] D58172: Embed swig version into lldb.py in a different way

zturner added a comment. Long term, I wonder if we can just delete this entire `modify-python-lldb.py` script. it seems like the entire purpose is to make sure that the SB methods support iteration, equality, and some other basic stuff, and it does this by inserting lines of python code into

[Lldb-commits] [PATCH] D57840: Convert TestCommandRegex to lit (from expect)

zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. Can you rename these two tests to `command-regex-delete.test` and `command-regex-unalias.test`? Otherwise, lgtm Repository: rL LLVM CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D57780: Don't include UnixSignals.h from Host

zturner created this revision. zturner added a reviewer: labath. Herald added a reviewer: jfb. This file lives in Target, which makes sense since it is supposed to represent the various types of signals that can occur on any platform (since you might be remote debugging a platform with

[Lldb-commits] [PATCH] D56904: [NativePDB] Process virtual bases in the correct order

zturner added a comment. In D56904#1383148 , @aleksandr.urakov wrote: > Ping! What do you think about it? Generally the change looks good, but I'm not sure why the compiler can't properly compile the source file. I know it's something to do with the

[Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

zturner added inline comments. Comment at: source/Target/PathMappingList.cpp:204 FileSpec _spec) const { if (!m_pairs.empty()) { +std::string orig_path = orig_spec.GetPath(); Can we put some early returns in here? ```

[Lldb-commits] [PATCH] D57213: [Scalar] Add support for 512-bits values.

zturner added inline comments. Comment at: lldb/source/Utility/Scalar.cpp:168 + swapped_words[6] = apint_words[1]; + swapped_words[7] = apint_words[0]; + apint_words = swapped_words; davide wrote: > davide wrote: > > aprantl wrote: > > >

[Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. This is really cool, thanks for doing this! Comment at: source/Expression/DWARFExpression.cpp:3253 - return false; + return true; } Why do we change

[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB352557: Fix some warnings in building LLDB. (authored by zturner, committed by ). Herald added a subscriber: abidh. Changed prior to commit: https://reviews.llvm.org/D57413?vs=184163=184186#toc

[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

zturner marked an inline comment as done. zturner added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:304-314 -union { - struct { -uint64_t _mutations; - }; - struct { -uint32_t _muts; -uint32_t

[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

zturner marked an inline comment as done. zturner added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:304-314 -union { - struct { -uint64_t _mutations; - }; - struct { -uint32_t _muts; -uint32_t

[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

zturner created this revision. zturner added reviewers: davide, JDevlieghere, jingham. Most of these are surrounding the ObjectiveC formatters. They use a lot of unions containing unnamed structs, which are not ISO C++ compliant and which gcc warns about. There are several other minor

[Lldb-commits] [PATCH] D56602: Move FileAction, ProcessInfo and ProcessLaunchInfo from Target to Host

zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. Seems I had forgotten about this. It looks pretty good to me, and I'm glad we're very close to breaking this dependency. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56602/new/

[Lldb-commits] [PATCH] D56904: [NativePDB] Process virtual bases in the correct order

zturner added a comment. Sorry, due to the way Phabricator re-orders feedback across files when sending the email notification, if you're reading my comments in email you have to read the previous email from the bottom up. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D56904: [NativePDB] Process virtual bases in the correct order

zturner added a comment. The above suggestion is admittedly minor, but since it's both a minor performance improvement **and** a minor readability/maintainability improvement, I think it's probably worth doing. Comment at: lit/SymbolFile/NativePDB/tag-types.cpp:5 // Test

[Lldb-commits] [PATCH] D56126: [NativePDB] Add basic support of methods recostruction in AST

zturner added a comment. Thanks for the reminder! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56126/new/ https://reviews.llvm.org/D56126 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D57213: [Scalar] Add support for 512-bits values.

zturner added inline comments. Comment at: lldb/source/Utility/Scalar.cpp:161 +if (endian::InlHostByteOrder() == eByteOrderBig) { + swapped_words[0] = apint_words[7]; + swapped_words[1] = apint_words[6]; I'm confused. You say it returns a pointer

  1   2   3   4   5   6   7   8   >