aprantl added a comment.
This is very exciting!
================
Comment at:
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/Makefile:4
+include $(LEVEL)/Makefile.rules
+CXXFLAGS += -g -O1 -glldb -Xclang -femit-debug-entry-values
----------------
The -g should be redundant, and if we leave out -glldb (which should be the
default for clang anyway) then this test in theory could also work with other
compilers such as GCC (which I think implements the same option under the same
-f option). I have no idea whether anyone runs such a bot, but it's still nice.
================
Comment at:
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/main.cpp:7
+//
+//===----------------------------------------------------------------------===//
+
----------------
I would just drop the header for testcases.
================
Comment at:
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values/main.cpp:35
+ /* Clobbers */ : "rsi"
+ );
+
----------------
Cool.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:497
+
+ finish_subexpressions_to(end_offset);
}
----------------
We should probably just let llvm's libDebugInfo do the printing here, but this
works fine for now.
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:1117
+ if ((!exe_ctx || !exe_ctx->HasTargetScope()) || !reg_ctx) {
+ LLDB_LOG(log, "Evaluate_DW_OP_entry_value: no exe/reg context");
+ return false;
----------------
The Evaluate function allows setting an Error with a message. Should we do the
same thing here so we can return the error to the caller?
================
Comment at: lldb/source/Expression/DWARFExpression.cpp:2856
+ error_ptr, log)) {
+ LLDB_LOGF(log, "Could not evaluate %s.", DW_OP_value_to_name(op));
+ return false;
----------------
See above, we can set `error_ptr` here.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp:62
DRC_class DW_OP_value_to_class(uint32_t val) {
+ // FIXME: This switch should be simplified by using DW_OP_* names.
switch (val) {
----------------
Don't spend too much time on that, we should just use the llvm DWARFExpression
iterator instead.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3712
+/// Collect call site parameters in a TAG_call_site DIE.
+static CallSiteParameterArray
----------------
DW_TAG_call_site
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3752
+ LocationInCaller = parse_simple_location(i);
+ break;
+ }
----------------
default?
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:795
if (Asm->TM.Options.EnableDebugEntryValues &&
- tuneForGDB()) {
+ (tuneForGDB() || tuneForLLDB())) {
ParamSet Params;
----------------
This should also be tested inside of LLVM itself and probably should be a
separate commit.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67376/new/
https://reviews.llvm.org/D67376
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
