[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

jasonmolenda added a comment. I've been meaning to look at this - for the command line style "cd test; ./dotest.py" approach, we could use similar logic to what packages/Python/lldbsuite/test/dotest.py does in getOutputPaths() where it finds an lldb binary in the "typical" location. for an Xco

[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error

jasonmolenda added a comment. (obviously this would only be the behavior if a filecheck path was not specified) https://reviews.llvm.org/D53175 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[Lldb-commits] [PATCH] D53305: Simplify LocateDSYMInVincinityOfExecutable a bit, and call Symbols::DownloadObjectAndSymbolFile for .dSYM.yaa archives

jasonmolenda created this revision. jasonmolenda added a reviewer: jingham. jasonmolenda added a project: LLDB. Herald added a subscriber: abidh. Host/common/Symbols.cpp has a method LocateDSYMInVincinityOfExecutable which looks for a dSYM next to a binary ("foo" + "foo.dSYM") and it looks for a

[Lldb-commits] [PATCH] D50155: Delete MacOSXFrameBackchain unwind logic (NFC)

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Sorry for not replying to this. Yes the code is unused - it was the initial unwinder implementation when lldb is very young, before we had our current one. The main reason to kee

[Lldb-commits] [PATCH] D53435: [x86] Fix issues with a realigned stack in MSVC compiled applications

jasonmolenda added a comment. Thanks for the ping aleksandr, I have somehow managed to set up my mail rules so I didn't see this. I'll try to look it over later today. Yes, the x86 instruction profiler has definitely overgrown over the years - the ARM64 instruction profiler is probably a bett

[Lldb-commits] [PATCH] D53435: [x86] Fix issues with a realigned stack in MSVC compiled applications

jasonmolenda added a comment. Thanks Aleksandr, this all looks good. I'd like to see a definition of AFA (and we can say "CFA is DWARF's Canonical Frame Address" in the same spot) maybe in UnwindPlan.h, non-Windows people (ok, me) will not know what that is referring to. I was a little worrie

[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

jasonmolenda added a comment. To me --jit sounds like an imperative (i.e. don't use the IR interpreter, jit and execute this expression), whereas --allow-jit seems closer to the behavior here. Repository: rLLDB LLDB https://reviews.llvm.org/D54056

[Lldb-commits] [PATCH] D54460: Don't keep a global ABI plugin per architecture

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. LGTM, my guess is that this was Greg's code originally. He may have made a singleton object out of caution. Repository: rLLDB LLDB https://reviews.llvm.org/D54460 _

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

jasonmolenda accepted this revision. jasonmolenda added a comment. LGTM. Comment at: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp:104 +if (image_infos_address != m_maybe_image_infos_address) + did_exec = true; + } Do you

[Lldb-commits] [PATCH] D55399: If we still have all_image_infos use it in DynamicLoaderMacOSDYLD to detect exec's

jasonmolenda added a comment. I don't feel strongly about it, your choice. But if I misunderstood it this time, I'll likely misunderstand it again in the future. :) Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55399/new/ https://reviews.llvm.org/D55399 __

[Lldb-commits] [PATCH] D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Yep, looks good. It would be nice if we found a dSYM with a Spotlight search (mdfind) if we could look NEXT to the dSYM bundle and see if there is a real binary, and load that. B

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

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D46669 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

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

jasonmolenda accepted this revision. jasonmolenda added a comment. Thanks for untangling this Pavel, I hadn't noticed we weren't building this plugin for non-darwin systems. I'd agree with Adrian's comment that we should have a constants like LLDB_KERNEL_SUCCESS / LLDB_KERNEL_INVALID_ARGUMENT,

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

jasonmolenda added a comment. This looks good. My original change in r311622 to add this "remove the last two file path components" should have explicitly said that this is only done for DBGVersion == 2 to make it clearer for people reading the code. At this point the parsing code behaves like

[Lldb-commits] [PATCH] D47539: [Platform] Accept arbitrary kext variants

jasonmolenda added a comment. LGTM. If we added more knowledge specifically about kext bundle layouts, we could restrict which files we test to see if they are valid binaries - but we'd need to parse the Info.plist at the top (to get the CFBundleExecutable name, and look for variations on that

[Lldb-commits] [PATCH] D48302: Search for kext variants is searching from parent directory when it should not be

jasonmolenda created this revision. jasonmolenda added a reviewer: JDevlieghere. Herald added a subscriber: llvm-commits. There's a small perf issue after the changes to r334205; the patch is intended to search all the executables in a kext bundle to discover variants, e.g. /System/Library/Exten

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

jasonmolenda added a comment. I could imagine there being a bug in this ProcessGDBRemote::UpdateThreadIDList -> ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue sequence when talking to a gdb-remote stub that does not implement the lldb-enhancement jThreadsInfo. If jThreadsInfo isn't

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

jasonmolenda added a comment. (just to be clear, the m_thread_pcs.clear() in ProcessGDBRemote::UpdateThreadIDList that I called a bug -- today we only have two ways of populating that, jThreadsInfo or the thread-pcs: value in the stop packet. So clearing it unconditionally here, and then popul

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

jasonmolenda added a comment. I think the m_thread_pcs.clear() in UpdateThreadIDList() is to clear out any old thread pc values that we might have populated in the past. The only way I can see this happening is if the remote stub SOMETIMES returned the thread-pcs: list in the stop packet. Upd

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. looks good. https://reviews.llvm.org/D48868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

jasonmolenda added a subscriber: ramana-nvr. jasonmolenda added a comment. That's a good point Pavel. I tried to write one (below) but I never saw what the original failure mode was. Venkata, can you help to make a test case that fails before the patch and works after? Or explain what bug was

[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

jasonmolenda added a comment. Haven't had time to read through the main part of the patch, but I think the changes to debugserver's JSON parser would be good additions even if you've switched away from using it. The debugserver JSON parser is a copy of lldb's with all the StringRef etc llvm cl

[Lldb-commits] [PATCH] D40812: Remove no-op null checks, NFC

jasonmolenda added a comment. all: echo 'int foo() { return 5; }' > mylib.c echo "int foo() __attribute__((weak_import)); int printf(const char *, ...); int main() { if (foo) printf (\"have foo() %d\\\n\", foo()); else printf(\"do not have foo\\\n\"); return 0; }" > main.c

[Lldb-commits] [PATCH] D40812: Remove no-op null checks, NFC

jasonmolenda added a comment. I don't think that is correct. here's an example in the form of a makefile. the main binary calls foo() which is weak linked from a library. In the first run, foo() is present in the library. In the second run, the library is rebuilt so foo() is absent. The be

[Lldb-commits] [PATCH] D40812: Remove no-op null checks, NFC

jasonmolenda added a comment. The output looks like this: % make echo 'int foo() { return 5; }' > mylib.c echo "int foo() __attribute__((weak_import)); int printf(const char *, ...); int main() { if (foo) printf (\"have foo() %d\\\n\", foo()); else printf(\"do not have foo\\\n\"); return 0; }"

[Lldb-commits] [PATCH] D40812: Remove no-op null checks, NFC

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Ah I see, I bet I assumed it was marked weak but it never was. patch looks good to me. https://reviews.llvm.org/D40812 ___ lldb-com

[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)

jasonmolenda added a comment. I guess I don't have an opinion on this one. The correct way to pass environment variables to the inferior is through SBLaunchInfo::SetEnvironmentEntries or in cmd line lldb, process launch -v ENV=val. A test that assumes an environment variable set in lldb will

[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Thanks for the background Pavel. I'm fine with changing the default behavior. I don't see this having any impact on users of debugserver, and if it does, we can add a flag like G

[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

jasonmolenda added a comment. fwiw I'm working on upstreaming on zmm (avx512) patches that we have locally (there's one testsuite fail I still need to find time to fix) and the TestZMMRegister.py test that ChrisB wrote to test this is written as skip-unless-darwin, and there's a new skipUnlessF

[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

jasonmolenda added a comment. I suppose a possible alternative would be to figure out the avx2 / avx512 features manually based on the cpuid instead of letting the compiler do it for us. e.g. https://stackoverflow.com/questions/1666093/cpuid-implementations-in-c and then checking the bits as

[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

jasonmolenda added a comment. In https://reviews.llvm.org/D41962#975168, @aprantl wrote: > > there's a new skipUnlessFeature() method added to decorators.py which runs > > sysctl to detect hardware features (in this case, hw.optional.avx512f) > > How does one execute a program like `sysctl` on t

[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

jasonmolenda created this revision. jasonmolenda added a reviewer: davide. jasonmolenda added a project: LLDB. Herald added subscribers: llvm-commits, JDevlieghere. I hit this while trying to debug lldb-server. When lldb-server is waiting for connections, it will be in MainLoop::RunImpl::Poll(),

[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

jasonmolenda added a comment. Hmmm, testing this would require launching lldb-server, attaching to it from a debugger, detaching, and seeing if lldb-server is still running. Repository: rLLD LLVM Linker https://reviews.llvm.org/D42206 ___ lldb-c

[Lldb-commits] [PATCH] D42210: Re-enable logging on the child side of a fork() in lldb-server platform mode

jasonmolenda created this revision. jasonmolenda added a reviewer: labath. jasonmolenda added a project: LLDB. Herald added a subscriber: llvm-commits. When lldb-server is in platform mode and waiting for a connection, when it receives a connection it fork()'s and then runs an lldb-server/debugse

[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

jasonmolenda added a comment. I tried sending signals to lldb-server via kill() and the signal handler caught them, the bit of code I had printing out the return value & errno value never got executed. The only way I was able to repo this was by debugging lldb-server. Repository: rLLD LLVM

[Lldb-commits] [PATCH] D42210: Re-enable logging on the child side of a fork() in lldb-server platform mode

jasonmolenda added a comment. Jim remembers some problem with logging across a fork()'ed process, Pavel does this ring a bell? This change might be completely bogus -- but it's very difficult to debug the child side of an lldb-server platform today. Repository: rL LLVM https://reviews.llvm

[Lldb-commits] [PATCH] D42215: [CMake] Make check-lldb work with LLDB_CODESIGN_IDENTITY=''

jasonmolenda added a comment. That's a good suggestion. We have some bots using the system debugserver -- and once in a while there's a change to both lldb and debugserver, and the bots running the older prebuilt debugserver will fail any tests for that patchset. https://reviews.llvm.org/D422

[Lldb-commits] [PATCH] D42336: Fix memory leaks in TestArm64InstEmulation

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Thanks! https://reviews.llvm.org/D42336 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[Lldb-commits] [PATCH] D42828: Fix for read-past-end-of-array buglet in ProcessElfCore.cpp while reading linux notes

jasonmolenda created this revision. jasonmolenda added a reviewer: labath. jasonmolenda added a project: LLDB. Herald added a subscriber: llvm-commits. I caught this while running the testsuite against lldb built with address sanitizer (ASAN) enabled - it found one problem when running the TestL

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

jasonmolenda added a comment. No, the unwind unittests that exist today should stay written as unit tests. These are testing the conversion of native unwind formats -- for instance, eh_frame, compact unwind, or instruction analysis -- into the intermediate UnwindPlan representation in lldb. T

[Lldb-commits] [PATCH] D43419: Fix TestBreakpointInGlobalConstructor for Windows

jasonmolenda added a comment. On Darwin we load all the libraries that the binary links against pre-execution, if possible. So I see: % lldb a.out (lldb) ima li libfoo.dylib [ 0] 35C5FE62-5327-3335-BBCF-5369CB07D1D6 0x /Volumes/ssdhome/jmolenda/k/svn/lldb/packages/Python/lldbs

[Lldb-commits] [PATCH] D43419: Fix TestBreakpointInGlobalConstructor for Windows

jasonmolenda added a comment. Yeah, it has a location because we're resolved it to a File address (section+offset), and when the dylib actually gets loaded we'll resolve that to a load address and insert the breakpoint instruction. https://reviews.llvm.org/D43419 ___

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

jasonmolenda added a comment. I think we're being a little hasty here. Greg's last suggestion is worth investigation -- how many tests would actually be run for this new variant? Greg, maybe you could remove the make clean targets from packages/Python/lldbsuite/test/plugins/builder_base.py (I

[Lldb-commits] [PATCH] D44139: Update selected thread after loading mach core

jasonmolenda accepted this revision. jasonmolenda added a comment. Looks good, thanks for doing this Jonas. I thought there would be a problem with ThreadMachCore::CalculateStopInfo() in source/Plugins/Process/mach-core/ThreadMachCore.cpp which will mark every thread that comes from the actual

[Lldb-commits] [PATCH] D44139: Update selected thread after loading mach core

jasonmolenda added a comment. (or rather, not "conflict with the OperatingSystemPlugIn stop reason" -- but would make it confusing to users. I think it might be best to remove ThreadMachCore::CalculateStopInfo.) https://reviews.llvm.org/D44139 __

[Lldb-commits] [PATCH] D45011: Prevent double release of mach ports

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. valgrind/asan/msan aren't going to help - this is a pretty obscure stuff. We could verify by hand that we're not leaking a port if this code was actually executed via lsmp(1) on m

[Lldb-commits] [PATCH] D45298: [debugserver] Fix LC_BUILD_VERSION load command handling.

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: packages/Python/lldbsuite/test/decorators.py:359 +output = subprocess.check_output(["xcodebuild", "-showsdks"], stderr=

[Lldb-commits] [PATCH] D45348: Don't return error for settings set .experimental. settings that are absent

jasonmolenda created this revision. jasonmolenda added a reviewer: jingham. jasonmolenda added a project: LLDB. Herald added a subscriber: llvm-commits. setting paths that include .experimental. are intended for settings that may be promoted to "real" settings in the future, or may be removed. W

[Lldb-commits] [PATCH] D45348: Don't return error for settings set .experimental. settings that are absent

jasonmolenda updated this revision to Diff 141473. jasonmolenda added a comment. rewrote the test cases in terms of self.expect. Haven't looked at being totally correct with flagging paths with .experimental. as never-errored. https://reviews.llvm.org/D45348 Files: packages/Python/lldbsuite

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

jasonmolenda added a comment. My two cents, Why print the '### Start STATISTICS dump ###' header/footer when printing the results? I don't think we demarcate result output like that anywhere else in lldb. I don't think it adds anything for the user. I would probably name the command statisti

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

jasonmolenda added a comment. Ah, no you couldn't set up a command alias like that. Still, if the full name was statistics, you could type 'stat' and it would match. 'stats' wouldn't, though. I do think decoupling the disabling action from the dumping action would be an improvement. You may

[Lldb-commits] [PATCH] D31823: Update LLDB Host to support IPv6 over TCP

jasonmolenda added a comment. Looks good. I'd recommend adding clayborg as a reviewer and giving him a couple days to comment on this if he has time. He wrote all of this code originally. Comment at: include/lldb/Host/common/TCPSocket.h:55 + + std::map m_listen_sockets; }

[Lldb-commits] [PATCH] D31880: Fix libc++ vector data formatter (bug #32553)

jasonmolenda added a comment. This all looks good to me, thanks for doing this Pavel. Tamas asked in an earlier comment, "The previous version of the data formatter was triggering for std::vector> as well. Jason, do you know why was it the case? Do we need that functionality because of a broke

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

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Looks fine to me, I don't know the debug_types sections that you're supporting but I read over the code and don't have any problems. Comment at: source/Plugins/S

[Lldb-commits] [PATCH] D32022: Fix backtrace of noreturn functions situated at the end of a module

jasonmolenda added a comment. Sorry Pavel, I kept meaning to look at this patch this week but hadn't yet. I will later today. When I read over your description of the problem, it sounded like a good fix - haven't looked at the code yet. https://reviews.llvm.org/D32022

[Lldb-commits] [PATCH] D32316: Change UniqueCStringMap to use ConstString as the key

jasonmolenda added a comment. I'd recommend Greg Clayton as a reviewer. Repository: rL LLVM https://reviews.llvm.org/D32316 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32316: Change UniqueCStringMap to use ConstString as the key

jasonmolenda added a comment. You should also add lldb-commits as a subscriber, fwiw, so comments/questions/etc are cc:'ed to the -commits list. Repository: rL LLVM https://reviews.llvm.org/D32316 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D32022: Fix backtrace of noreturn functions situated at the end of a module

jasonmolenda added a comment. Hi Pavel, I'd document the new flag in include/lldb/Core/Address.h where we have documentation for the other flags being used. It seems like we're fixing this a little indirectly, and I'm not sure it's the best approach. I want to make sure I understand the situat

[Lldb-commits] [PATCH] D32022: Fix backtrace of noreturn functions situated at the end of a module

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Pavel, my apologies for not following up on this, we had a big push to get ready for a beta release/conference this week, but it wasn't fair to leave you hanging with this change.

[Lldb-commits] [PATCH] D34199: Tweak SysV_arm64 function entry unwind plan

jasonmolenda accepted this revision. jasonmolenda added a comment. Looks good to me. https://reviews.llvm.org/D34199 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D34322: [lldb] Correctly escape newlines and backslashes in the JSON serializer

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D34322 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[Lldb-commits] [PATCH] D34274: Remove home-grown thread-local storage wrappers

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. We have thread local storage support on all our current darwin platforms. https://reviews.llvm.org/D34274 ___ lldb-commits mailing li

[Lldb-commits] [PATCH] D34750: [UnwindAssembly/x86] Add support for "lea imm(%ebp), %esp" pattern

jasonmolenda added a comment. Yeah, looks good. You'll only see this on i386 because the SysV ABI require 4-byte stack alignment. On x86_64 it's 16-byte so the compiler doesn't need to emit the AND instruction to realign it. On darwin we required 16 byte alignment on i386 too so we never saw

[Lldb-commits] [PATCH] D34929: respect target.x86-disassembly-flavor when using `memory read`

jasonmolenda closed this revision. jasonmolenda added a comment. I committed this for Jeffrey in r307618. Repository: rL LLVM https://reviews.llvm.org/D34929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[Lldb-commits] [PATCH] D36620: Fix crash on parsing gdb remote packets

jasonmolenda added a comment. This looks good, the documentation for isprint() say that the value of the argument must be representable as an unsigned char, or EOF. https://reviews.llvm.org/D36620 ___ lldb-commits mailing list lldb-commits@lists.ll

[Lldb-commits] [PATCH] D36620: Fix crash on parsing gdb remote packets

jasonmolenda closed this revision. jasonmolenda added a comment. Sendingsource/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Transmitting file data .done Committing transaction... Committed revision 311207. https://reviews.llvm.org/D36620 __

[Lldb-commits] [PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

jasonmolenda added a comment. Sorry for missing this back in August. I think it'd be clearer to import your python once in the startup, like -o "script import $module" \ Multiple imports are a no-op IIUC so it's harmless to re-import the module every time the breakpoint is hit (I'm guessing it

[Lldb-commits] [PATCH] D39199: [lldbtests] Handle errors instead of crashing

jasonmolenda added a comment. I think this is a good idea, thanks for writing the patch Davide. https://reviews.llvm.org/D39199 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

jasonmolenda added a comment. > There was a talk at cppcon a few weeks ago from someone at backtrace.io who > had some insightful metrics on debugger performance memory consumption, and > LLDB had ~2x the memory consumption of GDB. I haven't seen the paper, but my guess is that this is on linux

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

jasonmolenda added a comment. I'll test this later today. The cleanup looks good; the goal of the most recent changes was to make HAVE_LIBCOMPESSION always enabled on Darwin systems - the setting would occasionally get lost in the Xcode project settings and we'd have slow channels using uncomp

[Lldb-commits] [PATCH] D57011: Refactor HAVE_LIBCOMPRESSION and related code in GDBRemoteCommunication

jasonmolenda added a comment. Sorry for the delay in checking this out. It works fine. You need to add a #include "lldb/Host/Config.h" to GDBRemoteCommunication.h. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57011/new/ https://reviews.llvm.org/D57011

[Lldb-commits] [PATCH] D57745: [x64] Process the B field of the REX prefix correctly for the PUSH and POP instructions

jasonmolenda accepted this revision. jasonmolenda added a comment. The change looks good to me, thanks for fixing this. I'm not sure you're testing what you meant to test in TestPopRBPWithREX because you're pushing/popping r13. You could get the unwind state at byte offset 2 (after the push r

[Lldb-commits] [PATCH] D57928: Fix x86 return pattern detection

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Thanks for fixing this and adding a testcase. I don't know why 0xc9 leave was here; it's handled over in x86AssemblyInspectionEngine::leave_pattern_p. Do you have commit access?

[Lldb-commits] [PATCH] D58347: Reinitialize UnwindTable when the SymbolFile changes

jasonmolenda marked an inline comment as done. jasonmolenda added inline comments. Comment at: source/Core/Module.cpp:1451-1454 +// Clear the unwind table too, as that may also be affected by the +// symbol file information. +m_unwind_table.reset(); +

[Lldb-commits] [PATCH] D58912: [debugserver] Fix IsUserReady thread filtering

jasonmolenda added a comment. Will this hide a thread that jumps through a null function pointer? That's the only user process case where a pc of 0 needs to be reported to the developer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58912/new/ https://reviews.llvm.org/D58912 __

[Lldb-commits] [PATCH] D58912: [debugserver] Fix IsUserReady thread filtering

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Looks good, that was my only concern. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58912/new/ https://reviews.llvm.org/D58912 _

[Lldb-commits] [PATCH] D58962: Sanity check --max-gdbserver-port

jasonmolenda added a comment. LGTM fwiw I think the --min-gdbserver-port and --max-gdbserver-port options were primarily/only used for iOS etc testsuite runs, where a fixed set of ports is relayed between the devices. I was having some problems with lldb-server in platform mode on these device

[Lldb-commits] [PATCH] D59030: Pass ConstString by value

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: lldb/www/architecture/varformats.html:121 virtual size_t -

[Lldb-commits] [PATCH] D58347: Reinitialize UnwindTable when the SymbolFile changes

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Yeah, I'm ok with this, thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58347/new/ https://reviews.llvm.org/D58347 ___

[Lldb-commits] [PATCH] D59495: Fix an out-of-bounds error in RegisterContextDarwin_arm64

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Sorry for not weighing in earlier, yes this is good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59495/new/ https://reviews.llvm.org/D59495 __

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

jasonmolenda added a comment. In D55718#1442965 , @clayborg wrote: > It would be really nice to get the GDB remote server for ARC so that it vends > the correct regs to begin with, but if that isn't possible I guess we need to > do what we are doing here

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

jasonmolenda added a comment. In D55718#1443487 , @clayborg wrote: > As long as the numbers _can_ still come from the GDB server, I am all for > that. If they are not supplied, then we use arch defaults. That allows new > system/arch bringups where the i

[Lldb-commits] [PATCH] D59896: [ObjectFileMachO] Disable memory caching for savecore.

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Looks good. Removing that printf is good. Could you also remove the printf("mach_header:...") in the same function? It would be nice if include/lldb/Target/Process.h had the decl

[Lldb-commits] [PATCH] D60022: [Process] Fix WriteMemory return value

jasonmolenda marked an inline comment as done. jasonmolenda added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py:19-20 +target = self.dbg.CreateTarget('') +if self.TraceOn(): +self.runC

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

jasonmolenda created this revision. jasonmolenda added reviewers: clayborg, jingham. Herald added a subscriber: abidh. Herald added a project: LLDB. I'm addressing a perf issue where DynamicLoaderDarwin has been notified that a batch of solibs have been loaded. It adds them to the target one-by-

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

jasonmolenda marked 2 inline comments as done. jasonmolenda added a comment. Thanks for looking the patch over. Comment at: include/lldb/Target/Target.h:535 + bool notify, + Status *error_ptr = nullptr); cla

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

jasonmolenda added a comment. OK looked things over a bit. I think there's a case for adopting llvm::ErrorOr here, and elsewhere across, but today the only place we're using that return type is when the error is encoded in a std::error_code (aka fancy errno). We should have an ErrorOr that us

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

jasonmolenda updated this revision to Diff 193609. jasonmolenda added a comment. Remove const bool notify's. Rename method to Target::GetOrCreateModule. Refine the method headerdoc a bit. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60172/new/ https://revie

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

jasonmolenda added a comment. Thanks Pavel, I'll convert this to use Expected<> while I'm working on it. I'm writing a test case right now, and looking at how DynamicPluginDarwin works more closely for adding & removing binaries, e.g. there's also no way to batch remove libraries and have a gr

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

jasonmolenda updated this revision to Diff 194000. jasonmolenda added a comment. Added a testcase, TestModuleLoadedNotifys. Renamed the pure virtual notifier methods in ModuleList to add 'Notify' at the start of their names, updated the names in subclass Target as well. Added a NotifyModulesRe

[Lldb-commits] [PATCH] D60655: Fix typo in ArmUnwindInfo::GetUnwindPlan

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Agreed, this change is correct. I double checked this in the http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038b/IHI0038B_ehabi.pdf doc (mod 24 Nov 2015) and the only differ

[Lldb-commits] [PATCH] D60829: FuncUnwinders: remove "current_offset" from function arguments

jasonmolenda added a comment. Thanks for digging in to this Pavel. I haven't had time to look over the patch today, but I'd like to; I'll make time for it in the next couple days if that's OK. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60829/new/ https://reviews.llvm.org/D60829

[Lldb-commits] [PATCH] D60949: UnwindPlan: pretty-print dwarf expressions

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. very nice. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60949/new/ https://reviews.llvm.org/D60949 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D60957: Read ObjC class names in one large read, instead of reading them individually

jasonmolenda created this revision. jasonmolenda added a reviewer: jingham. Herald added subscribers: teemperor, abidh. Herald added a project: LLDB. There's a perf problem with Objective-C programs as we add more classes to the Darwin libraries over time - when lldb goes to run its first express

[Lldb-commits] [PATCH] D60829: FuncUnwinders: remove "current_offset" from function arguments

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Sorry for the delay in looking at this. It looks like a good change, an improvement on the original for sure. I've never seen multiple eh_frame FDEs for a single function, so tha

[Lldb-commits] [PATCH] D27289: Return "thread-pcs" in jstopinfo on Linux/Android.

jasonmolenda added a comment. For what it's worth, this change was part of the changes we made to reduce packet traffic for "private stops". When a high-level "step" or "next" command is done, we instruction step (or fast-step with Jim's fast-step code when it sees a sequence of instructions t

[Lldb-commits] [PATCH] D27289: Return "thread-pcs" in jstopinfo on Linux/Android.

jasonmolenda added a comment. @labath ah I see I hadn't looked at the lldb-server packets so I didn't know you had jThreadsInfo, good to hear. Yes, if your target is built mostly -fomit-frame-pointer, lldb-server won't be able to do a stack walk without reading eh_frame or the arm unwind info

[Lldb-commits] [PATCH] D27289: Return "thread-pcs" in jstopinfo on Linux/Android.

jasonmolenda added a comment. @labath intersting idea, sending a blob of stack memory above the current stack pointer reg value to accelerate an unwind. If you have a lot of small stack frames, it could be a performance benefit. Common/simple methods do their work in 32-64 bytes of stack spac

[Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax

jasonmolenda added a comment. A couple of thoughts / two cents. I don't mind the current if (log) { ... } style of logging, even with the PRIx64's and having to do filepath.GetPath().c_str() and all that. I like being able to do extra work in a if (log) block of code -- create a SymbolContext

[Lldb-commits] [PATCH] D28035: Stop limiting the number of TSan backtrace size to 8

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Looks fine to me, the only reason I could think to cap it is if the method used to backtrace the thread could get stuck in a loop & provide infinite number of frames. but we could

  1   2   3   4   5   6   7   8   9   10   >