[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good to me with a few minor cleanups. Comment at: lldb/include/lldb/Core/ModuleList.h:73-74 +public: + explicit ModuleIterator(const ModuleList *module_list, size_

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In D91734#2480474 , @probinson wrote: > This version of the patch avoids the weirdness I was seeing with prolog > instructions in certain cases. >

[Lldb-commits] [PATCH] D91734: [FastISel] Flush local value map on every instruction

dblaikie added a comment. In D91734#2482280 , @probinson wrote: > In D91734#2480930 , @dblaikie wrote: > >> I haven't looked closely, but I take it this one test modification seems >> reasonable to you? I guess we'

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

dblaikie added a comment. In D78978#2483327 , @paolosev wrote: > In D78978#2481358 , @vwzm228 wrote: > >> Is there any progress about such patch and D78801 >> ？ >> >> I have impleme

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

dblaikie updated this revision to Diff 315035. dblaikie added a comment. Update test to avoid running the program Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94063/new/ https://reviews.llvm.org/D94063 Files: lldb/source/Plugins/SymbolFile/DWAR

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

dblaikie added a comment. In D94063#2481867 , @labath wrote: > The fix is good, but the test could be improved. Yeah - haven't written lldb patches before so totally open to suggestions on the testing front for sure. Thanks! > Combining assembly input w

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

dblaikie updated this revision to Diff 315036. dblaikie added a comment. Use image lookup to make test runnable without executing the test code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94064/new/ https://reviews.llvm.org/D94064 Files: lldb/

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

dblaikie added a comment. In D94064#2481925 , @labath wrote: > I don't think that simply setting func_lo_pc to zero will be sufficient to > make this work. I would expect this to break in more complicated scenarios > (like, even if we just have two of th

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG274afac9a17f: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms (authored by dblaikie). Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

dblaikie added a comment. In D94063#2485271 , @labath wrote: > In D94063#2483546 , @dblaikie wrote: > >> If it's better to write it using C++ source and custom clang flags I can do >> that instead (it'll be an -mll

[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

dblaikie added a comment. In D94063#2492450 , @DavidSpickett wrote: > Do you have python enabled in the build? > (https://lldb.llvm.org/resources/build.html#preliminaries) > > The cmake option LLDB_ENABLE_PYTHON defaults to Auto which has tripped me up

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

dblaikie added a comment. (Works for me, FWIW - takes my local run down from 42 failures in check-lldb-api to 1 (a timeout)) Out of curiousity did you find a way to get lldb-test to print out the stdout/stderr of the underlying lldb process to see what error messages it was producing that demo

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

dblaikie updated this revision to Diff 318654. dblaikie added a comment. Retrieve the lowest address in the address ranges, rather than just hardcoding 0 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94064/new/ https://reviews.llvm.org/D94064 File

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

dblaikie added a comment. In D94064#2485222 , @labath wrote: > In D94064#2483578 , @dblaikie wrote: > >> In D94064#2481925 , @labath wrote: >> >>> I don't think that simply s

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

dblaikie updated this revision to Diff 318711. dblaikie added a comment. Include some more details about/in the test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94064/new/ https://reviews.llvm.org/D94064 Files: lldb/source/Plugins/SymbolFile/D

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

dblaikie added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test:22 +# initialization/no prologue instructions, so the location of "var" is valid +# at the start of the function, so 'image lookup -v -s main' will include it. +# -

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG78d41a1295d9: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms (authored by dblaikie). Repository: rG LLVM Github

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

dblaikie added a comment. If I had to guess what this might've related to is the fact that LLVM puts a DW_AT_low_pc on the CU even if the CU uses discontiguous ranges - and in that case the low_pc has the constant value 0. So that all address values are resolved "relative" to that zero, making

[Lldb-commits] [PATCH] D78489: [lldb/DWARF] Trust CU DW_AT_low/high_pc information when building address tables

dblaikie added a comment. In D78489#1993806 , @clayborg wrote: > One thing to think about that my flawed example above did show: dead stripped > functions cause problems for debug info. Both in DW_AT_ranges on the compile > unit and in all of the DWARF b

[Lldb-commits] [PATCH] D78882: [IR] Replace all uses of CallBase::getCalledValue() with getCalledOperand().

dblaikie accepted this revision. dblaikie added a comment. Looks great - thanks for the pointer type fixup(s) along the way too! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78882/new/ https://reviews.llvm.org/D78882 ___

[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

dblaikie added a comment. @vsk (assuming you're the original author of this test - couldn't quite figure out the revision history, sorry) - could you check if some of this could be simplified a bit to make it more clear what's being tested/what's "interesting" here? (I've provided some inline c

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

dblaikie added a comment. In D79811#2036112 , @labath wrote: > Here's another interesting use of aritificial functions: inherited > constructors. > > struct A { A(int); }; > struct B:A { using A::A; }; > B b(2); > > > The constructor B::B(int) will

[Lldb-commits] [PATCH] D79811: WIP: Reenable creation of artificial methods in AddMethodToCXXRecordType(...)

dblaikie added a comment. FWIW I agree that based on the current DWARF LLVM and GCC produce there's no way to allow users to default construct structs in C++11 (where non-static data member initializers were added) or above in the absence of a user-provided constructor - I don't think it'd be u

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

dblaikie added a subscriber: aprantl. dblaikie added inline comments. Comment at: lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8 + // FROM-FUNC1-NEXT: func1 + // FROM-FUNC1-SAME: [artificial] + // FROM-FUNC1-NEXT: main

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

dblaikie added inline comments. Comment at: lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8 + // FROM-FUNC1-NEXT: func1 + // FROM-FUNC1-SAME: [artificial] + // FROM-FUNC1-NEXT: main vsk wrote: > labath wrote: > > la

[Lldb-commits] [PATCH] D80519: [lldb/DWARF] Add support for pre-standard GNU call site attributes

dblaikie added inline comments. Comment at: lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp:8 + // FROM-FUNC1-NEXT: func1 + // FROM-FUNC1-SAME: [artificial] + // FROM-FUNC1-NEXT: main vsk wrote: > dblaikie wrote: > >

[Lldb-commits] [PATCH] D79147: Switch to using -debug-info-kind=constructor as default (from =limited)

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Could you include some comparative data in the description/commit message demonstrating this generally ends up emitting all the same types for a clang build before/after (& explanations fo

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

dblaikie added a subscriber: rnk. dblaikie added a comment. In D70840#1765219 , @labath wrote: > Throwing some additional dwarf people in here... > > In D70840#1763750 , @mstorsjo wrote: > > > In D70840#1763705

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

dblaikie added a comment. In D70840#1765876 , @mstorsjo wrote: > In D70840#1765219 , @labath wrote: > > > debug_aranges is a sort of a lookup table for speeding up address->compile > > unit searches. llvm does not

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

dblaikie added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:72 + } + llvm_unreachable("Fully covered switch!"); +} I think usually the unreachable comment that's used in this sort of case is "Invalid LocationListFormat!" or similar,

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

dblaikie added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:2829 +if (!loc) + LLDB_LOG_ERROR(log, loc.takeError(), "{0}"); +if (loc->Range) { labath wrote: > labath wrote: > > dblaikie wrote: > > > Does LLDB_LOG_ERROR stop

[Lldb-commits] [PATCH] D72489: [DWARF] Emit DW_AT_call_return_pc as an address

dblaikie added a comment. Thanks for looking into this! Could you measure the size of the object files of, for example, the clang binary before/after this change - and, if possible, on Linux (where relocations are required to fixup these addresses)? I'm concerned this might increase the size s

[Lldb-commits] [PATCH] D72489: [DWARF] Emit DW_AT_call_return_pc as an address

dblaikie added a comment. I ran some size analysis (& hadn't realized that this special case was only done in the DWARFv5 codepath... that leads me to wonder about size impact in DWARFv4, so maybe I'll need to go back and run some general size analysis cost of these call sites). The .rela.debu

[Lldb-commits] [PATCH] D72489: [DWARF] Emit DW_AT_call_return_pc as an address

dblaikie accepted this revision. dblaikie added a comment. While the related design discussions continue - the patch itself is good/correct & there's nothing much to be done about the address pool size/relocations increase for now, for GDB at least, which is what I care about. Perhaps it's best

[Lldb-commits] [PATCH] D72624: [WIP] TargetMachine Hook for Module Metadata

dblaikie added a comment. (just a general comment that this code review should be only in service of the design discussion happening on llvm-dev - please don't approve/commit this without closing out the design discussion there if there are actionable conclusions from this review) Repository:

<    1   2   3