date:20181212

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

2018-12-12 Thread Pavel Labath via lldb-commits

Author: labath
Date: Wed Dec 12 00:54:14 2018
New Revision: 348918

URL: http://llvm.org/viewvc/llvm-project?rev=348918&view=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 host. Where it
made sense, I tried to share code with the msvc builder by moving stuff
to the base class.

Reviewers: zturner

Subscribers: mehdi_amini, dexonsmith, lldb-commits

Differential Revision: https://reviews.llvm.org/D55430

Added:
lldb/trunk/lit/BuildScript/toolchain-clang.test
Modified:
lldb/trunk/lit/Breakpoint/case-sensitive.test
lldb/trunk/lit/BuildScript/modes.test
lldb/trunk/lit/BuildScript/script-args.test
lldb/trunk/lit/helper/build.py   (contents, props changed)

Modified: lldb/trunk/lit/Breakpoint/case-sensitive.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Breakpoint/case-sensitive.test?rev=348918&r1=348917&r2=348918&view=diff
==
--- lldb/trunk/lit/Breakpoint/case-sensitive.test (original)
+++ lldb/trunk/lit/Breakpoint/case-sensitive.test Wed Dec 12 00:54:14 2018
@@ -1,6 +1,6 @@
 # REQUIRES: nowindows
 #
-# RUN: %clang %p/Inputs/case-sensitive.c -g -o %t
+# RUN: %build %p/Inputs/case-sensitive.c --nodefaultlib -o %t
 # RUN: lldb-test breakpoints %t %s | FileCheck %s
 
 breakpoint set -f case-sensitive.c -l 3

Modified: lldb/trunk/lit/BuildScript/modes.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/BuildScript/modes.test?rev=348918&r1=348917&r2=348918&view=diff
==
--- lldb/trunk/lit/BuildScript/modes.test (original)
+++ lldb/trunk/lit/BuildScript/modes.test Wed Dec 12 00:54:14 2018
@@ -1,5 +1,3 @@
-REQUIRES: system-windows
-
 RUN: %build -n --verbose --arch=32 --mode=compile --compiler=any -o %t/foo.out 
foobar.c \
 RUN:| FileCheck --check-prefix=COMPILE %s
 
@@ -21,17 +19,17 @@ RUN:| FileCheck --check-prefix=BOTH-
 
 COMPILE: compiling foobar.c -> foo.out
 
-COMPILE-MULTI: compiling foo.c -> foo.obj
-COMPILE-MULTI: compiling bar.c -> bar.obj
+COMPILE-MULTI: compiling foo.c -> foo.o{{(bj)?}}
+COMPILE-MULTI: compiling bar.c -> bar.o{{(bj)?}}
 
 
 LINK: linking foobar.obj -> foo.exe
 
 LINK-MULTI: linking foo.obj+bar.obj -> foobar.exe
 
-BOTH: compiling foobar.c -> foobar.exe-foobar.obj
-BOTH: linking foobar.exe-foobar.obj -> foobar.exe
+BOTH: compiling foobar.c -> [[OBJFOO:foobar.exe-foobar.o(bj)?]]
+BOTH: linking [[OBJFOO]] -> foobar.exe
 
-BOTH-MULTI: compiling foo.c -> foobar.exe-foo.obj
-BOTH-MULTI: compiling bar.c -> foobar.exe-bar.obj
-BOTH-MULTI: linking foobar.exe-foo.obj+foobar.exe-bar.obj -> foobar.exe
+BOTH-MULTI: compiling foo.c -> [[OBJFOO:foobar.exe-foo.o(bj)?]]
+BOTH-MULTI: compiling bar.c -> [[OBJBAR:foobar.exe-bar.o(bj)?]]
+BOTH-MULTI: linking [[OBJFOO]]+[[OBJBAR]] -> foobar.exe

Modified: lldb/trunk/lit/BuildScript/script-args.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/BuildScript/script-args.test?rev=348918&r1=348917&r2=348918&view=diff
==
--- lldb/trunk/lit/BuildScript/script-args.test (original)
+++ lldb/trunk/lit/BuildScript/script-args.test Wed Dec 12 00:54:14 2018
@@ -1,5 +1,3 @@
-REQUIRES: system-windows
-
 RUN: %build -n --verbose --arch=32 --mode=compile --compiler=any -o %t/foo.out 
foobar.c \
 RUN:| FileCheck %s
 RUN: %build -n --verbose --arch=32 --mode=compile --compiler=any --outdir %t 
foo.c bar.c \
@@ -31,4 +29,4 @@ MULTI-INPUT-NEXT:   Clean: True
 MULTI-INPUT-NEXT:   Verbose: True
 MULTI-INPUT-NEXT:   Dryrun: True
 MULTI-INPUT-NEXT:   Inputs: foo.c
-MULTI-INPUT-NEXT:   bar.c
\ No newline at end of file
+MULTI-INPUT-NEXT:   bar.c

Added: lldb/trunk/lit/BuildScript/toolchain-clang.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/BuildScript/toolchain-clang.test?rev=348918&view=auto
==
--- lldb/trunk/lit/BuildScript/toolchain-clang.test (added)
+++ lldb/trunk/lit/BuildScript/toolchain-clang.test Wed Dec 12 00:54:14 2018
@@ -0,0 +1,14 @@
+RUN: %build -n --verbose --arch=32 --compiler=clang --mode=compile-and-link -o 
%t/foo.exe foobar.c \
+RUN:| FileCheck --check-prefix=CHECK --check-prefix=CHECK-32 %s
+
+RUN: %build -n --verbose --arch=64 --compiler=clang --mode=compile-and-link -o 
%t/foo.exe foobar.c \
+RUN:| FileCheck --check-prefix=CHECK --check-prefix=CHECK-64 %s
+
+CHECK: Cleaning {{.*}}toolchain-clang.test.tmp{{.}}foo.exe-foobar.o
+CHECK: Cleaning {{.*}}toolchain-clang.test.tmp{{.}}foo.exe
+CHECK: compiling foobar.c -> foo.exe-foobar.o
+CHECK-32: {{.*}}clang++{{(.exe)?}} -m32 -g -O0 -c -o {{.*}}foo.exe-foobar.o 
{{.*}}foobar.c
+CHECK-64: {{.*}}clang++{{(.exe)?}} -m64 -g -O0 -c -o {{.*}}foo.exe-foobar.o 
{{.*}}foobar.c
+CHECK: linking foo.ex

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

2018-12-12 Thread Pavel Labath via Phabricator via lldb-commits

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 --check-prefix=CHECK-32 %s

stella.stamenova wrote:
> There are a couple of tests for the build script that we should enable on 
> non-windows as well:
> * script-args
> * modes
> Both should pass with your change
I've enabled the two tests. I've had to tweak the "modes" test a bit to account 
for the different object file extension (.obj vs .o).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55430/new/

https://reviews.llvm.org/D55430



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Pavel Labath via Phabricator via lldb-commits

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:
  https://reviews.llvm.org/D55430?vs=177195&id=177824#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55430/new/

https://reviews.llvm.org/D55430

Files:
  lldb/trunk/lit/Breakpoint/case-sensitive.test
  lldb/trunk/lit/BuildScript/modes.test
  lldb/trunk/lit/BuildScript/script-args.test
  lldb/trunk/lit/BuildScript/toolchain-clang.test
  lldb/trunk/lit/helper/build.py

Index: lldb/trunk/lit/Breakpoint/case-sensitive.test
===
--- lldb/trunk/lit/Breakpoint/case-sensitive.test
+++ lldb/trunk/lit/Breakpoint/case-sensitive.test
@@ -1,6 +1,6 @@
 # REQUIRES: nowindows
 #
-# RUN: %clang %p/Inputs/case-sensitive.c -g -o %t
+# RUN: %build %p/Inputs/case-sensitive.c --nodefaultlib -o %t
 # RUN: lldb-test breakpoints %t %s | FileCheck %s
 
 breakpoint set -f case-sensitive.c -l 3
Index: lldb/trunk/lit/BuildScript/toolchain-clang.test
===
--- lldb/trunk/lit/BuildScript/toolchain-clang.test
+++ lldb/trunk/lit/BuildScript/toolchain-clang.test
@@ -0,0 +1,14 @@
+RUN: %build -n --verbose --arch=32 --compiler=clang --mode=compile-and-link -o %t/foo.exe foobar.c \
+RUN:| FileCheck --check-prefix=CHECK --check-prefix=CHECK-32 %s
+
+RUN: %build -n --verbose --arch=64 --compiler=clang --mode=compile-and-link -o %t/foo.exe foobar.c \
+RUN:| FileCheck --check-prefix=CHECK --check-prefix=CHECK-64 %s
+
+CHECK: Cleaning {{.*}}toolchain-clang.test.tmp{{.}}foo.exe-foobar.o
+CHECK: Cleaning {{.*}}toolchain-clang.test.tmp{{.}}foo.exe
+CHECK: compiling foobar.c -> foo.exe-foobar.o
+CHECK-32: {{.*}}clang++{{(.exe)?}} -m32 -g -O0 -c -o {{.*}}foo.exe-foobar.o {{.*}}foobar.c
+CHECK-64: {{.*}}clang++{{(.exe)?}} -m64 -g -O0 -c -o {{.*}}foo.exe-foobar.o {{.*}}foobar.c
+CHECK: linking foo.exe-foobar.o -> foo.exe
+CHECK-32: {{.*}}clang++{{(.exe)?}} -m32 -o {{.*}}foo.exe {{.*}}foo.exe-foobar.o
+CHECK-64: {{.*}}clang++{{(.exe)?}} -m64 -o {{.*}}foo.exe {{.*}}foo.exe-foobar.o
Index: lldb/trunk/lit/BuildScript/script-args.test
===
--- lldb/trunk/lit/BuildScript/script-args.test
+++ lldb/trunk/lit/BuildScript/script-args.test
@@ -1,5 +1,3 @@
-REQUIRES: system-windows
-
 RUN: %build -n --verbose --arch=32 --mode=compile --compiler=any -o %t/foo.out foobar.c \
 RUN:| FileCheck %s
 RUN: %build -n --verbose --arch=32 --mode=compile --compiler=any --outdir %t foo.c bar.c \
@@ -31,4 +29,4 @@
 MULTI-INPUT-NEXT:   Verbose: True
 MULTI-INPUT-NEXT:   Dryrun: True
 MULTI-INPUT-NEXT:   Inputs: foo.c
-MULTI-INPUT-NEXT:   bar.c
\ No newline at end of file
+MULTI-INPUT-NEXT:   bar.c
Index: lldb/trunk/lit/BuildScript/modes.test
===
--- lldb/trunk/lit/BuildScript/modes.test
+++ lldb/trunk/lit/BuildScript/modes.test
@@ -1,5 +1,3 @@
-REQUIRES: system-windows
-
 RUN: %build -n --verbose --arch=32 --mode=compile --compiler=any -o %t/foo.out foobar.c \
 RUN:| FileCheck --check-prefix=COMPILE %s
 
@@ -21,17 +19,17 @@
 
 COMPILE: compiling foobar.c -> foo.out
 
-COMPILE-MULTI: compiling foo.c -> foo.obj
-COMPILE-MULTI: compiling bar.c -> bar.obj
+COMPILE-MULTI: compiling foo.c -> foo.o{{(bj)?}}
+COMPILE-MULTI: compiling bar.c -> bar.o{{(bj)?}}
 
 
 LINK: linking foobar.obj -> foo.exe
 
 LINK-MULTI: linking foo.obj+bar.obj -> foobar.exe
 
-BOTH: compiling foobar.c -> foobar.exe-foobar.obj
-BOTH: linking foobar.exe-foobar.obj -> foobar.exe
+BOTH: compiling foobar.c -> [[OBJFOO:foobar.exe-foobar.o(bj)?]]
+BOTH: linking [[OBJFOO]] -> foobar.exe
 
-BOTH-MULTI: compiling foo.c -> foobar.exe-foo.obj
-BOTH-MULTI: compiling bar.c -> foobar.exe-bar.obj
-BOTH-MULTI: linking foobar.exe-foo.obj+foobar.exe-bar.obj -> foobar.exe
+BOTH-MULTI: compiling foo.c -> [[OBJFOO:foobar.exe-foo.o(bj)?]]
+BOTH-MULTI: compiling bar.c -> [[OBJBAR:foobar.exe-bar.o(bj)?]]
+BOTH-MULTI: linking [[OBJFOO]]+[[OBJBAR]] -> foobar.exe
Index: lldb/trunk/lit/helper/build.py
===
--- lldb/trunk/lit/helper/build.py
+++ lldb/trunk/lit/helper/build.py
@@ -1,3 +1,5 @@
+#! /usr/bin/env python
+
 from __future__ import print_function
 
 import argparse
@@ -207,7 +209,7 @@
 return 'unknown'
 
 class Builder(object):
-def __init__(self, toolchain_type, args):
+def __init__(self, toolchain_type, args, obj_ext):
 self.toolchain_type = toolchain_type
 self.inputs = args.inputs
 self.arch = args.arch
