[Lldb-commits] [PATCH] D70932: [EditLine] Fix RecallHistory that makes it move in the opposite direction.

JDevlieghere created this revision. JDevlieghere added reviewers: jingham, labath, teemperor, clayborg. Herald added a subscriber: abidh. Herald added a project: LLDB. The naming used by editline for the history operations is counter intuitive to how it's used in lldb for the REPL. - The H_PREV

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

shafik added a comment. In D70846#1763731 , @labath wrote: > There's `lldb-shell :: SymbolFile/DWARF/find-basic-function.cpp`, which > probably didn't get run for you as you didn't have lld enabled (cmake > -DLLVM_ENABLE_PROJECTS=...;lld). You'll need

[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] D70885: [lldb] Use explicit lldb commands on tests

jingham added a comment. I don't think it is wise to use shortened command names like "br" in a test if you aren't testing shortest command string. We don't guarantee that we will keep all currently unique shortened command names unique in perpetuity... Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D70932: [EditLine] Fix RecallHistory that makes it move in the opposite direction.

JDevlieghere added a comment. (No test because this code is only used by the REPL) Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70932/new/ https://reviews.llvm.org/D70932 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D70934: [Reproducer] Add version check

JDevlieghere created this revision. JDevlieghere added a reviewer: labath. Herald added subscribers: teemperor, abidh. Herald added a project: LLDB. To ensure a reproducer works correctly, the version of LLDB used for capture and replay must match. Right now the reproducer already contains the

[Lldb-commits] [PATCH] D70722: [lldb/IRExecutionUnit] Stop searching based on demangled names

JDevlieghere added a comment. Seems reasonable. @teemperor wdyt? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70722/new/ https://reviews.llvm.org/D70722 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D70882: Add environment var LLDBVSCODE_SKIP_INIT_FILES to lldb-vscode

aadsm updated this revision to Diff 231807. aadsm added a comment. Put the logic into a function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70882/new/ https://reviews.llvm.org/D70882 Files:

[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

mstorsjo marked an inline comment as done. mstorsjo added inline comments. Comment at: lldb/source/Symbol/ObjectFile.cpp:480-486 + size_t ret = data.SetData(m_data, offset, length); + // DataExtractor::SetData copies the address byte size from m_data, but + // m_data's

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to use a vector

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Breakpoint/BreakpointList.h:71 /// \bfalse if the input name was not a legal breakpoint name. - bool FindBreakpointsByName(const char *name, BreakpointList _bps); + bool FindBreakpointsByName(const char

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to use a vector

JosephTremoulet created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Using a BreakpointList corrupts the breakpoints' IDs because BreakpointList::Add sets the ID, so use a vector instead. Note that, despite the similar name,

[Lldb-commits] [PATCH] D70885: [lldb] Use explicit lldb commands on tests

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. We shouldn't be running the test suite that allows your ~/.lldbinit file to be run. This can really hose up many things, so we should change the lldb-vscode test suite to not

[Lldb-commits] [PATCH] D70884: [lldb] Fix TestFormattersSBAPI test

clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: lldb/packages/Python/lldbsuite/test/python_api/formatters/TestFormattersSBAPI.py:71 -format.format = lldb.eFormatOctal +

[Lldb-commits] [PATCH] D70886: [lldb-vscode] capture the debuggers file handles before lldbinit runs

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Would be good to write a test for this where the init file does "script print('hello')" which would hose up the connection Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D70886: [lldb-vscode] capture the debuggers file handles before lldbinit runs

clayborg added inline comments. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1207-1210 + auto arguments = request.getObject("arguments"); + const auto skipInitFiles = GetBoolean(arguments, "skipInitFiles", false); + g_vsc.debugger = +

[Lldb-commits] [PATCH] D70882: Add skipInitFiles option to lldb-vscode initialize

clayborg added a comment. If MS VS Code IDE itself will never pass this to the "initialize" packet, then we might just want to use an env var instead? You should probably get rid of https://reviews.llvm.org/D70886 and just modify this patch? Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D70886: [lldb-vscode] capture the debuggers file handles before lldbinit runs

clayborg added a comment. Abandon this one and modify https://reviews.llvm.org/D70882? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70886/new/ https://reviews.llvm.org/D70886 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D70827: [lldb] Remove FileSpec->CompileUnit inheritance

JDevlieghere added a comment. Awesome, thank you! This has been on my todo-list for awhile :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70827/new/ https://reviews.llvm.org/D70827 ___ lldb-commits

[Lldb-commits] [PATCH] D70851: [lldb] s/FileSpec::Equal/FileSpec::Match

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM assuming the answer to my inline comment is negative :-) Comment at: lldb/include/lldb/Utility/FileSpec.h:196 static bool Equal(const FileSpec , const

[Lldb-commits] [PATCH] D70830: [LLDB] Disable MSVC warning C4190

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70830/new/ https://reviews.llvm.org/D70830 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D70906: [lldb] Move register info "augmentation" from gdb-remote into ABI

labath created this revision. labath added reviewers: jasonmolenda, clayborg, tatyana-krasnukha. Herald added a subscriber: aprantl. Herald added a project: LLDB. Previously the ABI plugin exposed some "register infos" and the gdb-remote code used those to fill in the missing bits. Now, the

[Lldb-commits] [PATCH] D70906: [lldb] Move register info "augmentation" from gdb-remote into ABI

clayborg accepted this revision. clayborg 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/D70906/new/ https://reviews.llvm.org/D70906

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

JDevlieghere added a comment. Personally I'd prefer moving the test cases, but maybe there's a good historical reason for the current layout. I'll ask around during out team meeting later today and report back. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

clayborg added a comment. So the main goal here is to let the Architecture plug-in handle the address masking in all places and make sure there is no code like: if (ArchSpec arch = m_objfile_sp->GetArchitecture()) { if (arch.GetTriple().getArch() == llvm::Triple::arm ||

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

clayborg added a comment. So some background on how address masks are handled in LLDB: Typically the way we have tried to take care of the extra Thumb bit for ARM using: lldb::addr_t Address::GetCallableLoadAddress(Target *target, bool is_indirect = false) const; lldb::addr_t

[Lldb-commits] [PATCH] D70887: [lldb] Use realpath to avoid issues with symlinks in source paths

clayborg added a comment. Might be a better idea to realpath this once in the setup code instead of calling realpath thousands and thousands of times? Maybe we just santize "LLDB_TEST" and "LLDB_BUILD" one time with top level code like: os.environ["LLDB_TEST"] =

[Lldb-commits] [PATCH] D70883: [vscode.py] Make read_packet only return None when EOF

clayborg added a comment. So what is this fix actually doing? Allowing random STDOUT to be ignored? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70883/new/ https://reviews.llvm.org/D70883 ___

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to use a vector

JosephTremoulet marked an inline comment as done. JosephTremoulet added inline comments. Comment at: lldb/include/lldb/Breakpoint/BreakpointList.h:71 /// \bfalse if the input name was not a legal breakpoint name. - bool FindBreakpointsByName(const char *name,

[Lldb-commits] [PATCH] D70851: [lldb] s/FileSpec::Equal/FileSpec::Match

clayborg added inline comments. Comment at: lldb/include/lldb/Utility/FileSpec.h:196 static bool Equal(const FileSpec , const FileSpec , bool full); JDevlieghere wrote: > Why do we still need the `Equal` method? Are there cases where `full` is only >

[Lldb-commits] [PATCH] D70882: Add skipInitFiles option to lldb-vscode initialize

aadsm planned changes to this revision. aadsm added a comment. Yes, I'll change this to env it makes more sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70882/new/ https://reviews.llvm.org/D70882

[Lldb-commits] [PATCH] D70886: [lldb-vscode] capture the debuggers file handles before lldbinit runs

aadsm added a comment. The way phabricator shows the diff is tricky, this change has nothing to do with D70882 and stands by itself. The important part here is making the `g_vsc.debugger.SetOutputFileHandle(out, true); g_vsc.debugger.SetErrorFileHandle(out,

[Lldb-commits] [PATCH] D70884: [lldb] Fix TestFormattersSBAPI test

aadsm marked an inline comment as done. aadsm added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/python_api/formatters/TestFormattersSBAPI.py:71 -format.format = lldb.eFormatOctal +format.SetFormat(lldb.eFormatOctal)

[Lldb-commits] [PATCH] D70885: [lldb] Use explicit lldb commands on tests

aadsm added a comment. Yeah, that's what I do in D70882 . I still think tests can be more robust by using explicit lldb command names but don't really care much :). I'm happy to abandon it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D70883: [vscode.py] Make read_packet only return None when EOF

aadsm added a comment. Yes. This happens when lldb-vscode is run in stdin/stdout mode (which is what happens in tests and I've also seen some IDEs doing the same) and installed commands are doing "print". In this case the client gets data that does not start with `Content-Length`. This fixes

[Lldb-commits] [PATCH] D70887: [lldb] Use realpath to avoid issues with symlinks in source paths

aadsm updated this revision to Diff 231740. aadsm added a comment. Solve this at the failing test level. We can't be sure of the filename will end up in the debug info so we just add both keys to the mapping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to use a vector

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Breakpoint/BreakpointList.h:71 /// \bfalse if the input name was not a legal breakpoint name. - bool FindBreakpointsByName(const char *name, BreakpointList _bps); + bool FindBreakpointsByName(const char

[Lldb-commits] [lldb] 160a504 - [lldb][NFC] Add 'breakpoint command list' test

Author: Raphael Isemann Date: 2019-12-02T11:57:55+01:00 New Revision: 160a5045c699ac523eac3c7a1984705c3e86720e URL: https://github.com/llvm/llvm-project/commit/160a5045c699ac523eac3c7a1984705c3e86720e DIFF:

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

labath added reviewers: JDevlieghere, aprantl. labath added a subscriber: dblaikie. labath added a comment. 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 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] D69273: ValueObject: Fix a crash related to children address type computation

labath added a comment. This seems to have fizzled out without any sort of a conclusion.. Did you guys do anything about the crashes you were seeing on the swift side? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69273/new/

[Lldb-commits] [PATCH] D69273: ValueObject: Fix a crash related to children address type computation

davide added a comment. We've been off all the past week. I'll circle back with Jim about this once I get to the office. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69273/new/ https://reviews.llvm.org/D69273

[Lldb-commits] [lldb] 4f728bf - [lldb][NFC] Use raw_ostream instead of Stream in Baton::GetDescription

Author: Raphael Isemann Date: 2019-12-02T13:27:21+01:00 New Revision: 4f728bfc13c45bc744bfdbfc3086bed74a8cbb4c URL: https://github.com/llvm/llvm-project/commit/4f728bfc13c45bc744bfdbfc3086bed74a8cbb4c DIFF:

[Lldb-commits] [lldb] f8fb372 - [lldb][NFC] Make Stream's IndentLevel an unsigned integers.

Author: Raphael Isemann Date: 2019-12-02T13:01:26+01:00 New Revision: f8fb3729e9d794f174aa737351235f76e6ac46db URL: https://github.com/llvm/llvm-project/commit/f8fb3729e9d794f174aa737351235f76e6ac46db DIFF:

[Lldb-commits] [lldb] d62026e - [lldb][NFC] Don't calculate member indices in DWARFASTParserClang::ParseChildMembers

Author: Raphael Isemann Date: 2019-12-02T14:43:40+01:00 New Revision: d62026e2dde1d27c7d1c702f11b0464e1d470d4f URL: https://github.com/llvm/llvm-project/commit/d62026e2dde1d27c7d1c702f11b0464e1d470d4f DIFF:

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

shafik added subscribers: clayborg, shafik. shafik added a comment. In D70846#1763802 , @teemperor wrote: > This LGTM, but the TODO directly above the change in > ClangExpressionDeclMap.cpp worries me a bit. I am not sure how good our test > coverage is

[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

clayborg added inline comments. Comment at: lldb/source/Symbol/ObjectFile.cpp:480-486 + size_t ret = data.SetData(m_data, offset, length); + // DataExtractor::SetData copies the address byte size from m_data, but + // m_data's address byte size is only set from sizeof(void*),

[Lldb-commits] [PATCH] D70847: [lldb-vscode] Ensure that target matches the executable file

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. I think the fix is better done to fix the root cause issue instead of working around it. I have suggested a fix in inline comments. Comment at:

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

JDevlieghere added a comment. No objections to moving the test. Jordan, let me know if you're up for it, otherwise I'm happy to take care of it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70622/new/ https://reviews.llvm.org/D70622

[Lldb-commits] [PATCH] D70884: [lldb] Fix TestFormattersSBAPI test

This revision was automatically updated to reflect the committed changes. Closed by commit rGafd5d912812e: [lldb] Fix TestFormattersSBAPI test (authored by aadsm, committed by António Afonso aa...@fb.com). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [lldb] 8f2c100 - [lldb/CMake] Add in_call_stack to the utilities package

Author: Jonas Devlieghere Date: 2019-12-02T13:03:24-08:00 New Revision: 8f2c100f6fa5250b987d11e53d24119bc1f8850e URL: https://github.com/llvm/llvm-project/commit/8f2c100f6fa5250b987d11e53d24119bc1f8850e DIFF:

[Lldb-commits] [PATCH] D70847: [lldb-vscode] Ensure that target matches the executable file

clayborg added a comment. The other fix would be to add a new LLDB API function: lldb::SBModule lldb::SBTarget::SetExecutable(const char *path, const char *triple, const char *symfile); Where we would pass in "nullptr" for "triple" and "symfile" from lldb-vscode and we call that. But I do

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

amccarth resigned from this revision. amccarth added a comment. I'm following along, but I don't think I have enough domain knowledge to qualify as a reviewer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70840/new/ https://reviews.llvm.org/D70840

[Lldb-commits] [PATCH] D70882: Add environment var LLDBVSCODE_SKIP_INIT_FILES to lldb-vscode

aadsm updated this revision to Diff 231759. aadsm added a comment. Use an env var instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70882/new/ https://reviews.llvm.org/D70882 Files:

[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

This revision was automatically updated to reflect the committed changes. Closed by commit rG7d019d1a3be2: [LLDB] Set the right address size on output DataExtractors from ObjectFile (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

mstorsjo marked an inline comment as done. mstorsjo added a comment. In D70840#1765219 , @labath wrote: > debug_aranges is a sort of a lookup table for speeding up address->compile > unit searches. llvm does not generate it by default since, and I think

[Lldb-commits] [PATCH] D70882: Add environment var LLDBVSCODE_SKIP_INIT_FILES to lldb-vscode

clayborg added inline comments. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:64 +const bool skip_init_files = getenv("LLDBVSCODE_SKIP_INIT_FILES") != NULL; + make singleton function: ``` bool GetSkipInitFiles() { static const bool skip_init =

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to use a vector

jingham added a comment. In other places, I've used "XList" to mean the container that manages the things contained, and "XCollection" to be a random possibly unrelated collection of items. It doesn't make any sense to have a collection of breakpoints from more than one target, so the

[Lldb-commits] [PATCH] D70906: [lldb] Move register info "augmentation" from gdb-remote into ABI

jasonmolenda accepted this revision. jasonmolenda added a comment. Good change, lgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70906/new/ https://reviews.llvm.org/D70906 ___ lldb-commits mailing