[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81001#2179690 , @gedatsu217 wrote: > For example, I execute "help frame variable" and save it as command history. > Then, when I type "hel", "helhelp [me variable]" (gray characters are in []) > is displayed, probably because

[Lldb-commits] [PATCH] D84815: [LLDB] Improve PDB discovery

2020-07-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The idea is fine -- the same logic is used in `Symbols::LocateExecutableSymbolFile`. In fact, one of the problems of our pdb support is that it does not use that function to locate symbol files (that way it would also respect the `target.debug-file-search-paths`

[Lldb-commits] [PATCH] D84809: [lldb] Fix invalid error message in TargetList::CreateTargetInternal

2020-07-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The changes don't seem contentious to me, though I am also not very familiar with this code. What surprised me was the `OptionGroupPlatform` argument to `TargetList::CreateTargetInternal`. It seems like a slighly unusual thing to have, as that means the equivalent SB

[Lldb-commits] [PATCH] D84748: [cmake] Make gtest macro definitions a part the library interface

2020-07-28 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: ldionne, logan-5, beanz. Herald added subscribers: lldb-commits, dexonsmith, hiraditya, mgorny. Herald added projects: LLDB, LLVM. labath requested review of this revision. These definitions are needed by any file which uses gtest. Previously

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks. When trying this out I did notice one subtle bug related to backspacing. When backspacing over the first character, the command line does not get redrawn, which means the autosuggested text remains on screen. The "deleted" character also remains, and is printed

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81001#2176076 , @gedatsu217 wrote: > > Could you create a patch to change the definition of ANSI_UNFAINT ? (might > > be worth taking a quick look at git history if there is no good reason for > > why it uses the color code

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

2020-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath 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

[Lldb-commits] [PATCH] D83302: [lldb/DWARF] Don't treat class declarations with children as definitions

2020-07-27 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1956cf1042d3: [lldb/DWARF] Dont treat class declarations with children as definitions (authored by labath). Changed prior to commit: https://reviews.llvm.org/D83302?vs=276048=280847#toc Repository:

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81001#2172658 , @gedatsu217 wrote: > I do not intend for this feature to work with colors disabled. Right in that case, we should probably disable the feature if colors are disabled. > I found that pexpect output the below

[Lldb-commits] [PATCH] D79699: Add ptrace register access for AArch64 SVE registers

2020-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h:22 +#define INCLUDE_LINUX_PTRACE_DEFINITIONS_FOR_SVE_ARM64 +#include "Plugins/Process/Utility/LinuxPTraceDefines_arm64sve.h" +#endif omjavaid wrote: >

[Lldb-commits] [PATCH] D84126: Enable -Wsuggest-override in the LLVM build

2020-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Instead of adding -Wno-suggest-override to every user it might be cleaner to add the flag to the INTERFACE_COMPILE_OPTIONS property of the relevant targets (gtest, gmock, and gbenchmark ?). That way, anything which links against these will inherit it automatically.

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81001#2172035 , @labath wrote: > In D81001#2171999 , @gedatsu217 > wrote: > > > > That said, are you sure this is the right sequence? \x1b[1Gl seems like > > > it should print the l at

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81001#2171999 , @gedatsu217 wrote: > > That said, are you sure this is the right sequence? \x1b[1Gl seems like it > > should print the l at column one, which does not sound right... > > I thought it did not add up too, but

[Lldb-commits] [PATCH] D84401: [lldb] Add SectionList::ContainsFileAddressRange

2020-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath planned changes to this revision. labath added a comment. Waiting for the resolution of the discussion in the follow-up patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84401/new/ https://reviews.llvm.org/D84401

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

2020-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath 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

[Lldb-commits] [PATCH] D84501: NativeThreadLinux invalidate register cache on stop

2020-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Yep, looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84501/new/ https://reviews.llvm.org/D84501 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81001#2170570 , @gedatsu217 wrote: > I checked what's the sequence that actually gets output, and it was like > below. > > h\x1b[2melp frame\x1b[0m\x1b[1Ghe\x1b[2mlp frame\x1b[0m\x1b[1Gel\x1b[2mp > frame\x1b[0m\x1b[1Gl > >

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

2020-07-23 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s:35-36 # CHECK-NEXT: Line table for main.cpp -# CHECK-NEXT: 0x1000: main.cpp:1 -# CHECK-NEXT: 0x1001: main.cpp:1 +#

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

2020-07-23 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: JDevlieghere, MaskRay, clayborg. Herald added a subscriber: aprantl. Herald added a project: LLDB. The intention here is to prune out line sequences addresses referring outside any known sections. Such line sequences are typically produced by

[Lldb-commits] [PATCH] D84401: [lldb] Add SectionList::ContainsFileAddressRange

2020-07-23 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: JDevlieghere, clayborg. Herald added a subscriber: mgorny. Herald added a project: LLDB. This is similar to FindSectionContainingFileAddress, except it doesn't return a section (because that would be ambiguous), and it checks for an address

[Lldb-commits] [PATCH] D84254: [lldb] Skip overlapping hardware and external breakpoints when writing memory

2020-07-23 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D84254#2166562 , @tatyana-krasnukha wrote: > `BreakpointSite::IntersectsRange` returns `false` for hardware breakpoints, > that's why the

[Lldb-commits] [PATCH] D79699: Add ptrace register access for AArch64 SVE registers

2020-07-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:240 + // Update SVE registers in case there is change in configuration. + ConfigureRegisterContext(); + omjavaid wrote: > labath wrote: > > What's

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Some quibbles about the test. Otherwise, this looks good to me. Comment at: lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py:33 +self.child.send("hel") +self.child.expect_exact(faint_color + "p frame" + reset + "\x1b[1G") +

[Lldb-commits] [PATCH] D84008: [DWARFYAML] Refactor emitDebugInfo() to make the length be inferred.

2020-07-23 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. Yeah, looks good. Some small suggestions inline, though some of them may be better off as separate patches... Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:207-215 +static unsigned getOffsetSize(const DWARFYAML::Unit )

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81001#2162743 , @gedatsu217 wrote: > In addition to it, I tried the below code, but it did not go well. ("\x1b[nD" > moves the cursor n steps to the left.) > > self.child.send("hel") > self.child.expect_exact(faint_color +

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc9d5a3058fcd: [lldb] add printing of stdout compile errors to lldbsuite (authored by bbli, committed by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [Differential] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGc9d5a3058fcd: [lldb] add printing of stdout compile errors to lldbsuite (authored by bbli, committed by labath). Changed

[Lldb-commits] [PATCH] D84307: [lldb/interpreter] Move the history subcommand to session (NFCI)

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I don't have any strong opinions about this either way. The `history` alias is kind of nice because it matches the bash builtin, but I don't use either of them often enough to care. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D83552: [lldb/test] Do a better job at setting (DY)LD_LIBRARY_PATH

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5f4c850e7b4f: [lldb/test] Do a better job at setting (DY)LD_LIBRARY_PATH (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83552/new/

[Lldb-commits] [Differential] D83552: [lldb/test] Do a better job at setting (DY)LD_LIBRARY_PATH

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG5f4c850e7b4f: [lldb/test] Do a better job at setting (DY)LD_LIBRARY_PATH (authored by labath). Repository: rG LLVM

[Lldb-commits] [PATCH] D79699: Add ptrace register access for AArch64 SVE registers

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I haven't looked at all the sve details, but they seem very similar to the core file stuff, so I think they should be fine. I have some other comments though... Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:240 +

[Lldb-commits] [PATCH] D84257: [lldb] Don't use hardware index to determine whether a breakpoint site is hardware

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a subscriber: omjavaid. labath added a comment. In D84257#2164967 , @tatyana-krasnukha wrote: > Probably fixes llvm.org/PR44659, though I cannot check on arm/aarch64. @omjavaid might be interested in checking that. Repository: rG LLVM

[Lldb-commits] [PATCH] D84257: [lldb] Don't use hardware index to determine whether a breakpoint site is hardware

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Could you clarify what's the functional change here? Does this change the kind of breakpoint that lldb sets, or just how it gets reported? E.g., does the `ProcessGDBRemote` acutally cause lldb to send a different packet, or is that just reacting to the change in the

[Lldb-commits] [PATCH] D84254: [lldb] Skip overlapping hardware and external breakpoints when writing memory

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. When does this assertion fire? Is this breaking an existing test? Should we be adding a new one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84254/new/ https://reviews.llvm.org/D84254

[Lldb-commits] [PATCH] D82155: [lldb/interpreter] Add ability to save lldb session to a file

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath added inline comments. Comment at: lldb/source/Commands/CommandObjectQuit.cpp:75-80 + if (m_interpreter.GetSaveSessionOnQuit()) { +if (!m_interpreter.SaveTranscript(result)) { + result.SetStatus(eReturnStatusFailed); +

[Lldb-commits] [PATCH] D84269: [lldb] Add some example type anotations to python.swig

2020-07-22 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. IIUC, this would make the file incompatible with python2, which we still kind of support (though hopefully not for long). This seems like a good idea, but we first need to sort out our py2 story. Given that we've just branched for the 11.0 release, this may actually be

[Lldb-commits] [PATCH] D83552: [lldb/test] Do a better job at setting (DY)LD_LIBRARY_PATH

2020-07-20 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83552#2162555 , @friss wrote: > The `lldbtest.py` part LGTM, but I'm failing to see how this interacts with > `TestWeakSymbols.py`? IIRC, `registerSharedLibraryWithTarget` is an API we > call explicitly in the tests, but I

[Lldb-commits] [PATCH] D83552: [lldb/test] Do a better job at setting (DY)LD_LIBRARY_PATH

2020-07-20 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Any thoughts on this? It should fix the failures from D83306 ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83552/new/ https://reviews.llvm.org/D83552

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-20 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath added a comment. Looks fine to me, modulo the clang-format issues. Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:74 + + // First separate the file and the line specification: + llvm::StringRef

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-20 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a subscriber: aprantl. labath added a comment. This revision is now accepted and ready to land. In D83425#2161255 , @bbli wrote: > > Setting this no longer makes sense, as it will always be empty. Please > >

[Lldb-commits] [PATCH] D84008: [DWARFYAML][WIP] Refactor emitDebugInfo() to make the length be inferred.

2020-07-20 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D84008#2161243 , @Higuoxing wrote: > In D84008#2160426 , @MaskRay wrote: > > > The number of changed tests is large. Is it worth moving the > > `IO.mapOptional("Length", Unit.Length);`

[Lldb-commits] [PATCH] D84070: [LLDB] [COFF] Fix handling of symbols with more than one aux symbol

2020-07-20 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. Thanks. If you think this is worth cherry-picking to 11.0 (it sounds like it is), please file the appropriate bug. Comment at: lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml:104 StorageClass:IMAGE_SYM_CLASS_STATIC

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-20 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81001#2158645 , @teemperor wrote: > Not sure if there is a good way to test that the text is directly behind the > cursor, but we can test that it's there. I guess that would be done by expecting the appropriate cursor

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I wouldn't mind separate (internal) cache variable, though I am somewhat surprised by this problem. A (non-cache) variable set in one of the parent scopes should still take precedence over a cache variable with the same name. And since config-ix.cmake is included from

[Lldb-commits] [PATCH] D83904: [lldb] Unify sleep and time outs in GDB remote testcases

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Sounds like a good idea. Comment at: lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py:63-64 # Wait until all threads have started. -

[Lldb-commits] [PATCH] D83840: [lldb][test] Prevent infinite loop while looking for use_lldb_suite_root.py.

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83840#2156040 , @JDevlieghere wrote: > In D83840#2155308 , @labath wrote: > > > In D83840#2154263 , @shafik wrote: > > > > > @labath why do we

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I think this is as good as we can get right now. Thanks for sticking through. Two quick comments inline. Comment at:

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The test looks wrong to me. If the feature is supposed to be off by default, then how can it test anything without first turning the feature on? Either the feature isn't really off, or the test is broken. I would expect it to be the latter, because the test sequence

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:120-127 + // The setting value may have whitespace, double-quotes, or single-quotes + // around the file path to indicate that internal spaces are not word + // breaks.

[Lldb-commits] [PATCH] D83881: [lldb/COFF] Remove strtab zeroing hack

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGede7c02b38c0: [lldb/COFF] Remove strtab zeroing hack (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83881/new/

[Lldb-commits] [PATCH] D83881: [lldb/COFF] Remove strtab zeroing hack

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
labath marked 2 inline comments as done. labath added a comment. Thanks for checking this out. Sorry for forgetting you Martin, I'll have to remember to add you to my future coff patches. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:672

[Lldb-commits] [PATCH] D83957: [lldb/DWARF] Don't get confused by line sequences with tombstone values

2020-07-17 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. labath marked 3 inline comments as done. Closed by commit rGf3fab392f574: [lldb/DWARF] Dont get confused by line sequences with tombstone values (authored by labath). Changed prior to commit:

[Lldb-commits] [PATCH] D83957: [lldb/DWARF] Don't get confused by line sequences with tombstone values

2020-07-16 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: JDevlieghere, MaskRay. Herald added a subscriber: aprantl. Herald added a project: LLDB. Recently, lld has started debug info resolving relocations to garbage-collected symbols as -1 (instead of 0). For an unaware consumer this generated

[Lldb-commits] [PATCH] D83023: [lldb/ObjectFileMachO] Fetch shared cache images from our own shared cache

2020-07-16 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:474-481 +typedef struct dyld_shared_cache_dylib_text_info +dyld_shared_cache_dylib_text_info; + +extern "C" int dyld_shared_cache_iterate_text( +

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83425#2154593 , @bbli wrote: > Ok, I have revised the patch with the code from the first pic. I also moved > the `decode` back to the `format_build_error` since it was there to begin > with(not sure how much of a difference

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp:63-66 +m_sve_note_payload.resize(m_sveregset.GetByteSize()); +::memcpy(GetSVEBuffer(), m_sveregset.GetDataStart(), + m_sveregset.GetByteSize());

[Lldb-commits] [PATCH] D83023: [lldb/ObjectFileMachO] Fetch shared cache images from our own shared cache

2020-07-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I like how this has turned out. Some small requests inline. Comment at: lldb/include/lldb/Host/HostInfoBase.h:107 + static SharedCacheImageInfo + GetSharedCacheImageInfo(llvm::StringRef image_name) { Some doxygen here? "Try to find

[Lldb-commits] [PATCH] D83840: [lldb][test] Prevent infinite loop while looking for use_lldb_suite_root.py.

2020-07-16 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83840#2154263 , @shafik wrote: > @labath why do we need two copies of `use_lldb_suite.py`? This script is responsible for setting up an appropriate python import path. Before we can import any code in

[Lldb-commits] [PATCH] D83815: [lldb/Test] Use a process group for subprocesses

2020-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83815#2153438 , @JDevlieghere wrote: > In D83815#2152688 , @labath wrote: > > > > > > Given that we were already creating a process group on fork, I'll keep the > change on line 902

[Lldb-commits] [PATCH] D83881: [lldb/COFF] Remove strtab zeroing hack

2020-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: amccarth, markmentovai. Herald added a project: LLDB. This code (recently responsible for a unaligned access sanitizer failure) claims that the string table offset zero should result in an empty string. I cannot find any mention of this

[Lldb-commits] [PATCH] D83876: [lldb] Clean orphaned modules in Debugger::Destroy

2020-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: jingham. labath added a comment. I have no opinion on this. Jim may have one... Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83876/new/ https://reviews.llvm.org/D83876 ___ lldb-commits

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83425#2151003 , @bbli wrote: > Yeah so in this pic, F12338615: original.png > , you can see in the code that both > stdout and stderr point to PIPE, and what gets printed out is the >

[Lldb-commits] [PATCH] D82064: [ARM64] Add QEMU testing environment setup guide for SVE testing

2020-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D82064#2152807 , @omjavaid wrote: > Updated after incorporating review comments. > > @rovka I gave your idea a thought about moving included scripts to > lldb/utils. IMO these scripts have no relationship with LLDB and no such

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. At this point, I think I (finally) have a good understanding of both how this patch works and interacts with the rest of the world. I have one more batch of comments, but hopefully none are too controversial, and I really do hope this is the last iteration.

[Lldb-commits] [PATCH] D83840: [lldb][test] Prevent infinite loop while looking for use_lldb_suite_root.py.

2020-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Heh I fixed the same thing back in 67f6d842fab6 for `scripts/use_lldb_suite.py`, but of course I forgot to update this copy... The idea is good, just please make sure both files end up using

[Lldb-commits] [PATCH] D83815: [lldb/Test] Use a process group for subprocesses

2020-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. hmm... I have a lot thoughts here.. - `setsid` is overkill. If you want to create process group, create a process group (`setpgid`), not a session. - this solution does not seem very windows-friendly. In fact, I'd be surprised if it works there at all. - going for

[Lldb-commits] [PATCH] D83787: [lldb/Test] Always set the cleanupSubprocesses tear down hook

2020-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I suppose this might have been because someone wanted to ensure cleanupSubprocesses is not called twice (if multiple subprocesses are spawned). It looks like it should be safe to do so, though it's not completely ideal. TBH, I'm not sure why this needs to be a tear down

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp:82-106 +uint32_t +RegisterContextCorePOSIX_arm64::CalculateSVEOffset(uint32_t reg_num) const { + uint32_t sve_offset = 0; + if (m_sve_state ==

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp:82-106 +uint32_t +RegisterContextCorePOSIX_arm64::CalculateSVEOffset(uint32_t reg_num) const { + uint32_t sve_offset = 0; + if (m_sve_state ==

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp:82-106 +uint32_t +RegisterContextCorePOSIX_arm64::CalculateSVEOffset(uint32_t reg_num) const { + uint32_t sve_offset = 0; + if (m_sve_state ==

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Some more comments (and questions) from me. Sorry about the back-and-forth. It's taking me a while to wrap my head around this, and as I start to understand things, new questions arise... Comment at:

[Lldb-commits] [PATCH] D83512: [lldb/Module] Allow for the creation of memory-only modules

2020-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath marked an inline comment as done. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:543-544 + if (m_data.ValidOffsetForDataOfSize(offset,

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Using two headers is ok, though I'd like to know what you meant by the "mixed up" comment. Are you saying that the contents comes out nondeterministically? Can you provide an example of that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` blocking in synchronous mode.

2020-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. The code looks fine. The test could be cleaned up a bit... Comment at: lldb/source/Target/Platform.cpp:1834 if (error.Fail()) return nullptr; JDevlieghere wrote: > jingham wrote: > > If you fail

[Lldb-commits] [PATCH] D83545: [lldb/dotest] Remove the "xunit" result formatter

2020-07-13 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa5803765d8e0: [lldb/dotest] Remove the xunit result formatter (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83545/new/

[Lldb-commits] [PATCH] D83302: [lldb/DWARF] Don't treat class declarations with children as definitions

2020-07-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83302#2142155 , @aprantl wrote: > I tried to do some radar archeology for more context, but I could neither > find a radar mentioning that commit, nor a mention of a radar in that commit. Thanks for the effort. Repository:

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I didn't notice this before, but I see now that there's more register number overlap in `lldb-arm64-register-enums.h`. Having one overlapping enum is bad enough, but two seems really too much? Can we avoid the second enum somehow, at least? Perhaps by switching

[Lldb-commits] [PATCH] D83541: Remove Linux sysroot dependencies of SVE PT macros

2020-07-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. To use this code from Process/elf-core, we also need to move this file. `source/Plugins/Process/Utility` is the best place we got for this right now. Comment at: lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h:9-10 #ifndef

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks. It was fairly easy to reproduce the problem. I have a proposed fix in D83552 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83306/new/ https://reviews.llvm.org/D83306

[Lldb-commits] [PATCH] D83552: [lldb/test] Do a better job at setting (DY)LD_LIBRARY_PATH

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: davide, jingham. Herald added a project: LLDB. registerSharedLibrariesWithTarget was setting the library path environment variable to the process build directory, but the function is also accepting libraries in other directories (in which case

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. PS: Don't worry about the harbormaster failures too much. It's very picky these days... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83425/new/ https://reviews.llvm.org/D83425

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/packages/Python/lldbsuite/test_event/build_exception.py:8 self.build_error = called_process_error.lldb_extensions.get( -"stderr_content", "") +"stderr_content", "").decode("utf-8") +

[Lldb-commits] [PATCH] D83545: [lldb/dotest] Remove the "xunit" result formatter

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added a reviewer: JDevlieghere. Herald added a project: LLDB. My understanding is that this was added to make dotest interact well with the GreenDragon bots, back when dotest was the main test driver. Now that everything goes through lit (which has its own

[Lldb-commits] [PATCH] D83441: [ldb/Reproducers] Add YamlRecorder and MultiProvider

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This seems fine. A bit more doxygen wouldn't hurt... Comment at: lldb/source/Utility/Reproducer.cpp:10 #include "lldb/Utility/Reproducer.h" +#include "lldb/Utility/Event.h"

[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Right. It does bug me a little that this has no test, but I don't think it's the worst that could happen. And you're the one who's going to have to debug this all over again if the next value

[Lldb-commits] [PATCH] D83512: [lldb/Module] Allow for the creation of memory-only modules

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I like this. The coff thingy is unfortunate, but it's not a hard limitation. It's just that nobody cared about making this work from memory (until now), and (re)loading the file from disk was the easiest solution. If my d372a8e8

[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83446#2142564 , @JDevlieghere wrote: > Seems like my thoughts got lost a bit with all the inline replies: we can > solve this particular issue by making `process connect` block in synchronous > mode. The fact that that's not

[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D83446#2142030 , @JDevlieghere wrote: > In D83446#2141713 , @labath wrote: > > > I'm wondering if this problem does not go beyond reproducers... The fact > > that we can have two

[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Assuming the goal is to make this code ValueObjectVariable-only, the code is fine. I can't say I understand why should it be ValueObjectVariable-only (*), but given how different the ValueObjectConstResult hierarchy is, I think that may be for the best. It would still

[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: jingham. labath added a comment. Well, this is an interesting problem... IIUC, what's happening is that the printing of the "stop reason" in the "event handler thread" is generating some packets (to fetch the stop reason), and then the "cont" command produces some (c)

[Lldb-commits] [PATCH] D77043: Fix process gdb-remote usage of value_regs/invalidate_regs

2020-07-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The idea seems ok (though could use a test), but I am wondering if it wouldn't be better to do this conversion during (or immediately after) parsing. If I remember the previous discussions correctly, we've established that the value/invalidate_regs fields in the

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

2020-07-08 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG695b33a56919: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true) (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D81697: Add support for batch-testing to the LLDB testsuite.

2020-07-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81697#2138024 , @bbli wrote: > F12311388: image.png > > Hi, so I think I got the fix working. Attached is a screenshot of the new > output, with title "Build Command Stdout Ouput". Should I

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

2020-07-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks for fixing this. Comment at: lldb/source/Core/ValueObject.cpp:690 - if (!valobj && synthetic_array_member) -valobj = GetSyntheticValue() - ->GetChildAtIndex(synthetic_index, synthetic_array_member) - .get();

[Lldb-commits] [PATCH] D81697: Add support for batch-testing to the LLDB testsuite.

2020-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81697#2131004 , @bbli wrote: > > Ideally this error should include the actual command line > > So to clarify, you want to also print out the exact clang command/what flags > were passed to clang whenever compile errors happen?

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

2020-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. this is the line that motivated this patch -- if one has a LD_LIBRARY_PATH environment variable already set (as I happen to do), then that line will not overwrite it

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

2020-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, wallace, jingham. Herald added a project: LLDB. This function was documented to overwrite entries with D76111 , which was adding a couple of similar functions. However, this function (unlike the

[Lldb-commits] [PATCH] D83302: [lldb/DWARF] Don't treat class declarations with children as definitions

2020-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: clayborg, aprantl, JDevlieghere. Herald added a reviewer: shafik. Herald added a project: LLDB. This effectively reverts r188124, which added code to handle (DW_AT_)declarations of structures with some kinds of children as definitions. The

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:690-694 + if (!valobj && synthetic_array_member) +valobj = GetSyntheticValue() + ->GetChildAtIndex(synthetic_index, synthetic_array_member) + .get(); +

[Lldb-commits] [PATCH] D80105: [LLDB] Combine multiple defs of arm64 register sets

2020-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. ship it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80105/new/ https://reviews.llvm.org/D80105 ___ lldb-commits mailing list

  1   2   3   4   5   6   7   8   9   10   >