[Lldb-commits] [PATCH] D52953: [lldb-mi] Implement -gdb-set breakpoint pending on/off

2018-10-18 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: tools/lldb-mi/MICmdCmdGdbSet.cpp:447 +bool CMICmdCmdGdbSet::OptionFnBreakpoint( + const CMIUtilString::VecString_t ) { +bool bPending = false; clang-format Repository:

[Lldb-commits] [PATCH] D52953: [lldb-mi] Implement -gdb-set breakpoint pending on/off

2018-10-18 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. And there is no reviewers set :) Repository: rL LLVM https://reviews.llvm.org/D52953 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

2018-11-28 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. If we decide to optimize DWARF garbage collection, something generic will be cool. This generic-abi thread has some discussion about that https://groups.google.com/d/msg/generic-abi/A-1rbP8hFCA/EDA7Sf3KBwAJ (e.g. using COMDAT but it seems challenging and it comes with

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

2019-03-21 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D54747#1342666 , @rocallahan wrote: > Here are some results for the rusoto test in > https://github.com/rust-lang/rust/issues/56068#issue-382175735: > > | LLD | Binary size in bytes | > | LLD 6.0.1

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

2019-03-21 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lld/ELF/MarkLive.cpp:192 + Sec->Live = true; + if (Sec->kind() != SectionBase::Kind::Regular && + Sec->kind() != SectionBase::Kind::Merge) This check can be changed to `!isa && !isa`. But do you just want to

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

2019-03-22 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. @rocallahan I find that people are discussing a generic approach in D59553 Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/ https://reviews.llvm.org/D54747

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

2019-05-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D54747#1459175 , @rocallahan wrote: > Updated results for the rusoto test in > https://github.com/rust-lang/rust/issues/56068#issue-382175735. The test > changed a bit because I'm using an updated Rust toolchain and

[Lldb-commits] [PATCH] D62593: DWARFDebugInfoEntry: delete unused Extract() and rename FastExtract() to Extract()

2019-05-29 Thread Fangrui Song via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB362049: DWARFDebugInfoEntry: delete unused Extract() and rename FastExtract() to… (authored by MaskRay, committed by ). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Changed

[Lldb-commits] [PATCH] D64398: [LLDB] Fix FreeBSD build

2019-07-11 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. I think the description should mention that this is to fix FreeBSD build after the LaunchThread interface change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64398/new/

[Lldb-commits] [PATCH] D64398: [LLDB] Fix FreeBSD build

