[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-29 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision. JDevlieghere 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/D89842/new/ https://reviews.llvm.org/D89842 _

[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-29 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGefe62b637d51: [lldb/DWARF] Add support for DW_OP_implicit_value (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D89842#2345947 , @aprantl wrote: > Perhaps the test fits in here > https://github.com/llvm/llvm-project/blob/master/lldb/unittests/Expression/DWARFExpressionTest.cpp > ? Yes, that's even better. :) Repository: rG LLVM Gith

[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-22 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. Perhaps the test fits in here https://github.com/llvm/llvm-project/blob/master/lldb/unittests/Expression/DWARFExpressionTest.cpp ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89842/new/ https://reviews.llvm.org/D89842

[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The code looks pretty straight-forward, but I believe the test should be asm-based with hardcoded dwarf (if you're feeling adventurous, you can also try yaml). My reasons for that are: - it makes it guarantees (and makes it explicit) the input that is being tested - the

[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-22 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 299941. mib edited the summary of this revision. mib added a comment. Change shell test for unit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89842/new/ https://reviews.llvm.org/D89842 Files: lldb/source/

[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. +1 on everything Pavel said. Comment at: lldb/source/Expression/DWARFExpression.cpp:856-867 + const uint32_t len = opcodes.GetULEB128(&opcode_offset); + const void *data = opcodes.GetData(&opcode_offset, len); + + if (!data) { +LLDB_LOG(log,

[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-22 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 299540. mib added a comment. Add test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89842/new/ https://reviews.llvm.org/D89842 Files: lldb/source/Expression/DWARFExpression.cpp lldb/test/Shell/SymbolFile/DWAR

[Lldb-commits] [PATCH] D89842: [lldb/DWARF] Add support for DW_OP_implicit_value

2020-10-22 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 299539. mib retitled this revision from "[lldb/DWARF] Add support DW_OP_implicit_value" to "[lldb/DWARF] Add support for DW_OP_implicit_value". mib edited the summary of this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev