[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. This looks good to me, thanks for reviving this and finishing it up. We should land before phabracator is flash frozen, we can iterate issues are found in the future. =

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

jasonmolenda added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:135 +process_sp->SetCanInterpretFunctionCalls(true); +process_sp->SetCanJIT(false); + } These should not be set in the ABI, please remove these. These are pr

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

This revision was automatically updated to reflect the committed changes. Closed by commit rG2cab996192cf: Add "process metadata" Mach-O LC_NOTE for corefiles (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158785/new/ http

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda updated this revision to Diff 556505. jasonmolenda added a comment. Update patch to not use malloc/free for a temporary heap allocation; a std::string works fine. One caveat is that the c-strings we read in may have a nul byte terminator, and left alone the std::string will conside

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda updated this revision to Diff 556327. jasonmolenda added a comment. All of the feedback is addressed at this point. I had one decision I didn't like - the LC_NOTE has an explicit size in the load command, but I said that the JSON string needed to be nul byte ('\0') terminated, whic

[Lldb-commits] [PATCH] D158035: [lldb] Protect RNBRemote from a data race

jasonmolenda added inline comments. Comment at: lldb/tools/debugserver/source/RNBRemote.cpp:780 PThreadMutex::Locker locker(m_mutex); if (m_rx_packets.empty()) { JDevlieghere wrote: > This is an RAII object, right? Can we just block scope it? Right now it

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. LGTM this is just mechanical passing around of the user's setting though the layers. Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h:76

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda added a comment. Oh, and I did change the LC_NOTE name that I'm adding to "process metadata" and specified that it *may* contain a `threads` key, instead of *shall* contain a `threads` key. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda updated this revision to Diff 554541. jasonmolenda marked an inline comment as done. jasonmolenda added a comment. Address feedback from Alex and Jonas. Most significantly, add a `ObjectFileMachO::FindLC_NOTEByName` method which all the LC_NOTE readers in ObjectFileMachO call. One

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda marked 6 inline comments as done. jasonmolenda added a comment. Thanks for the feedback Alex & Jonas. Jonas' comment that we have a lot of different methods manually stepping over load commands to find their LC_NOTEs was something I'd been meaning to deal with some day, but hadn't

[Lldb-commits] [PATCH] D159101: [RISC-V] Add RISC-V ABI plugin

jasonmolenda added inline comments. Comment at: lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp:107 +static size_t word_size = 4U; +static size_t reg_size = word_size; + ted wrote: > DavidSpickett wrote: > > What's the reason to do this this way? It seems like ad

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda added a comment. Thanks for the feedback @bulbazord & @JDevlieghere , I will address those points. I am still trying to decide on the scope of this JSON LC_NOTE. Right now it's "thread extrainfo" and a dictionary with a "threads" key by definition. But I have a feeling it should

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda added a comment. One interesting question about the basic design of this LC_NOTE. If I call it "thread extrainfo" and it is a JSON dictionary with a `threads` array -- how long before someone wants to stick some non-thread specific piece of data in here. Should it be a `proc meta

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda updated this revision to Diff 553324. jasonmolenda added a subscriber: jingham. jasonmolenda added a comment. After thinking about this more, and talking with @jingham I rewrote the patch so LC_NOTE "thread extrainfo" is a JSON dictionary with a key `threads` that has an array. The

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda added a comment. As soon as I started thinking about thread names and queue names and mach exception data, all variable length things, a binary format doesn't seem ideal. We do already send a lot of thread information about processes in JSON in gdb remote serial protocol from debu

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda added a comment. Or possibly we should just make "thread extrainfo" a JSON blob with a dictionary for each thread, which would make it easier to extend, instead of a binary format. I'm not sure I have a strong opinion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

jasonmolenda created this revision. jasonmolenda added reviewers: JDevlieghere, bulbazord. jasonmolenda added a project: LLDB. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. This adds a new LC_NOTE to ObjectFileMachO to save th

[Lldb-commits] [PATCH] D158506: [lldb][AArch64] Add release notes and documentation for SME

jasonmolenda added a comment. LGTM, fwiw. Comment at: lldb/docs/use/aarch64-linux.rst:37 +The example above has a vector length of 16 bytes. Within LLDB you will always +see "vg" as in the ``vg`` register, which is 2 in this case (8*2 = 16). +Elsewhere you may see "vq" which is

[Lldb-commits] [PATCH] D157764: [LLDB] Allow expression evaluators to set arbitrary timeouts

jasonmolenda added a comment. In D157764#4608265 , @wallace wrote: > @jasonmolenda , yep. I have already reverted this patch. I'll figure out how > to make that work nicely with these changes. oh lol sorry took me a little while to confirm and I missed

[Lldb-commits] [PATCH] D157764: [LLDB] Allow expression evaluators to set arbitrary timeouts

jasonmolenda added a comment. @wallace this is breaking TestIRInterpreter.py on macOS now; https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ has been failing since it went in (there was another llvm issue causing failures right before this went in, so it was easy to miss). On my des

[Lldb-commits] [PATCH] D158237: Change LLGSTest.cpp to run environment_check inferior with stdin/stdout disabled, to work around ASAN CI bot issue

This revision was automatically updated to reflect the committed changes. Closed by commit rG18b211cb1521: Disable stdin/stdout for environment_check inferior process (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158237/ne

[Lldb-commits] [PATCH] D158312: [debugserver] align received mach exception data before accessing it as array of uint64_t's, fix UB sanitizer failure

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 rG1a2122e9e9d1: Align mach exception data before accessing it (authored by jasonmolenda). Repository: rG LLVM Github Mono

[Lldb-commits] [PATCH] D158312: [debugserver] align received mach exception data before accessing it as array of uint64_t's, fix UB sanitizer failure

jasonmolenda added a comment. I'm going to land this one sans-approval to fix the CI bot. but I still think self.runCmd("settings set target.process.extra-startup-command QSetLogging:bitmask=LOG_PROCESS|LOG_EXCEPTIONS|LOG_RNB_PACKETS|LOG_STEP;") shouldn't be in a test case, even though it he

[Lldb-commits] [PATCH] D158312: [debugserver] align received mach exception data before accessing it as array of uint64_t's, fix UB sanitizer failure

jasonmolenda created this revision. jasonmolenda added a reviewer: jingham. jasonmolenda added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. The mach exception data rec

[Lldb-commits] [PATCH] D158237: Change LLGSTest.cpp to run environment_check inferior with stdin/stdout disabled, to work around ASAN CI bot issue

jasonmolenda created this revision. jasonmolenda added a reviewer: labath. jasonmolenda added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. We have an ASAN+UBSAN bot fo

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

jasonmolenda added inline comments. Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:49 + if (TT.isOSBinFormatELF()) +return filename.starts_with("libobjc.so"); + if (TT.isOSWindows()) theraven wrote: > This is

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

This revision was automatically updated to reflect the committed changes. Closed by commit rG6f4a0c762fe2: hi/low addr space bits can be sent in stop-rely packet (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158041/new/ h

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda updated this revision to Diff 550924. jasonmolenda added a comment. Updated back to my most recent version of this patch, thanks for the help all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155905/new/ https://reviews.llvm.org/D1559

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

jasonmolenda updated this revision to Diff 550919. jasonmolenda added a comment. Update patch to remove the HasValue/IsValue and Clear methods from AddressableBits. Change methods that may fill in an AddressableBits object with values to return an AddressableBits object unconditionally. Change

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added a comment. ah dangit, I meant to update the patch in https://reviews.llvm.org/D158041 and accidentally blew away the last patch I had for this one. let's see if I can extract it from here somehow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda updated this revision to Diff 550917. jasonmolenda added a comment. Update patch to remove the HasValue/IsValue and Clear methods from AddressableBits. Change methods that may fill in an AddressableBits object with values to return an AddressableBits object unconditionally. Change

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

jasonmolenda added inline comments. Comment at: lldb/source/Utility/AddressableBits.cpp:62-64 +void AddressableBits::Clear() { + m_low_memory_addr_bits = m_high_memory_addr_bits = 0; +} JDevlieghere wrote: > jasonmolenda wrote: > > jasonmolenda wrote: > > > JDev

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

jasonmolenda updated this revision to Diff 550868. jasonmolenda added a comment. Update patch to change the `AddressableBits::IsValid` to `AddressableBits::HasValue` which is a clearer description. The `IsValid` methods are used across the lldb_private classes everywhere. I tried changing thi

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

jasonmolenda added inline comments. Comment at: lldb/source/Utility/AddressableBits.cpp:62-64 +void AddressableBits::Clear() { + m_low_memory_addr_bits = m_high_memory_addr_bits = 0; +} jasonmolenda wrote: > JDevlieghere wrote: > > Where is this called? > Hm. W

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

jasonmolenda added inline comments. Comment at: lldb/source/Utility/AddressableBits.cpp:58-60 +bool AddressableBits::IsValid() { + return m_low_memory_addr_bits != 0 || m_high_memory_addr_bits != 0; +} JDevlieghere wrote: > Please implement `operator bool()` ins

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

jasonmolenda created this revision. jasonmolenda added a reviewer: JDevlieghere. jasonmolenda added a project: LLDB. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. A small follow on patch to https://reviews.llvm.org/D157667 to

[Lldb-commits] [PATCH] D157756: Support corefiles that put the uuid of their binary in a fixed address in low memory; load that binary

This revision was automatically updated to reflect the committed changes. Closed by commit rGd70d5e9f6fa2: Get binary UUID from fixed location in special Mach-O corefiles (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15775

[Lldb-commits] [PATCH] D157667: Define qHostInfo and Mach-O LC_NOTE "addrable bits" methods to describe high and low memory addressing bits

This revision was automatically updated to reflect the committed changes. Closed by commit rG3ad618f4aea1: Update qHostInfo/LC_NOTE so multiple address bits can be specified (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15

[Lldb-commits] [PATCH] D157667: Define qHostInfo and Mach-O LC_NOTE "addrable bits" methods to describe high and low memory addressing bits

jasonmolenda updated this revision to Diff 550148. jasonmolenda added a comment. Incorporate Jonas' suggestion of a AddressableBits class that GDBRemoteCommunicationClient and ObjectFileMachO could use to store the zero/one/two addressable bits values back to a Process, and centralizing the log

[Lldb-commits] [PATCH] D157659: Have SBTarget::AddModule force a possibly-slow search for the binary, and if the Target has no arch, initialize it with the binary's arch

This revision was automatically updated to reflect the committed changes. Closed by commit rGebf249066ac5: [lldb] SBTarget::AddModule do all searches by UUID, set Target arch (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[Lldb-commits] [PATCH] D157756: Support corefiles that put the uuid of their binary in a fixed address in low memory; load that binary

jasonmolenda updated this revision to Diff 549516. jasonmolenda added a comment. Fix size check when making sure there's enough data. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157756/new/ https://reviews.llvm.org/D157756 Files: lldb/source/P

[Lldb-commits] [PATCH] D157756: Support corefiles that put the uuid of their binary in a fixed address in low memory; load that binary

jasonmolenda created this revision. jasonmolenda added a reviewer: jingham. jasonmolenda added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. There are some Mach-O coref

[Lldb-commits] [PATCH] D157659: Have SBTarget::AddModule force a possibly-slow search for the binary, and if the Target has no arch, initialize it with the binary's arch

jasonmolenda updated this revision to Diff 549507. jasonmolenda added a comment. Incorporate Alex's suggestions, not sure about using llvm-dwarfdump yet tho. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157659/new/ https://reviews.llvm.org/D157659

[Lldb-commits] [PATCH] D157659: Have SBTarget::AddModule force a possibly-slow search for the binary, and if the Target has no arch, initialize it with the binary's arch

jasonmolenda marked 2 inline comments as done. jasonmolenda added inline comments. Comment at: lldb/test/API/python_api/target-arch-from-module/TestTargetArchFromModule.py:39 +dwarfdump_cmd_output = subprocess.check_output( +('/usr/bin/dwarfdump --uuid "%s"'

[Lldb-commits] [PATCH] D157667: Define qHostInfo and Mach-O LC_NOTE "addrable bits" methods to describe high and low memory addressing bits

jasonmolenda updated this revision to Diff 549504. jasonmolenda added a comment. Fix the problems Alex found on review, thanks Alex. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157667/new/ https://reviews.llvm.org/D157667 Files: lldb/docs/lldb

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. The lldb side looks good to me, and I think you've addressed the llvm parts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/new

[Lldb-commits] [PATCH] D157667: Define qHostInfo and Mach-O LC_NOTE "addrable bits" methods to describe high and low memory addressing bits

jasonmolenda updated this revision to Diff 549214. jasonmolenda added a comment. update comments a tiny bit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157667/new/ https://reviews.llvm.org/D157667 Files: lldb/docs/lldb-gdb-remote.txt lldb/i

[Lldb-commits] [PATCH] D157667: Define qHostInfo and Mach-O LC_NOTE "addrable bits" methods to describe high and low memory addressing bits

jasonmolenda created this revision. jasonmolenda added reviewers: JDevlieghere, bulbazord. jasonmolenda added a project: LLDB. Herald added a subscriber: kristof.beyls. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. To recap, o

[Lldb-commits] [PATCH] D157659: Have SBTarget::AddModule force a possibly-slow search for the binary, and if the Target has no arch, initialize it with the binary's arch

jasonmolenda added a comment. One more bit of explanation about the changes: There is an SBTarget::AddModule which takes individual parts of a module spec, and an SBTarget::AddModule which takes an SBModuleSpec. I was going to need to duplicate my code to force the expensive search & update t

[Lldb-commits] [PATCH] D157659: Have SBTarget::AddModule force a possibly-slow search for the binary, and if the Target has no arch, initialize it with the binary's arch

jasonmolenda created this revision. jasonmolenda added reviewers: JDevlieghere, bulbazord. jasonmolenda added a project: LLDB. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. This patch addresses two issues I found while looking

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

jasonmolenda added a comment. Maybe const char *ent_name = #if TARGET_OS_OSX "com.apple.security.get-task-allow"; #else "get-task-allow"; #endif debugserver is running on the target device so compile time checks are fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15764

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

jasonmolenda added a comment. Maybe get-task-allow is allowed (lol) on macOS too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157640/new/ https://reviews.llvm.org/D157640 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lis

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

jasonmolenda added a comment. I'm fine with this but on macOS I believe it's com.apple.security.get-task-allow v. "Entitlements on macOS" https://developer.apple.com/documentation/technotes/tn3125-inside-code-signing-provisioning-profiles CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15

[Lldb-commits] [PATCH] D157165: [lldb] [darwin kernel debug] When looking for a Darwin kernel symbol file, call GetSharedModules before DownloadObjectAndSymbolFile

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0d8d31bbf55c: When loading kernel binary, use DownloadObjectAndSymbolFile last (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CH

[Lldb-commits] [PATCH] D157165: [lldb] [darwin kernel debug] When looking for a Darwin kernel symbol file, call GetSharedModules before DownloadObjectAndSymbolFile

jasonmolenda updated this revision to Diff 548414. jasonmolenda added a comment. Rebase patch on current top of tree sources. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157165/new/ https://reviews.llvm.org/D157165 Files: lldb/source/Plugins/D

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

This revision was automatically updated to reflect the committed changes. Closed by commit rG1290869ef2f7: Show error messages from DebugSymbols DBGShellCommand agent (authored by jasonmolenda). Changed prior to commit: https://reviews.llvm.org/D157160?vs=548015&id=548409#toc Repository: rG

[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

This revision was automatically updated to reflect the committed changes. Closed by commit rGc5f81100e447: When ProcessMachCore has metadata for a binary, don't scan (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157168/new

[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

jasonmolenda updated this revision to Diff 548364. jasonmolenda added a comment. Rebase on current TOT sources, and add a comment to the ProcessMachCore::LoadBinariesViaMetadata prototype in the header documenting the meaning of the return value, I agree with Alex I should make this a little cl

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

jasonmolenda updated this revision to Diff 548015. jasonmolenda added a comment. Update patch to send these error messages to the ErrorStream not OutputStream. Also change LocateSymbolFileMacOSX.cpp so the full command that was used to find the binary is included in the error message, to make i

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

jasonmolenda added inline comments. Comment at: lldb/source/Core/DynamicLoader.cpp:241-243 +Stream &s = target.GetDebugger().GetOutputStream(); +s.Printf("Tried DBGShellCommands cmd, got error: %s\n", + error.AsCString()); JDevlieg

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

jasonmolenda updated this revision to Diff 548009. jasonmolenda added a comment. fix typeo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157160/new/ https://reviews.llvm.org/D157160 Files: lldb/source/Core/DynamicLoader.cpp lldb/source/Plugins

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

jasonmolenda updated this revision to Diff 548001. jasonmolenda added a comment. Change calls to Log::Printf to use LLDB_LOGF(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157160/new/ https://reviews.llvm.org/D157160 Files: lldb/source/Core/D

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG57cbd26a68ab: Flag for LoadBinaryWithUUIDAndAddress, to create memory image or not (authored by jasonmolenda). Changed prior to commit: https://reviews.llvm.org/D157167?vs=547866&id=547978#toc Reposito

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

jasonmolenda updated this revision to Diff 547866. jasonmolenda added a comment. Fix my `script print`'s to just `print`, I can't repo a buffering problem that I thought I had seen in the past. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157167/n

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

jasonmolenda added inline comments. Comment at: lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py:38-39 ) +if self.TraceOn(): +self.runCmd("script print('Creating corefile with command %s')" % cmd) call(cmd

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

jasonmolenda updated this revision to Diff 547864. jasonmolenda added a comment. Incorporate Jonas' feedback about the naming of the new DynamicLoader method argument. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157167/new/ https://reviews.llvm.

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

jasonmolenda added inline comments. Comment at: lldb/include/lldb/Target/DynamicLoader.h:267 /// + /// \param[in] allow_use_memory_image_last_resort + /// If no better binary image can be found, allow reading the binary JDevlieghere wrote: > Nit: seems l

[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

jasonmolenda created this revision. jasonmolenda added reviewers: bulbazord, JDevlieghere. jasonmolenda added a project: LLDB. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. We have some corefiles which are intended to debug a

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

jasonmolenda created this revision. jasonmolenda added reviewers: bulbazord, JDevlieghere. jasonmolenda added a project: LLDB. Herald added a project: All. jasonmolenda requested review of this revision. Herald added subscribers: lldb-commits, wangpc. DynamicLoader::LoadBinaryWithUUIDAndAddress()

[Lldb-commits] [PATCH] D157165: [lldb] [darwin kernel debug] When looking for a Darwin kernel symbol file, call GetSharedModules before DownloadObjectAndSymbolFile

jasonmolenda created this revision. jasonmolenda added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. ModuleList::GetSharedModule will call in to the DebugSymbols framew

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

jasonmolenda created this revision. jasonmolenda added a reviewer: bulbazord. jasonmolenda added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. On Darwin systems, lldb w

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

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Ah, I missed that I removed the check when we were trying to clean up old pre-macOS 10.12 ifdef's. LGTM thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D156817: _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

jasonmolenda accepted this revision. jasonmolenda 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/D156817/new/ https://reviews.llvm.org/D156817 __

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

jasonmolenda added a comment. 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 seems like you're changing the filename to AppleUuidCompatbility.h but the two places it's

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added a comment. Given the AddressMaskType enum, I wonder if instead of `FixCodeAddress`, `FixDataAddress`, and `FixAddress`, there should be one `SBProcess::FixAddress(lldb::AddressMaskType type, lldb::addr_t address)` and it would be used with `eMaskTypeCode`, `eMaskTypeData`, or

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added inline comments. Comment at: lldb/source/API/SBProcess.cpp:1260-1261 + return process_sp->GetHighmemDataAddressMask(); +case eMaskTypeAny: + return process_sp->GetDataAddressMask(); +} jasonmolenda wrote: > clayborg wrote: > >

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added inline comments. Comment at: lldb/include/lldb/API/SBProcess.h:445 + lldb::addr_t FixDataAddress(lldb::addr_t addr); + lldb::addr_t FixAnyAddress(lldb::addr_t addr); + clayborg wrote: > What does this function do? Does it try to auto detect c

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda updated this revision to Diff 544936. jasonmolenda added a comment. Update the patch to express the mask getter/setters in terms of a type enum, after discussions with Greg and David. I still need to write a test for this API but I'm liking where this is now. Repository: rG LLV

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added a comment. In D155905#4537892 , @DavidSpickett wrote: >> but I could imagine some harvard architecture target that behaved >> differently (surely this is why Linux has two address masks) > > I'm not privy to the exact reasoning, but a

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added a comment. In D155905#4537036 , @clayborg wrote: > > I like it the above approach with more enums for the high and low code/data. > Not sure if eTypeAny makes sense in the GetAddressMask(eTypeAny) scenario, > but I can see the use

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Looking at Ted's earlier riscv disassembly with this enabled, it is more readable, I'm surprised these instructions don't print this way by default like they are for other targets.

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

jasonmolenda added a comment. In D156086#4530507 , @RamNalamothu wrote: > In D156086#4529791 , @jasonmolenda > wrote: > >> >> Does `isBranch` include other variants like `isUnconditionalBranch`? > > No. They

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added a comment. Also interesting to consider if there should be an "Any" define. e.g. enum AddressMaskType { eTypeCode = 0, eTypeData, eTypeHighmemCode, eTypeHighmemData, eTypeAny }; lldb::addr_t GetAddressMask(AddressMaskType mask_type); void SetAddress

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added inline comments. Comment at: lldb/include/lldb/API/SBProcess.h:415-416 + /// (unsigned long long) 0xfc00 + lldb::addr_t GetCodeAddressMask(); + lldb::addr_t GetDataAddressMask(); + clayborg wrote: > Will there only ever be two va

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

jasonmolenda added a comment. I'm not super familiar with the MCInst class from llvm, and hadn't heard of MCInstrAnalysis. I was looking through the llvm targets - are these MCInstrAnalysis primitives going to be implemented for all targets we support today? I see them defined for e.g. MIPS a

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda added a comment. Thanks for the feedback David. > You are missing a way to fix a high mem address, is that intentional? Or > would you get the high mem mask, change the setting, fix the address, as > needed by context. The FixAddress method will choose whether to use the high mem

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

jasonmolenda created this revision. jasonmolenda added reviewers: DavidSpickett, jingham, clayborg. jasonmolenda added a project: LLDB. Herald added subscribers: JDevlieghere, kristof.beyls. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb

[Lldb-commits] [PATCH] D155768: [lldb] [NFC] Remove some dead code from Watchpoint class, and a method that makes no sense

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG259e3f2a4fef: Remove IncrementFalseAlarmsAndReviseHitCount, unused ivars (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D155768: [lldb] [NFC] Remove some dead code from Watchpoint class, and a method that makes no sense

jasonmolenda added a comment. Thanks David, yeah I came to the same conclusion about that method. I made hacked up lldb that behaved like the MIPS case to confirm that the hit count is incremented before we decide it's a fake stop & decrement it again. Repository: rG LLVM Github Monorepo C

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

jasonmolenda added a comment. PrintBranchImmAsAddress is used in the RISCV, aarch64, PPC, Mips, and X86 instruction printers - most of them in one or two spots. I tried a quick attempt at writing a program that would change output with the aarch64 instruction printer but didn't succeed at firs

[Lldb-commits] [PATCH] D155768: [lldb] [NFC] Remove some dead code from Watchpoint class, and a method that makes no sense

jasonmolenda created this revision. jasonmolenda added a project: LLDB. Herald added subscribers: JDevlieghere, atanasyan, kristof.beyls, arichardson, sdardis. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. Johnny Chen added t

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

jasonmolenda added a comment. In D155269#4509364 , @DavidSpickett wrote: > I think in https://reviews.llvm.org/D154926, > `lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py` > addresses this. If

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

jasonmolenda added a comment. In D155269#4509130 , @DavidSpickett wrote: > > Ideally we would have as few routes to mode switch via the debugger as > possible. Writing to the streaming vector control register is the single > route I'd support given

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

jasonmolenda added a comment. In D155269#4505021 , @DavidSpickett wrote: >> As you say above, we can show svg when in non-streaming mode but we can't >> show vg when in streaming mode. Should we only show a single vg for the >> currently-available regi

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

jasonmolenda added a comment. Isn't it better to print branches within a function as an offset, given that our disassembly format by default lists the offset of each instruction. So instead of looking for a 6-digit long hex address, you're looking for a decimal offset in the output? I'm not s

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix LocateModuleCallbackTest

jasonmolenda accepted this revision. jasonmolenda added a comment. Great catch, thanks for digging in on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155333/new/ https://reviews.llvm.org/D155333 _

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

jasonmolenda added a comment. FWIW when I was testing this locally, I was seeing paths like (lldb) p uuid_view (lldb_private::FileSpec) $0 = file: "AndroidModule.so" dir: "/var/folders/1b/976z5_5513l64wvrj15vkt80gn/T/lldb/49966/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModuleAn

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

jasonmolenda added a comment. Like reading third-party/unittest/googletest/include/gtest/gtest-printers.h, I thought I could add a static PrintTo method? but index 6eb5b805d9d..93d98baa5be 100644 --- a/lldb/include/lldb/Utility/FileSpec.h +++ b/lldb/include/lldb/Utility/FileSpec.h @@ -1

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

jasonmolenda added a comment. My first thought was, but what about all the other unit tests doing this, and git grepping around, I think there's one in Interpreter/TestOptionValue.cpp and one in Interpreter/TestOptionValueFileColonLine.cpp. I'd like to see what @JDevlieghere thinks, this patc

  1   2   3   4   5   6   7   8   9   10   >