2019-07-09 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp:798 + if (!m_operation_thread) { + Status e("Start launch thread failed"); + error = e; See D64163/r365226 Repository: rLLDB LLDB CHANGES SINCE LAST

[Lldb-commits] [PATCH] D64398: [LLDB] Fix FreeBSD build

2019-07-09 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D64398#1575713 , @devnexen wrote: > By the way quick question, I did not commit since quite a time, is it still > subversion ? Many people switched to the git monorepo github.com/llvm/llvm-project && `git llvm push` (I

[Lldb-commits] [PATCH] D67644: [ScriptInterpreter] Initialize globals when loading a scripting module.

2019-09-16 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/lit/Commands/Inputs/frame.py:3 +print(lldb.frame) \ No newline at end of file No newline at end of file Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67644/new/

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

2019-09-09 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Host/common/LZMA.cpp:124 + llvm::SmallVectorImpl ) { + if (InputBuffer.size() == 0) +return llvm::createStringError(llvm::inconvertibleErrorCode(), empty Comment

[Lldb-commits] [PATCH] D67520: Add pretty printing of Clang "bitfield" enums

2019-09-16 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: source/Symbol/ClangASTContext.cpp:9529 + // in `enum {A, B, ALL = A|B }` we visit ALL first. + std::stable_sort( + values.begin(), values.end(), [](const auto , const auto ) { You can simply sort by magnitude and

[Lldb-commits] [PATCH] D65789: A more robust way of testing debug_line parser near the end of module

2019-08-06 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: lit/SymbolFile/DWARF/Inputs/debug-line-basic.script:6 + .shstrtab : { *(.shstrtab) } + .debug_info0 : { *(.debug_info ) } + .debug_line0 : {

[Lldb-commits] [PATCH] D65282: ObjectFileELF: permit thread-local sections with overlapping file addresses

2019-07-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml:44 +# LOOKUP-LABEL: image lookup -a 0x1010 +# LOOKUP: Address: {{.*}}.PT_LOAD[0]..data + 16) + Do you mind explaining more how you'd like to improve file-address-based

[Lldb-commits] [PATCH] D65282: ObjectFileELF: permit thread-local sections with overlapping file addresses

2019-07-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. > Are you referring to the "image lookup" command specifically, or is it a more > general question about the internals of lldb too? Both:) This patch doesn't change the `Address: a.o[0x1010] (a.o.PT_LOAD[0]..tdata + 0)` output so I was puzzled what this patch

[Lldb-commits] [PATCH] D65282: ObjectFileELF: permit thread-local sections with overlapping file addresses

2019-07-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lit/Modules/ELF/PT_LOAD-overlap-PT_TLS.yaml:62 +Flags: [ SHF_ALLOC, SHF_WRITE ] +Address: 0x1000 +AddressAlign:0x4 `.data = .tbss = 0x1010` is a more realistic scenario. Normally, a

[Lldb-commits] [PATCH] D65282: ObjectFileELF: permit thread-local sections with overlapping file addresses

2019-07-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. In D65282#1602293 , @labath wrote: > In D65282#1602244 , @MaskRay wrote: > > > > Are you referring to the

[Lldb-commits] [PATCH] D65282: ObjectFileELF: permit thread-local sections with overlapping file addresses

2019-07-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. > It turns out this was too aggressive because thread-local sections typically will have file addresses which apear to overlap regular data/code. This does not cause a problem at runtime because thread-local sections are loaded into memory using special logic, but it can

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

2019-12-03 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py:58 +while True: +line = f.readline().decode("utf-8") +if len(line) == 0: Prefer single quotes. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

2019-10-16 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:389 + for (size_t Index = 0; Index < Exception.MaxParameters; ++Index) { +SmallString<16> Name = formatv("Parameter {0}", Index); +support::ulittle64_t =

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

2019-10-16 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D68943#1709998 , @grimar wrote: > Ok, I was able to debug it finally. > > I think we should add a .symtab in a few more cases implicitly. > For example, when we have a SHT_RELA/SHT_REL section that has an empty Link, > i.e.

[Lldb-commits] [PATCH] D68872: SBCommandReturnObject: change LLDB_RECORD_METHOD(..., FILE *, ...) to use LLDB_RECORD_DUMMY

2019-10-16 Thread Fangrui Song via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG56ee31964f5a: SBCommandReturnObject: change LLDB_RECORD_METHOD(..., FILE *, ...) to use… (authored by MaskRay). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Changed prior to

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D49466#1761044 , @thakis wrote: > There's still one failing test on Windows after the fix attempt: > http://45.33.8.238/win/3052/step_6.txt > > Please take a look and revert if it's not an easy fix. (And please watch bots >

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. The ugly path separator pattern `{{(/|)}}` appears in 60+ tests. Can we teach clang and other tools to 1. accept both `/` and `\` input 2. but only output `/` on Windows? We can probably remove `llvm::sys::path::Style::{windows,posix,native}` from

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Does this work on Windows? --- i/clang/test/Preprocessor/file_test.c +++ w/clang/test/Preprocessor/file_test.c @@ -1,8 +1,7 @@ -// XFAIL: system-windows // RUN: %clang -E -ffile-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | FileCheck %s // RUN: %clang -E

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 231142. MaskRay added a comment. Minimize diff in Options.td Properly rebase remapDIPath on top of D69213 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49466/new/

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 231140. MaskRay added a comment. Add back remapDIPath that was unintentionally deleted by D69213 , caught by a test. Small adjustment of the code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6c92cdff7225: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map (authored by dankm, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

2019-11-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D49466#1760765 , @dankm wrote: > Ping? The tests need fixing... I can commit it. Now that we've migrated to the llvm monorepo, the git commit message can retain the author info properly... Repository: rG LLVM Github

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. This was probably due to: Expected(Error Err) : HasError(true) #if LLVM_ENABLE_ABI_BREAKING_CHECKS // Expected is unchecked upon construction in Debug builds. , Unchecked(true) #endif { assert(Err && "Cannot create Expected

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added inline comments. Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:111 + + explicit MemoryInfoListStream(std::vector Infos) + : Stream(StreamKind::MemoryInfoList, grimar wrote: > Maybe be more explicit here,

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D68723#1703322 , @devnexen wrote: > @dim I LGTMed this but would it a real issue for you to take on a less > disruptive approach proposed by MaskRay ? I think we should do what @labath suggested (changing Expected to

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D68723#1703051 , @devnexen wrote: > LGTM otherwise. I don't think this change should be reverted. It can just be repaired by adding some `(void)!xxx;` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68723/new/

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

2019-10-14 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. This is not correct. A lot of yaml2obj tests fail with: > yaml2obj: error: unknown section referenced: '.symtab' by YAML section > '.rela.text' The problem is that .symtab can be the sh_link field of relocation sections, .llvm_addrsig, and some sections that may be

[Lldb-commits] [PATCH] D71800: [CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS AND UNIX

2019-12-21 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay created this revision. MaskRay added reviewers: clayborg, JDevlieghere, jingham, labath. Herald added subscribers: lldb-commits, kristof.beyls, mgorny. Herald added a project: LLDB. For -DBUILD_SHARED_LIBS=On builds, lib/liblldb.so depends on lib/*.so .

[Lldb-commits] [PATCH] D71800: [CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS AND UNIX

2019-12-21 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: lldb/source/API/CMakeLists.txt:124 + set(_install_rpath "\$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}" "\$ORIGIN/../../../../lib${LLVM_LIBDIR_SUFFIX}" ${extra_libdir}) + set_target_properties(liblldb

[Lldb-commits] [PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-02 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. I really like the new feature. Thanks for making the efforts! However, I am afraid I don't like some of the fixes here. You can replace `const auto` with `const auto &` and call that a fix... IMHO if the type is not obvious, `const ConcreteType &` will be better. In

[Lldb-commits] [PATCH] D71800: [CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS or LLVM_LINK_LLVM_DYLIB on *nix

2020-01-03 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Ping:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71800/new/ https://reviews.llvm.org/D71800 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D71800: [CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS or LLVM_LINK_LLVM_DYLIB on *nix

2019-12-23 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 235170. MaskRay marked 3 inline comments as done. MaskRay retitled this revision from "[CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS AND UNIX" to "[CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS or LLVM_LINK_LLVM_DYLIB

[Lldb-commits] [PATCH] D71800: [CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS or LLVM_LINK_LLVM_DYLIB on *nix

2019-12-23 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/API/CMakeLists.txt:117 +# loaded from Python. +if(LLDB_ENABLE_PYTHON AND BUILD_SHARED_LIBS AND UNIX) + if(LLVM_INSTALL_PREFIX AND NOT (LLVM_INSTALL_PREFIX STREQUAL CMAKE_INSTALL_PREFIX)) labath wrote: > I

[Lldb-commits] [PATCH] D71857: [NFC] Fixes -Wrange-loop-analysis warnings

2020-01-06 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D71857#1804307 , @Mordante wrote: > In D71857#1800663 , @MaskRay wrote: > > > I think there is a false positive. > > > >

[Lldb-commits] [PATCH] D71800: [CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS or LLVM_LINK_LLVM_DYLIB on *nix

2020-01-06 Thread Fangrui Song via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbbfebd7b8a67: [CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS or… (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D75810: [lldb] Add .clang-tidy with customization to disable readability-identifier-naming

2020-03-09 Thread Fangrui Song via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG71269a1f172c: [lldb] Add .clang-tidy with customization to disable readability-identifier… (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D75810: [lldb] Add .clang-tidy with customization to readability-identifier-naming.{Function, Member, Parameter, Variable}Case

2020-03-07 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay created this revision. MaskRay added reviewers: JDevlieghere, jingham, labath. Herald added subscribers: lldb-commits, aheejin. Herald added a project: LLDB. MaskRay added a comment. https://github.com/google/llvm-premerge-checks/issues/142 This should suppress the annoying `clang-tidy

[Lldb-commits] [PATCH] D75810: [lldb] Add .clang-tidy with customization to readability-identifier-naming.{Function, Member, Parameter, Variable}Case

2020-03-07 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. https://github.com/google/llvm-premerge-checks/issues/142 This should suppress the annoying `clang-tidy linux` diagnostics in `Diff Detail - Build Status`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75810/new/

[Lldb-commits] [PATCH] D75810: [lldb] Add .clang-tidy with customization to disable readability-identifier-naming

2020-03-09 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 249139. MaskRay retitled this revision from "[lldb] Add .clang-tidy with customization to readability-identifier-naming.{Function,Member,Parameter,Variable}Case" to "[lldb] Add .clang-tidy with customization to disable readability-identifier-naming".

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

2020-03-30 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:102 + +Streamer.emitIntValue(ELFAttrs::Format_Version, 1); + } `emitInt8` Comment at:

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

2020-03-30 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. The code generally looks good. For unittests, I think we can either make llvm-readobj -A canonical or the unittests canonical. If we decide to place tests on one place, we should delete most tests on the other side. My current preference is that we use more of

[Lldb-commits] [PATCH] D77186: [source maps] Ensure all valid source maps are added instead of failing with the first invalid one

2020-04-01 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D77186#1955451 , @wallace wrote: > address comments @wallace labath made comments after the differential had been approved. So it looks like he may have further questions. It was probably better waiting for an explicit ack

[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

2020-03-26 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. I remember the patch caused many failures yesterday. The new diff is good. Comment at: llvm/unittests/Support/ELFAttributeParserTest.cpp:16 + +TagNameMap emptyTagNameMap; + `static const` Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

2020-03-23 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. commit fd868f517d2c5ca8c0f160dbec0857b14ecf74c1 should be associated with `Differential Revision: https://reviews.llvm.org/D76111`. The revert of fd868f517d2c5ca8c0f160dbec0857b14ecf74c1

[Lldb-commits] [PATCH] D86857: [Test] Simplify DWARF test cases. NFC.

2020-08-30 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86857/new/ https://reviews.llvm.org/D86857

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

2020-09-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: llvm/lib/Support/MemoryBuffer.cpp:460 - if (shouldUseMmap(FD, FileSize, MapSize, Offset, RequiresNullTerminator, -PageSize, IsVolatile)) { + if (false) { std::error_code EC; I guess this was

[Lldb-commits] [PATCH] D88728: [lldb] Check for and use ptsname_r if available

2020-10-06 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Worth mentioning that this will be in POSIX issue 8 https://www.austingroupbugs.net/bug_view_page.php?bug_id=508 Comment at:

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

2020-10-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/tools/lldb-server/CMakeLists.txt:61 LINK_COMPONENTS Support ) Otherwise it fails in a BUILD_SHARED_LIBS=on build because the -Wl,-z,defs linker option requires a module to have fully specified

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

2020-10-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/tools/lldb-server/lldb-gdbserver.cpp:404 + unsigned MissingArgCount; + opt::InputArgList Args = Opts.ParseArgs(makeArrayRef(argv + 2, argc - 2), + MissingArgIndex, MissingArgCount);

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

2020-10-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/tools/lldb-server/lldb-gdbserver.cpp:369 + + If no target is selected a startup, the lldb-server can be directed to launch + or attach a process by the LLDB client. `a startup` -> `at startup` Repository: rG

[Lldb-commits] [PATCH] D88728: [lldb] Check for and use ptsname_r if available

2020-10-02 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Host/common/PseudoTerminal.cpp:149 + int r = ptsname_r(m_primary_fd, buf, sizeof(buf)); + assert(r == 0); + return buf; MaskRay wrote: > labath wrote: > > mgorny wrote: > > > labath wrote: > > > > mgorny

[Lldb-commits] [PATCH] D88728: [lldb] Check for and use ptsname_r if available

2020-10-02 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Host/common/PseudoTerminal.cpp:149 + int r = ptsname_r(m_primary_fd, buf, sizeof(buf)); + assert(r == 0); + return buf; labath wrote: > mgorny wrote: > > labath wrote: > > > mgorny wrote: > > > > I would

[Lldb-commits] [PATCH] D85968: [lldb] Forcefully complete a type when adding nested classes

2020-08-18 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s:7 -# RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj > %t -# RUN: %lldb %t -o "expr a" -o exit 2>&1 | FileCheck %s --check-prefix=EXPR -# RUN: %lldb %t -o

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

2020-09-21 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Target/TraceSettingsParser.cpp:123 + if (schema.empty()) { +std::ostringstream schema_builder; +schema_builder << "{\n \"trace\": "; This requires `sstream`. Fixed. Comment at:

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

2020-05-29 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. I haven't been paying close attention to this patch, but allowing 20 bytes makes sense. In GNU ld and gold, `--build-id` is `--build-id=sha1` (20 bytes). The 3 linkers (plus LLD) don't have a way to produce a build ID longer than 20 bytes. CHANGES SINCE LAST ACTION

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

2020-07-17 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1040 std::unique_ptr sequence = LineTable::CreateLineSequenceContainer(); std::vector> sequences; While here, delete the initialization and move

[Lldb-commits] [PATCH] D85169: [test] Exit with an error if no tests are run.

2020-08-03 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:1043 +if configuration.suite.countTestCases() == 0: +print("error: did not discover any tests.") +exitTestSuite(1) JDevlieghere wrote: > Maybe `did not

[Lldb-commits] [PATCH] D85248: [test] Support git commit version ids for clang.

2020-08-04 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85248/new/ https://reviews.llvm.org/D85248

[Lldb-commits] [PATCH] D85100: [ELF] Allow sections after a non-SHF_ALLOC section to be covered by PT_LOAD

2020-08-06 Thread Fangrui Song via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rGa6db64ef4a99: [ELF] Allow sections after a non-SHF_ALLOC section to be covered by PT_LOAD (authored by MaskRay). Herald

[Lldb-commits] [PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

2020-07-09 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks like a good cleanup. Hope someone with more CMake-fu to confirm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83454/new/

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

2020-07-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Hi, your git commit contains extra Phabricator tags. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git commit with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk

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

2020-07-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Hi, your git commit contains extra Phabricator tags. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git commit with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

2020-07-11 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a subscriber: mehdi_amini. MaskRay added a comment. In D82477#2145967 , @clayborg wrote: > In D82477#2145565 , @MaskRay wrote: > > > Hi, your git commit contains extra Phabricator tags. You can drop

[Lldb-commits] [PATCH] D81200: [vscode] set default values for terminateDebuggee for the disconnect request

2020-06-24 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Hi, your git commit contains extra Phabricator tags. You can drop `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git commit with the following script: arcfilter () { arc amend git log -1 --pretty=%B | awk

[Lldb-commits] [PATCH] D82238: lldb whitelist -> allowed

2020-06-19 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay 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/D82238/new/ https://reviews.llvm.org/D82238

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

2020-07-24 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. What is the state of the patch? Does lldb support cgdb-style u/d/f/b etc now? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68541/new/ https://reviews.llvm.org/D68541 ___ lldb-commits

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

2020-07-24 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Core/Section.cpp:570 + while (file_addr < end) { +SectionSP sect_sp = FindSectionContainingFileAddress(file_addr); +if (!sect_sp) I am a bit concerned about the potential large time complexity

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

2020-07-24 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. For ELF, there are non-pic cases (i.e. -no-pie) and pic cases (-pie or -shared). I think it is sufficient just testing -pie (image base is zero). If filtering for -pie works, filter for -no-pie or -shared should work as well. Comment at:

[Lldb-commits] [PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

2020-07-17 Thread Fangrui Song via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG53880b8cb9c6: [CMake] Make `intrinsics_gen` dependency unconditional. (authored by michele.scandale, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-07-18 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. The number of changed tests is large. Is it worth moving the `IO.mapOptional("Length", Unit.Length);` change to a separate patch to make the refactoring more focused? Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-07-16 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. > Recently, lld has started debug info resolving relocations to > garbage-collected symbols as -1 Just mention D81784 :) > (instead of 0). This is not accurate. It resolves the relocation to `r_addend`. In many cases, r_addend is 0.

[Lldb-commits] [PATCH] D84623: Remove HAVE_VCS_VERSION_INC, not needed

2020-10-29 Thread Fangrui Song via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9bb9b737c557: Remove HAVE_VCS_VERSION_INC, not needed (authored by hlopko, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D84623: Remove HAVE_VCS_VERSION_INC, not needed

2020-10-29 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. Herald added a subscriber: dexonsmith. Thanks! Assuming you don't have a commit bit, I'll commit on your behalf. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84623/new/

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

2021-01-20 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Is this good? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94890/new/ https://reviews.llvm.org/D94890 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

2021-01-20 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 317985. MaskRay retitled this revision from "Makefile.rules: Avoid redundant .d generation and make restart" to "Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test". MaskRay edited the summary of this

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

2021-01-20 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 317983. MaskRay added a comment. Inline archive rule Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94890/new/ https://reviews.llvm.org/D94890 Files: lldb/packages/Python/lldbsuite/test/make/Makefile.rules

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

2021-01-20 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D94890#2510627 , @JDevlieghere wrote: > In D94890#2510446 , @MaskRay wrote: > >> Is this good? :) > > I think you forgot to update the patch? Ah, looks like you want to do inline

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

2021-01-20 Thread Fangrui Song via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6afdf13ae4cc: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive… (authored by MaskRay). Repository: rG LLVM Github

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation (make restart) and inline archive rule to the only test

2021-01-20 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 318011. MaskRay marked an inline comment as done. MaskRay added a comment. $(RM) a.o b.o Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94890/new/ https://reviews.llvm.org/D94890 Files:

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

2021-01-17 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay created this revision. MaskRay added reviewers: dblaikie, labath, mgorny, rupprecht. Herald added a subscriber: krytarowski. MaskRay requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. On my Debian machine, system libc++/libc++abi is

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

2021-01-17 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay created this revision. MaskRay added reviewers: friss, JDevlieghere, labath, rupprecht. MaskRay requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Take an example when `CXX_SOURCES` is main.cpp. main.d is a included file. make will

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

2021-01-21 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 318414. MaskRay marked an inline comment as done. MaskRay added a comment. Adopt rupprecht's suggestion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94888/new/ https://reviews.llvm.org/D94888 Files:

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

2021-01-22 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 318546. MaskRay added a comment. Drop fancy check in dotest.py (restore the previous behavior) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94888/new/ https://reviews.llvm.org/D94888 Files:

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

2021-01-19 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 317599. MaskRay added a comment. Delete `if os.path.isdir("/usr/include/c++/v1"):` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94888/new/ https://reviews.llvm.org/D94888 Files:

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

2021-01-18 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay updated this revision to Diff 317382. MaskRay edited the summary of this revision. MaskRay added a comment. Don't use double-colon. https://lists.gnu.org/archive/html/help-make/2021-01/msg00016.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

2021-01-19 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D94890#2507856 , @JDevlieghere wrote: > In D94890#2507241 , @MaskRay wrote: > >> In D94890#2505988 , @labath wrote: >> >>> Looks like a nice

[Lldb-commits] [PATCH] D94890: Makefile.rules: Avoid redundant .d generation and make restart

2021-01-19 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D94890#2505988 , @labath wrote: > Looks like a nice cleanup. The only part I am not sure of is the part about > removing `$(RM) $(ARCHIVE_OBJECTS)`. Is that necessary? > I'm not sure why is that line there, but if I had to

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

2021-01-19 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D94888#2505992 , @labath wrote: > It looks like this is removing the ability to build libc++ tests with gcc (as > it does not have the `-stdlib` option). While having that ability would be > nice, I don't believe there's

[Lldb-commits] [PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

2021-01-27 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. `#!/usr/bin/env perl -w` (2 arguments) unfortunately does not work on Linux and many other systems. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95119/new/ https://reviews.llvm.org/D95119

[Lldb-commits] [PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

2021-01-27 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Several `llvm/utils/` scripts can probably just be removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95119/new/ https://reviews.llvm.org/D95119 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

2021-05-07 Thread Fangrui Song via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG632ebc4ab437: [MC] Untangle MCContext and MCObjectFileInfo (authored by flip1995, committed by MaskRay). Repository: rG LLVM Github Monorepo

  1   2   3   >