[Lldb-commits] [PATCH] D95683: [lldb] Fix fallout caused by D89156 on 11.0.1 for MacOS

JosephTremoulet added a comment. LGTM (and thanks for the fix and apologies for the break) )but I think @tstellar should have a look given it's fixing a build break in the release/11.x branch caused by my porting of changes from master for https://bugs.llvm.org/show_bug.cgi?id=48053

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet added a subscriber: tstellar. JosephTremoulet added a comment. Oh, I see. I made this change in the master branch and then ported it to the release/11.x branch. It changed the signature of a virtual method, which had a couple more overrides in the release/11.x branch, and when

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet added a comment. In D89156#2530353 , @Abpostelnicu wrote: > Shouldn't this also update PlatformiOSSimulator.h? There is no PlatformOSSimulator.h file (or I can't find it). There is a type `PlatformiOSSumulator`, but it doesn't have its

[Lldb-commits] [PATCH] D94629: [LLDB] MinidumpParser: Prefer executable module even at higher address

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG85dfcaadc5f0: [LLDB] MinidumpParser: Prefer executable module even at higher address (authored by JosephTremoulet). Repository: rG LLVM Github Mon

[Lldb-commits] [PATCH] D94629: [LLDB] MinidumpParser: Prefer executable module even at higher address

JosephTremoulet updated this revision to Diff 316498. JosephTremoulet added a comment. - Fix comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94629/new/ https://reviews.llvm.org/D94629 Files: lldb/source/Plugins/Process/minidump/MinidumpPar

[Lldb-commits] [PATCH] D94629: [LLDB] MinidumpParser: Prefer executable module even at higher address

