[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-13 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:152 +ConstString func_name = sym_ctx.GetFunctionName(); +if (func_name == ConstString(function_name) || +alternate_function_name.empty() || JDevlieghere

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:152 +ConstString func_name = sym_ctx.GetFunctionName(); +if (func_name == ConstString(function_name) || +alternate_function_name.empty() || teemperor

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-11 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added inline comments. Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:152 +ConstString func_name = sym_ctx.GetFunctionName(); +if (func_name == ConstString(function_name) || +alternate_function_name.empty() || JDevlieghere

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-10 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a subscriber: davide. jankratochvil added a comment. @davide Sorry for the revert rG6b2979c12300b90a1e69791d43ee9cff14f4265e - I will find some way to test stuff on OSX, I already made too many OSX

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: lldb/source/Commands/CommandObjectFrame.cpp:966 name = "(internal)"; result.GetOutputStream().Printf( + "%d: %s, module %s, function %s{%s}%s\n", recognizer_id, We should stream

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-10 Thread Jan Kratochvil via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1a39f1b966a8: [lldb] Fix+re-enable Assert StackFrame Recognizer on Linux (authored by jankratochvil). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-10 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. In D74252#1866476 , @mib wrote: > Has the latest version of this patch landed yet or does it need some extra > work ? I was giving some time to other reviewers but it is checked-in now. Repository: rG LLVM Github

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment. Hello Jan, Has the latest version of this patch landed yet or does it need some extra work ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74252/new/ https://reviews.llvm.org/D74252

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. In D74252#1865504 , @mib wrote: > but let's leave symbol as is and add a `ConstString alternate_symbol` FYI this had a purpose. Any code touching `symbol` should be rechecked if it should not handle also

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. LGTM! Thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74252/new/ https://reviews.llvm.org/D74252

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 243358. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74252/new/ https://reviews.llvm.org/D74252 Files: lldb/include/lldb/Target/StackFrameRecognizer.h lldb/source/Commands/CommandObjectFrame.cpp

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment. In D74252#1865501 , @jankratochvil wrote: > In D74252#1865500 , @mib wrote: > > > Knowing that this will be called every time a thread stops, it would be > > better if we could avoid

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. In D74252#1865500 , @mib wrote: > Knowing that this will be called every time a thread stops, it would be > better if we could avoid processing a regex every time we try to recognise a > frame. Or

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment. In D74252#1865496 , @jankratochvil wrote: > OK, then it could be changed to a regex. Is it fine afterwards? Knowing that this will be called every time a thread stops, it would be better if we could avoid processing a regex every

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment. In D74252#1865493 , @mib wrote: > We'd like to avoid registering recognizers multiple times for the same > purpose ... OK, then it could be changed to a regex. Is it fine afterwards? Repository: rG LLVM Github

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment. We'd like to avoid registering recognizers multiple times for the same purpose ... This could cause some issues down the road, since those recognizers can - indirectly - perform an action (change the current frame or the stop reason). Also, from a usability stand-point,

[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer on Linux

2020-02-08 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 243353. jankratochvil retitled this revision from "Fix+re-enable Assert StackFrame Recognizer" to "Fix+re-enable Assert StackFrame Recognizer on Linux". jankratochvil edited the summary of this revision. Herald added a subscriber: jfb. Repository: rG