[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

sas accepted this revision. sas added a comment. This revision is now accepted and ready to land. LGTM for now. https://reviews.llvm.org/D47792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

sas added a comment. > One idea I had was to introduce another target for the framwork itself, e.g. > lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would > depend on liblldb and all the necessary tools, headers, etc, that the > framework would need. That way liblldb can

[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

sas added inline comments. Comment at: source/API/CMakeLists.txt:184 + set(EXECUTABLE_NAME "LLDB") + set(CURRENT_PROJECT_VERSION "360.99.0") set_target_properties(liblldb PROPERTIES Hardcoding this here isn't ideal. Where did you get this number from? Is

[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

sas added a comment. Don't we risk creating circular dependencies with this? What happens when a tool depends on liblldb? you'll have theTool depend on liblldb and liblldb depend on theTool. https://reviews.llvm.org/D47801 ___ lldb-commits

[Lldb-commits] [PATCH] D47742: Change SWIG output directory when building LLDB.framework with CMake

sas added inline comments. Comment at: scripts/CMakeLists.txt:38 OUTPUT ${LLDB_WRAP_PYTHON} - OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/lldb.py DEPENDS ${SWIG_SOURCES} xiaobai wrote: > sas wrote: > > ...and this path were not the same before your change. Was

[Lldb-commits] [PATCH] D47742: Change SWIG output directory when building LLDB.framework with CMake

sas accepted this revision. sas added inline comments. This revision is now accepted and ready to land. Comment at: CMakeLists.txt:149 DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py -DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/scripts/lldb.py

[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

sas added a comment. In https://reviews.llvm.org/D40539#937900, @clayborg wrote: > In https://reviews.llvm.org/D40539#937854, @sas wrote: > > > Basically, if you have a `.debug` directory in the same directory where the > > original object file is, you can have debug symbols there. For

[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

sas added a comment. In https://reviews.llvm.org/D40537#937880, @zturner wrote: > In https://reviews.llvm.org/D40537#937866, @sas wrote: > > > In https://reviews.llvm.org/D40537#937196, @zturner wrote: > > > > > You could use llvm's range adapters to make this slightly better. > > > > > > auto

[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

sas added a comment. In https://reviews.llvm.org/D40537#937862, @clayborg wrote: > A better solution would be to initialize UUID::m_num_uuid_bytes with zero and > only set it to a valid value if we set bytes into it. Then UUID::IsValid() > becomes easy: > > bool UUID::IsValid() const {

[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

sas added a comment. In https://reviews.llvm.org/D40537#937196, @zturner wrote: > You could use llvm's range adapters to make this slightly better. > > auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes); > return llvm::find(Bytes, 0) != Bytes.end(); > > > Another option would just be

[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

sas added a comment. Basically, if you have a `.debug` directory in the same directory where the original object file is, you can have debug symbols there. For instance, you can have: /my/project/myElf.exe /my/project/.debug/myElf.exe with the first file being a standard stripped elf file, and

[Lldb-commits] [PATCH] D40536: Simplify UUID constructors

This revision was automatically updated to reflect the committed changes. Closed by commit rL319191: Simplify UUID constructors (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D40536 Files: lldb/trunk/source/Utility/UUID.cpp Index: lldb/trunk/source/Utility/UUID.cpp

[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

sas created this revision. Herald added subscribers: JDevlieghere, emaste. In ObjectFileELF we try to read the `.gnu_debuglink` section of the ELF file to determine the name of the debug symbols file. If this section does not exist, we stop the search. Instead, what we should do is locate a file

[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

sas created this revision. This change also prevents looking further than the size of the current UUID. https://reviews.llvm.org/D40537 Files: source/Utility/UUID.cpp Index: source/Utility/UUID.cpp === ---

[Lldb-commits] [PATCH] D40536: Simplify UUID constructors

sas created this revision. This remove a small amount of duplicated code. https://reviews.llvm.org/D40536 Files: source/Utility/UUID.cpp Index: source/Utility/UUID.cpp === --- source/Utility/UUID.cpp +++

[Lldb-commits] [PATCH] D40519: Remove some duplicated code in UUID.cpp

This revision was automatically updated to reflect the committed changes. Closed by commit rL319132: Remove some duplicated code in UUID.cpp (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D40519 Files: lldb/trunk/source/Utility/UUID.cpp Index:

[Lldb-commits] [PATCH] D40519: Remove some duplicated code in UUID.cpp

sas updated this revision to Diff 124499. sas added a comment. Address @zturner's comment and remove a useless temporary variable. https://reviews.llvm.org/D40519 Files: source/Utility/UUID.cpp Index: source/Utility/UUID.cpp

[Lldb-commits] [PATCH] D40519: Remove some duplicated code in UUID.cpp

sas created this revision. Formatting needs to be done only once. Ran check-lldb and nothing changes. https://reviews.llvm.org/D40519 Files: source/Utility/UUID.cpp Index: source/Utility/UUID.cpp === ---

[Lldb-commits] [PATCH] D40517: Mark UUID::GetByteSize() const

This revision was automatically updated to reflect the committed changes. Closed by commit rL319095: Mark UUID::GetByteSize() const (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D40517 Files: lldb/trunk/include/lldb/Utility/UUID.h lldb/trunk/source/Utility/UUID.cpp

[Lldb-commits] [PATCH] D40517: Mark UUID::GetByteSize() const

sas created this revision. This method doesn't modify anything in the object it's called on so we can mark it const to make it usable in a const context. https://reviews.llvm.org/D40517 Files: include/lldb/Utility/UUID.h source/Utility/UUID.cpp Index: source/Utility/UUID.cpp

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

sas added a comment. Friendly ping. https://reviews.llvm.org/D39315 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39692: Disable tests in lang/c/shared_lib on Windows

This revision was automatically updated to reflect the committed changes. Closed by commit rL317529: Disable tests in lang/c/shared_lib on Windows (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D39692 Files:

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

This revision was automatically updated to reflect the committed changes. Closed by commit rL317501: Add a dependency from check-lldb on lld (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D39689 Files: lldb/trunk/test/CMakeLists.txt Index: lldb/trunk/test/CMakeLists.txt

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

sas updated this revision to Diff 121760. sas added a comment. Move check out of if (TARGET clang) block. https://reviews.llvm.org/D39689 Files: test/CMakeLists.txt Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

sas updated this revision to Diff 121750. sas added a comment. Check only on Windows. https://reviews.llvm.org/D39689 Files: test/CMakeLists.txt Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++ test/CMakeLists.txt @@

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

sas added a comment. @davide: I agree, the extra dependency is annoying. FWIW, I was trying to run tests on Windows and nothing worked because lld wasn't built. Maybe the behavior is different on Windows and on other hosts? I just assumed this was the new default because I saw a couple of

[Lldb-commits] [PATCH] D39429: Use LLVM version string

sas created this revision. macOS builds of LLDB use the bundle version from `tools/driver/lldb-Info.plist`. That file hasn't been updated since the 4.0 release so the version we print provides no value. I also think that even if it were up to date, that number has no meaning and displaying the

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

sas added a subscriber: fjricci. sas added a comment. In https://reviews.llvm.org/D39315#908284, @zturner wrote: > So when you're using ds2 as the remote, are you still using the normal lldb > test suite? `dotest.py`? If so, then we could have a test decorator that > says

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

sas added a comment. In https://reviews.llvm.org/D39315#908251, @zturner wrote: > In https://reviews.llvm.org/D39315#908246, @sas wrote: > > > Same thing here, I have no idea how to go about testing something specific > > like this. Given that this is Windows Phone, it's even worse than simply

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

sas added a comment. Same thing here, I have no idea how to go about testing something specific like this. Given that this is Windows Phone, it's even worse than simply Windows Desktop because the only way to test is by doing remote debugging. I suppose checking in binaries to the tree so we

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

sas added a comment. @clayborg: the follow-up to this change is in https://reviews.llvm.org/D39337, where we implement `GetAddressClass()`. https://reviews.llvm.org/D39315 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D39337: Implement a simple GetAddressClass for PECOFF

sas created this revision. This function apparently is called only by addresses that are valid. We mainly need to use the ISA class whenever is needed. Depends on https://reviews.llvm.org/D39315. Change by Walter Erquinigo https://reviews.llvm.org/D39337 Files:

[Lldb-commits] [PATCH] D19603: Fix entry point lookup for ObjectFilePECOFF

sas added a comment. In https://reviews.llvm.org/D19603#908130, @clayborg wrote: > Does this need some ARM support where we strip bit zero in case the entry > point is Thumb? Good question. Somehow we never had any issue with this but I don't remember explicitly checking for difference with

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

sas updated this revision to Diff 120447. sas added a comment. Address comments by @clayborg. https://reviews.llvm.org/D39315 Files: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

[Lldb-commits] [PATCH] D39335: Allow SysV-i386 ABI on everything other than Apple targets

This revision was automatically updated to reflect the committed changes. Closed by commit rL316673: Allow SysV-i386 ABI on everything other than Apple targets (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D39335 Files:

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

sas requested review of this revision. sas added a comment. I saw the `bool is_arm = ` pattern only in `ObjectFileMachO.cpp`. I'm not sure about the specifics for Darwin targets, but having an object file with some functions in ARM and some functions in Thumb is valid, so I think checking for

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

sas added a comment. In https://reviews.llvm.org/D39314#908065, @davide wrote: > can you please try adding a test for this one? :) To be fully honest, I'm not 100% sure how I'm supposed to add tests for this. I looked through the directories containing unit tests and didn't find anything

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

sas created this revision. Herald added subscribers: kristof.beyls, aemerson. This assumes that the environment is always Thumb Change by Walter Erquinigo https://reviews.llvm.org/D39315 Files: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Index:

[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

sas created this revision. This commit implements basic DidAttach and DidLaunch for the windows DynamicLoader plugin which allow us to load shared libraries from the inferior. This is an improved version of https://reviews.llvm.org/D12245 and should work much better.

[Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.

This revision was automatically updated to reflect the committed changes. Closed by commit rL316532: Allow ObjectFilePECOFF to initialize with ARM binaries. (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D19604 Files:

[Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.

sas updated this revision to Diff 120146. sas added a comment. Herald added a subscriber: kristof.beyls. Rebase. https://reviews.llvm.org/D19604 Files: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

[Lldb-commits] [PATCH] D38373: Add a few missing newlines in lldb-server messages

This revision was automatically updated to reflect the committed changes. Closed by commit rL314455: Add a few missing newlines in lldb-server messages (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D38373 Files: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp Index:

[Lldb-commits] [PATCH] D38373: Add a few missing newlines in lldb-server messages

sas created this revision. https://reviews.llvm.org/D38373 Files: tools/lldb-server/lldb-gdbserver.cpp Index: tools/lldb-server/lldb-gdbserver.cpp === --- tools/lldb-server/lldb-gdbserver.cpp +++

[Lldb-commits] [PATCH] D22231: Implement trampoline step-through for Windows-x86.

This revision was automatically updated to reflect the committed changes. Closed by commit rL314045: Implement trampoline step-through for Windows-x86. (authored by sas). Repository: rL LLVM https://reviews.llvm.org/D22231 Files:

[Lldb-commits] [PATCH] D22231: Implement trampoline step-through for Windows-x86.

sas updated this revision to Diff 116440. sas added a comment. Rebase + clang-format. https://reviews.llvm.org/D22231 Files: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp

[Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

sas accepted this revision. sas added a comment. This revision is now accepted and ready to land. Seems fine. https://reviews.llvm.org/D37651 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D35036: switch on enum should be exhaustive and warning-free

sas added a comment. Putting that check in an inline function instead of doing ad-hoc validation in the call-site seems better indeed. https://reviews.llvm.org/D35036 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D35036: switch on enum should be exhaustive and warning-free

sas accepted this revision. sas added a comment. This revision is now accepted and ready to land. Looks like the numeric values of `TypeCode` are contiguous. It would probably be cleaner to a check after the cast to validate the value instead of having to add a line for each label in the

[Lldb-commits] [PATCH] D33853: Avoid invalid string access in ObjCLanguage::MethodName::SetName

This revision was automatically updated to reflect the committed changes. Closed by commit rL304725: Avoid invalid string access in ObjCLanguage::MethodName::SetName (authored by sas). Changed prior to commit: https://reviews.llvm.org/D33853?vs=101284=101429#toc Repository: rL LLVM

[Lldb-commits] [PATCH] D33853: Avoid invalid string access in ObjCLanguage::MethodName::SetName

sas added a comment. This seems like a fairly straightforward fix so I'll merge it. https://reviews.llvm.org/D33853 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D33853: Avoid invalid string access in ObjCLanguage::MethodName::SetName

sas created this revision. Don't access `name[1] if the string is only of length 1. Avoids a crash/assertion failure when parsing the string `-`. Test Plan: Debug a swift binary, set a breakpoint, watch lldb not crash Original change by Paul Menage

[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning

sas accepted this revision. sas added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp:59 + assert(m_num_registers == + static_cast(m_registers_count[gpr_registers_count] + +

[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient

This revision was automatically updated to reflect the committed changes. Closed by commit rL299239: Verify memory address range validity in GDBRemoteCommunicationClient (authored by sas). Changed prior to commit: https://reviews.llvm.org/D31485?vs=93674=93685#toc Repository: rL LLVM

[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient

sas added a reviewer: clayborg. sas added a subscriber: clayborg. sas added a comment. Doing additional checking on the packets returned over the wire seems decent to me. CC'ing @clayborg to see what he thinks about it also. https://reviews.llvm.org/D31485

[Lldb-commits] [PATCH] D31086: Remove FileSystem::MakeDirectory

sas added a comment. In https://reviews.llvm.org/D31086#705159, @labath wrote: > In https://reviews.llvm.org/D31086#704518, @sas wrote: > > > The second behavioral change seems good but the first thing you described > > is a bit odd. Creating folders with 770 does not seem like a common > >

[Lldb-commits] [PATCH] D31086: Remove FileSystem::MakeDirectory

sas accepted this revision. sas added a comment. This revision is now accepted and ready to land. The second behavioral change seems good but the first thing you described is a bit odd. Creating folders with 770 does not seem like a common behavior, and 700 or 755 is usually more standard.

[Lldb-commits] [PATCH] D31108: Delete LLDB code for MD5'ing a file. Use LLVM instead

sas accepted this revision. sas added a comment. This revision is now accepted and ready to land. So nice. https://reviews.llvm.org/D31108 ___ lldb-commits mailing list lldb-commits@lists.llvm.org