[Lldb-commits] [lldb] r323855 - Fix TestTargetSymbolsAddCommand makefile

2018-01-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Wed Jan 31 01:14:12 2018
New Revision: 323855

URL: http://llvm.org/viewvc/llvm-project?rev=323855&view=rev
Log:
Fix TestTargetSymbolsAddCommand makefile

We started passing the "all" target to make, which rendered the
"localall" trick in this Makefile inoperable.

I implement the strip step differently, and also reformat the Makefile.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/Makefile

Modified: lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/Makefile?rev=323855&r1=323854&r2=323855&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/Makefile 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/linux/add-symbols/Makefile Wed 
Jan 31 01:14:12 2018
@@ -2,11 +2,12 @@ LEVEL = ../../make
 CXX_SOURCES := main.cpp
 LD_EXTRAS += -Wl,--build-id=none
 
-localall : stripped.out all
+all: stripped.out
+
 stripped.out : a.out
-  $(OBJCOPY) 
--remove-section=.note.gnu.build-id --remove-section=.gnu_debuglink 
--strip-debug a.out stripped.out
+   $(OBJCOPY) --remove-section=.note.gnu.build-id 
--remove-section=.gnu_debuglink --strip-debug $< $@
 
 clean::
-   $(RM) stripped.out
+   $(RM) stripped.out
 
-include $(LEVEL)/Makefile.rules
\ No newline at end of file
+include $(LEVEL)/Makefile.rules


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