@@ -219,10 +221,50 @@
 self.mode = args.mode
 self.nodefaultlib = args.nodefaultlib
 self.verbose = args.verbose
+

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

2018-12-12 Thread Pavel Labath via Phabricator via lldb-commits

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 input command stream instead of the real one.




Comment at: source/Interpreter/CommandInterpreter.cpp:131
+// Explicitly ignore reproducer commands.
+if (command.find("reproducer") == 0)
+  return;

This seems pretty hacky, and easy to break in multiple ways:
- `alias rep reproducer`; `rep generate`
- `file /tmp/reproducer/bin/lldb`
Couldn't you just have the actual reproducer CommandObject check what mode 
we're in, and behave like a nop if there is nothing to do? (So, the commands 
would still be captured, but they wouldn't do anything when replaying.)



Comment at: source/Interpreter/CommandInterpreter.cpp:2154
+  options.SetSilent(false);
+  options.SetStopOnError(true);
+  options.SetStopOnContinue(false);

Is this correct? What if the command originally has ended with an error 
(perhaps because the user made a typo)? Will that abort the replay here?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55582/new/

https://reviews.llvm.org/D55582



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Pavel Labath via Phabricator via lldb-commits

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 mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Pavel Labath via Phabricator via lldb-commits

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 return Function(SomeLocal, 'a');
+// CHECK-NEXT:19   }
+// CHECK-NEXT:20

I'm not thrilled by all of the hard-coded information (line numbers, the 
default format of stop-information printing) here, which is irrelevant for the 
tested functionality here. This will make it very hard to update this test if 
there is ever a need for (perhaps one would like to remove the line break in 
the RUN: command once the `env LLDB_USE_NATIVE_PDB_READER=1` part disappears), 
or the default way of printing stop information in lldb changes. It doesn't 
seem like a maintainable long term strategy.



Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:1929
+}
+#include "lldb/Target/MemoryRegionInfo.h"
+CompilerDeclContext

Huh?



Comment at: llvm/include/llvm/Support/BinaryStreamArray.h:142
 
-  void drop_front() { Stream = Stream.drop_front(begin()->length()); }
 

It looks like this could be done (and tested, in llvm) separately.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55575/new/

https://reviews.llvm.org/D55575



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Pavel Labath via lldb-commits


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,
Zachary, what is the relation ship between "image base" of an object
file and its sections? Is there any way we could arrange so that the
base address of a module always belongs to one of its sections?


Historically, an image base of N was used as a way to tell the loader 
"map the file in so that byte 0 of the file is virtual address N in the 
process's address space".  as in *((char *)N) would be the first byte of 
the file in a running process.  Then, everything else in the file is 
written as an offset from N.  This includes section addresses.  So for 
example, if we use dumpbin on a simple executable we can see something 
like this:


Dump of file bin\count.exe

PE signature found

File Type: EXECUTABLE IMAGE

OPTIONAL HEADER VALUES
                   ...
        14000 image base (00014000 to 000140011FFF)
                   ...
SECTION HEADER #1
    .text name
     1000 virtual address (000140001000 to 0001400089AE)

So under this scheme, the first byte of the first section would be at 
virtual address 000140001000 in the running process.


Later, ASLR came along and threw a wrench in all of that, and so these 
days the image base is mostly meaningless.  The loader will probably 
never actually load your module at the address specified in image base.  
But the rest of the rules still hold true.  Wherever it *does* load your 
module, the first byte of .text will still be at offset 1000 from that.


So, if you want to return this value from the PE/COFF header, or even if 
you want to return the actual address that the module was loaded at, 
then no, it will never belong to any section (because the bytes at that 
address will be the PE/COFF file header).


Does this make sense?


I think it does.

I am aware that this address is not going to represent a valid address 
in target memory (the same is true for elf and macho targets), but what 
we're trying to ensure is that when we take this address, and ask the 
running target to give us the "load" address for it, it will return the 
actual place in memory (and conversely if the target is not running it 
should give us an invalid address instead of returning something bogus.


So, if I understand correctly, the PE/COFF file will always be loaded 
into one contiguous chunk of memory, ranging from ImageBase (modulo 
ASLR) to ImageBase+SizeOfImage. Then various sections are mapped into 
that range (according to their RVAs).


If that's the case, then we could model this as one big 
segment/container section/whateever, and the individual (loadable) 
sections would be sub-sections of that. Apart from solving my current 
problem, this should also improve the address lookup for these modules. 
E.g. right now if you ask lldb to lookup the address corresponding to 
the memory image of the header, it will say it does not belong anywhere, 
but that address is clearly associated with the module.


I'll try looking at what kind of changes are needed to make this happen. 
I'll start with the elf case, as I am more familiar with that (and it'll 
probably be more complicated).


thanks,
pl
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Pavel Labath via lldb-commits

Author: labath
Date: Wed Dec 12 04:35:25 2018
New Revision: 348924

URL: http://llvm.org/viewvc/llvm-project?rev=348924&view=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 teaches lldb-test to
print nested sections too.

Added:
lldb/trunk/lit/Modules/MachO/
lldb/trunk/lit/Modules/MachO/subsections.yaml
Modified:
lldb/trunk/tools/lldb-test/lldb-test.cpp

Added: lldb/trunk/lit/Modules/MachO/subsections.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/MachO/subsections.yaml?rev=348924&view=auto
==
--- lldb/trunk/lit/Modules/MachO/subsections.yaml (added)
+++ lldb/trunk/lit/Modules/MachO/subsections.yaml Wed Dec 12 04:35:25 2018
@@ -0,0 +1,106 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+#CHECK: Showing 2 sections
+#CHECK-NEXT:  Index: 0
+#CHECK-NEXT:  Name: __PAGEZERO
+#CHECK-NEXT:  Type: container
+#CHECK-NEXT:  VM size: 4294967296
+#CHECK-NEXT:  File size: 0
+#CHECK-NEXT:  There are no subsections
+#
+#CHECK:   Index: 1
+#CHECK-NEXT:  Name: __TEXT
+#CHECK-NEXT:  Type: container
+#CHECK-NEXT:  VM size: 4096
+#CHECK-NEXT:  File size: 4096
+#CHECK-NEXT:  Showing 3 subsections
+#CHECK-NEXT:Index: 0
+#CHECK-NEXT:Name: __text
+#CHECK-NEXT:Type: code
+#CHECK-NEXT:VM size: 22
+#CHECK-NEXT:File size: 22
+#
+#CHECK: Index: 1
+#CHECK-NEXT:Name: __unwind_info
+#CHECK-NEXT:Type: compact-unwind
+#CHECK-NEXT:VM size: 76
+#CHECK-NEXT:File size: 76
+#
+#CHECK: Index: 2
+#CHECK-NEXT:Name: __eh_frame
+#CHECK-NEXT:Type: eh-frame
+#CHECK-NEXT:VM size: 104
+#CHECK-NEXT:File size: 104
+
+--- !mach-o
+FileHeader:  
+  magic:   0xFEEDFACF
+  cputype: 0x0107
+  cpusubtype:  0x0003
+  filetype:0x0002
+  ncmds:   12
+  sizeofcmds:  728
+  flags:   0x0085
+  reserved:0x
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 312
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  4096
+fileoff: 0
+filesize:4096
+maxprot: 7
+initprot:5
+nsects:  3
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x00010F30
+size:22
+offset:  0x0F30
+align:   4
+reloff:  0x
+nreloc:  0
+flags:   0x8400
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x00010F48
+size:76
+offset:  0x0F48
+align:   2
+reloff:  0x
+nreloc:  0
+flags:   0x
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+  - sectname:__eh_frame
+segname: __TEXT
+addr:0x00010F98
+size:104
+offset:  0x0F98
+align:   3
+reloff:  0x
+nreloc:  0
+flags:   0x000B
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+...

Modified: lldb/trunk/tools/lldb-test/lldb-test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-test/lldb-test.cpp?rev=348924&r1=348923&r2=348924&view=diff
==
--- lldb/trunk/tools/lldb-test/lldb-test.cpp (original)
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp Wed Dec 12 04:35:25 2018
@@ -718,6 +718,37 @@ int opts::symbols::dumpSymbols(Debugger
   return HadErrors;
 }
 
+static void dumpSectionList(LinePrinter &Printer, const SectionList &List, 
bool is_subsection) {
+  size_t Count = List.GetNumSections(0);
+  if (Count == 0) {
+Printer.formatLine("There are no {0}sections", is_subsection ? "sub" : "");
+return;
+  }
+  Printer.formatLine("Showing {0} {1}sections", Count,
+ is_subsection ? "sub" : "");
+  for (size_t I = 0; I < Count; ++I) {
+auto S = List.GetSectionAtIndex(I);
+assert(S);
+AutoIndent Indent(Printer, 2);
+Printer.formatLine("In

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

2018-12-12 Thread Pavel Labath via Phabricator via lldb-commits

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 working.

Equally annoying was the fact that the LinePrinter did not terminate
it's output with a newline (instead it would leave the unterminated hanging
indent from the last NewLine() command), which meant that the shell prompt
following the lldb-test command came out wrong.

This fixes both issues by changing how newlines are handled. NewLine(),
which was ending the previous line ('\n') *and* begging the next line by
printing the indent, is now "demoted" to just printing literal "\n".
Instead, lines are now delimited via a helper Line object, which makes
sure the line is indented and terminated in an RIAA fashion. The typical
usage would be:
Printer.line() << "This text will be indented and terminated";
If one needs to do more work than it will fit into a single statement,
one can also assign the result of the line() function to a local
variable. The line will then be terminated when that object goes out of
scope.


https://reviews.llvm.org/D55597

Files:
  lit/Modules/MachO/subsections.yaml
  tools/lldb-test/FormatUtil.cpp
  tools/lldb-test/FormatUtil.h

Index: tools/lldb-test/FormatUtil.h
===
--- tools/lldb-test/FormatUtil.h
+++ tools/lldb-test/FormatUtil.h
@@ -26,27 +26,36 @@
   int CurrentIndent;
 
 public:
+  class Line {
+LinePrinter *P;
+
+  public:
+Line(LinePrinter &P) : P(&P) { P.OS.indent(P.CurrentIndent); }
+~Line();
+
+Line(Line &&RHS) : P(RHS.P) { RHS.P = nullptr; }
+void operator=(Line &&) = delete;
+
+operator llvm::raw_ostream &() { return P->OS; }
+  };
+
   LinePrinter(int Indent, llvm::raw_ostream &Stream);
 
   void Indent(uint32_t Amount = 0);
   void Unindent(uint32_t Amount = 0);
   void NewLine();
 
-  void printLine(const llvm::Twine &T);
-  void print(const llvm::Twine &T);
+  void printLine(const llvm::Twine &T) { line() << T; }
   template  void formatLine(const char *Fmt, Ts &&... Items) {
 printLine(llvm::formatv(Fmt, std::forward(Items)...));
   }
-  template  void format(const char *Fmt, Ts &&... Items) {
-print(llvm::formatv(Fmt, std::forward(Items)...));
-  }
 
   void formatBinary(llvm::StringRef Label, llvm::ArrayRef Data,
 uint32_t StartOffset);
   void formatBinary(llvm::StringRef Label, llvm::ArrayRef Data,
 uint64_t BaseAddr, uint32_t StartOffset);
 
-  llvm::raw_ostream &getStream() { return OS; }
+  Line line() { return Line(*this); }
   int getIndentLevel() const { return CurrentIndent; }
 };
 
@@ -64,12 +73,6 @@
   uint32_t Amount = 0;
 };
 
-template 
-inline llvm::raw_ostream &operator<<(LinePrinter &Printer, const T &Item) {
-  Printer.getStream() << Item;
-  return Printer.getStream();
-}
-
 } // namespace lldb_private
 
 #endif
Index: tools/lldb-test/FormatUtil.cpp
===
--- tools/lldb-test/FormatUtil.cpp
+++ tools/lldb-test/FormatUtil.cpp
@@ -14,6 +14,11 @@
 using namespace lldb_private;
 using namespace llvm;
 
+LinePrinter::Line::~Line() {
+  if (P)
+P->NewLine();
+}
+
 LinePrinter::LinePrinter(int Indent, llvm::raw_ostream &Stream)
 : OS(Stream), IndentSpaces(Indent), CurrentIndent(0) {}
 
