[Lldb-commits] [PATCH] D55631: Delay replacing the process till after we've finalized it

jingham created this revision. jingham added a reviewer: clayborg. Herald added subscribers: lldb-commits, abidh. Because of the way we use weak pointers in Process, it is not safe to just destroy a fully live Process object. For instance, because the Thread holds a ProcessWP, if the Process

[Lldb-commits] [lldb] r348996 - [NFC] Small code cleanups in utility.

Author: jdevlieghere Date: Wed Dec 12 16:15:17 2018 New Revision: 348996 URL: http://llvm.org/viewvc/llvm-project?rev=348996=rev Log: [NFC] Small code cleanups in utility. Fix a few small annoyances in Utility I ran into. Modified: lldb/trunk/source/Utility/FileSpec.cpp

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

aprantl added inline comments. Comment at: lit/Reproducer/Functionalities/TestStepping.test:39 +# CHECK: 1 breakpoints disabled. +# CHECK: 1 breakpoints disabled. + aprantl wrote: > perhaps copy the commands in here so it is easier to understand what is being >

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

aprantl added inline comments. Comment at: lit/Reproducer/Functionalities/Inputs/stepping.c:8 +// +//===--===// +#include Why does only this file have a header? Comment at:

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

JDevlieghere created this revision. JDevlieghere added reviewers: labath, davide, aprantl, zturner. JDevlieghere added a project: LLDB. Herald added a subscriber: teemperor. This patch adds test that check that functionality in lldb continues to work when replaying a reproducer. Currently the

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere updated this revision to Diff 177950. JDevlieghere added a comment. When sourcing a file we should also ignore the individual commands otherwise they end up getting executed twice, once as part of the `command source` and once for every individual command in the file. CHANGES

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere updated this revision to Diff 177906. JDevlieghere added a comment. Skip driver logic when replaying, otherwise commands are executed or sourced twice, once in the driver and once by replaying the commands. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55582/new/

[Lldb-commits] [PATCH] D55614: Fix MinidumpParser::GetFilteredModuleList() and test it

clayborg added a comment. The order in the modules list is maintained by changing the llvm::StringMap to map from module name to "filtered_modules" index. This avoids having to iterate across the StringMap in the end and make the filtered_modules in a different ordering. CHANGES SINCE LAST

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

I ended up implementing the support for "target symbols add" since it's something we needed anyway. This allowed the removal of the contentious implicit search in the current directory. I tried to verify this behavior, but it seems like it should already work > out of the box? So we're on the

[Lldb-commits] [PATCH] D55614: Fix MinidumpParser::GetFilteredModuleList() and test it

clayborg created this revision. clayborg added reviewers: zturner, labath, markmentovai, lemo. The MinidumpParser::GetFilteredModuleList() code was attempting to iterate through the entire module list and if it found more than one entry for a given module name, it wanted to pick the

[Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

lemo marked an inline comment as done. lemo added inline comments. Comment at: source/Commands/CommandObjectTarget.cpp:4246 if (symbol_file) { -ObjectFile *object_file = symbol_file->GetObjectFile(); note I had to bypass this check: we

[Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

lemo updated this revision to Diff 177898. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55142/new/ https://reviews.llvm.org/D55142 Files: lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.cpp lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.dmp

[Lldb-commits] [PATCH] D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Yep, looks good. It would be nice if we found a dSYM with a Spotlight search (mdfind) if we could look NEXT to the dSYM bundle and see if there is a real binary, and load that.

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

aprantl updated this revision to Diff 177890. aprantl added a comment. Be more verbose about what is a valid arch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55608/new/ https://reviews.llvm.org/D55608 Files: examples/python/crashlog.py Index: examples/python/crashlog.py

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

aprantl updated this revision to Diff 177889. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55608/new/ https://reviews.llvm.org/D55608 Files: examples/python/crashlog.py Index: examples/python/crashlog.py === ---

[Lldb-commits] [PATCH] D55608: Make crashlog.py work or binaries with spaces in their names

aprantl created this revision. aprantl added reviewers: jasonmolenda, jingham. This is a little dangerous since the crashlog files aren't 100% unambiguous, but the risk is mitigated by using a non-greedy `+?` pattern. rdar://problem/38478511 https://reviews.llvm.org/D55608 Files:

[Lldb-commits] [PATCH] D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors

sgraenitz added a comment. Ninja builds work well without it. In order to catch **actual** double-signing problems here, I would prefer to pass it only in case of Xcode generator. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55116/new/

[Lldb-commits] [PATCH] D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors

kubamracek accepted this revision. kubamracek added a comment. This revision is now accepted and ready to land. I'd slightly prefer to use --force for *all* builds, not just Xcode builds, to have uniformity. But LGTM even in this form if you feel strongly about it. Repository: rL LLVM

[Lldb-commits] [PATCH] D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing

aprantl created this revision. aprantl added a reviewer: jasonmolenda. Often users have a crash log an d a .dSYM bundle, but not the original application binary. It turns out that for crash symbolication, we can safely fall back to using the binary inside the .dSYM bundle.

[Lldb-commits] [PATCH] D55332: [CMake] Python bindings generation polishing

sgraenitz marked an inline comment as done. sgraenitz added inline comments. Comment at: CMakeLists.txt:134 --srcRoot=${LLDB_SOURCE_DIR} - --targetDir=${LLDB_PYTHON_TARGET_DIR} - --cfgBldDir=${LLDB_PYTHON_TARGET_DIR} +

[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

jingham added a comment. This change looks good to me too. I don't think this will fix the parameter/local variable lookup inversion that we've been hacking around. The problem comes because to make an JITted object we can easily call we make a wrapper function with a simple argument list -

[Lldb-commits] [lldb] r348951 - NFC: fix compiler warning about code never being executed when compiling on non windows platform.

Author: gclayton Date: Wed Dec 12 10:14:27 2018 New Revision: 348951 URL: http://llvm.org/viewvc/llvm-project?rev=348951=rev Log: NFC: fix compiler warning about code never being executed when compiling on non windows platform. Modified:

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere updated this revision to Diff 177878. JDevlieghere added a comment. Address Pavel's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55582/new/ https://reviews.llvm.org/D55582 Files: include/lldb/Core/Debugger.h include/lldb/Interpreter/CommandInterpreter.h

[Lldb-commits] [PATCH] D55575: [NativePDB] Support local variables

zturner updated this revision to Diff 177877. zturner added a comment. Updated test to be less dependent on the exact format of the source. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55575/new/ https://reviews.llvm.org/D55575 Files:

[Lldb-commits] [PATCH] D55584: [LLDB] Simplify Boolean expressions

shafik accepted this revision as: shafik. shafik added a comment. LGTM after comment are addressed. Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5894 +if (!success) { + offset = MachHeaderSizeFromMagic(m_header.magic); + for (uint32_t i = 0;

[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB348941: [ast] CreateParameterDeclaration should use an appropriate DeclContext. (authored by zturner, committed by ). Herald added a subscriber: teemperor. Changed prior to commit:

[Lldb-commits] [PATCH] D55584: [LLDB] Simplify Boolean expressions

JDevlieghere added a comment. In D55584#1328087 , @labath wrote: > I also find the `static_cast` thingy weird. The rest of the changes > seem to be towards the better (based on a pseudo-random sample), but the > change is a quite big. Me and Adrian

[Lldb-commits] [lldb] r348941 - [ast] CreateParameterDeclaration should use an appropriate DeclContext.

Author: zturner Date: Wed Dec 12 09:17:53 2018 New Revision: 348941 URL: http://llvm.org/viewvc/llvm-project?rev=348941=rev Log: [ast] CreateParameterDeclaration should use an appropriate DeclContext. Previously CreateParameterDeclaration was always using the translation unit DeclContext. We

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

There is another option which I was just made aware of. LLDB already has a setting called `target.debug-file-search-paths`. This is basically a symbol path. If you call Symbols::LocateExecutableSymbolFile, it will already add use this setting, and moreover it will implicitly add current working

Re: [Lldb-commits] [PATCH] D55430: build.py: Implement "gcc" builder

It's fine, I was mostly just curious. On Wed, Dec 12, 2018 at 8:52 AM Pavel Labath via Phabricator < revi...@reviews.llvm.org> wrote: > labath marked an inline comment as done. > labath added inline comments. > > > > Comment at: lldb/trunk/lit/helper/build.py:630 > +

[Lldb-commits] [PATCH] D55430: build.py: Implement "gcc" builder

labath marked an inline comment as done. labath added inline comments. Comment at: lldb/trunk/lit/helper/build.py:630 +args.append('-nostdinc') +args.append('-static') +args.append('-c') zturner wrote: > Why do we need this?

[Lldb-commits] [PATCH] D55430: build.py: Implement "gcc" builder

zturner added inline comments. Comment at: lldb/trunk/lit/helper/build.py:630 +args.append('-nostdinc') +args.append('-static') +args.append('-c') Why do we need this? Repository: rL LLVM CHANGES SINCE LAST ACTION

[Lldb-commits] [lldb] r348936 - ELF: Clean up section type computation

Author: labath Date: Wed Dec 12 07:46:18 2018 New Revision: 348936 URL: http://llvm.org/viewvc/llvm-project?rev=348936=rev Log: ELF: Clean up section type computation Move code into a separate function, and replace the if-else chain with llvm::StringSwitch. A slight behavioral change is that

[Lldb-commits] [PATCH] D55575: [NativePDB] Support local variables

labath added inline comments. Comment at: lldb/lit/SymbolFile/NativePDB/local-variables.cpp:31-37 +// CHECK-NEXT:14 } +// CHECK-NEXT:15 +// CHECK-NEXT:16 int main(int argc, char **argv) { +// CHECK-NEXT: -> 17 int SomeLocal = argc * 2; +// CHECK-NEXT:18

[Lldb-commits] [PATCH] D55575: [NativePDB] Support local variables

zturner marked an inline comment as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:1929 +} +#include "lldb/Target/MemoryRegionInfo.h" +CompilerDeclContext labath wrote: > Huh? Oops :-/ CHANGES

[Lldb-commits] [PATCH] D55575: [NativePDB] Support local variables

zturner marked an inline comment as done. zturner added inline comments. Comment at: lldb/lit/SymbolFile/NativePDB/local-variables.cpp:31-37 +// CHECK-NEXT:14 } +// CHECK-NEXT:15 +// CHECK-NEXT:16 int main(int argc, char **argv) { +// CHECK-NEXT: -> 17 int

[Lldb-commits] [lldb] r348928 - ELF: Simplify program header iteration

Author: labath Date: Wed Dec 12 06:20:28 2018 New Revision: 348928 URL: http://llvm.org/viewvc/llvm-project?rev=348928=rev Log: ELF: Simplify program header iteration Instead of GetProgramHeaderCount+GetProgramHeaderByIndex, expose an ArrayRef of all program headers, to enable range-based

[Lldb-commits] [PATCH] D55597: lldb-test: Improve newline handling

labath created this revision. labath added a reviewer: zturner. Previously lldb-test's LinePrinter would output the indentation spaces even on completely empty lines. This is not nice, as trailing spaces get flagged as errors in some tools/editors, and it prevents FileCheck's CHECK-EMPTY from

[Lldb-commits] [lldb] r348924 - lldb-test: Add ability to dump subsections

Author: labath Date: Wed Dec 12 04:35:25 2018 New Revision: 348924 URL: http://llvm.org/viewvc/llvm-project?rev=348924=rev Log: lldb-test: Add ability to dump subsections Previously, lldb-test would only print top-level sections. However, in lldb, sections can contain other sections. This

Re: [Lldb-commits] [PATCH] D55356: Add a method to get the "base" file address of an object file

On 11/12/2018 23:54, Zachary Turner wrote: On Tue, Dec 11, 2018 at 11:57 AM Pavel Labath > wrote: The part I know nothing about is whether something similar could be done for PE/COFF files (and I'll need something like that there too). Adrian,

[Lldb-commits] [PATCH] D55575: [NativePDB] Support local variables

labath added inline comments. Comment at: lldb/lit/SymbolFile/NativePDB/local-variables.cpp:31-37 +// CHECK-NEXT:14 } +// CHECK-NEXT:15 +// CHECK-NEXT:16 int main(int argc, char **argv) { +// CHECK-NEXT: -> 17 int SomeLocal = argc * 2; +// CHECK-NEXT:18

[Lldb-commits] [PATCH] D55584: [LLDB] Simplify Boolean expressions

labath added a comment. I also find the `static_cast` thingy weird. The rest of the changes seem to be towards the better (based on a pseudo-random sample), but the change is a quite big. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55584/new/ https://reviews.llvm.org/D55584

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

labath added a comment. My main question is why do we need the separate `SBDebugger::RunReplay` API for this. Shouldn't the record/replay be as transparent as possible? I would expect that `SBDebugger::RunCommandInterpreter` will detect that it is running in replay mode and use the recorded

[Lldb-commits] [PATCH] D55430: build.py: Implement "gcc" builder

This revision was automatically updated to reflect the committed changes. labath marked an inline comment as done. Closed by commit rL348918: build.py: Implement gcc builder (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D55430: build.py: Implement "gcc" builder

labath marked 3 inline comments as done. labath added inline comments. Comment at: lit/BuildScript/toolchain-clang.test:1 +RUN: %build -n --verbose --arch=32 --compiler=clang --mode=compile-and-link -o %t/foo.exe foobar.c \ +RUN:| FileCheck --check-prefix=CHECK

[Lldb-commits] [lldb] r348918 - build.py: Implement "gcc" builder

Author: labath Date: Wed Dec 12 00:54:14 2018 New Revision: 348918 URL: http://llvm.org/viewvc/llvm-project?rev=348918=rev Log: build.py: Implement "gcc" builder Summary: This implements the gcc builder in build.py script to allow it to compile host executables when running on a non-windows