[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

2018-01-31 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D42145#992289, @owenpshaw wrote:

> > So the main question is: do we want WriteMemory to work anywhere and always 
> > try to do the right thing, or return an error an the user would be expected 
> > to know to check the memory region you are writing to and know to call 
> > "Process::WriteFlash(...)". I vote to keep things simple and have 
> > Process::WriteMemory() just do the right thing. I am open to differing 
> > opinions, so let me know what you guys think.
>
> What are other use cases for writing to flash memory from lldb?  Since I 
> can't think of any, I'm biased towards keeping this implementation simple and 
> narrowly focused on object loading, which means not worrying about preserving 
> the unwritten parts of erased blocks, and giving an error for other write 
> attempts to flash regions.


How does the flash memory behave from the POV of the debugged process? E.g. if 
it does something like:

  char *Flash = /*pointer to flash memory*/
  *Flash = 47;

will that succeed, or will it get some exception? I'm guessing it's the 
latter.. If that's the case, then I'm also leaning towards *not* making 
WriteMemory "just work", as we cannot make the existence of flash memory 
transparent without e.g. fixing how breakpoint setting in the flashed region 
works.

However, I'm not sure it should be the responsibility of the ObjectFile to know 
about the various memory kinds. Maybe we could still keep that in the process 
class. We could have the ObjectFile create a structure describing the desired 
memory layout (just like in the batch-WriteMemory version), and then pass it to 
some Process function, which would handle the flash details. (So in essence, 
the implementation would remain the same, you just wouldn't call the function 
WriteMemory, but something else).


https://reviews.llvm.org/D42145



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


[Lldb-commits] [lldb] r323879 - [lldb] Enable debugging of binaries with mixed (splitted/regular) dwarf

2018-01-31 Thread Davide Italiano via lldb-commits
Author: davide
Date: Wed Jan 31 07:17:47 2018
New Revision: 323879

URL: http://llvm.org/viewvc/llvm-project?rev=323879&view=rev
Log:
[lldb] Enable debugging of binaries with mixed (splitted/regular) dwarf

This recommits the patch, now that I verified that the bot instability
is due to something else. Sorry for the noise.

Added:

lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile

lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py

lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c

lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile?rev=323879&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/Makefile
 Wed Jan 31 07:17:47 2018
@@ -0,0 +1,10 @@
+LEVEL := ../../make
+
+C_SOURCES := a.c b.c
+a.o: CFLAGS_EXTRAS += -gsplit-dwarf
+
+include $(LEVEL)/Makefile.rules
+
+.PHONY: clean
+clean::
+   $(RM) -f a.dwo a.o b.o main

Added: 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py?rev=323879&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
 Wed Jan 31 07:17:47 2018
@@ -0,0 +1,46 @@
+""" Testing debugging of a binary with "mixed" dwarf (with/without fission). 
"""
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestMixedDwarfBinary(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@no_debug_info_test  # Prevent the genaration of the dwarf version of this 
test
+@add_test_categories(["dwo"])
+@skipUnlessPlatform(["linux"])
+def test_mixed_dwarf(self):
+"""Test that 'frame variable' works
+for the executable built from two source files compiled
+with/whithout -gsplit-dwarf correspondingly."""
+
+self.build()
+exe = os.path.join(os.getcwd(), "a.out")
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("g", "a.out")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertTrue(self.process, PROCESS_IS_VALID)
+
+# The stop reason of the thread should be breakpoint.
+self.assertTrue(self.process.GetState() == lldb.eStateStopped,
+STOPPED_DUE_TO_BREAKPOINT)
+
+frame = self.process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+x = frame.FindVariable("x")
+self.assertTrue(x.IsValid(), "x is not valid")
+y = frame.FindVariable("y")
+self.assertTrue(y.IsValid(), "y is not valid")
+

Added: 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c?rev=323879&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c 
(added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/a.c 
Wed Jan 31 07:17:47 2018
@@ -0,0 +1,3 @@
+int f() {
+  return 1;
+}

Added: 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c?rev=323879&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c 
(added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/b.c 
Wed Jan 31 07:17:47 2018
@@ -0,0 +1,11 @@
+extern int f();
+
+void g() {
+  int y = 14;
+  int x = f();
+}
+
+int main() {
+  g();
+  return 0;
+}

Modified: lldb/trunk/so

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

2018-01-31 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D42281#992359, @aprantl wrote:

> In https://reviews.llvm.org/D42281#992295, @aprantl wrote:
>
> > There are a few test failures, but they look manageable. I will need help 
> > resolving them though:
>
>
> Updated list:
>
> http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7843
>  FAIL: 
> TestLoadUnload.LoadUnloadTestCase.test_lldb_process_load_and_unload_commands_dwarf
>  test3 HT4A1JT02945,aarch64-linux-android-clang,aarch64,tcp,-lldb-mi 8 
> unexpected failures failed ( 41 mins, 48 secs )
> stdio
>  FAIL: 
> TestBreakpointInGlobalConstructor.TestBreakpointInGlobalConstructors.test
>  FAIL: TestMoveNearest.TestMoveNearest.test
>  FAIL: 
> TestGlobalVariables.GlobalVariablesTestCase.test_c_global_variables_dwarf
>  FAIL: TestGlobalVariables.GlobalVariablesTestCase.test_c_global_variables_dwo
>  FAIL: 
> TestGlobalVariables.GlobalVariablesTestCase.test_c_global_variables_gmodules
>  FAIL: 
> TestLoadUnload.LoadUnloadTestCase.test_lldb_process_load_and_unload_commands_dwarf
>  FAIL: 
> TestLoadUnload.LoadUnloadTestCase.test_lldb_process_load_and_unload_commands_dwo
>  FAIL: 
> TestLoadUnload.LoadUnloadTestCase.test_lldb_process_load_and_unload_commands_gmodules
>
> http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-android/builds/10506
>  test7 emulator-5554,i686-linux-android-clang,i386,abstract,-lldb-mi failed
>
> http://lab.llvm.org:8011/builders/lldb-x86_64-darwin-13.4/builds/8462
>  test2 5A24000830,aarch64-linux-android-gcc,aarch64,tcp,-lldb-mi:-watchpoint 
> 2 unexpected failures failed 
>  FAIL: TestMoveNearest.TestMoveNearest.test
>  FAIL: 
> TestBreakpointInGlobalConstructor.TestBreakpointInGlobalConstructors.test


Most of these turned out to be caused by leftover shared libraries in the 
source tree (fun fact: "svn status" refuses to list the .so files even though 
they are not present in the svn:ignore list), when I cleaned those up, the 
tests started passing again.

There is still the TestLoadUnload failure on the windows bot, which I suspect 
to be a real issue (we've changed the test a bit, and now it passes full paths 
to lldb, so I suspect a directory-separator issue), but I have not looked into 
that yet. I'll handle that on our side in the coming days.

All in all, I would say this was rather a painless experience. I think we are 
ready for phase two.


Repository:
  rL LLVM

https://reviews.llvm.org/D42281



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


[Lldb-commits] [lldb] r323884 - Fix TestMixedDwarfBinary.py

2018-01-31 Thread Pavel Labath via lldb-commits
Author: labath
Date: Wed Jan 31 08:23:24 2018
New Revision: 323884

URL: http://llvm.org/viewvc/llvm-project?rev=323884&view=rev
Log:
Fix TestMixedDwarfBinary.py

As of yesterday, tests need to use self.getBuildArtifact() to get the
built executable.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py?rev=323884&r1=323883&r2=323884&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/linux/mix-dwo-and-regular-objects/TestMixedDwarfBinary.py
 Wed Jan 31 08:23:24 2018
@@ -22,7 +22,7 @@ class TestMixedDwarfBinary(TestBase):
 with/whithout -gsplit-dwarf correspondingly."""
 
 self.build()
-exe = os.path.join(os.getcwd(), "a.out")
+exe = self.getBuildArtifact("a.out")
 
 self.target = self.dbg.CreateTarget(exe)
 self.assertTrue(self.target, VALID_TARGET)


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


[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

2018-01-31 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Thanks a lot for your help, Pavel!

We also discovered the most amazing test failure on the green dragon bots 
yesterday (fix still underway) that was made much worse by my patch.
On Darwin LLDB automatically tries to find a .dSYM bundle for an executable by 
querying the Spotlight index for the executable's UUID. Because the UUID is a 
hash of filename and .text section, and the testsuite compiles many short 
hello-world-style files, all alike, and named `a.out`, two tests can have the 
same UUID and LLDB then sometimes picks up debug info from a .dSYM that belongs 
to different testcase! We are working on a mechanism to disable the UUID lookup 
inside the testsuite.


Repository:
  rL LLVM

https://reviews.llvm.org/D42281



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


[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

2018-01-31 Thread Owen Shaw via Phabricator via lldb-commits
owenpshaw added a comment.

> How does the flash memory behave from the POV of the debugged process?

Just tested on a couple targets and they both silently failed to change the 
flash memory.  So no exception, but no flash write either.

Unless there are objections, I'll work on some changes along the lines you 
described.


https://reviews.llvm.org/D42145



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


[Lldb-commits] [PATCH] D42443: [SymbolFilePDB] Add support for function symbols

2018-01-31 Thread Zachary Turner via Phabricator via lldb-commits
zturner added inline comments.



Comment at: lit/SymbolFile/PDB/Inputs/FuncSymbols.cpp:2
+// Static function
+static long StaticFunction(int a)
+{

Would it be worth trying one in an anonymous namespace?



Comment at: lit/SymbolFile/PDB/func-symbols.test:4
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/FuncSymbols.cpp /o 
%T/FuncSymbols.cpp.obj
+RUN: link %T/FuncSymbolsTestMain.cpp.obj %T/FuncSymbols.cpp.obj /DEBUG 
/nodefaultlib /Entry:main /OUT:%T/FuncSymbolsTest.exe
+RUN: lldb-test symbols %T/FuncSymbolsTest.exe | FileCheck %s

I bet we could get rid of `REQUIRES: windows` if we changed this to `lld-link`. 
 You're already specifying `/nodefaultlib /entry:main`, and no windows header 
files are included, so maybe it's worth a try?



Comment at: lit/SymbolFile/PDB/func-symbols.test:12
+CHECK-DAG: {{.*}}: SymbolVendor ([[MD]])
+CHECK-DAG: [[TY0:.*]]:   Type{[[UID0:.*]]} , name = "Func_arg_array", decl = 
FuncSymbolsTestMain.cpp:3, compiler_type = {{.*}} int (int *)
+CHECK-DAG: [[TY1:.*]]:   Type{[[UID1:.*]]} , name = "Func_arg_void", decl = 
FuncSymbolsTestMain.cpp:4, compiler_type = {{.*}} void (void)

Not required, but just as a note, you can do 

```
CHECK-DAG: [[TY0:.*]]
CHECK-SAME: name = "Func_arg_array"
CHECK-SAME: decl=FuncSymbolsTestMain.cpp:3

CHECK-DAG: [[TTY1:.*]]
```

etc if the long lines bothers you.



Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:292
+} else
+  return nullptr;
+

You can probably put an `llvm_unreachable` here.  If it has one of those 2 enum 
values, it had better cast to one type or the other.



Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:342
+auto array_type = llvm::dyn_cast(&type);
+lldbassert(array_type);
 uint32_t num_elements = array_type->getCount();

I think these can all be real asserts.  Like before, if it has the proper enum 
value, it really should cast.  And we're about to dereference it anyway, so no 
harm no foul.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:270-271
+const lldb_private::SymbolContext &sc) {
+  if (!pdb_func)
+return nullptr;
+  lldbassert(sc.comp_unit && sc.module_sp.get());

Can we either change `pdb_func` to be a reference, or assert that it's non 
null?  We have complete control over all entry points to this function, so we 
should be safe to make the assumption about the inputs.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:303-305
+  if (func_sp.get() != nullptr) {
+sc.comp_unit->AddFunction(func_sp);
+  }

This null check isn't necessary, since you just unconditionally newed it.  
Also, in the above line, can we use `make_shared` instead of `reset`?



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:376-380
+SymbolFilePDB::ParseFunctionBlocks(const lldb_private::SymbolContext &sc,
+   uint64_t func_file_vm_addr,
+   const llvm::pdb::PDBSymbol *pdb_symbol,
+   lldb_private::Block *parent_block,
+   bool is_top_parent) {

This function does not appear to access any member data.  Can you remove it 
from the class and mark it `static` at the file level?



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:381-382
+   bool is_top_parent) {
+  if (!pdb_symbol || !parent_block)
+return 0;
+

This is another case that since we control all the inputs, I think we can 
either accept references or hard assert.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:404-405
+
+  block = new Block(pdb_symbol->getSymIndexId());
+  BlockSP block_sp(block);
+  parent_block->AddChild(block_sp);

can you do `auto block_sp = 
std::make_shared(pdb_symbol->getSymIndexId());`



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:549-550
+if (!cu_sp) {
+  if (resolved_flags | eSymbolContextVariable) {
+  }
+  return 0;

Did you mean to do something here?



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:561
+auto *pdb_func = llvm::dyn_cast(symbol_up.get());
+lldbassert(pdb_func);
+auto func_uid = pdb_func->getSymIndexId();

actual `assert` is ok here.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:693-695
+  if (resolve_scope & (eSymbolContextFunction | eSymbolContextBlock |
+   eSymbolContextLineEntry)) {
+if (ParseCompileUnitLineTable(sc, line)) {

Can you refactor this block a bit to include a bunch of early continues and/or 
breaks?  The ind

[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions

2018-01-31 Thread Owen Shaw via Phabricator via lldb-commits
owenpshaw updated this revision to Diff 132260.
owenpshaw added a comment.

- Remove Begin/End memory batch API
- Add Process::WriteObjectFile(llvm::ArrayRef) method.  Open to 
other names since it's not necessarily specific to object files, but wanted to 
somehow indicate that the method may do an uncommon write (like write to flash).
- Kept vFlashWrite as part of DoWriteMemory, and changed the is_batch_write 
flag to an allow_flash_writes flag.  ProcessGDB:: WriteObjectFile sets 
allow_flash_writes, and then calls the regular WriteMemory.  Didn't seem like 
there was a need to duplicate the WriteMemory logic.
- Any other memory write attempts to flash regions will now fail

I think this gets us closer to what we're talking about, but let me know if 
anything should change.


https://reviews.llvm.org/D42145

Files:
  include/lldb/Host/XML.h
  include/lldb/Target/MemoryRegionInfo.h
  include/lldb/Target/Process.h
  
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteLoad.py
  source/Host/common/XML.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  source/Symbol/ObjectFile.cpp
  source/Target/Process.cpp

Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -2533,6 +2533,18 @@
   return 0;
 }
 
+bool Process::WriteObjectFile(llvm::ArrayRef entries,
+  Status &error) {
+  error.Clear();
+  for (const auto &Entry : entries) {
+WriteMemory(Entry.Dest, Entry.Contents.data(), Entry.Contents.size(),
+error);
+if (!error.Success())
+  break;  
+  }
+  return error.Success();
+}
+
 #define USE_ALLOCATE_MEMORY_CACHE 1
 addr_t Process::AllocateMemory(size_t size, uint32_t permissions,
Status &error) {
Index: source/Symbol/ObjectFile.cpp
===
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -659,22 +659,30 @@
   SectionList *section_list = GetSectionList();
   if (!section_list)
 return Status("No section in object file");
+
+  std::vector writeEntries;
+
+  // Create a list of write entries from loadable sections
   size_t section_count = section_list->GetNumSections(0);
   for (size_t i = 0; i < section_count; ++i) {
+Process::WriteEntry entry;
 SectionSP section_sp = section_list->GetSectionAtIndex(i);
-addr_t addr = target.GetSectionLoadList().GetSectionLoadAddress(section_sp);
-if (addr != LLDB_INVALID_ADDRESS) {
-  DataExtractor section_data;
-  // We can skip sections like bss
-  if (section_sp->GetFileSize() == 0)
-continue;
-  section_sp->GetSectionData(section_data);
-  lldb::offset_t written = process->WriteMemory(
-  addr, section_data.GetDataStart(), section_data.GetByteSize(), error);
-  if (written != section_data.GetByteSize())
-return error;
-}
+entry.Dest = target.GetSectionLoadList().GetSectionLoadAddress(section_sp);
+if (entry.Dest == LLDB_INVALID_ADDRESS)
+  continue;
+// We can skip sections like bss
+if (section_sp->GetFileSize() == 0)
+  continue;
+DataExtractor section_data;
+section_sp->GetSectionData(section_data);
+entry.Contents = llvm::ArrayRef(section_data.GetDataStart(),
+ section_data.GetByteSize());
+writeEntries.push_back(entry);
   }
+
+  if (!process->WriteObjectFile(writeEntries, error))
+return error;
+
   if (set_pc) {
 ThreadList &thread_list = process->GetThreadList();
 ThreadSP curr_thread(thread_list.GetSelectedThread());
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -144,6 +144,9 @@
   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
   Status &error) override;
 
+  bool WriteObjectFile(llvm::ArrayRef entries, Status &error)
+  override;
+
   size_t DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size,
Status &error) override;
 
@@ -302,6 +305,11 @@
   int64_t m_breakpoint_pc_offset;
   lldb::tid_t m_initial_tid; // The initial thread ID, given by stub on attach
 
+  bool m_allow_flash_writes;
+  using FlashRangeVector = lldb_private::RangeVector;
+  using FlashRange = FlashRangeVector::Entry;
+  FlashRangeVector m_erased_flash_ranges;
+
   //--
   // Accessors
   //---

[Lldb-commits] [lldb] r323917 - replace os.mkdirs with lldbutil.mkdir_p (NFC)

2018-01-31 Thread Adrian Prantl via lldb-commits
Author: adrian
Date: Wed Jan 31 13:39:00 2018
New Revision: 323917

URL: http://llvm.org/viewvc/llvm-project?rev=323917&view=rev
Log:
replace os.mkdirs with lldbutil.mkdir_p (NFC)

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_launch/TestProcessLaunch.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_launch/TestProcessLaunch.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_launch/TestProcessLaunch.py?rev=323917&r1=323916&r2=323917&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_launch/TestProcessLaunch.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_launch/TestProcessLaunch.py
 Wed Jan 31 13:39:00 2018
@@ -133,8 +133,7 @@ class ProcessLaunchTestCase(TestBase):
 err_file_name = "my_working_dir_test.err"
 
 my_working_dir_path = self.getBuildArtifact(mywd)
-try: os.makedirs(my_working_dir_path)
-except: pass
+lldbutil.mkdir_p(my_working_dir_path)
 out_file_path = os.path.join(my_working_dir_path, out_file_name)
 err_file_path = os.path.join(my_working_dir_path, err_file_name)
 

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py?rev=323917&r1=323916&r2=323917&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/single-quote-in-filename-to-lldb/TestSingleQuoteInFilename.py
 Wed Jan 31 13:39:00 2018
@@ -35,8 +35,7 @@ class SingleQuoteInCommandLineTestCase(T
 """Test that 'lldb my_file_name' works where my_file_name is a string 
with a single quote char in it."""
 import pexpect
 self.buildDefault()
-try: os.makedirs(self.getBuildArtifact("path with '09"))
-except: pass
+lldbutil.mkdir_p(self.getBuildArtifact("path with '09"))
 system([["cp",
  self.getBuildArtifact("a.out"),
  "\"%s\"" % self.getBuildArtifact(self.myexe)]])


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


[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

2018-01-31 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision.
aprantl added reviewers: labath, zturner, jingham.
Herald added subscribers: JDevlieghere, eraman.

This patch creates a .dwarf, .dwo, etc., build directory for each 
testcase variant.
Most importantly, this eliminates the need for the per-test lock file in the 
source directory.

Tests that are marked as NO_DEBUG_INFO_TESTCASE and build with buildDefault() 
are built in a .default build directory.

The implementation changes self.mydir to a tuple (test_subdir, test_name). I 
chose not to rename the variable as to not break anyone's out-of-tree testcases 
any more than necessary.

It would be awesome if someone could test this on Windows / Linux / Android 
again.


https://reviews.llvm.org/D42763

Files:
  packages/Python/lldbsuite/test/api/listeners/TestListener.py
  
packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
  packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
  
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_on_vectors/TestValueOfVectorVariable.py
  packages/Python/lldbsuite/test/lldbinline.py
  packages/Python/lldbsuite/test/lldbtest.py
  packages/Python/lldbsuite/test/plugins/builder_base.py
  packages/Python/lldbsuite/test/plugins/builder_darwin.py
  packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
  packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
  packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
  
packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py

Index: packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
===
--- packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
+++ packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
@@ -20,16 +20,16 @@
 def setUp(self):
 # Call super's setUp().
 TestBase.setUp(self)
-# Get the full path to our executable to be attached/debugged.
-self.exe = self.getBuildArtifact(self.testMethodName)
-self.d = {'EXE': self.testMethodName}
 
 @add_test_categories(['pyapi'])
 def test_with_process_launch_api(self):
 """Test SBValue::GetValueDidChange"""
-self.build(dictionary=self.d)
-self.setTearDownCleanup(dictionary=self.d)
-target = self.dbg.CreateTarget(self.exe)
+# Get the full path to our executable to be attached/debugged.
+exe = self.getBuildArtifact(self.testMethodName)
+d = {'EXE': exe}
+self.build(dictionary=d)
+self.setTearDownCleanup(dictionary=d)
+target = self.dbg.CreateTarget(exe)
 
 breakpoint = target.BreakpointCreateBySourceRegex(
 "break here", lldb.SBFileSpec("main.c"))
Index: packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
===
--- packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
+++ packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
@@ -73,10 +73,7 @@
 str(compileUnit),
 "The compile unit should match",
 exe=False,
-substrs=[
-os.path.join(
-self.mydir,
-'main.c')])
+substrs=[self.getSourcePath('main.c')])
 
 function = context.GetFunction()
 self.assertTrue(function)
@@ -88,12 +85,12 @@
 
 lineEntry = context.GetLineEntry()
 #print("line entry:", lineEntry)
+reldir, _ = self.mydir
 self.expect(
 lineEntry.GetFileSpec().GetDirectory(),
 "The line entry should have the correct directory",
 exe=False,
-substrs=[
-self.mydir])
+substrs=[reldir])
 self.expect(
 lineEntry.GetFileSpec().GetFilename(),
 "The line entry should have the correct filename",
Index: packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
===
--- packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
+++ packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
@@ -19,6 +19,8 @@
 def setUp(self):
 # Call super's setUp().
 TestBase.setUp(self)
+
+def setup_test(self):
 # Get the full path to our executable to be debugged.
 self.exe = self.getBuildArtifact("process_io")
 self.local_input_file = self.getBuildArtifact("input.txt")
@@ -38,6 +40,7 @@
 @expectedFlakeyLinux(bugnumber="llvm.org/pr26437")
 def test_stdin_by_api(self):
 """Exercise SBProcess.PutSTDIN()."""
+self.setup_test()
 self.build()
 self.create_target()
 self.run_process(True)
@@ -49

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

2018-01-31 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added inline comments.



Comment at: 
packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py:39-40
 (_COMP_DIR_SYM_LINK_PROP, pwd_symlink))
-lldbutil.run_break_set_by_file_and_line(self, self.src_path, self.line)
+src_path = self.getBuildArtifact(_SRC_FILE)
+lldbutil.run_break_set_by_file_and_line(self, src_path, self.line)
 

Instead of making a temporary variable `src_path` that gets used once in each 
instance, why not just insert `self.getBuildArtifact(_SRC_FILE)` directly into 
`lldbutil.run_break_set_by_file_and_line()`?



Comment at: 
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_on_vectors/TestValueOfVectorVariable.py:25-26
 """Test verify displayed value of vector variable."""
-self.build(dictionary=self.d)
-self.setTearDownCleanup(dictionary=self.d)
+exe = self.getBuildArtifact("a.out")
+d = {'C_SOURCES': self.source, 'EXE': exe}
+self.build(dictionary=d)

You should be able to insert the call to `getBuildArtifact()` into the 
construction of `d`, since `exe` is used nowhere else.



Comment at: 
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_on_vectors/TestValueOfVectorVariable.py:40
 exe = self.getBuildArtifact("a.out")
+d = {'C_SOURCES': self.source, 'EXE': exe}
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)

Is this `d` used?



Comment at: packages/Python/lldbsuite/test/lldbinline.py:96-98
+return "-N dwarf %s" % (testdir)
 else:
+return "-N dsym %s" % (testdir)

Good opportunity to move away from the old style of formatting strings to a 
newer style? It doesn't make a huge difference, I just think it'd be nice to 
do. :)
`return "-N dwarf {}".format(testdir)`
`return "-N dwarf {}".format(testdir)`

This is supported in Python 2.7, but I'm not sure if we can assume that version 
or greater to be present.



Comment at: packages/Python/lldbsuite/test/lldbtest.py:533-539
+if rel_prefix[-3:] == ".py":
+rel_prefix = rel_prefix[:-3]
+elif rel_prefix[-4:] == ".pyc":
+rel_prefix = rel_prefix[:-4]
+else:
+raise Exception("test_file is not a python file")
+return os.path.split(rel_prefix)

Might I suggest this?
```
(rel_test_path, extension) = os.path.splitext(rel_prefix)
if extension not in [".py", ".pyc"]:
raise Exception("test_file is not a python file")
return rel_test_path
```
Feels cleaner to me, at least. :)



Comment at: packages/Python/lldbsuite/test/lldbtest.py:712
 """Return the full path to the current test."""
-return os.path.join(os.environ["LLDB_TEST"], self.mydir)
+subdir, stem = self.mydir
+return os.path.join(os.environ["LLDB_TEST"], subdir)

`stem` is unused, you could make this a `_`.



Comment at: packages/Python/lldbsuite/test/lldbtest.py:718
+variant = self.debug_info
+subdir, stem = self.mydir
+if not variant:

`subdir` and `stem` are kind of confusing to me. It looks like you used 
`test_subdir` below, so it might be good to use that here too, I think. Also, I 
think the name `test_name` is less confusing than `test_stem`. What do you 
think?



Comment at: packages/Python/lldbsuite/test/lldbtest.py:719
+subdir, stem = self.mydir
+if not variant:
+variant = 'default'

I recommend `if variant is None` instead of `if not variant`



Comment at: packages/Python/lldbsuite/test/plugins/builder_base.py:63
 # Construct the base make invocation.
+test_subdir, test_stem = dir_stem
 lldb_test = os.environ["LLDB_TEST"]

I find `test_stem` kind of confusing. How do you feel about `test_name`?


https://reviews.llvm.org/D42763



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2018-01-31 Thread Alex Langford via Phabricator via lldb-commits
xiaobai abandoned this revision.
xiaobai added a comment.

It's clear this is not the way forward. The problem I am trying to solve should 
be attacked differently. Thanks everyone for your input!


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

2018-01-31 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

@rsmith: lldb does as you suggest, if it can find a name in the local context, 
it returns that, if it can find it in the current CU, it returns that, etc...  
It only returns multiple matches if it can't find a single instance along that 
chain.

The problem with this case is that from lldb's standpoint the symbol wasn't 
ambiguous, the only match lldb knew about was the internal libc symbol.  The 
collision was between that libc symbol and a local variable that was defined in 
the expression that lldb was submitting to clang.  At present, we don't know 
anything about those in flight definitions, we just reply with names from 
outside the current compilation.  So we didn't know there was a closer 
definition.  Is it possible for lldb to peek at those in flight definitions 
ourselves and return that as the most proximate instance?  I think that's the 
behavior we want.  The other thing to do is have clang know to prioritize name 
matches from decls it made out of the expression over those provided to it by 
an externalAST source, if that's a thing it is possible to do.


https://reviews.llvm.org/D39307



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


[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

2018-01-31 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

Oh, FWIW I tested this on Linux (specifically CentOS) and the test result 
summary showed no difference in Success/Failure rate.


https://reviews.llvm.org/D42763



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