@@ -31,39 +36,31 @@
 
 void LinePrinter::NewLine() {
   OS << "\n";
-  OS.indent(CurrentIndent);
-}
-
-void LinePrinter::print(const Twine &T) { OS << T; }
-
-void LinePrinter::printLine(const Twine &T) {
-  NewLine();
-  OS << T;
 }
 
 void LinePrinter::formatBinary(StringRef Label, ArrayRef Data,
uint32_t StartOffset) {
-  NewLine();
-  OS << Label << " (";
-  if (!Data.empty()) {
-OS << "\n";
-OS << format_bytes_with_ascii(Data, StartOffset, 32, 4,
-  CurrentIndent + IndentSpaces, true);
-NewLine();
+  if (Data.empty()) {
+line() << Label << " ()";
+return;
   }
-  OS << ")";
+  line() << Label << " (";
+  OS << format_bytes_with_ascii(Data, StartOffset, 32, 4,
+CurrentIndent + IndentSpaces, true);
+  NewLine();
+  line() << ")";
 }
 
 void LinePrinter::formatBinary(StringRef Label, ArrayRef Data,
uint64_t Base, uint32_t StartOffset) {
-  NewLine();
-  OS << Label << " (";
-  if (!Data.empty()) {
-OS << "\n";
-Base += StartOffset;
-OS << format_bytes_with_ascii(Data, Base, 32, 4,
-  CurrentIndent + IndentSpaces, true);
-NewLine();
+  if (Data.empty()) {
+line() << Label << " ()";
+return;
   }
-  OS << ")";
+  line() << Label << " (";
+  Base += StartOffset;
+  OS << format_bytes_with_ascii(Data, Base, 32, 4, CurrentIndent + IndentSpaces,
+

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

2018-12-12 Thread Pavel Labath via lldb-commits

Author: labath
Date: Wed Dec 12 06:20:28 2018
New Revision: 348928

URL: http://llvm.org/viewvc/llvm-project?rev=348928&view=rev
Log:
ELF: Simplify program header iteration

Instead of GetProgramHeaderCount+GetProgramHeaderByIndex, expose an
ArrayRef of all program headers, to enable range-based iteration.
Instead of GetSegmentDataByIndex, expose GetSegmentData, taking a
program header (reference).

This makes the code simpler by enabling range-based loops and also
allowed to remove some null checks, as it became locally obvious that
some pointers can never be null.

Modified:
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=348928&r1=348927&r2=348928&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Wed Dec 12 
06:20:28 2018
@@ -539,14 +539,13 @@ static uint32_t calc_gnu_debuglink_crc32
 
 uint32_t ObjectFileELF::CalculateELFNotesSegmentsCRC32(
 const ProgramHeaderColl &program_headers, DataExtractor &object_data) {
-  typedef ProgramHeaderCollConstIter Iter;
 
   uint32_t core_notes_crc = 0;
 
-  for (Iter I = program_headers.begin(); I != program_headers.end(); ++I) {
-if (I->p_type == llvm::ELF::PT_NOTE) {
-  const elf_off ph_offset = I->p_offset;
-  const size_t ph_size = I->p_filesz;
+  for (const ELFProgramHeader &H : program_headers) {
+if (H.p_type == llvm::ELF::PT_NOTE) {
+  const elf_off ph_offset = H.p_offset;
+  const size_t ph_size = H.p_filesz;
 
   DataExtractor segment_data;
   if (segment_data.SetData(object_data, ph_offset, ph_size) != ph_size) {
@@ -806,15 +805,11 @@ bool ObjectFileELF::SetLoadAddress(Targe
 if (section_list) {
   if (!value_is_offset) {
 bool found_offset = false;
-for (size_t i = 1, count = GetProgramHeaderCount(); i <= count; ++i) {
-  const elf::ELFProgramHeader *header = GetProgramHeaderByIndex(i);
-  if (header == nullptr)
+for (const ELFProgramHeader &H : ProgramHeaders()) {
+  if (H.p_type != PT_LOAD || H.p_offset != 0)
 continue;
 
-  if (header->p_type != PT_LOAD || header->p_offset != 0)
-continue;
-
-  value = value - header->p_vaddr;
+  value = value - H.p_vaddr;
   found_offset = true;
   break;
 }
@@ -1158,8 +1153,8 @@ size_t ObjectFileELF::GetProgramHeaderIn
 //--
 // ParseProgramHeaders
 //--
-size_t ObjectFileELF::ParseProgramHeaders() {
-  return GetProgramHeaderInfo(m_program_headers, m_data, m_header);
+bool ObjectFileELF::ParseProgramHeaders() {
+  return GetProgramHeaderInfo(m_program_headers, m_data, m_header) != 0;
 }
 
 lldb_private::Status
@@ -1705,27 +1700,6 @@ size_t ObjectFileELF::GetSectionHeaderIn
   return 0;
 }
 
-size_t ObjectFileELF::GetProgramHeaderCount() { return ParseProgramHeaders(); }
-
-const elf::ELFProgramHeader *
-ObjectFileELF::GetProgramHeaderByIndex(lldb::user_id_t id) {
-  if (!id || !ParseProgramHeaders())
-return NULL;
-
-  if (--id < m_program_headers.size())
-return &m_program_headers[id];
-
-  return NULL;
-}
-
-DataExtractor ObjectFileELF::GetSegmentDataByIndex(lldb::user_id_t id) {
-  const elf::ELFProgramHeader *segment_header = GetProgramHeaderByIndex(id);
-  if (segment_header == NULL)
-return DataExtractor();
-  return DataExtractor(m_data, segment_header->p_offset,
-   segment_header->p_filesz);
-}
-
 llvm::StringRef
 ObjectFileELF::StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const 
{
   size_t pos = symbol_name.find('@');
@@ -3181,11 +3155,9 @@ void ObjectFileELF::DumpELFProgramHeader
   s->PutCString(" ---    "
 "  - \n");
 
-  uint32_t idx = 0;
-  for (ProgramHeaderCollConstIter I = m_program_headers.begin();
-   I != m_program_headers.end(); ++I, ++idx) {
-s->Printf("[%2u] ", idx);
-ObjectFileELF::DumpELFProgramHeader(s, *I);
+  for (const auto &H : llvm::enumerate(m_program_headers)) {
+s->Format("[{0,2}] ", H.index());
+ObjectFileELF::DumpELFProgramHeader(s, H.value());
 s->EOL();
   }
 }
@@ -3305,18 +3277,13 @@ bool ObjectFileELF::GetArchitecture(Arch
   m_arch_spec.TripleOSIsUnspecifiedUnknown()) {
 // Core files don't have section headers yet they have PT_NOTE program
 // header

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

2018-12-12 Thread Zachary Turner via Phabricator via lldb-commits

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 SomeLocal = argc * 2;
+// CHECK-NEXT:18 return Function(SomeLocal, 'a');
+// CHECK-NEXT:19   }
+// CHECK-NEXT:20

labath wrote:
> I'm not thrilled by all of the hard-coded information (line numbers, the 
> default format of stop-information printing) here, which is irrelevant for 
> the tested functionality here. This will make it very hard to update this 
> test if there is ever a need for (perhaps one would like to remove the line 
> break in the RUN: command once the `env LLDB_USE_NATIVE_PDB_READER=1` part 
> disappears), or the default way of printing stop information in lldb changes. 
> It doesn't seem like a maintainable long term strategy.
I felt the same way TBH, but wanted to see what the feedback would be like.  
What do you think about just removing all the backtrace lines from the check 
output and only checking the print commands and their output?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55575/new/

https://reviews.llvm.org/D55575



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Zachary Turner via Phabricator via lldb-commits

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 SINCE LAST ACTION
  https://reviews.llvm.org/D55575/new/

https://reviews.llvm.org/D55575



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Pavel Labath via Phabricator via lldb-commits

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 return Function(SomeLocal, 'a');
+// CHECK-NEXT:19   }
+// CHECK-NEXT:20

zturner wrote:
> labath wrote:
> > I'm not thrilled by all of the hard-coded information (line numbers, the 
> > default format of stop-information printing) here, which is irrelevant for 
> > the tested functionality here. This will make it very hard to update this 
> > test if there is ever a need for (perhaps one would like to remove the line 
> > break in the RUN: command once the `env LLDB_USE_NATIVE_PDB_READER=1` part 
> > disappears), or the default way of printing stop information in lldb 
> > changes. It doesn't seem like a maintainable long term strategy.
> I felt the same way TBH, but wanted to see what the feedback would be like.  
> What do you think about just removing all the backtrace lines from the check 
> output and only checking the print commands and their output?
I think that would be **much** better.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55575/new/

https://reviews.llvm.org/D55575



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Pavel Labath via lldb-commits

Author: labath
Date: Wed Dec 12 07:46:18 2018
New Revision: 348936

URL: http://llvm.org/viewvc/llvm-project?rev=348936&view=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 now I use the section flags
(SHF_TLS) instead of the section name to set the thread-specific
property. There is no explanation in the original commit introducing
this (r153537) as to why that was done this way, but the new behavior
should be more correct.

Modified:
lldb/trunk/lit/Modules/MachO/subsections.yaml
lldb/trunk/lit/Modules/build-id-case.yaml
lldb/trunk/lit/Modules/compressed-sections.yaml
lldb/trunk/lit/Modules/elf-section-types.yaml
lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/lit/Modules/MachO/subsections.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/MachO/subsections.yaml?rev=348936&r1=348935&r2=348936&view=diff
==
--- lldb/trunk/lit/Modules/MachO/subsections.yaml (original)
+++ lldb/trunk/lit/Modules/MachO/subsections.yaml Wed Dec 12 07:46:18 2018
@@ -5,6 +5,7 @@
 #CHECK-NEXT:  Index: 0
 #CHECK-NEXT:  Name: __PAGEZERO
 #CHECK-NEXT:  Type: container
+#CHECK-NEXT:  Thread specific: no
 #CHECK-NEXT:  VM size: 4294967296
 #CHECK-NEXT:  File size: 0
 #CHECK-NEXT:  There are no subsections
@@ -12,24 +13,28 @@
 #CHECK:   Index: 1
 #CHECK-NEXT:  Name: __TEXT
 #CHECK-NEXT:  Type: container
+#CHECK-NEXT:  Thread specific: no
 #CHECK-NEXT:  VM size: 4096
 #CHECK-NEXT:  File size: 4096
 #CHECK-NEXT:  Showing 3 subsections
 #CHECK-NEXT:Index: 0
 #CHECK-NEXT:Name: __text
 #CHECK-NEXT:Type: code
+#CHECK-NEXT:Thread specific: no
 #CHECK-NEXT:VM size: 22
 #CHECK-NEXT:File size: 22
 #
 #CHECK: Index: 1
 #CHECK-NEXT:Name: __unwind_info
 #CHECK-NEXT:Type: compact-unwind
+#CHECK-NEXT:Thread specific: no
 #CHECK-NEXT:VM size: 76
 #CHECK-NEXT:File size: 76
 #
 #CHECK: Index: 2
 #CHECK-NEXT:Name: __eh_frame
 #CHECK-NEXT:Type: eh-frame
+#CHECK-NEXT:Thread specific: no
 #CHECK-NEXT:VM size: 104
 #CHECK-NEXT:File size: 104
 

Modified: lldb/trunk/lit/Modules/build-id-case.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/build-id-case.yaml?rev=348936&r1=348935&r2=348936&view=diff
==
--- lldb/trunk/lit/Modules/build-id-case.yaml (original)
+++ lldb/trunk/lit/Modules/build-id-case.yaml Wed Dec 12 07:46:18 2018
@@ -6,8 +6,6 @@
 
 # CHECK: Name: .debug_frame
 # CHECK-NEXT: Type: dwarf-frame
-# CHECK-NEXT: VM size: 0
-# CHECK-NEXT: File size: 8
 
 --- !ELF
 FileHeader:

Modified: lldb/trunk/lit/Modules/compressed-sections.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/compressed-sections.yaml?rev=348936&r1=348935&r2=348936&view=diff
==
--- lldb/trunk/lit/Modules/compressed-sections.yaml (original)
+++ lldb/trunk/lit/Modules/compressed-sections.yaml Wed Dec 12 07:46:18 2018
@@ -19,6 +19,7 @@ Sections:
 
 # CHECK: Name: .hello_elf
 # CHECK-NEXT: Type: regular
+# CHECK-NEXT: Thread specific: no
 # CHECK-NEXT: VM size: 0
 # CHECK-NEXT: File size: 28
 # CHECK-NEXT: Data:
@@ -26,6 +27,7 @@ Sections:
 
 # CHECK: Name: .bogus
 # CHECK-NEXT: Type: regular
+# CHECK-NEXT: Thread specific: no
 # CHECK-NEXT: VM size: 0
 # CHECK-NEXT: File size: 8
 # CHECK-NEXT: Data: ()

Modified: lldb/trunk/lit/Modules/elf-section-types.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/elf-section-types.yaml?rev=348936&r1=348935&r2=348936&view=diff
==
--- lldb/trunk/lit/Modules/elf-section-types.yaml (original)
+++ lldb/trunk/lit/Modules/elf-section-types.yaml Wed Dec 12 07:46:18 2018
@@ -1,32 +1,36 @@
 # RUN: yaml2obj %s > %t
 # RUN: lldb-test object-file %t | FileCheck %s
 
-# CHECK: Name: .text
+# CHECK-LABEL: Name: .text
 # CHECK-NEXT: Type: code
 
-# CHECK: Name: .debug_info
+# CHECK-LABEL: Name: .debug_info
 # CHECK-NEXT: Type: dwarf-info
-# CHECK-NEXT: VM size: 0
-# CHECK-NEXT: File size: 8
 
-# CHECK: Name: .debug_types
+# CHECK-LABEL: Name: .debug_types
 # CHECK-NEXT: Type: dwarf-types
-# CHECK-NEXT: VM size: 0
-# CHECK-NEXT: File size: 8
 
-# CHECK: Name: .debug_names
+# CHECK-LABEL: Name: .debug_names
 # CHECK-NEXT: Type: dwarf-names
-# CHECK-NEXT: VM size: 0
-# CHECK-NEXT: File size: 8
 
-# CHECK: Name: .gnu_debugaltlink
+# CHECK-LABEL: Name: .gnu_debugaltlink
 # CHECK-NEXT: Type: dwarf-gnu-debugaltlink
-# CHECK-NEXT: VM size: 0
-# CHECK-NEXT: File size: 8
 
-# CHECK: Name: __codesection
+# CHECK-LABEL: Name: __codesection
 # CHECK-NEXT: Type: code
 
+# CHECK-LABEL: Name: .data
+

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

2018-12-12 Thread Zachary Turner via Phabricator via lldb-commits

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
  https://reviews.llvm.org/D55430/new/

https://reviews.llvm.org/D55430



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Pavel Labath via Phabricator via lldb-commits

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?
Without this I got errors when trying this out on a mac (something about 
"dynamically linked executables must link to libSystem.dylib"). Doing a static 
link in this case did not seem like a bad choice, since this script doesn't 
even support building shared libraries at this point. If we get to a point 
where we want to build shared libraries in --nodefaultlib mode, then we will 
have to revisit this.

(this could be made darwin-only, but it seemed better to be consistent).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55430/new/

https://reviews.llvm.org/D55430



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Zachary Turner via lldb-commits

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
> +args.append('-nostdinc')
> +args.append('-static')
> +args.append('-c')
> 
> zturner wrote:
> > Why do we need this?
> Without this I got errors when trying this out on a mac (something about
> "dynamically linked executables must link to libSystem.dylib"). Doing a
> static link in this case did not seem like a bad choice, since this script
> doesn't even support building shared libraries at this point. If we get to
> a point where we want to build shared libraries in --nodefaultlib mode,
> then we will have to revisit this.
>
> (this could be made darwin-only, but it seemed better to be consistent).
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55430/new/
>
> https://reviews.llvm.org/D55430
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Zachary Turner via lldb-commits

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 directory to this path.

So, if you want this behavior in a supported way that isn't temporary, we
should move the code for findMatchingPDBFile() out of SymbolFilePDB and
into this function.  Then everyone is happy I think.

What do you think?

On Tue, Dec 11, 2018 at 4:42 PM Zachary Turner  wrote:

> On Tue, Dec 11, 2018 at 4:22 PM Leonard Mosescu 
> wrote:
>
>> I guess I don't see why we need a temporary solution at all.  If we can
>>> have logic that can be rolled into the SymbolVendor when we get it, and
>>> makes sense there, and is also simple, why not go with it?  Failing that,
>>> doesn't the `target symbols add` solution also work fine?
>>>
>>
>> I just checked again, and "target symbols add" depends on having a real
>> SymbolVendor implementation. So unfortunately we still need a temporary
>> solution.
>>
>
> I tried to verify this behavior, but it seems like it should already work
> out of the box?   So we're on the same page, we already do have a real
> SymbolVendor implementation, it just happens to be the *default*
> SymbolVendor implementation.  It's not the case that one doesn't exist at
> all.
>
> So anyway, when I run "target symbols add foo.exe path/to/foo.pdb"
> internally what this does is set a member variable called m_symfile_spec on
> the Module object.  Then, it calls our normal CalculateAbilities() function
> which calls loadMatchingPDBFile.
>
> I think all that needs to happen is that we need to check this field.  If
> it's empty, try to load a matching PDB file.  If it's set to something,
> then load that file instead.
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Zachary Turner via lldb-commits

Author: zturner
Date: Wed Dec 12 09:17:53 2018
New Revision: 348941

URL: http://llvm.org/viewvc/llvm-project?rev=348941&view=rev
Log:
[ast] CreateParameterDeclaration should use an appropriate DeclContext.

Previously CreateParameterDeclaration was always using the translation
unit DeclContext.  We would later go and add parameters to the
FunctionDecl, but internally clang makes a copy when you do this, and
we'd end up with ParmVarDecl's at the global scope as well as in the
function scope.

This fixes the issue.  It's hard to say whether this will introduce
a behavioral change in name lookup, but I know there have been several
hacks introduced in previous years to deal with collisions between
various types of variables, so there's a chance that this patch could
obviate one of those hacks.

Differential Revision: https://reviews.llvm.org/D55571

Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=348941&r1=348940&r2=348941&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Wed Dec 12 09:17:53 2018
@@ -401,7 +401,8 @@ public:
type_quals, cc);
   }
 
-  clang::ParmVarDecl *CreateParameterDeclaration(const char *name,
+  clang::ParmVarDecl *CreateParameterDeclaration(clang::DeclContext *decl_ctx,
+ const char *name,
  const CompilerType 
¶m_type,
  int storage);
 

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=348941&r1=348940&r2=348941&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Wed Dec 
12 09:17:53 2018
@@ -3407,8 +3407,9 @@ size_t DWARFASTParserClang::ParseChildPa
 function_param_types.push_back(type->GetForwardCompilerType());
 
 clang::ParmVarDecl *param_var_decl =
-m_ast.CreateParameterDeclaration(
-name, type->GetForwardCompilerType(), storage);
+m_ast.CreateParameterDeclaration(containing_decl_ctx, name,
+ 
type->GetForwardCompilerType(),
+ storage);
 assert(param_var_decl);
 function_param_decls.push_back(param_var_decl);
 

Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp?rev=348941&r1=348940&r2=348941&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp Wed 
Dec 12 09:17:53 2018
@@ -635,8 +635,8 @@ lldb::FunctionSP SymbolFileNativePDB::Cr
 PdbCompilandSymId param_uid(func_id.modi, record_offset);
 TypeSP type_sp = GetOrCreateType(param_type);
 clang::ParmVarDecl *param = m_clang->CreateParameterDeclaration(
-param_name.str().c_str(), type_sp->GetForwardCompilerType(),
-clang::SC_None);
+function_decl, param_name.str().c_str(),
+type_sp->GetForwardCompilerType(), clang::SC_None);
 lldbassert(m_uid_to_decl.count(toOpaqueUid(param_uid)) == 0);
 
 m_uid_to_decl[toOpaqueUid(param_uid)] = param;

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp?rev=348941&r1=348940&r2=348941&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp Wed Dec 12 
09:17:53 2018
@@ -932,7 +932,8 @@ PDBASTParser::GetDeclForSymbol(const llv
 continue;
 
   clang::ParmVarDecl *param = m_ast.CreateParameterDeclaration(
-  nullptr, arg_type->GetForwardCompilerType(), clang::SC_None);
+  decl, nullptr, arg_type->GetForwardCompilerType(),

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

2018-12-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

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 went through all the changes and I didn't see anything that 
looked out of the ordinary.

I left the static cast because it was suggested to me before in a review, but 
happy to remove it as there is a consensus that it's worse :-)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55584/new/

https://reviews.llvm.org/D55584



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Zachary Turner via Phabricator via lldb-commits

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:
  https://reviews.llvm.org/D55571?vs=177761&id=177864#toc

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55571/new/

https://reviews.llvm.org/D55571

Files:
  include/lldb/Symbol/ClangASTContext.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Symbol/ClangASTContext.cpp


Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -2214,11 +2214,11 @@
 }
 
 ParmVarDecl *ClangASTContext::CreateParameterDeclaration(
-const char *name, const CompilerType ¶m_type, int storage) {
+clang::DeclContext *decl_ctx, const char *name,
+const CompilerType ¶m_type, int storage) {
   ASTContext *ast = getASTContext();
   assert(ast != nullptr);
-  return ParmVarDecl::Create(*ast, ast->getTranslationUnitDecl(),
- SourceLocation(), SourceLocation(),
+  return ParmVarDecl::Create(*ast, decl_ctx, SourceLocation(), 
SourceLocation(),
  name && name[0] ? &ast->Idents.get(name) : 
nullptr,
  ClangUtil::GetQualType(param_type), nullptr,
  (clang::StorageClass)storage, nullptr);
Index: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -635,8 +635,8 @@
 PdbCompilandSymId param_uid(func_id.modi, record_offset);
 TypeSP type_sp = GetOrCreateType(param_type);
 clang::ParmVarDecl *param = m_clang->CreateParameterDeclaration(
-param_name.str().c_str(), type_sp->GetForwardCompilerType(),
-clang::SC_None);
+function_decl, param_name.str().c_str(),
+type_sp->GetForwardCompilerType(), clang::SC_None);
 lldbassert(m_uid_to_decl.count(toOpaqueUid(param_uid)) == 0);
 
 m_uid_to_decl[toOpaqueUid(param_uid)] = param;
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -932,7 +932,8 @@
 continue;
 
   clang::ParmVarDecl *param = m_ast.CreateParameterDeclaration(
-  nullptr, arg_type->GetForwardCompilerType(), clang::SC_None);
+  decl, nullptr, arg_type->GetForwardCompilerType(),
+  clang::SC_None);
   if (param)
 params.push_back(param);
 }
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3407,8 +3407,9 @@
 function_param_types.push_back(type->GetForwardCompilerType());
 
 clang::ParmVarDecl *param_var_decl =
-m_ast.CreateParameterDeclaration(
-name, type->GetForwardCompilerType(), storage);
+m_ast.CreateParameterDeclaration(containing_decl_ctx, name,
+ 
type->GetForwardCompilerType(),
+ storage);
 assert(param_var_decl);
 function_param_decls.push_back(param_var_decl);
 
Index: include/lldb/Symbol/ClangASTContext.h
===
--- include/lldb/Symbol/ClangASTContext.h
+++ include/lldb/Symbol/ClangASTContext.h
@@ -401,7 +401,8 @@
type_quals, cc);
   }
 
-  clang::ParmVarDecl *CreateParameterDeclaration(const char *name,
+  clang::ParmVarDecl *CreateParameterDeclaration(clang::DeclContext *decl_ctx,
+ const char *name,
  const CompilerType 
¶m_type,
  int storage);
 


Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -2214,11 +2214,11 @@
 }
 
 ParmVarDecl *ClangASTContext::CreateParameterDeclaration(
-const char *name, const CompilerType ¶m_type, int storage) {
+clang::DeclContext *decl_ctx, const char *name,
+const CompilerType ¶m_type, int storage) {
   ASTContext *a

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

2018-12-12 Thread Shafik Yaghmour via Phabricator via lldb-commits

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; !success && i < m_header.ncmds; ++i) {

This looks like a big chunk of unrelated changes.



Comment at: 
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1725
 // not support qHostInfo or qWatchpointSupportInfo packets.
-if (atype == llvm::Triple::mips || atype == llvm::Triple::mipsel ||
-atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el ||
-atype == llvm::Triple::ppc64le)
-  after = false;
-else
-  after = true;
+after =
+!(atype == llvm::Triple::mips || atype == llvm::Triple::mipsel ||

Not sure if I like this one, the previous form feels more readable.



Comment at: source/Plugins/Process/mach-core/ProcessMachCore.cpp:305
   std::string corefile_identifier = core_objfile->GetIdentifierString();
-  if (found_main_binary_definitively == false 
-  && corefile_identifier.find("Darwin Kernel") != std::string::npos) {
-  UUID uuid;
-  addr_t addr = LLDB_INVALID_ADDRESS;
-  if (corefile_identifier.find("UUID=") != std::string::npos) {
-  size_t p = corefile_identifier.find("UUID=") + strlen("UUID=");
-  std::string uuid_str = corefile_identifier.substr(p, 36);
-  uuid.SetFromStringRef(uuid_str);
-  }
-  if (corefile_identifier.find("stext=") != std::string::npos) {
-  size_t p = corefile_identifier.find("stext=") + strlen("stext=");
-  if (corefile_identifier[p] == '0' && corefile_identifier[p + 1] == 
'x') {
-  errno = 0;
-  addr = ::strtoul(corefile_identifier.c_str() + p, NULL, 16);
-  if (errno != 0 || addr == 0)
-  addr = LLDB_INVALID_ADDRESS;
-  }
-  }
-  if (uuid.IsValid() && addr != LLDB_INVALID_ADDRESS) {
-  m_mach_kernel_addr = addr;
-  found_main_binary_definitively = true;
-  if (log)
-log->Printf("ProcessMachCore::DoLoadCore: Using the kernel address 
0x%" PRIx64
-" from LC_IDENT/LC_NOTE 'kern ver str' string: '%s'", 
addr, corefile_identifier.c_str());
+  if (!found_main_binary_definitively &&
+  corefile_identifier.find("Darwin Kernel") != std::string::npos) {

Another big chunk of unrelated changes.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp:1117
   if ((distance(range.first, range.second) == 1) &&
-  range.first->end_sequence == true) {
+  static_cast(range.first->end_sequence)) {
 *range.first = state;

The `static_cast` feels less readable here.



Comment at: source/Symbol/LineTable.cpp:217
 if (prev_pos->file_addr == search_entry.file_addr &&
-prev_pos->is_terminal_entry == false)
+!static_cast(prev_pos->is_terminal_entry))
   --pos;

static_cast



Comment at: source/Symbol/LineTable.cpp:236
 // terminating entry for a previous line...
-if (pos != end_pos && pos->is_terminal_entry == false) {
+if (pos != end_pos && !static_cast(pos->is_terminal_entry)) {
   uint32_t match_idx = std::distance(begin_pos, pos);

static_cast



Comment at: source/Symbol/Type.cpp:749
 bool TypeAndOrName::IsEmpty() const {
-  if ((bool)m_type_name || (bool)m_type_pair)
-return false;
-  else
-return true;
+  return !((bool)m_type_name || (bool)m_type_pair);
 }

Oh, even worse C-style casts ... can we remove these. I am assuming they are 
triggering a member conversion function.



Comment at: source/Utility/RegisterValue.cpp:478
   if (this == &rhs)
-return rhs.m_type == eTypeInvalid ? false : true;
+return rhs.m_type != eTypeInvalid;
 

Nice one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55584/new/

https://reviews.llvm.org/D55584



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Zachary Turner via Phabricator via lldb-commits

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/lit/SymbolFile/NativePDB/Inputs/local-variables.lldbinit
  lldb/lit/SymbolFile/NativePDB/local-variables.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.h
  lldb/source/Plugins/SymbolFile/NativePDB/PdbSymUid.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Symbol/ClangASTContext.cpp
  llvm/include/llvm/Support/BinaryStreamArray.h

Index: llvm/include/llvm/Support/BinaryStreamArray.h
===
--- llvm/include/llvm/Support/BinaryStreamArray.h
+++ llvm/include/llvm/Support/BinaryStreamArray.h
@@ -139,7 +139,7 @@
 this->Skew = Skew;
   }
 
-  void drop_front() { Stream = Stream.drop_front(begin()->length()); }
+  void drop_front() { Skew += begin()->length(); }
 
 private:
   BinaryStreamRef Stream;
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -2218,10 +2218,13 @@
 const CompilerType ¶m_type, int storage) {
   ASTContext *ast = getASTContext();
   assert(ast != nullptr);
-  return ParmVarDecl::Create(*ast, decl_ctx, SourceLocation(), SourceLocation(),
- name && name[0] ? &ast->Idents.get(name) : nullptr,
- ClangUtil::GetQualType(param_type), nullptr,
- (clang::StorageClass)storage, nullptr);
+  auto *decl =
+  ParmVarDecl::Create(*ast, decl_ctx, SourceLocation(), SourceLocation(),
+  name && name[0] ? &ast->Idents.get(name) : nullptr,
+  ClangUtil::GetQualType(param_type), nullptr,
+  (clang::StorageClass)storage, nullptr);
+  decl_ctx->addDecl(decl);
+  return decl;
 }
 
 void ClangASTContext::SetFunctionParameters(FunctionDecl *function_decl,
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -109,10 +109,10 @@
VariableList &variables) override;
 
   size_t ParseTypes(const SymbolContext &sc) override;
-  size_t ParseVariablesForContext(const SymbolContext &sc) override {
-return 0;
-  }
+  size_t ParseVariablesForContext(const SymbolContext &sc) override;
 
+  CompilerDecl GetDeclForUID(lldb::user_id_t uid) override;
+  CompilerDeclContext GetDeclContextForUID(lldb::user_id_t uid) override;
   CompilerDeclContext GetDeclContextContainingUID(lldb::user_id_t uid) override;
   Type *ResolveTypeUID(lldb::user_id_t type_uid) override;
   llvm::Optional GetDynamicArrayInfoForUID(
@@ -194,20 +194,30 @@
  clang::MSInheritanceAttr::Spelling inheritance);
 
   lldb::FunctionSP GetOrCreateFunction(PdbCompilandSymId func_id,
-   const SymbolContext &sc);
+   CompileUnit &comp_unit);
   lldb::CompUnitSP GetOrCreateCompileUnit(const CompilandIndexItem &cci);
   lldb::TypeSP GetOrCreateType(PdbTypeSymId type_id);
   lldb::TypeSP GetOrCreateType(llvm::codeview::TypeIndex ti);
   lldb::VariableSP GetOrCreateGlobalVariable(PdbGlobalSymId var_id);
+  Block &GetOrCreateBlock(PdbCompilandSymId block_id);
+  lldb::VariableSP GetOrCreateLocalVariable(PdbCompilandSymId scope_id,
+PdbCompilandSymId var_id,
+bool is_param);
 
   lldb::FunctionSP CreateFunction(PdbCompilandSymId func_id,
-  const SymbolContext &sc);
+  CompileUnit &comp_unit);
+  Block &CreateBlock(PdbCompilandSymId block_id);
+  lldb::VariableSP CreateLocalVariable(PdbCompilandSymId scope_id,
+   PdbCompilandSymId var_id, bool is_param);
   lldb::CompUnitSP CreateCompileUnit(const CompilandIndexItem &cci);
   lldb::TypeSP CreateType(PdbTypeSymId type_id);
   lldb::TypeSP CreateAndCacheType(PdbTypeSymId type_id);
   lldb::VariableSP CreateGlobalVariable(PdbGlobalSymId var_id);
   lldb::VariableSP CreateConstantSymbol(PdbGlobalSymId var_id,
 const llvm::codeview::CVSymbol &cvs);
+  size_t ParseVariablesForCompileUnit(CompileUnit &comp_unit,
+  VariableList &variables);
+  size_t ParseVariablesForBlock(PdbCompilandSymId block_id);
 
   llvm::B

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

2018-12-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

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
  lit/Reproducer/TestCommandRepro.test
  source/API/SBDebugger.cpp
  source/Commands/CommandObjectReproducer.cpp
  source/Core/Debugger.cpp
  source/Interpreter/CommandInterpreter.cpp

Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
@@ -74,6 +75,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace llvm;
 
 static const char *k_white_space = " \t\v";
 
@@ -116,6 +118,42 @@
   eEchoCommentCommands = 5
 };
 
+class lldb_private::CommandProvider
+: public repro::Provider {
+public:
+  CommandProvider(const FileSpec &directory) : Provider(directory) {
+m_info.name = "command-interpreter";
+m_info.files.push_back("command-interpreter.txt");
+  }
+
+  void CaptureCommand(std::string command) {
+m_commands.push_back(std::move(command));
+  }
+
+  void Keep() override {
+FileSpec file =
+GetRoot().CopyByAppendingPathComponent("command-interpreter.txt");
+
+std::error_code ec;
+llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::F_Text);
+
+if (ec)
+  return;
+
+for (auto &command : m_commands)
+  os << command << '\n';
+  }
+
+  void Discard() override {}
+
+  static char ID;
+
+private:
+  std::vector m_commands;
+};
+
+char CommandProvider::ID = 0;
+
 ConstString &CommandInterpreter::GetStaticBroadcasterClass() {
   static ConstString class_name("lldb.commandInterpreter");
   return class_name;
@@ -141,6 +179,9 @@
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
   CheckInWithManager();
   m_collection_sp->Initialize(g_properties);
+
+  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
+m_provider = &g->GetOrCreate();
 }
 
 bool CommandInterpreter::GetExpandRegexAliases() const {
@@ -1679,6 +1720,9 @@
 
   Status error(PreprocessCommand(command_string));
 
+  if (m_provider)
+m_provider->CaptureCommand(original_command_string);
+
   if (error.Fail()) {
 result.AppendError(error.AsCString());
 result.SetStatus(eReturnStatusFailed);
@@ -2074,6 +2118,45 @@
   return position;
 }
 
+void CommandInterpreter::ReplayCommands(CommandReturnObject &result) {
+  repro::Loader *loader = repro::Reproducer::Instance().GetLoader();
+  if (!loader) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);
+return;
+  }
+
+  auto provider_info = loader->GetProviderInfo("command-interpreter");
+  if (!provider_info) {
+result.AppendErrorWithFormat("no provider for command interpreter.");
+result.SetStatus(eReturnStatusFailed);
+return;
+  }
+
+  if (provider_info->files.empty()) {
+result.AppendErrorWithFormat(
+"provider for command interpreter contains no files.");
+result.SetStatus(eReturnStatusFailed);
+return;
+  }
+
+  FileSpec command_file = loader->GetRoot().CopyByAppendingPathComponent(
+  provider_info->files.front());
+
+  // Remember current batch mode.
+  const bool saved_batch = SetBatchCommandMode(true);
+
+  CommandInterpreterRunOptions options;
+  options.SetSilent(false);
+  options.SetStopOnError(false);
+  options.SetStopOnContinue(false);
+
+  // Run the commands in a new execution context.
+  HandleCommandsFromFile(command_file, nullptr, options, result);
+
+  // Reset current batch mode.
+  SetBatchCommandMode(saved_batch);
+}
+
 void CommandInterpreter::SourceInitFile(bool in_cwd,
 CommandReturnObject &result) {
   FileSpec init_file;
Index: source/Core/Debugger.cpp
===
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Host/Terminal.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/OptionValueSInt64.h"
@@ -1692,6 +1693,16 @@
   return GetDummyTarget();
 }
 
+void Debugger::RunReplay() {
+  // Create a command return object and hook up its streams.
+  CommandReturnObject result;
+  result.SetImmediateOutputStream(GetOutputFile());
+  result.SetImmediateErrorStream(GetErrorFile());
+
+  // Replay commands from reproducer.
+  GetCommandInterpreter().ReplayCommands(result);
+}
+
 S

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

2018-12-12 Thread Greg Clayton via lldb-commits

Author: gclayton
Date: Wed Dec 12 10:14:27 2018
New Revision: 348951

URL: http://llvm.org/viewvc/llvm-project?rev=348951&view=rev
Log:
NFC: fix compiler warning about code never being executed when compiling on non 
windows platform.


Modified:
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=348951&r1=348950&r2=348951&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Wed Dec 12 
10:14:27 2018
@@ -78,12 +78,13 @@ bool ShouldAddLine(uint32_t requested_li
 } // namespace
 
 static bool ShouldUseNativeReader() {
-#if !defined(_WIN32)
-  return true;
-#endif
+#if defined(_WIN32)
   llvm::StringRef use_native = ::getenv("LLDB_USE_NATIVE_PDB_READER");
   return use_native.equals_lower("on") || use_native.equals_lower("yes") ||
  use_native.equals_lower("1") || use_native.equals_lower("true");
+#else
+  return true;
+#endif
 }
 
 void SymbolFilePDB::Initialize() {


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Jim Ingham via Phabricator via lldb-commits

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 - 
that makes running it easy but means it doesn't share the original function's 
context.   Then we smuggle all the local Decls into the wrapper function using 
the FindExternalLexicalDecls from our ClangASTSource.  Unfortunately, that 
lookup gets consulted after the namespace/class lookup is done.  So names in 
the current class or namespace context shadow local variables/arguments.

This will clean up the AST Context's we make but sadly I don't think it will 
affect the expression parser name lookup issues.  If you wanted to test that 
proposition, you can do:

settings set target.experimental.inject-local-vars false

then try some tests where a local variable or parameter name shadows an ivar or 
type name in the current context.  That will find the wrong variable when this 
is false.  I'm sure there are also tests for this, but I don't remember where 
they are off-hand.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55571/new/

https://reviews.llvm.org/D55571



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Stefan Gränitz via Phabricator via lldb-commits

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}
+   --targetDir=$
+   --cfgBldDir=$

sgraenitz wrote:
> JDevlieghere wrote:
> > stella.stamenova wrote:
> > > I have a vague recollection that using TARGET_PROPERTY didn't work for 
> > > some changes I was working on a few months ago, but I don't remember the 
> > > details. It would be good to make sure that this works on mac, linux and 
> > > windows all before you commit.
> > IIRC and if we’re talking about the same thing, the generator expressions 
> > didn’t work because we were using them in a configured file (the 
> > lldb-dotest wrapper). Anyway, still worth double checking here.
> Yes, whenever we use a generator expression, there must be a processing step 
> at generation time that expands the expression. Debugging can become hairy, 
> because `message($)` doesn't have such a step (but the 
> `COMMAND` in `add_custom_target` has one).
> 
> Anyway, I had another look and indeed the genex's are not necessary here. I 
> would really like to keep the info in the target properties though, because 
> that's where CMake needs it and we can avoid duplicating it to global 
> variables.
> 
> I will double check that this works on the common platforms before I commit 
> all my related changes.
BTW removed generator expressions here and now use `get_target_property` to 
fetch property values at configuration time.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55332/new/

https://reviews.llvm.org/D55332



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Adrian Prantl via Phabricator via lldb-commits

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.


https://reviews.llvm.org/D55607

Files:
  examples/python/crashlog.py


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -246,6 +246,25 @@
 self.identifier = identifier
 self.version = version
 
+def find_matching_slice(self):
+dwarfdump_cmd_output = commands.getoutput(
+'dwarfdump --uuid "%s"' % self.path)
+self_uuid = self.get_uuid()
+for line in dwarfdump_cmd_output.splitlines():
+match = self.dwarfdump_uuid_regex.search(line)
+if match:
+dwarf_uuid_str = match.group(1)
+dwarf_uuid = uuid.UUID(dwarf_uuid_str)
+if self_uuid == dwarf_uuid:
+self.resolved_path = self.path
+self.arch = match.group(2)
+return True
+if not self.resolved_path:
+self.unavailable = True
+print("error\nerror: unable to locate '%s' with UUID %s"
+  % (self.path, uuid_str))
+return False
+
 def locate_module_and_debug_symbols(self):
 # Don't load a module twice...
 if self.resolved:
@@ -277,22 +296,25 @@
 plist['DBGSymbolRichExecutable'])
 self.resolved_path = self.path
 if not self.resolved_path and os.path.exists(self.path):
-dwarfdump_cmd_output = commands.getoutput(
-'dwarfdump --uuid "%s"' % self.path)
-self_uuid = self.get_uuid()
-for line in dwarfdump_cmd_output.splitlines():
-match = self.dwarfdump_uuid_regex.search(line)
-if match:
-dwarf_uuid_str = match.group(1)
-dwarf_uuid = uuid.UUID(dwarf_uuid_str)
-if self_uuid == dwarf_uuid:
-self.resolved_path = self.path
-self.arch = match.group(2)
-break
-if not self.resolved_path:
-self.unavailable = True
-print "error\nerror: unable to locate '%s' with UUID 
%s" % (self.path, uuid_str)
+if not self.find_matching_slice():
 return False
+if not self.resolved_path and not os.path.exists(self.path):
+try:
+import subprocess
+dsym = subprocess.check_output(
+["/usr/bin/mdfind",
+ "com_apple_xcode_dsym_uuids == %s"%uuid_str])[:-1]
+if dsym and os.path.exists(dsym):
+print('falling back to binary inside "%s"'%dsym)
+self.symfile = dsym
+dwarf_dir = os.path.join(dsym, 
'Contents/Resources/DWARF')
+for filename in os.listdir(dwarf_dir):
+self.path = os.path.join(dwarf_dir, filename)
+if not self.find_matching_slice():
+return False
+break
+except:
+pass
 if (self.resolved_path and os.path.exists(self.resolved_path)) or (
 self.path and os.path.exists(self.path)):
 print 'ok'


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -246,6 +246,25 @@
 self.identifier = identifier
 self.version = version
 
+def find_matching_slice(self):
+dwarfdump_cmd_output = commands.getoutput(
+'dwarfdump --uuid "%s"' % self.path)
+self_uuid = self.get_uuid()
+for line in dwarfdump_cmd_output.splitlines():
+match = self.dwarfdump_uuid_regex.search(line)
+if match:
+dwarf_uuid_str = match.group(1)
+dwarf_uuid = uuid.UUID(dwarf_uuid_str)
+if self_uuid == dwarf_uuid:
+self.resolved_path = self.path
+self.arch = match.group(2)
+return True
+if not self.resolved_path:
+self.unavailable = True
+print("error\nerror: unable to locate '%s' with UUID %s"
+  % (self.path, uuid_str))
+

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

2018-12-12 Thread Kuba (Brecka) Mracek via Phabricator via lldb-commits

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

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55116/new/

https://reviews.llvm.org/D55116



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Stefan Gränitz via Phabricator via lldb-commits

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/

https://reviews.llvm.org/D55116



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Adrian Prantl via Phabricator via lldb-commits

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:
  examples/python/crashlog.py


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
@@ -300,6 +300,7 @@
 return False
 if not self.resolved_path and not os.path.exists(self.path):
 try:
+import pdb; pdb.set_trace()
 import subprocess
 dsym = subprocess.check_output(
 ["/usr/bin/mdfind",


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
@@ -300,6 +300,7 @@
 return False
 if not self.resolved_path and not os.path.exists(self.path):
 try:
+import pdb; pdb.set_trace()
 import subprocess
 dsym = subprocess.check_output(
 ["/usr/bin/mdfind",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Adrian Prantl via Phabricator via lldb-commits

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
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([^/]+)/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Adrian Prantl via Phabricator via lldb-commits

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
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) 
+([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([0-9a-zA-Z_]+) 
+<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([0-9a-zA-Z_]+) 
+/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:


Index: examples/python/crashlog.py
===
--- examples/python/crashlog.py
+++ examples/python/crashlog.py
@@ -94,11 +94,11 @@
 thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
 app_backtrace_regex = re.compile(
 '^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-frame_regex = re.compile('^([0-9]+)\s+([^ ]+)\s+(0x[0-9a-fA-F]+) +(.*)')
+frame_regex = re.compile('^([0-9]+)\s+(.+?)\s+(0x[0-9a-fA-F]+) +(.*)')
 image_regex_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^<]+)<([-0-9a-fA-F]+)> (.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([0-9a-zA-Z_]+) +<([-0-9a-fA-F]+)> (.*)')
 image_regex_no_uuid = re.compile(
-'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?([^ ]+) +([^/]+)/(.*)')
+'(0x[0-9a-fA-F]+)[- ]+(0x[0-9a-fA-F]+) +[+]?(.+?) +([0-9a-zA-Z_]+) +/(.*)')
 empty_line_regex = re.compile('^$')
 
 class Thread:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Jason Molenda via Phabricator via lldb-commits

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.  But this is a good first step, and it gets us the 
source-level information for symbolicating the crash.  More advanced crashlog 
users may want to look at the assembly instructions around the crash site, and 
for that we need the actual binary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55607/new/

https://reviews.llvm.org/D55607



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Leonard Mosescu via Phabricator via lldb-commits

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
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.lldbinit
  lit/Minidump/Windows/Sigsegv/Inputs/sigsegv.pdb
  lit/Minidump/Windows/Sigsegv/sigsegv.test
  source/Commands/CommandObjectTarget.cpp
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpParser.h
  source/Plugins/Process/minidump/ProcessMinidump.cpp
  source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
  source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.h
  source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp
  source/Plugins/SymbolFile/NativePDB/PdbIndex.h
  source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Index: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -30,6 +30,7 @@
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Symbol/VariableList.h"
+#include "lldb/Utility/UUID.h"
 
 #include "llvm/DebugInfo/CodeView/CVRecord.h"
 #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
@@ -102,33 +103,57 @@
 }
 
 static std::unique_ptr
-loadMatchingPDBFile(std::string exe_path, llvm::BumpPtrAllocator &allocator) {
-  // Try to find a matching PDB for an EXE.
+loadMatchingPDBFile(lldb_private::ObjectFile& obj_file,
+llvm::BumpPtrAllocator &allocator) {
   using namespace llvm::object;
-  auto expected_binary = createBinary(exe_path);
 
-  // If the file isn't a PE/COFF executable, fail.
-  if (!expected_binary) {
-llvm::consumeError(expected_binary.takeError());
-return nullptr;
-  }
-  OwningBinary binary = std::move(*expected_binary);
+  // Try to find a matching PDB for an EXE.
+  std::string pdb_file;
+  llvm::codeview::GUID guid;
 
-  auto *obj = llvm::dyn_cast(binary.getBinary());
-  if (!obj)
-return nullptr;
-  const llvm::codeview::DebugInfo *pdb_info = nullptr;
+  const auto module = obj_file.GetModule();
+  const auto symbol_file_filespec = module->GetSymbolFileFileSpec();
+  if (symbol_file_filespec.IsResolved()) {
+// If we have the symbol file filespec set, use it
+pdb_file = symbol_file_filespec.GetPath();
+
+// Extract the debug GUID from the ObjectFile
+lldb_private::UUID uuid;
+obj_file.GetUUID(&uuid);
+auto uuid_bytes = uuid.GetBytes();
+if (uuid_bytes.size() != sizeof(guid) + 4) // CvRecordPdb70
+  return nullptr;
+memcpy(&guid, uuid_bytes.data(), sizeof(guid));
+  } else {
+// Use the PDB path embedded in the binary
+auto expected_binary = createBinary(obj_file.GetFileSpec().GetPath());
+if (!expected_binary) {
+  llvm::consumeError(expected_binary.takeError());
+  return nullptr;
+}
 
-  // If it doesn't have a debug directory, fail.
-  llvm::StringRef pdb_file;
-  auto ec = obj->getDebugPDBInfo(pdb_info, pdb_file);
-  if (ec)
-return nullptr;
+OwningBinary binary = std::move(*expected_binary);
+
+auto *obj = llvm::dyn_cast(binary.getBinary());
+if (!obj)
+  return nullptr;
+
+const llvm::codeview::DebugInfo *pdb_info = nullptr;
+llvm::StringRef pdb_file_name_ref;
+if (obj->getDebugPDBInfo(pdb_info, pdb_file_name_ref)) {
+  // If it doesn't have a debug directory, fail.
+  return nullptr;
+}
+pdb_file = pdb_file_name_ref;
+
+// Extract the debug GUID from the debug directory
+memcpy(&guid, pdb_info->PDB70.Signature, sizeof(guid));
+  }
 
   // if the file doesn't exist, is not a pdb, or doesn't have a matching guid,
   // fail.
   llvm::file_magic magic;
-  ec = llvm::identify_magic(pdb_file, magic);
+  auto ec = llvm::identify_magic(pdb_file, magic);
   if (ec || magic != llvm::file_magic::pdb)
 return nullptr;
   std::unique_ptr pdb = loadPDBFile(pdb_file, allocator);
@@ -140,11 +165,11 @@
 llvm::consumeError(expected_info.takeError());
 return nullptr;
   }
-  llvm::codeview::GUID guid;
-  memcpy(&guid, pdb_info->PDB70.Signature, 16);
 
+  // TODO: we need to compare the age, in addition to the GUID
   if (expected_info->getGuid() != guid)
 return nullptr;
+
   return pdb;
 }
 
@@ -521,7 +546,7 @@
   if (!m_index) {
 // Lazily load and match the PDB file, but only do this once.
 std::unique_ptr file_up =
-loadMatchingPDBFile(m_obj_file->GetFileSpec().GetPath(), m_allocator);
+loadMatchingPDBFile(*m_obj_file, m_allocator);
 
 if (!file_up) {
   auto module_sp = m_obj_file->GetModule();
@@ -1459,11 +1484,10 @@
 llvm::Optional modi = m_index->GetModuleIndexForVa(file_addr);
 if (!modi)
   return 0;
-CompilandIndexItem *cci = m_index->compilands().GetCompiland(*modi);
-if (!cci)

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

2018-12-12 Thread Leonard Mosescu via Phabricator via lldb-commits

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 don't (yet) have a ObjectFilePDB so the 
SymbolFileNativePDB always points to the associated PE binary. 

the check itself seems valuable as a diagnostic but not strictly required. 
Should I add a TODO comment and/or open a bug to revisit this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55142/new/

https://reviews.llvm.org/D55142



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Greg Clayton via Phabricator via lldb-commits

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 MinidumpModule with the lowest address. A 
bug existed where it wasn't doing that due to "exists" variable being inverted. 
"exists" was set to try if it was inserted. Furthermore, the order of the 
modules would be modified by sorting all modules from low address to high 
address (using MinidumpModule::base_of_image). This fix also maintains the 
original order which means your executable is at index 0 as intended instead of 
some random shared library.

Tests were added to ensure this functionality doesn't regress.


https://reviews.llvm.org/D55614

Files:
  source/Plugins/Process/minidump/MinidumpParser.cpp
  unittests/Process/minidump/Inputs/modules-dup-min-addr.dmp
  unittests/Process/minidump/Inputs/modules-order.dmp
  unittests/Process/minidump/MinidumpParserTest.cpp

Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -533,3 +533,41 @@
 }
   }
 }
+
+TEST_F(MinidumpParserTest, MinidumpDuplicateModuleMinAddress) {
+  SetUpData("modules-dup-min-addr.dmp");
+  // Test that if we have two modules in the module list:
+  ///tmp/a with range [0x2000-0x3000)
+  ///tmp/a with range [0x1000-0x2000)
+  // That we end up with one module in the filtered list with the
+  // range [0x1000-0x2000). MinidumpParser::GetFilteredModuleList() is
+  // trying to ensure that if we have the same module mentioned more than
+  // one time, we pick the one with the lowest base_of_image.
+  std::vector filtered_modules =
+  parser->GetFilteredModuleList();
+  EXPECT_EQ(1, filtered_modules.size());
+  EXPECT_EQ(0x1000, filtered_modules[0]->base_of_image);
+}
+
+TEST_F(MinidumpParserTest, MinidumpModuleOrder) {
+  SetUpData("modules-order.dmp");
+  // Test that if we have two modules in the module list:
+  ///tmp/a with range [0x2000-0x3000)
+  ///tmp/b with range [0x1000-0x2000)
+  // That we end up with two modules in the filtered list with the same ranges
+  // and in the same order. Previous versions of the
+  // MinidumpParser::GetFilteredModuleList() function would sort all images
+  // by address and modify the order of the modules.
+  std::vector filtered_modules =
+  parser->GetFilteredModuleList();
+  llvm::Optional name;
+  EXPECT_EQ(2, filtered_modules.size());
+  EXPECT_EQ(0x2000, filtered_modules[0]->base_of_image);
+  name = parser->GetMinidumpString(filtered_modules[0]->module_name_rva);
+  ASSERT_TRUE((bool)name);
+  EXPECT_EQ(std::string("/tmp/a"), *name);
+  EXPECT_EQ(0x1000, filtered_modules[1]->base_of_image);
+  name = parser->GetMinidumpString(filtered_modules[1]->module_name_rva);
+  ASSERT_TRUE((bool)name);
+  EXPECT_EQ(std::string("/tmp/b"), *name);
+}
Index: source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- source/Plugins/Process/minidump/MinidumpParser.cpp
+++ source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -274,36 +274,45 @@
 
 std::vector MinidumpParser::GetFilteredModuleList() {
   llvm::ArrayRef modules = GetModuleList();
-  // map module_name -> pair(load_address, pointer to module struct in memory)
-  llvm::StringMap> lowest_addr;
+  // map module_name -> filtered_modules index
+  typedef llvm::StringMap MapType;
+  MapType module_name_to_filtered_index;
 
   std::vector filtered_modules;
-
+  
   llvm::Optional name;
   std::string module_name;
 
   for (const auto &module : modules) {
 name = GetMinidumpString(module.module_name_rva);
-
+
 if (!name)
   continue;
-
+
 module_name = name.getValue();
+
+MapType::iterator iter;
+bool inserted;
+// See if we have inserted this module aready into filtered_modules. If we
+// haven't insert an entry into module_name_to_filtered_index with the
+// index where we will insert it if it isn't in the vector already.
+std::tie(iter, inserted) = module_name_to_filtered_index.try_emplace(
+module_name, filtered_modules.size());
 
-auto iter = lowest_addr.end();
-bool exists;
-std::tie(iter, exists) = lowest_addr.try_emplace(
-module_name, std::make_pair(module.base_of_image, &module));
-
-if (exists && module.base_of_image < iter->second.first)
-  iter->second = std::make_pair(module.base_of_image, &module);
+if (inserted) {
+  // This module has not been seen yet, insert it into filtered_modules at
+  // the index that was inserted into module_name_to_filtered_index using
+  // "filtered_modules.size()" above.
+  filtered_modules.push_back(&module);

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

2018-12-12 Thread Leonard Mosescu via lldb-commits

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 same page, we already do have a real
> SymbolVendor implementation, it just happens to be the *default*
> SymbolVendor implementation.  It's not the case that one doesn't exist at
> all.


There were a few missing pieces, although you're right, it works with the
default SymbolVendor as you pointed out (btw, what I meant by "real"
SymbolVendor is a "PDB specific" SymbolVendor, sorry for the confusion)

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 directory to this path.
> So, if you want this behavior in a supported way that isn't temporary, we
> should move the code for findMatchingPDBFile() out of SymbolFilePDB and
> into this function.  Then everyone is happy I think.
>

As far as I can tell, Symbols::LocateExecutableSymbolFile() is a helper
intended for specialized SymbolVendors. So yes, when we get around to build
a specialized SymbolVendorPDB we'd be able to use it but until then I don't
think that moving that logic inside SymbolFileNativePDB is appropriate.

On Wed, Dec 12, 2018 at 1:19 PM Leonard Mosescu via Phabricator <
revi...@reviews.llvm.org> wrote:

> 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 don't (yet) have a ObjectFilePDB so
> the SymbolFileNativePDB always points to the associated PE binary.
>
> the check itself seems valuable as a diagnostic but not strictly required.
> Should I add a TODO comment and/or open a bug to revisit this?
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55142/new/
>
> https://reviews.llvm.org/D55142
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Greg Clayton via Phabricator via lldb-commits

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 ACTION
  https://reviews.llvm.org/D55614/new/

https://reviews.llvm.org/D55614



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

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/

https://reviews.llvm.org/D55582

Files:
  include/lldb/Core/Debugger.h
  include/lldb/Interpreter/CommandInterpreter.h
  lit/Reproducer/TestCommandRepro.test
  source/API/SBDebugger.cpp
  source/Commands/CommandObjectReproducer.cpp
  source/Core/Debugger.cpp
  source/Interpreter/CommandInterpreter.cpp
  tools/driver/Driver.cpp
  tools/driver/Driver.h

Index: tools/driver/Driver.h
===
--- tools/driver/Driver.h
+++ tools/driver/Driver.h
@@ -100,6 +100,7 @@
 bool m_wait_for = false;
 bool m_repl = false;
 bool m_batch = false;
+bool m_replay = false;
 
 // FIXME: When we have set/show variables we can remove this from here.
 bool m_use_external_editor = false;
Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -286,6 +286,10 @@
 m_option_data.m_core_file = arg_value;
   }
 
+  if (args.hasArg(OPT_replay)) {
+m_option_data.m_replay = true;
+  }
+
   if (args.hasArg(OPT_editor)) {
 m_option_data.m_use_external_editor = true;
   }
@@ -677,6 +681,8 @@
   else
 WithColor::error() << error.GetError() << '\n';
 }
+  } else if (m_option_data.m_replay) {
+m_debugger.RunCommandInterpreter(handle_events, spawn_thread);
   } else {
 // Check if we have any data in the commands stream, and if so, save it to a
 // temp file
Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
@@ -74,6 +75,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace llvm;
 
 static const char *k_white_space = " \t\v";
 
@@ -116,6 +118,42 @@
   eEchoCommentCommands = 5
 };
 
+class lldb_private::CommandProvider
+: public repro::Provider {
+public:
+  CommandProvider(const FileSpec &directory) : Provider(directory) {
+m_info.name = "command-interpreter";
+m_info.files.push_back("command-interpreter.txt");
+  }
+
+  void CaptureCommand(std::string command) {
+m_commands.push_back(std::move(command));
+  }
+
+  void Keep() override {
+FileSpec file =
+GetRoot().CopyByAppendingPathComponent("command-interpreter.txt");
+
+std::error_code ec;
+llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::F_Text);
+
+if (ec)
+  return;
+
+for (auto &command : m_commands)
+  os << command << '\n';
+  }
+
+  void Discard() override {}
+
+  static char ID;
+
+private:
+  std::vector m_commands;
+};
+
+char CommandProvider::ID = 0;
+
 ConstString &CommandInterpreter::GetStaticBroadcasterClass() {
   static ConstString class_name("lldb.commandInterpreter");
   return class_name;
@@ -141,6 +179,9 @@
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
   CheckInWithManager();
   m_collection_sp->Initialize(g_properties);
+
+  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
+m_provider = &g->GetOrCreate();
 }
 
 bool CommandInterpreter::GetExpandRegexAliases() const {
@@ -1679,6 +1720,9 @@
 
   Status error(PreprocessCommand(command_string));
 
+  if (m_provider)
+m_provider->CaptureCommand(original_command_string);
+
   if (error.Fail()) {
 result.AppendError(error.AsCString());
 result.SetStatus(eReturnStatusFailed);
@@ -2074,6 +2118,45 @@
   return position;
 }
 
+void CommandInterpreter::ReplayCommands(CommandReturnObject &result) {
+  repro::Loader *loader = repro::Reproducer::Instance().GetLoader();
+  if (!loader) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);
+return;
+  }
+
+  auto provider_info = loader->GetProviderInfo("command-interpreter");
+  if (!provider_info) {
+result.AppendErrorWithFormat("no provider for command interpreter.");
+result.SetStatus(eReturnStatusFailed);
+return;
+  }
+
+  if (provider_info->files.empty()) {
+result.AppendErrorWithFormat(
+"provider for command interpreter contains no files.");
+result.SetStatus(eReturnStatusFailed);
+return;
+  }
+
+  FileSpec command_file = loader->GetRoot().CopyByAppendingPathComponent(
+  provider_info->files.front());
+
+  // Remember current batch mode.
+  const bool saved_batch = SetBatchCommandMode(true);
+
+  CommandInterpreterRunOptions options;
+  options.SetSilent(false);
+  options.SetStopOn

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

2018-12-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

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 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
  lit/Reproducer/Inputs/CommandCapture.in
  lit/Reproducer/TestCommandRepro.test
  source/API/SBDebugger.cpp
  source/Commands/CommandObjectCommands.cpp
  source/Commands/CommandObjectReproducer.cpp
  source/Core/Debugger.cpp
  source/Interpreter/CommandInterpreter.cpp
  tools/driver/Driver.cpp
  tools/driver/Driver.h

Index: tools/driver/Driver.h
===
--- tools/driver/Driver.h
+++ tools/driver/Driver.h
@@ -100,6 +100,7 @@
 bool m_wait_for = false;
 bool m_repl = false;
 bool m_batch = false;
+bool m_replay = false;
 
 // FIXME: When we have set/show variables we can remove this from here.
 bool m_use_external_editor = false;
Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -286,6 +286,10 @@
 m_option_data.m_core_file = arg_value;
   }
 
+  if (args.hasArg(OPT_replay)) {
+m_option_data.m_replay = true;
+  }
+
   if (args.hasArg(OPT_editor)) {
 m_option_data.m_use_external_editor = true;
   }
@@ -677,6 +681,8 @@
   else
 WithColor::error() << error.GetError() << '\n';
 }
+  } else if (m_option_data.m_replay) {
+m_debugger.RunCommandInterpreter(handle_events, spawn_thread);
   } else {
 // Check if we have any data in the commands stream, and if so, save it to a
 // temp file
Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
@@ -74,6 +75,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace llvm;
 
 static const char *k_white_space = " \t\v";
 
@@ -116,6 +118,42 @@
   eEchoCommentCommands = 5
 };
 
+class lldb_private::CommandProvider
+: public repro::Provider {
+public:
+  CommandProvider(const FileSpec &directory) : Provider(directory) {
+m_info.name = "command-interpreter";
+m_info.files.push_back("command-interpreter.txt");
+  }
+
+  void CaptureCommand(std::string command) {
+m_commands.push_back(std::move(command));
+  }
+
+  void Keep() override {
+FileSpec file =
+GetRoot().CopyByAppendingPathComponent("command-interpreter.txt");
+
+std::error_code ec;
+llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::F_Text);
+
+if (ec)
+  return;
+
+for (auto &command : m_commands)
+  os << command << '\n';
+  }
+
+  void Discard() override {}
+
+  static char ID;
+
+private:
+  std::vector m_commands;
+};
+
+char CommandProvider::ID = 0;
+
 ConstString &CommandInterpreter::GetStaticBroadcasterClass() {
   static ConstString class_name("lldb.commandInterpreter");
   return class_name;
@@ -141,6 +179,9 @@
   SetEventName(eBroadcastBitQuitCommandReceived, "quit");
   CheckInWithManager();
   m_collection_sp->Initialize(g_properties);
+
+  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
+m_provider = &g->GetOrCreate();
 }
 
 bool CommandInterpreter::GetExpandRegexAliases() const {
@@ -1589,7 +1630,8 @@
CommandReturnObject &result,
ExecutionContext *override_context,
bool repeat_on_empty_command,
-   bool no_context_switching)
+   bool no_context_switching,
+   bool add_to_reproducer)
 
 {
 
@@ -1679,6 +1721,9 @@
 
   Status error(PreprocessCommand(command_string));
 
+  if (m_provider && add_to_reproducer)
+m_provider->CaptureCommand(original_command_string);
+
   if (error.Fail()) {
 result.AppendError(error.AsCString());
 result.SetStatus(eReturnStatusFailed);
@@ -2074,6 +2119,45 @@
   return position;
 }
 
+void CommandInterpreter::ReplayCommands(CommandReturnObject &result) {
+  repro::Loader *loader = repro::Reproducer::Instance().GetLoader();
+  if (!loader) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);
+return;
+  }
+
+  auto provider_info = loader->GetProviderInfo("command-interpreter");
+  if (!provider_info) {
+result.AppendErrorWith

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

2018-12-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

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 patch checks that:

- Entries in image list are identical.
- That stepping behaves the same.
- That the data formatters behave the same.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D55626

Files:
  lit/Reproducer/Functionalities/Inputs/DataFormatter.in
  lit/Reproducer/Functionalities/Inputs/Stepping.in
  lit/Reproducer/Functionalities/Inputs/foo.cpp
  lit/Reproducer/Functionalities/Inputs/stepping.c
  lit/Reproducer/Functionalities/TestDataFormatter.test
  lit/Reproducer/Functionalities/TestImagineList.test
  lit/Reproducer/Functionalities/TestStepping.test

Index: lit/Reproducer/Functionalities/TestStepping.test
===
--- /dev/null
+++ lit/Reproducer/Functionalities/TestStepping.test
@@ -0,0 +1,51 @@
+# UNSUPPORTED: system-windows, system-freebsd
+
+# This tests that stepping continues to work when replaying a reproducer.
+
+# RUN: rm -rf %T/stepping
+# RUN: %clang %S/Inputs/stepping.c -g -o %t.out
+
+# RUN: %lldb -x -b -s %S/Inputs/Stepping.in --capture %T/stepping %t.out | FileCheck %s --check-prefix CHECK --check-prefix CAPTURE
+# RUN: %lldb --replay %T/stepping %t.out | FileCheck %s --check-prefix CHECK --check-prefix REPLAY
+
+# CHECK: Breakpoint 1: {{.*}} stepping.c:42
+# CHECK: Breakpoint 2: {{.*}} stepping.c:20
+# CHECK: Breakpoint 3: {{.*}} stepping.c:29
+# CHECK: Breakpoint 4: {{.*}} stepping.c:34
+
+# CHECK: Process {{.*}} stopped
+# CHECK: stop reason = breakpoint 1.1
+# CHECK: Process {{.*}} stopped
+# CHECK: stop reason = breakpoint 2.1
+# CHECK: Process {{.*}} stopped
+# CHECK: stop reason = breakpoint 3.1
+# CHECK: Process {{.*}} stopped
+# CHECK: stop reason = breakpoint 4.1
+
+# CHECK: frame #0: {{.*}}`c
+# CHECK: frame #1: {{.*}}`b
+# CHECK: frame #2: {{.*}}`a
+
+# CHECK: Process {{.*}} resuming
+# CHECK: Process {{.*}} stopped
+
+# CHECK: stop reason = step over
+# CHECK: stop reason = step over
+# CHECK: stop reason = step over
+
+# CHECK: 1 breakpoints disabled.
+# CHECK: 1 breakpoints disabled.
+# CHECK: 1 breakpoints disabled.
+# CHECK: 1 breakpoints disabled.
+
+# CHECK: stop reason = step over
+# CHECK: stop reason = step over
+
+# CHECK: stop reason = step in
+# CHECK: stop reason = step in
+# CHECK: stop reason = step in
+# CHECK: stop reason = step in
+# CHECK: stop reason = step in
+
+# CHECK: Process {{.*}} resuming
+# CHECK: Process {{.*}} exited with status = 0
Index: lit/Reproducer/Functionalities/TestImagineList.test
===
--- /dev/null
+++ lit/Reproducer/Functionalities/TestImagineList.test
@@ -0,0 +1,32 @@
+# UNSUPPORTED: system-windows, system-freebsd
+
+# This tests that image list works when replaying. We arbitrarily assume
+# there's at least two entries and compare that they're identical.
+
+# RUN: %clang %S/Inputs/stepping.c -g -o %t.out
+
+# RUN: rm -rf %T/imagelist
+# RUN: rm -rf %t.txt
+
+# RUN: echo "CAPTURE" >> %t.txt
+# RUN: %lldb -x -b  --capture %T/imagelist \
+# RUN:-o 'image list' \
+# RUN:-o 'reproducer generate' \
+# RUN:%t.out >> %t.txt 2>&1
+
+# RUN: rm -rf
+
+# RUN: echo "REPLAY" >> %t.txt
+# RUN: %lldb -x -b --replay %T/imagelist  %t.out >> %t.txt 2>&1
+
+# RUN: cat %t.txt | FileCheck %s
+
+# CHECK: CAPTURE
+# CHECK: image list
+# CHECK: [  0] [[ZERO:.*]]
+# CHECK: [  1] [[ONE:.*]]
+
+# CHECK: REPLAY
+# CHECK: image list
+# CHECK: [  0] {{.*}}[[ZERO]]
+# CHECK: [  1] {{.*}}[[ONE]]
Index: lit/Reproducer/Functionalities/TestDataFormatter.test
===
--- /dev/null
+++ lit/Reproducer/Functionalities/TestDataFormatter.test
@@ -0,0 +1,16 @@
+# UNSUPPORTED: system-windows, system-freebsd
+
+# This tests that data formatters continue to work when replaying a reproducer.
+
+# RUN: rm -rf %T/dataformatter
+# RUN: %clang -x c++ %S/Inputs/foo.cpp -g -o %t.out
+
+# RUN: %lldb -x -b -s %S/Inputs/DataFormatter.in --capture %T/dataformatter %t.out | FileCheck %s
+# RUN: %lldb --replay %T/dataformatter %t.out | FileCheck %s
+
+# CHECK: stop reason = breakpoint 1.1
+# CHECK: (Foo) foo = (m_i = 0, m_d = 0)
+# CHECK: stop reason = step over
+# CHECK: (Foo) foo = (m_i = 1, m_d = 2)
+# CHECK: Process {{.*}} resuming
+# CHECK: Process {{.*}} exited with status = 0
Index: lit/Reproducer/Functionalities/Inputs/stepping.c
===
--- /dev/null
+++ lit/Reproducer/Functionalities/Inputs/stepping.c
@@ -0,0 +1,51 @@
+//===-- stepping.c --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Op

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

2018-12-12 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lit/Reproducer/Functionalities/Inputs/stepping.c:8
+//
+//===--===//
+#include 

Why does only this file have a header?



Comment at: lit/Reproducer/Functionalities/Inputs/stepping.c:9
+//===--===//
+#include 
+

If we don't need stdio then removing it will speed up compile time. You can 
still fwd-declare puts or printf.



Comment at: lit/Reproducer/Functionalities/TestStepping.test:39
+# CHECK: 1 breakpoints disabled.
+# CHECK: 1 breakpoints disabled.
+

perhaps copy the commands in here so it is easier to understand what is being 
checked for?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55626/new/

https://reviews.llvm.org/D55626



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Adrian Prantl via Phabricator via lldb-commits

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 
> checked for?
You could even generate the input file using `grep -v CHECK %s`... ?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55626/new/

https://reviews.llvm.org/D55626



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2018-12-12 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Wed Dec 12 16:15:17 2018
New Revision: 348996

URL: http://llvm.org/viewvc/llvm-project?rev=348996&view=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/trunk/source/Utility/Stream.cpp
lldb/trunk/source/Utility/StringList.cpp
lldb/trunk/source/Utility/StructuredData.cpp
lldb/trunk/source/Utility/TildeExpressionResolver.cpp
lldb/trunk/source/Utility/UUID.cpp

Modified: lldb/trunk/source/Utility/FileSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=348996&r1=348995&r2=348996&view=diff
==
--- lldb/trunk/source/Utility/FileSpec.cpp (original)
+++ lldb/trunk/source/Utility/FileSpec.cpp Wed Dec 12 16:15:17 2018
@@ -424,7 +424,7 @@ std::string FileSpec::GetPath(bool denor
 }
 
 const char *FileSpec::GetCString(bool denormalize) const {
-  return ConstString{GetPath(denormalize)}.AsCString(NULL);
+  return ConstString{GetPath(denormalize)}.AsCString(nullptr);
 }
 
 void FileSpec::GetPath(llvm::SmallVectorImpl &path,

Modified: lldb/trunk/source/Utility/Stream.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Stream.cpp?rev=348996&r1=348995&r2=348996&view=diff
==
--- lldb/trunk/source/Utility/Stream.cpp (original)
+++ lldb/trunk/source/Utility/Stream.cpp Wed Dec 12 16:15:17 2018
@@ -93,9 +93,9 @@ void Stream::QuotedCString(const char *c
 //--
 void Stream::Address(uint64_t addr, uint32_t addr_size, const char *prefix,
  const char *suffix) {
-  if (prefix == NULL)
+  if (prefix == nullptr)
 prefix = "";
-  if (suffix == NULL)
+  if (suffix == nullptr)
 suffix = "";
   //int addr_width = m_addr_size << 1;
   //Printf ("%s0x%0*" PRIx64 "%s", prefix, addr_width, addr, suffix);

Modified: lldb/trunk/source/Utility/StringList.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/StringList.cpp?rev=348996&r1=348995&r2=348996&view=diff
==
--- lldb/trunk/source/Utility/StringList.cpp (original)
+++ lldb/trunk/source/Utility/StringList.cpp Wed Dec 12 16:15:17 2018
@@ -83,7 +83,7 @@ size_t StringList::GetMaxStringLength()
 const char *StringList::GetStringAtIndex(size_t idx) const {
   if (idx < m_strings.size())
 return m_strings[idx].c_str();
-  return NULL;
+  return nullptr;
 }
 
 void StringList::Join(const char *separator, Stream &strm) {

Modified: lldb/trunk/source/Utility/StructuredData.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/StructuredData.cpp?rev=348996&r1=348995&r2=348996&view=diff
==
--- lldb/trunk/source/Utility/StructuredData.cpp (original)
+++ lldb/trunk/source/Utility/StructuredData.cpp Wed Dec 12 16:15:17 2018
@@ -144,7 +144,7 @@ static StructuredData::ObjectSP ParseJSO
 }
 
 StructuredData::ObjectSP StructuredData::ParseJSON(std::string json_text) {
-  JSONParser json_parser(json_text.c_str());
+  JSONParser json_parser(json_text);
   StructuredData::ObjectSP object_sp = ParseJSONValue(json_parser);
   return object_sp;
 }
@@ -169,11 +169,11 @@ StructuredData::Object::GetObjectForDotS
 
   if (this->GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');
-if (match.second.size() == 0) {
+if (match.second.empty()) {
   return this->shared_from_this();
 }
 errno = 0;
-uint64_t val = strtoul(match.second.str().c_str(), NULL, 10);
+uint64_t val = strtoul(match.second.str().c_str(), nullptr, 10);
 if (errno == 0) {
   return this->GetAsArray()->GetItemAtIndex(val);
 }

Modified: lldb/trunk/source/Utility/TildeExpressionResolver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/TildeExpressionResolver.cpp?rev=348996&r1=348995&r2=348996&view=diff
==
--- lldb/trunk/source/Utility/TildeExpressionResolver.cpp (original)
+++ lldb/trunk/source/Utility/TildeExpressionResolver.cpp Wed Dec 12 16:15:17 
2018
@@ -59,7 +59,7 @@ bool StandardTildeExpressionResolver::Re
   struct passwd *user_entry;
   Expr = Expr.drop_front();
 
-  while ((user_entry = getpwent()) != NULL) {
+  while ((user_entry = getpwent()) != nullptr) {
 StringRef ThisName(user_entry->pw_name);
 if (!ThisName.startswith(Expr))
   continue;

Modified: lldb/trunk/source/Utility/UUID.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/UUID.cpp?rev=348996&r1=348995&r2=348996&view=diff
==
--- lldb/trunk/source/Utility/UUI

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

2018-12-12 Thread Jim Ingham via Phabricator via lldb-commits

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 gets destroyed as a result of its SharedPointer count 
going to 0, the thread can't get back to it's Process anymore (the WP->SP 
conversion fails), and since it gets to the Target from the Process, it can't 
get its target either.

This is structurally weak, but we've worked around it so far by making sure we 
call Finalize on the Process before we allow it to be destroyed.  Finalize 
clears out all of the objects Process owns, and then the eventual destruction 
can be just reclamation of memory.

However, we shot ourselves in the foot in Target::Launch by storing away a 
ProcessWP for the previous process owned by the target, then setting our 
m_process_sp to the new process THEN reconstituting the ProcessWP and calling 
Finalize on it.  But if Target was the last holder of the old ProcessSP, then 
when we set m_process_sp to the new process, that would trigger the destruction 
of the old Process BEFORE finalizing it.  Tearing down the Process before 
finalizing it doesn't always crash, it depends on what work the process was 
doing at the time.  But sometimes it does crash.

This patch avoids this problem by first finalizing the old process, THEN 
resetting the shared pointer.  That way we know Finalize will happen before 
destruction.

This is a NFC commit, it only fixes a fairly obscure crash.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D55631

Files:
  source/Target/Target.cpp


Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2868,8 +2868,6 @@
 ProcessWP process_wp;
 if (m_process_sp)
   process_wp = m_process_sp;
-m_process_sp =
-GetPlatform()->DebugProcess(launch_info, debugger, this, error);
 
 // Cleanup the old process since someone might still have a strong
 // reference to this process and we would like to allow it to cleanup as
@@ -2880,6 +2878,10 @@
 ProcessSP old_process_sp(process_wp.lock());
 if (old_process_sp)
   old_process_sp->Finalize();
+
+m_process_sp =
+GetPlatform()->DebugProcess(launch_info, debugger, this, error);
+
   } else {
 if (log)
   log->Printf("Target::%s the platform doesn't know how to debug a "


Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2868,8 +2868,6 @@
 ProcessWP process_wp;
 if (m_process_sp)
   process_wp = m_process_sp;
-m_process_sp =
-GetPlatform()->DebugProcess(launch_info, debugger, this, error);
 
 // Cleanup the old process since someone might still have a strong
 // reference to this process and we would like to allow it to cleanup as
@@ -2880,6 +2878,10 @@
 ProcessSP old_process_sp(process_wp.lock());
 if (old_process_sp)
   old_process_sp->Finalize();
+
+m_process_sp =
+GetPlatform()->DebugProcess(launch_info, debugger, this, error);
+
   } else {
 if (log)
   log->Printf("Target::%s the platform doesn't know how to debug a "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

46 matches

Mail list logo