[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

kwk added a comment. @stella.stamenova I've reverted this patch and thanks for letting me know. @labath I didn't notice that tests failed, I'm really sorry. @labath how shall we go about this? We do have the situation that when you lookup a symbol you might find it twice if it is in `.dynsym`

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

kwk added a comment. @MaskRay I've addressed your comments. Comment at: lldb/source/Host/common/LZMA.cpp:129 + uint64_t uncompressedSize = 0; + auto err = getUncompressedSize(InputBuffer, uncompressedSize); + if (err) labath wrote: > MaskRay wrote: > > if

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

kwk updated this revision to Diff 219848. kwk marked 5 inline comments as done. kwk added a comment. - Addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66791/new/ https://reviews.llvm.org/D66791 Files:

[Lldb-commits] [PATCH] D67474: [Reproducer] Add `reproducer dump` command

jingham added a comment. If I've run lldb like: $ lldb -capture-path and then I want to dump the reproducer I've loaded, it seems annoying to have to say: (lldb) reproducer dump -f Why do I need to type it again? So it seems like you could make -f optional, and if it isn't provided use

[Lldb-commits] [PATCH] D67474: [Reproducer] Add `reproducer dump` command

JDevlieghere created this revision. JDevlieghere added reviewers: friss, jingham, labath, LLDB. JDevlieghere added a project: LLDB. Herald added subscribers: llvm-commits, teemperor, hiraditya. Herald added a project: LLVM. This adds a `reproducer dump` commands which makes it possible to inspect

[Lldb-commits] [lldb] r371688 - [NFC] Reformat SBDebugger before making changes

Author: jdevlieghere Date: Wed Sep 11 16:27:09 2019 New Revision: 371688 URL: http://llvm.org/viewvc/llvm-project?rev=371688=rev Log: [NFC] Reformat SBDebugger before making changes Modified: lldb/trunk/source/API/SBDebugger.cpp Modified: lldb/trunk/source/API/SBDebugger.cpp URL:

[Lldb-commits] [lldb] r371689 - [Reproducer] Move the command loader into the reproducer (NFC)

Author: jdevlieghere Date: Wed Sep 11 16:27:12 2019 New Revision: 371689 URL: http://llvm.org/viewvc/llvm-project?rev=371689=rev Log: [Reproducer] Move the command loader into the reproducer (NFC) This just moves the CommandLoader utility into the reproducer namespace and makes it accessible

[Lldb-commits] [PATCH] D67472: [Target] Move InferiorCall to Process

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. I was going to say LGTM unless Jim has concerns, but looks like he cut me to the chase :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D67472: [Target] Move InferiorCall to Process

jingham added a comment. This seems like the right place to put it, it's clearly something you would do with a process. I don't see any reason why this should be a private function. It's just a convenience wrapper around a bunch of other public functionality, and doesn't do anything that

[Lldb-commits] [lldb] r371685 - [Reproducer] Move GDB Remote Provider into Reproducer (NFC)

Author: jdevlieghere Date: Wed Sep 11 16:15:12 2019 New Revision: 371685 URL: http://llvm.org/viewvc/llvm-project?rev=371685=rev Log: [Reproducer] Move GDB Remote Provider into Reproducer (NFC) Originally the idea was for providers to be defined close to where they are used. While this helped

[Lldb-commits] [PATCH] D67472: [Target] Move InferiorCall to Process

xiaobai created this revision. xiaobai added reviewers: JDevlieghere, clayborg, compnerd, labath. Herald added a project: LLDB. InferiorCall is only ever used in Process, and it is not specific to POSIX. By moving it to Process, we can remove all dependencies on plugins from Process. Moving

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

This revision was automatically updated to reflect the committed changes. Closed by commit rL371668: [DWARF] Evaluate DW_OP_entry_value (authored by vedantk, committed by ). Changed prior to commit: https://reviews.llvm.org/D67376?vs=219779=219804#toc Repository: rL LLVM CHANGES SINCE LAST

[Lldb-commits] [lldb] r371667 - [Status] Add a LLDB_ERRORF macro for error reporting (similar to LLDB_LOGF)

Author: vedantk Date: Wed Sep 11 14:23:42 2019 New Revision: 371667 URL: http://llvm.org/viewvc/llvm-project?rev=371667=rev Log: [Status] Add a LLDB_ERRORF macro for error reporting (similar to LLDB_LOGF) Modified: lldb/trunk/include/lldb/Utility/Status.h Modified:

[Lldb-commits] [lldb] r371668 - [DWARF] Evaluate DW_OP_entry_value

Author: vedantk Date: Wed Sep 11 14:23:45 2019 New Revision: 371668 URL: http://llvm.org/viewvc/llvm-project?rev=371668=rev Log: [DWARF] Evaluate DW_OP_entry_value Add support for evaluating DW_OP_entry_value. This involves parsing DW_TAG_call_site_parameter and wiring the information through to

[Lldb-commits] [PATCH] D67427: [Plugins/Process] Remove direct use of ClangASTContext from InferiorCallPOSIX

This revision was automatically updated to reflect the committed changes. Closed by commit rL371654: [Plugins/Process] Remove direct use of ClangASTContext from InferiorCallPOSIX (authored by xiaobai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[Lldb-commits] [lldb] r371654 - [Plugins/Process] Remove direct use of ClangASTContext from InferiorCallPOSIX

Author: xiaobai Date: Wed Sep 11 13:36:28 2019 New Revision: 371654 URL: http://llvm.org/viewvc/llvm-project?rev=371654=rev Log: [Plugins/Process] Remove direct use of ClangASTContext from InferiorCallPOSIX Summary: InferiorCallPOSIX directly grabs a ClangASTContext from the Target it has and

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks, that's exactly what I was looking for! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67376/new/ https://reviews.llvm.org/D67376

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:258 + +using CallSiteParameterArray = std::unique_ptr>; + aprantl wrote: > vsk wrote: > > vsk wrote: > > > aprantl wrote: > > > > vsk wrote: > > > > > grandinj wrote: > > > > > > the way

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

amccarth added a comment. I've been trying to figure out how to implement the same functionality you've got here, so I'm very interested in helping you land this in some form. I think Clayborg and Pavel makes some good points about where the right layer of abstraction is for the unwind plans.

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

aprantl added a comment. Otherwise this LGTM now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67111/new/ https://reviews.llvm.org/D67111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

aprantl added inline comments. Comment at: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp:196 + +auto contains_lambda_identifier = []( llvm::StringRef & str_ref ) { +return str_ref.contains("$_") || str_ref.contains("'lambda'");

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

aprantl added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:497 + + finish_subexpressions_to(end_offset); } labath wrote: > aprantl wrote: > > We should probably just let llvm's libDebugInfo do the printing here, but > > this works

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

aprantl added a comment. Looking great! I have one more question inline, but that is mostly about documenting the algorithm. Comment at: lldb/include/lldb/Symbol/Function.h:258 + +using CallSiteParameterArray = std::unique_ptr>; + vsk wrote: > vsk wrote: > >

[Lldb-commits] [PATCH] D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols

friss accepted this revision. friss added a comment. This revision is now accepted and ready to land. This LGTM, in principle, but it's weird to add the testing in a skipped test. I trust that it works on your machine, but we won't know if it works on all the tested configurations. Can

[Lldb-commits] [lldb] r371625 - Revert "[LLDB][ELF] Load both, .symtab and .dynsym sections"

Author: kwk Date: Wed Sep 11 07:33:37 2019 New Revision: 371625 URL: http://llvm.org/viewvc/llvm-project?rev=371625=rev Log: Revert "[LLDB][ELF] Load both, .symtab and .dynsym sections" This reverts commit 3a4781bbf4f39a25562b4c61c9a9ab2483a96b41. Removed:

[Lldb-commits] [lldb] r371623 - [lldb][NFC] Make include directories in Clang expression parser a std::string

Author: teemperor Date: Wed Sep 11 07:33:11 2019 New Revision: 371623 URL: http://llvm.org/viewvc/llvm-project?rev=371623=rev Log: [lldb][NFC] Make include directories in Clang expression parser a std::string We never compare these directories (where ConstString would be good) and essentially

[Lldb-commits] [lldb] r371624 - Revert "[LLDB][ELF] Fixup for comments in D67390"

Author: kwk Date: Wed Sep 11 07:33:21 2019 New Revision: 371624 URL: http://llvm.org/viewvc/llvm-project?rev=371624=rev Log: Revert "[LLDB][ELF] Fixup for comments in D67390" This reverts commit 813f05915d29904878d926f9849ca3dbe78096af. Modified:

[Lldb-commits] [lldb] r371618 - [lldb][NFC] Remove dead code in SBAddress::GetDescription

Author: teemperor Date: Wed Sep 11 06:57:41 2019 New Revision: 371618 URL: http://llvm.org/viewvc/llvm-project?rev=371618=rev Log: [lldb][NFC] Remove dead code in SBAddress::GetDescription Modified: lldb/trunk/source/API/SBAddress.cpp Modified: lldb/trunk/source/API/SBAddress.cpp URL:

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This looks great now. Thank you for your patience. Just make sure to deal with the breakages from the other patch first. If the fix is non trivial, or if it's going to take a while, then

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

labath added inline comments. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2668-2671 + if (!m_symtab_up) { +auto sec = symtab ? symtab : dynsym; +m_symtab_up.reset(new Symtab(sec->GetObjectFile())); + } kwk wrote: >

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

labath added a comment. It broke on linux too. You did run the tests before committing, did you? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67390/new/ https://reviews.llvm.org/D67390 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D67376: [DWARF] Evaluate DW_OP_entry_value

labath added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:497 + + finish_subexpressions_to(end_offset); } aprantl wrote: > We should probably just let llvm's libDebugInfo do the printing here, but > this works fine for now. FYI: I

[Lldb-commits] [lldb] r371614 - [lldb][NFC] Sort files in unittests/Expression/CMakeLists.txt

Author: teemperor Date: Wed Sep 11 05:57:49 2019 New Revision: 371614 URL: http://llvm.org/viewvc/llvm-project?rev=371614=rev Log: [lldb][NFC] Sort files in unittests/Expression/CMakeLists.txt Modified: lldb/trunk/unittests/Expression/CMakeLists.txt Modified:

[Lldb-commits] [PATCH] D67369: Implement DW_OP_convert

labath added a comment. The yaml object file is pretty impressive, but I am wondering if it is really necessary. Have you looked if it would be possible to reuse the existing `TestFile::fromYaml` functionality? I think it should be possible to reuse that by just inserting some MachO yaml

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

stella.stamenova added a comment. This broke the windows bot: http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/8741 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67390/new/ https://reviews.llvm.org/D67390

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

labath added a comment. In D67347#1664640 , @aleksandr.urakov wrote: > It's a good point, thank you! I had the same thoughts when I done it, but I'm > not completely sure. The problem is that an object file can't be completely > responsible for

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

kwk updated this revision to Diff 219680. kwk added a comment. Rebased on master to include changes from D67390 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66791/new/ https://reviews.llvm.org/D66791 Files:

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

kwk marked 2 inline comments as done. kwk added inline comments. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2711-2731 Section *symtab = section_list->FindSectionByType(eSectionTypeELFSymbolTable, true).get(); -if (!symtab) { - // The

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

kwk marked 3 inline comments as done. kwk added a comment. @labath I've addressed your comment rewrites in a fixup commit that I've commited without a review (llvm-svn: 371600): https://reviews.llvm.org/rLLDB371600 Comment at:

[Lldb-commits] [lldb] r371600 - [LLDB][ELF] Fixup for comments in D67390

Author: kwk Date: Wed Sep 11 03:12:36 2019 New Revision: 371600 URL: http://llvm.org/viewvc/llvm-project?rev=371600=rev Log: [LLDB][ELF] Fixup for comments in D67390 Modified: lldb/trunk/lit/Modules/ELF/load-from-dynsym-alone.test lldb/trunk/lit/Modules/ELF/load-symtab-and-dynsym.test

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

This revision was automatically updated to reflect the committed changes. Closed by commit rL371599: [LLDB][ELF] Load both, .symtab and .dynsym sections (authored by kwk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

kwk updated this revision to Diff 219677. kwk added a comment. Updated commit message and squashed commits into one rebased onto master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67390/new/ https://reviews.llvm.org/D67390 Files:

[Lldb-commits] [lldb] r371599 - [LLDB][ELF] Load both, .symtab and .dynsym sections

Author: kwk Date: Wed Sep 11 03:00:30 2019 New Revision: 371599 URL: http://llvm.org/viewvc/llvm-project?rev=371599=rev Log: [LLDB][ELF] Load both, .symtab and .dynsym sections Summary: This change ensures that the .dynsym section will be parsed even when there's already is a .symtab. It is

[Lldb-commits] [PATCH] D67022: Enhance SymbolFileDWARF::ParseDeclsForContext performance

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. looks fine to me Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67022/new/ https://reviews.llvm.org/D67022

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

labath accepted this revision. labath added a comment. lgtm. The reason I requested this to be put separately, is because it is implemented in a way that kicks in even without minidebuginfo. I think this is fine, because entries can be removed from the symtab even without the whole

[Lldb-commits] [PATCH] D67230: Remove call to obsolete gethostbyname, using getaddrinfo

davezarzycki added a comment. Great. See: r371596 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67230/new/ https://reviews.llvm.org/D67230 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] r371596 - [LLDB] Do not try to canonicalize gethostname() result

Author: davezarzycki Date: Wed Sep 11 01:32:37 2019 New Revision: 371596 URL: http://llvm.org/viewvc/llvm-project?rev=371596=rev Log: [LLDB] Do not try to canonicalize gethostname() result This code is trying too hard and failing. Either the result of gethostname() is canonical or it is not. If

[Lldb-commits] [PATCH] D66654: 2/2: Process formatters in reverse-chronological order

labath accepted this revision. labath added a comment. I think this looks great. Some operations may get less efficient with this, but OTOH, some will get actually faster. And it is definitely the most understandable solution out of everything we had so far. Comment at:

[Lldb-commits] [PATCH] D67230: Remove call to obsolete gethostbyname, using getaddrinfo

labath added a comment. In D67230#1661120 , @serge-sans-paille wrote: > @davezarzycki I tend to agree with you. We can just remove 35:44 and just > keep the hostname. sounds good to me Repository: rL LLVM CHANGES SINCE LAST ACTION