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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

jankratochvil added a comment. I have reverted this change as it fixed the testsuite but real world usage was broken: (lldb) run assert.test.tmp.out: /home/jkratoch/redhat/llvm-monorepo/lldb/test/Shell/Recognizer/Inputs/assert.c:7: int main(): Assertion `a == 42' failed. Process 12062

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

mib added a comment. I used an apt installed lldb so it was probably a build older than Jan 13th. 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

This revision was automatically updated to reflect the committed changes. Closed by commit rGcf1046c716b3: [lldb] Fix+re-enable Assert StackFrame Recognizer on Linux (authored by jankratochvil). Changed prior to commit: https://reviews.llvm.org/D74252?vs=243266=243274#toc Repository: rG

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

jankratochvil marked 2 inline comments as done. jankratochvil added a comment. In D74252#1864879 , @mib wrote: > Originally, when I implemented the patch and tested it on linux, the symbol > on the abort frame was (__GI___assert_fail) on my machine.

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

mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. Originally, when I implemented the patch and tested it on linux, the symbol on the abort frame was (__GI___assert_fail) on my machine. In D73303#1846281 ,

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

jankratochvil created this revision. jankratochvil added reviewers: mib, JDevlieghere, labath. jankratochvil added a project: LLDB. Herald added a subscriber: aprantl. D73303 was failing on Fedora Linux and so it was