[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

dblaikie added a comment. Might be worth checking who wrote some of this/nearby code and whether they're still active in the community and see if they can review this - not sure if Pavel is around/has the bandwidth to review this. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D104380: [lldb] Set return object failed status even if error string is empty

dblaikie added a comment. In D104380#2826465 , @DavidSpickett wrote: > This change would have been part of https://reviews.llvm.org/D103701 if I had > realised that this was in fact how it worked. I separated it from the follow > ons because of that

[Lldb-commits] [PATCH] D104525: [lldb] Assert that CommandResultObject error messages are not empty

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sounds good, if this already passes all tests/etc (ie: there actually aren't any callers passing empty strings/non-failed errors/etc) Comment at:

[Lldb-commits] [PATCH] D104380: [lldb] Set return object failed status even if error string is empty

dblaikie added a comment. This might be a case of an overly reduced patch - this patch alone I'd expect to be observable in some way, and tested. If the "instring.empty()" condition never fires, then I'd expect to change that to an assert, rather than changing the semantics of it in an

[Lldb-commits] [PATCH] D103842: NFC: .clang-tidy: Inherit configs from parents to improve maintainability

This revision was automatically updated to reflect the committed changes. Closed by commit rGc5d56fec502f: NFC: .clang-tidy: Inherit configs from parents to improve maintainability (authored by dblaikie). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D103842: NFC: .clang-tidy: Inherit configs from parents to improve maintainability

dblaikie created this revision. dblaikie added reviewers: hokein, aaron.ballman, jyknight, mehdi_amini, kuhnel. Herald added subscribers: dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob,

[Lldb-commits] [PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

dblaikie added a comment. In D100581#2793775 , @ldionne wrote: > Hello! There are still some false positives, for example this one is breaking > the libc++ CI: >

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

dblaikie added a comment. In D101921#2786245 , @MaskRay wrote: > Because of `new MCObjectFileInfo`, we cannot use a forward declaration > (incomplete class) to replace `#include "llvm/MC/MCObjectFileInfo.h"` in > `TargetRegistry.h`. > > I thought about

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

dblaikie added a comment. In D102942#2791204 , @mbenfield wrote: > I also heard via email from Amara Emerson that the change is fine for similar > reasons. Great, thanks for checking! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sure, all sounds good - if you can, please reach out to the authors of any of the semantics changing changes (the ones related to the `Changed` values in transformations) to see if they

[Lldb-commits] [PATCH] D102851: [lldb] Improve invalid DWARF DW_AT_ranges error reporting

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Yeah, looks pretty good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102851/new/ https://reviews.llvm.org/D102851

[Lldb-commits] [PATCH] D102851: [lldb] Improve invalid DWARF DW_AT_ranges error reporting

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good to me - at least includes more values from the file to point to the problem. Though phrasing might be better (I guess some other code prints the "DW_AT_ranges(0x0) attribute"

[Lldb-commits] [PATCH] D102630: [lit] Stop using PATH to lookup clang/lld/lldb unless requested

dblaikie added a comment. @thopre - as an aside: It'd be helpful if you could include some text in the text box when marking something "approved" through phabricator. There's a bug/limitation that approvals without text don't produce email to the mailing list - so it looks like a patch hasn't

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sounds good to me if - if the build (of ~everything in the monorepo) is clean - and do please keep an eye on the buildbots when this is committed as there's likely to be cleanup here and

[Lldb-commits] [PATCH] D102092: [lldb] Enable -Wmisleading-indentation

dblaikie added a comment. might as well enable it for LLVM more generally? (though probably lldb specifically, and lots of llvm more generally, would need cleanup before this is enabled? Have you checked if lldb or other parts of llvm build cleanly with this warning enabled?) Repository:

[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

dblaikie added a comment. In D101237#2728726 , @teemperor wrote: >> (and it could tell clang exactly how large the structure is too - from the >> DWARF) > > We are actually doing that to my knowledge and return the `DW_AT_byte_size` > value for the

[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

dblaikie added a comment. In D101237#2718456 , @shafik wrote: > I think @dblaikie original idea of adding a DWARF attribute for this case is > the right way to go here. AFAICT this will change the answer to basic > questions such as what size a

[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

dblaikie added a comment. In D101237#2714962 , @jankratochvil wrote: > In D101237#2714953 , @dblaikie > wrote: > >> Given that no_unique_address changes the layout of data structures ( >>

[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

dblaikie added a subscriber: aprantl. dblaikie added a comment. > As DWARF does not contain the [[no_unique_address]] attribute the patch adds > it to every record member, could it be a problem? Given that no_unique_address changes the layout of data structures (

[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

dblaikie added a comment. If this is going to be plumbed through everywhere (ie: a DWARFDIE is mostly useless without both its lexical DWARFUnit and its imported-into DWARFUnit) then maybe it makes more sense to add this DWARFUnit* to the DWARFBaseDie type alongside the lexical DWARFUnit?

[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

dblaikie added inline comments. Comment at: lldb/source/Core/RichManglingContext.cpp:23 +RichManglingContext::~RichManglingContext() { + std::free(m_ipd_buf); + ResetCxxMethodParser(); JDevlieghere wrote: > shafik wrote: > > rupprecht wrote: > > > JDevlieghere

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

dblaikie added inline comments. Comment at: lldb/tools/driver/Driver.cpp:856 if (const char *reproducer_path = SBReproducer::GetPath()) { -// Leaking the string on purpose. -std::string *finalize_cmd = new std::string(argv0); +static std::string

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

dblaikie added inline comments. Comment at: lldb/tools/driver/Driver.cpp:856 if (const char *reproducer_path = SBReproducer::GetPath()) { -// Leaking the string on purpose. -std::string *finalize_cmd = new std::string(argv0); +static std::string

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

dblaikie added a comment. In D100191#2689543 , @mgorny wrote: > In D100191#2689489 , @labath wrote: > >> We should also start thinking about tests. I suppose the smallest piece of >> functionality that could be

[Lldb-commits] [PATCH] D100447: [lldb] Silence GCC warnings about control reaching the end of non-void functions. NFC.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good - thanks! Do you need me to commit this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100447/new/

[Lldb-commits] [PATCH] D100447: [lldb] Silence GCC warnings about control reaching the end of non-void functions. NFC.

dblaikie added a comment. I'm guessing this change will break the build in a different way, triggering clang's -Wcovered-switch-default - the usual way this gcc/clang diagnostic pinch is resolved is by putting the unreachable after the switch/at the end of the function, instead of in a default

[Lldb-commits] [PATCH] D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

dblaikie added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s:11 +# Failure was the block range 1..2 was not printed plus: +# error: DW_AT_range-DW_FORM_sec_offset.s.tmp {0x003f}: DIE has DW_AT_ranges(0xc) attribute, but range

[Lldb-commits] [PATCH] D99702: [lldb-vscode] Use LLVM's ScopeExit to ensure we always terminate the debugger

dblaikie added a comment. @wallace - minor inconvenience, but approving a patch in Phabricator without any textual comment does not result in an email to the list, making it look like a patch is being committed without approval - could you include some text in your Phab approvals, as it makes

[Lldb-commits] [PATCH] D99401: Fix .debug_aranges parsing issues introduced with llvm error handling in LLDB

dblaikie added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp:56 + if (m_header.length > 0) +m_next_offset = m_offset + *offset_ptr - m_offset + m_header.length; + else clayborg wrote: > dblaikie wrote: > > This

[Lldb-commits] [PATCH] D99401: Fix .debug_aranges parsing issues introduced with llvm error handling in LLDB

dblaikie added a comment. (generally makes sense to me - but I'll leave it to some more lldb-focussed reviewers to do more review/approval) Usual caveat/question: Does this take us closer or further away from unifying this code with LLVM's libDebugInfoDWARF? (or neutral)

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

dblaikie added inline comments. Comment at: lldb/source/API/SBDebugger.cpp:857 +error = m_opaque_sp->GetTargetList().CreateTarget( +*m_opaque_sp, filename, arch, eLoadDependentsYes, platform_sp, +target_sp); clayborg wrote: >

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

dblaikie added inline comments. Comment at: lldb/source/API/SBDebugger.cpp:857 +error = m_opaque_sp->GetTargetList().CreateTarget( +*m_opaque_sp, filename, arch, eLoadDependentsYes, platform_sp, +target_sp); jingham wrote: >

[Lldb-commits] [PATCH] D99120: [lldb] Silence GCC warnings about format not being a string literal in LLDB_SCOPED_TIMER

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. Looks right to me Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99120/new/

[Lldb-commits] [PATCH] D98996: Teach DWARFExpression about DWARF 4+ Location Descriptions

dblaikie added a comment. (idle question: Is this code remotely related to any code in LLVM's libDebugInfoDWARF? Does this patch take us closer to or further away from unifying them? If it takes us further away, any chance of designing it in a direction that points towards each other rather

[Lldb-commits] [PATCH] D86110: [WIP][DebugInfo] Lazily parse debug_loclist offsets

dblaikie added inline comments. Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFListTable.h:116 + Optional getOffsetEntry(DataExtractor Data, uint32_t Index) const { +if (Index > HeaderData.OffsetEntryCount) + return None; jankratochvil wrote: >

[Lldb-commits] [PATCH] D98289: [lldb] Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

dblaikie added inline comments. Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFListTable.h:116 Optional getOffsetEntry(DataExtractor Data, uint32_t Index) const { -if (Index > HeaderData.OffsetEntryCount) +if (Index >= HeaderData.OffsetEntryCount) return

[Lldb-commits] [PATCH] D98289: [lldb] Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

dblaikie added a comment. In D98289#2618881 , @jankratochvil wrote: > Ah there is already a discussion from it: > http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2021-March/004765.html Yeah, happy to discuss/clarify/help with anything in

[Lldb-commits] [PATCH] D96778: [lldb] Fix handling of `DW_AT_decl_file` according to D91014

dblaikie added a comment. In D96778#2576980 , @jankratochvil wrote: > In D96778#2576927 , @dblaikie wrote: > >> What part of DWZ is specified by DWARFv5? > > Only `DW_*_sup` - being used by DWZ with DWARF-4 as

[Lldb-commits] [PATCH] D96778: [lldb] Fix handling of `DW_AT_decl_file` according to D91014

dblaikie added a comment. In D96778#2576913 , @jankratochvil wrote: > In D96778#2576881 , @dblaikie wrote: > >> In D96778#2573076 , @jankratochvil >> wrote: >> >>>

[Lldb-commits] [PATCH] D96778: [lldb] Fix handling of `DW_AT_decl_file` according to D91014

dblaikie added a comment. In D96778#2573076 , @jankratochvil wrote: > In D96778#2572816 , @dblaikie wrote: > >> rolls this into one file with two CUs - bit easier to deal with. > > Then one could not use the

[Lldb-commits] [PATCH] D96778: [lldb] Fix handling of `DW_AT_decl_file` according to D91014

dblaikie added a comment. In D96778#2572405 , @jankratochvil wrote: > In D96778#2566208 , @dblaikie wrote: > >> I expect it'd be good to have a test case showing the sort of DWARF that DWZ >> produces for

[Lldb-commits] [PATCH] D96817: Fix deep copying for OptionValue classes

dblaikie added a comment. In D96817#2569852 , @clayborg wrote: > In D96817#2569595 , @dblaikie wrote: > >>> CRTP was my first implementation, however, I discarded it as more >>> bug-prone. Virtual Clone function

[Lldb-commits] [PATCH] D96778: [lldb] Fix handling of `DW_AT_decl_file` according to D91014

dblaikie added a comment. In D96778#2568794 , @werat wrote: > To be honest, I'm not sure how to reproduce this kind of debug info. I've > tried a few examples (e.g. force inline the function from another CU) , but > it didn't work. > > Should we

[Lldb-commits] [PATCH] D96817: Fix deep copying for OptionValue classes

dblaikie added a comment. > CRTP was my first implementation, however, I discarded it as more bug-prone. > Virtual Clone function at the interface is so common that, I believe, > everyone knows it must be overridden by a new derived class. The necessity of > inheriting from base_clone_helper

[Lldb-commits] [PATCH] D96778: [lldb] Fix handling of `DW_AT_decl_file` according to D91014

dblaikie added a comment. In D96778#2567255 , @jankratochvil wrote: > In D96778#2566208 , @dblaikie wrote: > >> I expect it'd be good to have a test case showing the sort of DWARF that DWZ >> produces for

[Lldb-commits] [PATCH] D96817: Fix deep copying for OptionValue classes

dblaikie added inline comments. Comment at: lldb/unittests/Interpreter/TestOptionValue.cpp:173 + // Trigger the callback second time. + file_list_copy_ptr->SetValueFromString(llvm::StringRef("0 another/path"), + eVarSetOperationReplace);

[Lldb-commits] [PATCH] D96817: Fix deep copying for OptionValue classes

dblaikie added a comment. Perhaps all these clone implementations could be provided via mixin instead? struct base { virtual std::shared_ptr Clone() = 0; ... }; template struct base_clone_helper : IntermediateBase { static_assert(std::is_base_of::value); std::shared_ptr

[Lldb-commits] [PATCH] D96778: [lldb] Fix handling of `DW_AT_decl_file` according to D91014

dblaikie added a comment. In D96778#2565677 , @jankratochvil wrote: > In D96778#2565414 , @werat wrote: > >> I can't claim I fully understand what's the difference here, but this aligns >> with your comment at

[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] 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

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-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

[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

[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

[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] 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

[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] 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

[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-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

[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-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

[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

[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] 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,

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

dblaikie added inline comments. Comment at: lldb/include/lldb/Core/ModuleList.h:71 +: public llvm::iterator_facade_base< + ModuleIterator, std::bidirectional_iterator_tag, lldb::ModuleSP> { +public: On the fence, but this could be a random access

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

dblaikie added inline comments. Comment at: lldb/include/lldb/Core/ModuleList.h:70 +class ModuleIterator +: public std::iterator { +public: FWIW, std::iterator is deprecated since C++17 - probably best not to add new uses of it. (I think the idea is that

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

dblaikie created this revision. dblaikie added a reviewer: labath. dblaikie requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Finishing out the support (to the best of my knowledge/based on current testing running the whole check-lldb with a

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

dblaikie created this revision. dblaikie added a reviewer: labath. dblaikie requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: lldb-commits, sstefan1. Herald added a project: LLDB. gcc already produces debug info with this form

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

dblaikie added a comment. In D91734#2443231 , @probinson wrote: > In D91734#2432988 , @dblaikie wrote: > >> Yeah, it boils down to something like this: >> >> void f1(const char*, const char*) { } >> int main()

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

dblaikie added a comment. Also, FWIW, the other gdb test suite failures we saw were: FAIL: gdb.base/foll-exec.exp: step through execlp call FAIL: gdb.base/foll-exec.exp: step after execlp call FAIL: gdb.base/foll-exec.exp: print execd-program/global_i (after execlp) FAIL:

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

dblaikie added a comment. In D91734#2432188 , @probinson wrote: > See D92606 for a front-end patch to improve > locations in the IR. > That, plus reapplying this patch, should help out GDB. I haven't had a > chance to

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

dblaikie added a comment. In D91734#2431247 , @probinson wrote: >> Sometihng like this seems plausible to me: > > Yes, I was playing with essentially that exact patch last night. It has no > effect on the final assembly on its own, but combined with my

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

dblaikie added a comment. Sometihng like this seems plausible to me: $ git diff diff --git clang/lib/CodeGen/CGExprScalar.cpp clang/lib/CodeGen/CGExprScalar.cpp index c906af8a4afa..c85ce46508a6 100644 --- clang/lib/CodeGen/CGExprScalar.cpp +++ clang/lib/CodeGen/CGExprScalar.cpp @@

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

dblaikie added a comment. In D91734#2426897 , @rnk wrote: > I see. We should give that constant materialization a location. Yeah, likely - if this patch makes constant materialization local to the IR instruction - if there's a reasonable location the IR

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

dblaikie added a comment. This causes several regressions in the gdb test suite we're running internally - the failures are: FAIL: gdb.base/break.exp: breakpoint at start of multi line while conditional FAIL: gdb.base/break.exp: breakpoint info FAIL: gdb.base/foll-exec.exp: step through

[Lldb-commits] [PATCH] D90840: [lldb/DWARF] Fix sizes of DW_OP_const[1248][us] and DW_OP_litN results

dblaikie accepted this revision. dblaikie added a comment. Sounds good Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90840/new/ https://reviews.llvm.org/D90840 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D90840: [lldb/DWARF] Fix sizes of DW_OP_const[1248][us] and DW_OP_litN results

dblaikie added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:944 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS)); + auto to_generic = [addr_size = opcodes.GetAddressByteSize()](auto v) { +bool is_signed =

[Lldb-commits] [PATCH] D90598: [lldb/DWARF] Fix implementation of DW_OP_convert

dblaikie added a comment. In D90598#2368020 , @labath wrote: > Judging by http://lists.llvm.org/pipermail/llvm-dev/2020-October/145990.html, > dsymutil has a similar bug... That's my understanding. It means I'm not sure (though I'm not the expert here

[Lldb-commits] [PATCH] D86110: [WIP][DebugInfo] Lazily parse debug_loclist offsets

dblaikie added a comment. In D86110#2225970 , @labath wrote: > In D86110#2224163 , @dblaikie wrote: > >> In D86110#2223905 , @labath wrote: >> >>> This sounds perfectly

[Lldb-commits] [PATCH] D86110: [WIP][DebugInfo] Lazily parse debug_loclist offsets

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf7a49d2aa691: [WIP][DebugInfo] Lazily parse debug_loclist offsets (authored by dblaikie). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D86110: [WIP][DebugInfo] Lazily parse debug_loclist offsets

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In D86110#2223905 , @labath wrote: > This sounds perfectly reasonable to me. The offsets are present in memory > already so there's no point in

[Lldb-commits] [PATCH] D86110: [WIP][DebugInfo] Lazily parse debug_loclist offsets

dblaikie created this revision. dblaikie added a reviewer: labath. Herald added subscribers: llvm-commits, lldb-commits, hiraditya, aprantl. Herald added projects: LLDB, LLVM. dblaikie requested review of this revision. Herald added a subscriber: JDevlieghere. Parsing DWARFv5 debug_loclist

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

dblaikie added a subscriber: echristo. dblaikie added a comment. In D79147#2178104 , @russell.gallop wrote: > Hi, > > It looks like is causing one of the debuginfo-tests: > llgdb-tests/nrvo-string.cpp to fail, run on Linux. Failure as below. I don't >

[Lldb-commits] [PATCH] D84402: [lldb/DWARF] Add more line table validation

dblaikie added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1044 for (const llvm::DWARFDebugLine::Sequence : line_table->Sequences) { +if (!list.ContainsFileAddressRange(seq.LowPC, seq.HighPC - seq.LowPC)) + continue;

[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

[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] 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: > >

[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] 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

[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] 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

[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] 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

[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] D77141: [DebugInfo] Rename section identifiers which are deprecated in DWARFv5. NFC.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Sure - looks good CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77141/new/ https://reviews.llvm.org/D77141 ___ lldb-commits

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

dblaikie accepted this revision. dblaikie added a comment. In D75929#1954443 , @labath wrote: > Thanks for doing this. This looks fine to me. @dblaikie, @jhenderson, do you > have any additional comments? nah, nothing dire that comes to mind - thanks

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

dblaikie added inline comments. Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:47-48 +// For pre-standard ones, which correspond to sections being deprecated in +// DWARFv5, the values are chosen more or less arbitrary and a tag "_EXT_" +// is added to the names.

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

dblaikie added inline comments. Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h:47-48 +// For pre-standard ones, which correspond to sections being deprecated in +// DWARFv5, the values are chosen more or less arbitrary and a tag "_EXT_" +// is added to the names.

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

dblaikie added a comment. Side note: This has become complicated enough it might be worth separating these patches now rather than later - changes to dumping should be separate from changes to llvm-dwp, for instance. In D75929#1926834 , @labath wrote:

<    1   2   3   >