[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

xiaobai added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:42 /// This will compute the clang resource directory assuming that clang was -/// installed with the same prefix as lldb. +/// installed with the same prefix as lldb. Note: the verify

[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB357030: [ExpressionParser] Add swift-lldb case for finding clang resource dir (authored by xiaobai, committed by ). Herald added a subscriber: teemperor. Herald added a project: LLDB. Changed prior to

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

xiaobai added a comment. Thank you for taking the time to write this up! I wish I could have read this when I started working on LLDB. :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 ___ lldb-com

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

xiaobai created this revision. xiaobai added reviewers: compnerd, sgraenitz, labath. Herald added subscribers: kristof.beyls, javed.absar, mgorny. xiaobai added a reviewer: mgorny. This line is unnecessary because add_llvm_executable will handle linking the correct LLVM libraries for you. LLDB sta

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

xiaobai added a comment. In D60180#1453106 , @sgraenitz wrote: > Major concern from my side is that clang does something like this too: > > clang/CMakeLists.txt > 86: set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib") > 125: link_di

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

xiaobai added a comment. So I ran the lldb test suite from a standalone build tree, and this patch didn't change anything. I added some logging to the CMake and `LLVM_LIBRARY_DIR` is being set correctly. It appears to be getting it from the LLVMConfig we get from `find_package(LLVM)`. I think w

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

xiaobai added a comment. In D60180#1454634 , @sgraenitz wrote: > > LLVM_LIBRARY_DIR is being set correctly. It appears to be getting it from > > the LLVMConfig we get from find_package(LLVM). > > Yes LLVMConfig sets the variable, but it's not going to be

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

xiaobai added a comment. In D60180#1456263 , @sgraenitz wrote: > > No, everything is being built for android. Cross-compiling lldb-server > > involves cross-compiling llvm libraries, clang libraries, and if you've got > > swift in the picture, swift host

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB357817: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds (authored by xiaobai, committed by ). Herald added a subscriber: abidh. Herald added a project: LLDB. Changed prior to commit:

[Lldb-commits] [PATCH] D154513: [lldb][NFC] Factor out code from SymbolFileDWARF::ParseVariableDIE

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Makes sense to me. There are some potential improvements we could make to this code (which I'm sure you can already identify), but to keep this change relatively risk-free we should prob

[Lldb-commits] [PATCH] D152870: [lldb][NFCI] Remove StructuredData::Array::GetItemAtIndexAsString overloads with ConstString

bulbazord added inline comments. Comment at: lldb/source/Target/DynamicRegisterInfo.cpp:204 +m_sets.push_back( +{ConstString(set_name).AsCString(), nullptr, 0, nullptr}); } else { mib wrote: > I guess `m_sets` is a vector of `char*` ...

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

bulbazord added inline comments. Comment at: lldb/include/lldb/Core/Debugger.h:455-456 +InterruptionReport(std::string function_name, std::string description) : +m_function_name(function_name), +m_description(description), +m_interrupt_time(std::chron

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

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

[Lldb-commits] [PATCH] D154549: [lldb][NFCI] Remove use of Stream * from TypeSystem

This revision was automatically updated to reflect the committed changes. Closed by commit rG564ff8ffe75d: [lldb][NFCI] Remove use of Stream * from TypeSystem (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154549/new/ https:/

[Lldb-commits] [PATCH] D154752: runCmd should print the command before running it in case of crashes

bulbazord accepted this revision. bulbazord added a comment. Oh I really like this. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154752/new/ https://reviews.llvm.org/D154752 ___ lldb-commits mai

[Lldb-commits] [PATCH] D154757: [lldb][NFCI] TestEmulation should take a Stream ref

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, fdeazeve. Herald added subscribers: luke, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, s

[Lldb-commits] [PATCH] D154759: [lldb][NFCI] Change type of UnwindPlan::m_source_name

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jasonmolenda, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. m_source_name is currently a ConstString. Comparing

[Lldb-commits] [PATCH] D154757: [lldb][NFCI] TestEmulation should take a Stream ref

This revision was automatically updated to reflect the committed changes. Closed by commit rG1e7101a3d958: [lldb][NFCI] TestEmulation should take a Stream ref (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154757/new/ https:/

[Lldb-commits] [PATCH] D154883: [lldb][NFCI] Methods to load scripting resources should take a Stream by reference

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added subscribers: lldb-commits, wangpc. Herald added a project: LLDB. These methods all take a `Stream *` to get feedback about

[Lldb-commits] [PATCH] D154890: [lldb][NFCI] Avoid construction of temporary std::strings in Variable

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, fdeazeve. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. A common thing to do is to call `str().c_str()` to get

[Lldb-commits] [PATCH] D154759: [lldb][NFCI] Change type of UnwindPlan::m_source_name

bulbazord added a comment. In D154759#4487381 , @jasonmolenda wrote: > I guess simplest is we can have all the UnwindPlan creators have a `static > g_unwindplan_name` and save a pointer to that. Each one is unique, don't > need anything fancier than t

[Lldb-commits] [PATCH] D154890: [lldb][NFCI] Avoid construction of temporary std::strings in Variable

This revision was automatically updated to reflect the committed changes. Closed by commit rGe7c48ffde1c8: [lldb][NFCI] Avoid construction of temporary std::strings in Variable (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15

[Lldb-commits] [PATCH] D154883: [lldb][NFCI] Methods to load scripting resources should take a Stream by reference

This revision was automatically updated to reflect the committed changes. Closed by commit rG1d796b48e4d4: [lldb][NFCI] Methods to load scripting resources should take a Stream by… (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D152837: [lldb] Identify Swift-implemented ObjC classes

bulbazord added a comment. My concerns have been addressed, LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152837/new/ https://reviews.llvm.org/D152837 ___ lldb-commits mailing list lldb-commits@lis

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support to Rust enums in TypeSystemClang

bulbazord added a comment. I'm curious to know why you don't try and implement some kind of `TypeSystemRust`? I assume it's going to be a lengthy process, but eventually you (or somebody else) is going to want to implement a `TypeSystemRust` in some form and they'll have to revisit all of rust-

[Lldb-commits] [PATCH] D155030: [lldb][NFCI] Avoid construction of temporary std::strings in RegisterValue

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D153735: [lldb][LocateModuleCallback] Implement API, Python interface

bulbazord added inline comments. Comment at: lldb/source/API/SBPlatform.cpp:664-689 + if (platform_sp) { +if (callback) { + platform_sp->SetLocateModuleCallback( + [callback, callback_baton](const ModuleSpec &module_spec, + F

[Lldb-commits] [PATCH] D155035: [lldb][NFCI] Remove unneeded temporary std::string allocations in SBAPI

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This uses some friend class trickery to avoid some unne

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support to Rust enums in TypeSystemClang

bulbazord added a comment. In D149213#4491889 , @VladimirMakaev wrote: > In D149213#4491594 , @bulbazord > wrote: > >> I'm curious to know why you don't try and implement some kind of >> `TypeSystemRust`? I ass

[Lldb-commits] [PATCH] D155100: [lldb][NFC] Factor out code linking OSO addr of uninitialized GVs

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

[Lldb-commits] [PATCH] D155129: [lldb] Move CommandOverrideCallbackWithResult to lldb_private namespace

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This has an `lldb_private` type in its parameter, it sh

[Lldb-commits] [PATCH] D155137: [lldb] Forward declare SBPlatform and SBTypeMember in SBDefines

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.

[Lldb-commits] [PATCH] D155137: [lldb] Forward declare SBPlatform and SBTypeMember in SBDefines

This revision was automatically updated to reflect the committed changes. Closed by commit rG1d7a3dcff778: [lldb] Forward declare SBPlatform and SBTypeMember in SBDefines (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155137/n

[Lldb-commits] [PATCH] D155129: [lldb] Move CommandOverrideCallbackWithResult to lldb_private namespace

This revision was automatically updated to reflect the committed changes. Closed by commit rGc25e48c5d776: [lldb] Move CommandOverrideCallbackWithResult to lldb_private namespace (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

bulbazord added a comment. Very interesting, I think this will be pretty useful for the ergonomics of the SBAPI from python! Comment at: lldb/bindings/python/python-typemaps.swig:74-75 +if (!lldb_module) { + llvm::StringRef err_msg = llvm::toString(lldb_module.takeErr

[Lldb-commits] [PATCH] D155035: [lldb][NFCI] Remove unneeded temporary std::string allocations in SBAPI

This revision was automatically updated to reflect the committed changes. Closed by commit rG7ee633a24b8c: [lldb][NFCI] Remove unneeded temporary std::string allocations in SBAPI (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D155030: [lldb][NFCI] Avoid construction of temporary std::strings in RegisterValue

This revision was automatically updated to reflect the committed changes. Closed by commit rG1b29a848ffa2: [lldb][NFCI] Avoid construction of temporary std::strings in RegisterValue (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[Lldb-commits] [PATCH] D155653: [lldb][NFCI] Add some missing SB class forward declarations

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I noticed these were missing. Repository: rG LLVM Github Mono

[Lldb-commits] [PATCH] D155161: [lldb] Convert script native types to StructuredData counterpart

bulbazord added a comment. I'm good with this approach. One thing to note is that this change is explicitly **ABI breaking**. Specifically this change removes `lldb::ScriptedObject` so `SBProcess::GetScriptedImplementation`'s return value changes. `lldb::ScriptedObject` was originally added in

[Lldb-commits] [PATCH] D155653: [lldb][NFCI] Add some missing SB class forward declarations

bulbazord updated this revision to Diff 543631. bulbazord added a comment. Rebase onto main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155653/new/ https://reviews.llvm.org/D155653 Files: lldb/include/lldb/API/SBDefines.h Index: lldb/include

[Lldb-commits] [PATCH] D155653: [lldb][NFCI] Add some missing SB class forward declarations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5876eee1ff92: [lldb][NFCI] Add some missing SB class forward declarations (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SI

[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. As of 0beffb854209a41f31beb18f9631258349a99299

[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

This revision was automatically updated to reflect the committed changes. Closed by commit rGa809720102fa: [lldb][NFCI] Change logic to find clang resource dir in standalone builds (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D156447: [lldb] Split CTF parsing and type creation (NFC)

bulbazord added inline comments. Comment at: lldb/source/Plugins/SymbolFile/CTF/CTFTypes.h:1 +//===-- CTFType.h ---*- C++ -*-===// +// Comment at: lldb/source/Plugins/SymbolFile/CTF/CTFTypes.h:9-10

[Lldb-commits] [PATCH] D156367: [BSDArchive] NULL check the child object file ptr before accessing its member

bulbazord added a comment. Herald added a subscriber: JDevlieghere. Hey, I think this may have broken the green dragon bots. Could you take a look and fix forward or revert? Thanks https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/3262/ (You can ignore the `import-std-module` tests,

[Lldb-commits] [PATCH] D156447: [lldb] Split CTF parsing and type creation (NFC)

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Although I'm not super knowledgable about CTF, this kind of refactor makes sense. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156447/new/ https://reviews.llvm.org/D1564

[Lldb-commits] [PATCH] D156483: [lldb] Support CTF forward declarations

bulbazord added a comment. Looks like a straightforward follow-up to D156447 . One small nit but otherwise looks good. Comment at: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp:530 + CompilerType forward_compiler_type = m_ast->CreateRe

[Lldb-commits] [PATCH] D156490: [lldb] Fix CTF parsing of large structs

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM modulo spelling nit Comment at: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.h:257 static constexpr uint8_t g_ctf_version = 4; + static constexpr uint32_t

[Lldb-commits] [PATCH] D156562: [lldb] Clean up uses of UuidCompatibility.h

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This commit does a few related things: - Removes u

[Lldb-commits] [PATCH] D156562: [lldb] Clean up uses of UuidCompatibility.h

bulbazord added inline comments. Comment at: lldb/include/lldb/Utility/AppleUuidCompatibility.h:15 // uuid_t is guaranteed to always be a 16-byte array typedef unsigned char uuid_t[16]; #endif // utility_UUID_COMPATIBILITY_H mib wrote: > May be this could be m

[Lldb-commits] [PATCH] D156562: [lldb] Clean up uses of UuidCompatibility.h

bulbazord added a comment. In D156562#4543743 , @jasonmolenda wrote: > This is fine to me but maybe instead of `ifndef apple` we could do a `if > __has_include()` and include the system header if it's avail. I > must be misreading the patch but it see

[Lldb-commits] [PATCH] D156562: [lldb] Clean up uses of UuidCompatibility.h

bulbazord updated this revision to Diff 545289. bulbazord added a comment. Fix incorrect includes that @jasonmolenda pointed out Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156562/new/ https://reviews.llvm.org/D156562 Files: lldb/include/lldb/

[Lldb-commits] [PATCH] D156562: [lldb] Clean up uses of UuidCompatibility.h

bulbazord added a comment. In D156562#4543743 , @jasonmolenda wrote: > This is fine to me but maybe instead of `ifndef apple` we could do a `if > __has_include()` and include the system header if it's avail. One reason I decided not to do this is becau

[Lldb-commits] [PATCH] D156562: [lldb] Clean up uses of UuidCompatibility.h

This revision was automatically updated to reflect the committed changes. Closed by commit rGdb9087a696a6: [lldb] Clean up uses of UuidCompatibility.h (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156562/new/ https://reviews

[Lldb-commits] [PATCH] D156763: [lldb] Fix building LLDB standlone without framework

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In a809720102fae8d1b5a7073f99f9dae9395c5f41

[Lldb-commits] [PATCH] D156764: [lldb][NFCI] More forward declarations in SBDefines.h

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, clayborg. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I noticed these were missing. Repository: rG LLVM G

[Lldb-commits] [PATCH] D156804: [lldb] Bump SWIG minimum version to 4

bulbazord added a comment. `python.swig` has a few checks for SWIG versions 3.0.9 and 3.0.10, those can be removed as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156804/new/ https://reviews.llvm.org/D156804 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D156763: [lldb] Fix building LLDB standlone without framework

This revision was automatically updated to reflect the committed changes. Closed by commit rG6888de118707: [lldb] Fix building LLDB standlone without framework (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156763/new/ https:

[Lldb-commits] [PATCH] D156764: [lldb][NFCI] More forward declarations in SBDefines.h

This revision was automatically updated to reflect the committed changes. Closed by commit rGb8aba26c82f2: [lldb][NFCI] More forward declarations in SBDefines.h (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156764/new/ https

[Lldb-commits] [PATCH] D156838: [lldb] Re-add compile-time checks for SPI in HosInfoMacOSX

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jasonmolenda, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. There were some checks removed previously in bc1969

[Lldb-commits] [PATCH] D156838: [lldb] Re-add compile-time checks for SPI in HosInfoMacOSX

This revision was automatically updated to reflect the committed changes. Closed by commit rG6c2e3249160f: [lldb] Re-add compile-time checks for SPI in HosInfoMacOSX (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156838/new/

[Lldb-commits] [PATCH] D156919: [lldb/crashlog] Make register output match lldb ordering in legacy mode

bulbazord added a comment. In general looks good to me. I have one suggestion and one question. Comment at: lldb/examples/python/crashlog.py:104 +registers_info = None +if self.arch: +if "x86_64" == self.arch: Is there a

[Lldb-commits] [PATCH] D156934: [lldb][NFCI] Add SBTraceCursor.h to swig headers file

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D156934 F

[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

bulbazord added a comment. In D156270#4557902 , @mgorny wrote: > I'm not sure if it was really intended but the new API is kinda horrible, at > least for us in Gentoo. > > Our install prefix is `/usr/lib/llvm/NN`, whereas clang resource dir is > `/usr/l

[Lldb-commits] [PATCH] D156997: [LLDB][CMake][NFC] Remove unused LLDB_LINKER_SUPPORTS_GROUPS variable

bulbazord accepted this revision. bulbazord added a comment. LGTM. I don't see any actual uses of this on llvm.org. If somebody needs it they can re-add it and add their use case or they can maintain a patch downstream. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[Lldb-commits] [PATCH] D157022: Fix the NSIndexSet formatter for macOS Sonoma

bulbazord added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:278 + && descriptor->GetTaggedPointerInfo(nullptr, nullptr, &payload)) { +count = __builtin_popcountll(payload); +break; I'm pretty sure `__builtin_p

[Lldb-commits] [PATCH] D157022: Fix the NSIndexSet formatter for macOS Sonoma

bulbazord added a comment. It would be nice if we could clean up what `mode` is. There's a lot of bit-wise operations and setting it to 1 and 2 and it's not clear what each one might mean in each case... Some kind of enum or semantic meaning for each number would be nice. Either way, I think I'

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

bulbazord added a comment. A few questions: - Do all of these need to be protected with a mutex? In your description you're saying TSan is detecting data races. What piece of data are you observing the data race on? - Do we need a recursive mutex? I assume that these operations might call into

[Lldb-commits] [PATCH] D157041: [lldb] Protect OptionValue accesses from data races

bulbazord added a comment. In D157041#4559255 , @augusto2112 wrote: >> Do all of these need to be protected with a mutex? In your description >> you're saying TSan is detecting data races. What piece of data are you >> observing the data race on? > > O

[Lldb-commits] [PATCH] D157043: [lldb/crashlog] Make TextCrashLogParser more resilient to new lines

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Looks alright to me. I think it would be interesting to replace a lot of the index juggling code with python Iterators that can only iterate over the lines that are not empty, but that's

[Lldb-commits] [PATCH] D157044: [lldb/crashlog] Fix sticky image parsing logic

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Python default parameter values strike again! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157044/new/ https://reviews.llvm.org/D157044

[Lldb-commits] [PATCH] D156934: [lldb][NFCI] Add SBTraceCursor.h to swig headers file

This revision was automatically updated to reflect the committed changes. Closed by commit rG08dc847d55fb: [lldb][NFCI] Add SBTraceCursor.h to swig headers file (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156934/new/ https

[Lldb-commits] [PATCH] D157122: [lldb] Remove support for SBHostOS threading functionality

bulbazord created this revision. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. As stated on Discourse*, these methods have been deprecated. I am removing their implementation. They will now do nothin

[Lldb-commits] [PATCH] D157122: [lldb] Remove support for SBHostOS threading functionality

bulbazord updated this revision to Diff 547290. bulbazord added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Updating release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157122/new/ https://reviews.llvm

[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock

bulbazord added inline comments. Comment at: lldb/source/Core/Communication.cpp:41-43 Clear(); + std::unique_lock guard(m_shared_mutex); I think this one //may// be slightly incorrect. Here's how I imagine it going wrong: - Thread 1 calls Connect. It only

[Lldb-commits] [PATCH] D157137: [lldb/crashlog] Skip images with empty path and 0 UUID from loading

bulbazord added inline comments. Comment at: lldb/examples/python/symbolication.py:413-418 name = os.path.basename(self.path) +if not name: +if self.uuid == uuid.UUID(int=0): +return None +

[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock

bulbazord added inline comments. Comment at: lldb/source/Core/Communication.cpp:43 + std::unique_lock guard(m_shared_mutex); LLDB_LOG(GetLog(LLDBLog::Communication), JDevlieghere wrote: > augusto2112 wrote: > > Do you think it's possible that between the ca

[Lldb-commits] [PATCH] D157167: [lldb] Add flag to DynamicLoader::LoadBinaryWithUUIDAndAddress to control whether we fall back to reading the binary out of memory

bulbazord added a comment. I don't have any objections other than the questions Jonas has asked. LGTM otherwise Comment at: lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py:38-39 ) +if self.TraceOn(): +self.runCm

[Lldb-commits] [PATCH] D157168: [lldb] [mach-o corefiles] If we have LC_NOTE metadata and can't find a binary, don't fall back to an exhaustive scan

bulbazord added a comment. Patch looks good to me, one small suggestion about naming. Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h:89 void CreateMemoryRegions(); - void LoadBinariesViaMetadata(); + bool LoadBinariesViaMetadata(); void LoadBinaries

[Lldb-commits] [PATCH] D157152: [lldb] Make TSan errors fatal when running the test suite

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157152/new/ https://reviews.llvm.org/D157152 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock

bulbazord accepted this revision. bulbazord added a comment. LGTM Comment at: lldb/source/Core/Communication.cpp:115-118 +total_written += +WriteUnlocked(static_cast(src) + total_written, + src_len - total_written, status, error_ptr); while (

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in ConnectionFileDescriptor

bulbazord accepted this revision. bulbazord added a comment. Makes sense to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157347/new/ https://reviews.llvm.org/D157347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists

[Lldb-commits] [PATCH] D157168: [lldb] [mach-o corefiles] If we have LC_NOTE metadata and can't find a binary, don't fall back to an exhaustive scan

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

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

bulbazord added inline comments. Comment at: lldb/include/lldb/Host/File.h:425-433 + ValueGuard DescriptorIsValid() const { +m_descriptor_mutex.lock(); +return ValueGuard(m_descriptor_mutex, + File::DescriptorIsValid(m_descriptor)); + } + + ValueGu

[Lldb-commits] [PATCH] D157361: [lldb] FIx data race in ThreadedCommunication

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Looks straightforward and makes sense. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157361/new/ https://reviews.llvm.org/D157361 __

[Lldb-commits] [PATCH] D157450: [lldb][NFCI] Remove unused IOStreamMacros

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jasonmolenda, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. These are unused AFAICT. Repository: rG LLVM Gi

[Lldb-commits] [PATCH] D157455: [lldb][NFCI] Remove MappedHash.h

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jasonmolenda, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This is no longer used as of 8e71d14972b48df8c5b701

[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, jingham, mib, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. StreamFile subclasses Stream (from lldbUtility) and

[Lldb-commits] [PATCH] D157463: [lldb] Sink StreamBuffer into lldbUtility

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. lldbUtility seems like a more suitable place for St

[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

bulbazord added a comment. In D157460#4571772 , @mib wrote: > Shouldn't this be next to the other `Stream` class in `lldbUtility` ? `lldbUtility` is the lowest layer and does not depend on anything in `lldbHost`. That being said, `StreamFile` uses `File

[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

bulbazord added inline comments. Comment at: lldb/include/lldb/Host/StreamFile.h:12 #include "lldb/Host/File.h" #include "lldb/Utility/Stream.h" mib wrote: > bulbazord wrote: > > mib wrote: > > > Do we really need this header ? We could forward declare FileSP

[Lldb-commits] [PATCH] D157450: [lldb][NFCI] Remove unused IOStreamMacros

This revision was automatically updated to reflect the committed changes. Closed by commit rG335d0e3a6e8d: [lldb][NFCI] Remove unused IOStreamMacros (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157450/new/ https://reviews.l

[Lldb-commits] [PATCH] D157538: [lldb][NFCI] Remove use of ifdef __cpluplus where unneeded

bulbazord created this revision. bulbazord added reviewers: JDevlieghere, mib, jasonmolenda, jingham. Herald added a project: All. bulbazord requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Just about every file in the lldb project is built

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

bulbazord accepted this revision. bulbazord added a comment. This revision is now accepted and ready to land. Looks right to me now. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157347/new/ https://reviews.llvm.org/D157347 ___ lldb-c

[Lldb-commits] [PATCH] D157455: [lldb][NFCI] Remove MappedHash.h

This revision was automatically updated to reflect the committed changes. Closed by commit rG44c876fd75c2: [lldb][NFCI] Remove MappedHash.h (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157455/new/ https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

bulbazord added inline comments. Comment at: lldb/include/lldb/Target/Process.h:348-357 eBroadcastBitStateChanged = (1 << 0), eBroadcastBitInterrupt = (1 << 1), eBroadcastBitSTDOUT = (1 << 2), eBroadcastBitSTDERR = (1 << 3), eBroadcastBitProfileData = (1

[Lldb-commits] [PATCH] D157463: [lldb] Sink StreamBuffer into lldbUtility

This revision was automatically updated to reflect the committed changes. Closed by commit rG20c77a578933: [lldb] Sink StreamBuffer into lldbUtility (authored by bulbazord). Changed prior to commit: https://reviews.llvm.org/D157463?vs=548419&id=548826#toc Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

bulbazord updated this revision to Diff 548828. bulbazord added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157460/new/ https://reviews.llvm.org/D157460 Files: lldb/include/lldb/Core/Debugger.h lldb/include/lldb/Core/Stream

[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

This revision was automatically updated to reflect the committed changes. Closed by commit rGf2d32ddcec82: [lldb] Sink StreamFile into lldbHost (authored by bulbazord). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157460/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

bulbazord requested changes to this revision. bulbazord added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Host/common/File.cpp:609 num_bytes = bytes_written; - } else if (StreamIsValid()) { + } else if (ValueGuard stream_guard

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