JosephTremoulet created this revision. JosephTremoulet requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. When a program maps one of its own modules for reading, and then crashes, breakpad can emit two entries for that module in the ModuleList

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG61bfc703c3d3: [lldb] GetSharedModule: Collect old modules in SmallVector (authored by JosephTremoulet). Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd20aa7ca4221: [lldb] Report old modules from ModuleList::ReplaceEquivalent (authored by JosephTremoulet). Changed prior to commit: https://reviews

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet added a comment. OK, I believe this is correct now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89156/new/ https://reviews.llvm.org/D89156 ___ lldb-commits mailing list lldb-commits@lis

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet updated this revision to Diff 301964. JosephTremoulet added a comment. - Ignore old modules not in target list Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89156/new/ https://reviews.llvm.org/D89156 Files: lldb/include/lldb/Core

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet added inline comments. Comment at: lldb/source/Target/Target.cpp:2131-2132 +for (ModuleSP &old_module_sp : old_modules) { + if (m_images.GetIndexForModule(old_module_sp.get()) != + LLDB_INVALID_INDEX32) { +if (!did_replace

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet updated this revision to Diff 301935. JosephTremoulet added a comment. - Expand logging as suggested in D89157 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89156/new/ https://reviews.llvm.org/D8915

[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

JosephTremoulet added a comment. > But then you get to a point where you shouldn't really have multiple modules > replacing a single one so you aren't really sure what to do about it. That > part makes me a little uneasy. Yeah, I wouldn't claim that the handling of the multiple-old-module case

[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

JosephTremoulet added a comment. Thanks for the review! I'm still unsure how to proceed w.r.t. D89156 -- did you intend to include that when you approved this change, or do you have a sense when you'll have time to look at it, or should I change this to not de

[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

JosephTremoulet added a comment. I'd been hoping to get this bugfix into 11.0.1, but it (and D89156 , which is just some cleanup that occurred to me while working on this code) have been stalled for a few weeks, and I don't have any insight into the reason. If

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89156/new/ https://reviews.llvm.org/D89156 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm

[Lldb-commits] [PATCH] D89155: [lldb] Minidump: check for .text hash match with directory

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd30797b4041f: [lldb] Minidump: check for .text hash match with directory (authored by JosephTremoulet). Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet added a comment. In D89156#2328772 , @jingham wrote: > IIUC, the problem with ReplaceModule will get fixed in a dependent patch, > right? So the last bit of this patch will get cleaned up by that change. If > that's right, then this LGT

[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

JosephTremoulet added inline comments. Comment at: lldb/source/Core/ModuleList.cpp:779-781 if (module_sp->FileHasChanged()) { if (old_modules) old_modules->push_back(module_sp); JosephTremoulet wrote: > > If I run lldb on a binary

[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

JosephTremoulet marked an inline comment as done. JosephTremoulet added a comment. Comment at: lldb/source/Core/ModuleList.cpp:779-781 if (module_sp->FileHasChanged()) { if (old_modules) old_modules->push_back(module_sp); > If

[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

JosephTremoulet updated this revision to Diff 297939. JosephTremoulet added a comment. - Comment new parameter Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89157/new/ https://reviews.llvm.org/D89157 Files: lldb/include/lldb/Core/ModuleList.h

[Lldb-commits] [PATCH] D89155: [lldb] Minidump: check for .text hash match with directory

JosephTremoulet marked an inline comment as done. JosephTremoulet added a comment. Thanks for the speedy review! I've updated it to include a testcase that shows the problem you described, and include the fix you suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[Lldb-commits] [PATCH] D89155: [lldb] Minidump: check for .text hash match with directory

JosephTremoulet updated this revision to Diff 297422. JosephTremoulet added a comment. - Handle finding wrong version before right one Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89155/new/ https://reviews.llvm.org/D89155 Files: lldb/source/Pl

[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

JosephTremoulet created this revision. Herald added a reviewer: JDevlieghere. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. JosephTremoulet requested review of this revision. This allows the Target to update its module list when loading a shared module replaces an equivale

[Lldb-commits] [PATCH] D89156: [lldb] GetSharedModule: Collect old modules in SmallVector

JosephTremoulet created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. JosephTremoulet requested review of this revision. Herald added a subscriber: JDevlieghere. The various GetSharedModule methods have an optional out parameter for the old module when a fil

[Lldb-commits] [PATCH] D89155: [lldb] Minidump: check for .text hash match with directory

JosephTremoulet created this revision. Herald added a reviewer: JDevlieghere. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. JosephTremoulet requested review of this revision. When opening a minidump, we might discover that it reports a UUID for a module that doesn't match

[Lldb-commits] [PATCH] D88099: [lldb] Fix GetRemoteSharedModule fallback logic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20f84257ac4a: [lldb] Fix GetRemoteSharedModule fallback logic (authored by JosephTremoulet). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[Lldb-commits] [PATCH] D88099: [lldb] Fix GetRemoteSharedModule fallback logic

JosephTremoulet created this revision. Herald added a reviewer: JDevlieghere. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. JosephTremoulet requested review of this revision. When the various methods of locating the module in GetRemoteSharedModule fail, make sure we pass t

[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

JosephTremoulet added a comment. To clarify, the failure did not reproduce for me in the SUSE 15.02 container. I don't know what I'm doing differently. To create the environment, I just did `docker run ... opensuse/leap:15.2`, and then `zypper in gcc-c++ python3-devel` inside the container.

[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

JosephTremoulet added a comment. FWIW, I tried running the test in an SUSE 15.2 docker container (on an Ubuntu 18.04 host), and here's what I'm seeing. `raise` and `abort` indeed do not indicate that they had the `S` set in the augmentation: (lldb) bt * thread #1, name = 'a.out', stop rea

[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

JosephTremoulet added a comment. > eh_frame augmented UnwindPlan: > ... > This UnwindPlan is for a trap handler function: yes. From memory, I'd have expected that for `__restore_rt`, but not for `raise` or `abort`. Can you dump the augmentation fields from the CIEs (or instrument `FDEToUnwindP

[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

JosephTremoulet added a comment. > And its parent 'raise' is set so as well because of > 'GetNextFrame()->m_frame_type == eTrapHandlerFrame '. Sorry, can you clarify what code path you're referring to here? I see two occurrences of `GetNextFrame()->m_frame_type == eTrapHandlerFrame` in the cod

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to return Expected

This revision was automatically updated to reflect the committed changes. JosephTremoulet marked an inline comment as done. Closed by commit rG95b2e516bd3e: Change Target::FindBreakpointsByName to return Expected (authored by JosephTremoulet). Repository: rG LLVM Github Monorepo CHANGE

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to return Expected

JosephTremoulet marked 2 inline comments as done. JosephTremoulet added inline comments. Comment at: lldb/source/Breakpoint/BreakpointList.cpp:136 +"FindBreakpointsByName requires a name", +llvm::inconvertibleErrorCode()); JDevlieghere wrote: >

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to return Expected

JosephTremoulet updated this revision to Diff 231931. JosephTremoulet added a comment. - Use invalid_argument error code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70907/new/ https://reviews.llvm.org/D70907 Files: lldb/include/lldb/Breakpoint

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to use a vector

JosephTremoulet updated this revision to Diff 231914. JosephTremoulet added a comment. - Change signature to return Expected> Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70907/new/ https://reviews.llvm.org/D70907 Files: lldb/include/lldb/Break

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to use a vector

JosephTremoulet marked an inline comment as done. JosephTremoulet added inline comments. Comment at: lldb/include/lldb/Breakpoint/BreakpointList.h:71 /// \bfalse if the input name was not a legal breakpoint name. - bool FindBreakpointsByName(const char *name, BreakpointList

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to use a vector

JosephTremoulet created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Using a BreakpointList corrupts the breakpoints' IDs because BreakpointList::Add sets the ID, so use a vector instead. Note that, despite the similar name, SBTarget::FindBreakpointsByName

[Lldb-commits] [PATCH] D68096: ProcessMinidump: Suppress reporting stop for signal '0'

This revision was automatically updated to reflect the committed changes. Closed by commit rG77460d3888c5: ProcessMinidump: Suppress reporting stop for signal '0' (authored by JosephTremoulet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68096/new/

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

This revision was automatically updated to reflect the committed changes. Closed by commit rGa50272f8261f: Update MinidumpYAML to use minidump::Exception for exception stream (authored by JosephTremoulet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D68658: LLDB: Use LLVM's type for minidump ExceptionStream [NFC]

This revision was automatically updated to reflect the committed changes. Closed by commit rGd094d97d0223: LLDB: Use LLVM's type for minidump ExceptionStream [NFC] (authored by JosephTremoulet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68658/new

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 225459. JosephTremoulet marked an inline comment as done. JosephTremoulet added a comment. - Rebase - Use Twine instead of formatv Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://review

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet marked an inline comment as done. JosephTremoulet added inline comments. Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:389 + for (size_t Index = 0; Index < Exception.MaxParameters; ++Index) { +SmallString<16> Name = formatv("Parameter {0}", Index); +su

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet marked an inline comment as done. JosephTremoulet added inline comments. Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:389 + for (size_t Index = 0; Index < Exception.MaxParameters; ++Index) { +SmallString<16> Name = formatv("Parameter {0}", Index); +su

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 225044. JosephTremoulet added a comment. - punctuation fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 Files: lldb/packages/Python/lldbsuite/test/funct

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet marked an inline comment as done. JosephTremoulet added inline comments. Comment at: llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml:3 + +# Test that we report an error for an ExceptionStream where the specified +# number of parameters is greate

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 224857. JosephTremoulet marked 2 inline comments as done. JosephTremoulet added a comment. - Move test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 Files:

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet marked 4 inline comments as done. JosephTremoulet added inline comments. Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:377-383 + // TODO: We could provide a reasonable default for ThreadContext by searching + // the Thread stream for a thread with the given I

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 224853. JosephTremoulet added a comment. - Remove TODO, lit-ify negative test and tighten check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 Files: lldb/p

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 224805. JosephTremoulet added a comment. - ...and fix namespace... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 Files: lldb/packages/Python/lldbsuite/test

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 224804. JosephTremoulet added a comment. - Fix Expected<> types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 Files: lldb/packages/Python/lldbsuite/test/fu

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 224800. JosephTremoulet marked an inline comment as done. JosephTremoulet added a comment. - Apply review feedback (-auto, -memset, +comments) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ ht

[Lldb-commits] [PATCH] D68096: ProcessMinidump: Suppress reporting stop for signal '0'

JosephTremoulet added a comment. In D68096#1706258 , @clayborg wrote: > If this patch is solely meant to fix this issue, then I remove my "Requires > Changes" and this patch is good to go if it no longer hangs LLDB. Great, thanks. And just to confirm o

[Lldb-commits] [PATCH] D68096: ProcessMinidump: Suppress reporting stop for signal '0'

JosephTremoulet added a comment. Just to make sure I'm understanding the feedback correctly, I'll try to summarize. Please let me know if this is off track: - When one uses breakpad to generate a minidump for a process that *hasn't* crashed, depending which of its methods one calls to write th

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 224607. JosephTremoulet added a comment. Address review feedback - Add Exception stream to minidump-basic.yaml to test obj2yaml direction - Reorder ExceptionStream definition and constructors - Use the mapOptional helper - Re

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet marked 4 inline comments as done. JosephTremoulet added a comment. Added Exception stream to minidump-basic.yaml as suggested. Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:394 + mapOptionalHex(IO, "Exception Address", Exception.ExceptionAddress, 0); + IO.m

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

JosephTremoulet updated this revision to Diff 224469. JosephTremoulet added a comment. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. - Update test input yaml Exception stream Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D686

[Lldb-commits] [PATCH] D68096: ProcessMinidump: Suppress reporting stop for signal '0'

JosephTremoulet updated this revision to Diff 224470. JosephTremoulet added a comment. Herald added a reviewer: jfb. - Rebase - Add testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68096/new/ https://reviews.llvm.org/D68096 Files: lldb/pa

[Lldb-commits] [PATCH] D68658: LLDB: Use LLVM's type for minidump ExceptionStream [NFC]

JosephTremoulet updated this revision to Diff 224356. JosephTremoulet added a comment. - Define DumpRequested constant in-line, per feedback in D68656 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68658/new/ https

[Lldb-commits] [PATCH] D68658: LLDB: Use LLVM's type for minidump ExceptionStream

JosephTremoulet created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. JosephTremoulet added a parent revision: D68657: Update MinidumpYAML to use minidump::Exception for exception stream. JosephTremoulet added reviewers: labath, clayborg. The types defined

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the thread list stream

JosephTremoulet added a comment. Nit: Title says "thread" rather than "memory info" Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68645/new/ https://reviews.llvm.org/D68645 ___ lldb-commits mailing list lldb-commits@l

[Lldb-commits] [PATCH] D68096: ProcessMinidump: Suppress reporting stop for signal '0'

JosephTremoulet marked 2 inline comments as done. JosephTremoulet added a comment. In D68096#1687790 , @labath wrote: > It doesn't look like it should be too hard to add yaml support for the > exceptions stream -- it should only be a matter of adapting th

[Lldb-commits] [PATCH] D68096: ProcessMinidump: inject SIGSTOP on Linux if no thread has a signal

JosephTremoulet updated this revision to Diff 222444. JosephTremoulet added a comment. - Review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68096/new/ https://reviews.llvm.org/D68096 Files: lldb/source/Plugins/Process/minidump/Process

[Lldb-commits] [PATCH] D68096: ProcessMinidump: inject SIGSTOP on Linux if no thread has a signal

JosephTremoulet updated this revision to Diff 25. JosephTremoulet added a comment. - Use accessor for m_unix_signals_sp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68096/new/ https://reviews.llvm.org/D68096 Files: lldb/source/Plugins/Proce

[Lldb-commits] [PATCH] D68096: ProcessMinidump: inject SIGSTOP on Linux if no thread has a signal

JosephTremoulet marked 2 inline comments as done. JosephTremoulet added inline comments. Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:219-237 + + if (arch.GetTriple().isOSLinux()) { + +SetUnixSignals(UnixSignals::Create(GetArchitecture())); + +if

[Lldb-commits] [PATCH] D68096: Add Linux signal support to ProcessMinidump

JosephTremoulet updated this revision to Diff 18. JosephTremoulet added a comment. - Move artificial SIGSTOP injection to RefreshStateAfterStop Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68096/new/ https://reviews.llvm.org/D68096 Files: l

[Lldb-commits] [PATCH] D68096: Add Linux signal support to ProcessMinidump

JosephTremoulet marked an inline comment as done. JosephTremoulet added a comment. > Fortunately, for the functionality you're testing, I don't think you really > need the executable file, so you can just ignore the elf bit and test with a > plain `lldb -c foo.dmp` (obviously, you won't get the

[Lldb-commits] [PATCH] D68096: Add Linux signal support to ProcessMinidump

JosephTremoulet added a comment. I'm trying to fix an issue where opening a minidump created by breakpad in lldb just hangs, but if I use breakpad's minidump-2-core on that same dump then opening the core dump works fine. From debugging the two cases I can see that the critical logic that Proc

[Lldb-commits] [PATCH] D68096: Add Linux signal support to ProcessMinidump

JosephTremoulet created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Set the UnixSignals object when isOSLinux is true, and force a SIGSTOP if there's no other signal when loading a core dump (the same as ProcessElfCore::DoLoadCore does) since the loading p

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet marked an inline comment as done. JosephTremoulet added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:1760 +void RegisterContextLLDB::PropagateTrapHandlerFlag( +lldb::UnwindPlanSP unwind_plan) { + if (unwind_plan->Ge

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet added a comment. In D64993#1612746 , @JDevlieghere wrote: > Hey Joseph, > > The test `trap_frame_sym_ctx` is failing on GreenDragon. Can you please have > a look? > > http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/32449/testRepor

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet updated this revision to Diff 213067. JosephTremoulet added a comment. - Expand comment about return trampolines - rename PropagateTrapHandlerFlag -> PropagateTrapHandlerFlagFromUnwindPlan Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet added a comment. In D64993#1608452 , @labath wrote: > I haven't looked at the implementation, but I did run into the problem you > are fixing in the past, and I am happy that someone is finally implementing > this. I just wanted to say th

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet updated this revision to Diff 212642. JosephTremoulet added a comment. - add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64993/new/ https://reviews.llvm.org/D64993 Files: lldb/include/lldb/Core/Address.h lldb/include/lld

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet updated this revision to Diff 212615. JosephTremoulet added a comment. - Move resolution helper from RegisterContextLLDB to lldb_private::Address - Defensively initialize out arguments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet marked an inline comment as done. JosephTremoulet added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:595-624 +bool RegisterContextLLDB::TryResolveSymbolContextAndAddressRange( +lldb_private::Address pc, lldb_private:

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet added a comment. @jasonmolenda @clayborg ping. To clarify, the issue this fixes is, using test functionalities/signal/handle-abrt as an example: #include #include #include void handler(int sig) { printf("Set a breakpoint here.\n"); exit(0); } v

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet added a comment. In D63667#1594718 , @jankratochvil wrote: > You did not have DWARF for glibc installed: `dnf debuginfo-install glibc` > > Then in fact your test was better as only real frames matter for this patch. > There is some LLDB b

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet marked 2 inline comments as done. JosephTremoulet added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:1760 +void RegisterContextLLDB::PropagateTrapHandlerFlag( +lldb::UnwindPlanSP unwind_plan) { + if (unwind_plan->Ge

[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

JosephTremoulet created this revision. JosephTremoulet added reviewers: jasonmolenda, clayborg. Herald added a reviewer: jfb. Herald added a project: LLDB. Update StackFrame::GetSymbolContext to mirror the logic in RegisterContextLLDB::InitializeNonZerothFrame that knows not to do the pc decrement

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet added a comment. > I'm guessing on this Linux system, you've got a trap receiver function on the > stack that is on its first instruction or something? So backing up the pc > value for *that* frame is the problem you're solving. > ... > Just just to check, we've got a backtrace

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet added a subscriber: clayborg. JosephTremoulet added a comment. ping @jasonmolenda -- do the updates look like what you had in mind when you said " It'd be a good change to capture that information from the eh_frame though, even if we don't see a clear way to use it right now"? A

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet added a comment. Herald added subscribers: wuzish, MaskRay. > That LLDB frame #5 is a bit bogus Thanks for taking a look at this. I'm trying to reproduce what you're seeing and failing. This is what I see, in a fedora:latest docker container on an Ubuntu host on an x86_64 mach

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet added a comment. In D63667#1561365 , @jankratochvil wrote: > Shouldn't you add also symbol `__restore_rt` (Fedora 30 x86_64)? Thanks, yes, looks like I should. Updated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet updated this revision to Diff 206931. JosephTremoulet added a comment. - Include __restore_rt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63667/new/ https://reviews.llvm.org/D63667 Files: lldb/include/lldb/Symbol/UnwindPlan.h

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet updated this revision to Diff 206516. JosephTremoulet added a comment. - fix copy pasta Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63667/new/ https://reviews.llvm.org/D63667 Files: lldb/include/lldb/Symbol/UnwindPlan.h lldb

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet added a comment. I've updated this with code to recognize the 'S' in the eh_frame augmentation and record it in a new bool member of UnwindPlan, `m_plan_is_for_signal_trap`. I haven't hooked up any consumers of the new bit; as you say, with the current code flow we don't parse

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet updated this revision to Diff 206505. JosephTremoulet added a comment. Herald added subscribers: jsji, atanasyan, jrtc27, kbarton, nemanjai. - Fix typos - Convey 'S' eh_frame augmentation to UnwindPlan Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet added a comment. FYI, I sent mail about this to lldb-dev.. I'll copy the contents here for the benefit of anybody who didn't see it there but could use the context: > Hi, > > I'm trying to use lldb in a project where I need to report stack traces from > signal handlers, and nee

[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

JosephTremoulet created this revision. JosephTremoulet added reviewers: labath, jankratochvil, compnerd. Herald added subscribers: kristof.beyls, javed.absar. Herald added a reviewer: jfb. Herald added a project: LLDB. Add __kernel_rt_sigreturn to the list of trap handlers for Linux (it's used as

[Lldb-commits] [PATCH] D61036: [lldb] Use local definition of get_cpuid_count

JosephTremoulet added a comment. Switched to static function, and yes I do have commit access. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61036/new/ https://reviews.llvm.org/D61036

[Lldb-commits] [PATCH] D61036: [lldb] Use local definition of get_cpuid_count

JosephTremoulet updated this revision to Diff 196459. JosephTremoulet added a comment. - Use static function rather than anonymous namespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61036/new/ https://reviews.llvm.org/D61036 Files: lldb/sou

[Lldb-commits] [PATCH] D61036: [lldb] Use local definition of get_cpuid_count

JosephTremoulet added a comment. @labath, as discussed in D59991 , I went with a local copy of the function since I didn't see an obvious opportunity to tweak the calling code, but maybe I overlooked something. My copy of cpuid.h matches https://github.com/gcc-

[Lldb-commits] [PATCH] D61036: [lldb] Use local definition of get_cpuid_count

JosephTremoulet created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This is needed for gcc/cstdlib++ 5.4.0, where __get_cpuid_count is not defined in cpuid.h. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D61036 Files: lldb/source/Pl

[Lldb-commits] [PATCH] D59991: [Linux/x86] Fix writing of non-gpr registers on newer processors

JosephTremoulet added a comment. In D59991#1475694 , @labath wrote: > gcc-5.4 technically supported, so we can try to make things work for you. I'd > be best if you can create a patch that will make things work for you (since > it's kinda hard for me to

[Lldb-commits] [PATCH] D59991: [Linux/x86] Fix writing of non-gpr registers on newer processors

JosephTremoulet added inline comments. Comment at: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:282 + // Then fetch the maximum size of the area. + if (!__get_cpuid_count(0x0d, 0, &eax, &ebx, &ecx, &edx)) +return sizeof(FPR); --