[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Nice cleanup, thanks. I have one suggestion inline you can implement if you 
think it's a good idea.

FTR, I believe using pipes here is wrong altogether because it can lead to 
deadlock. The size of the pipe buffer is implementation-defined, and since 
there's noone reading from it while we write it, we can block indefinitely if 
we encounter a particularly long sequence of -o commands. I guess the fact that 
we haven't hit this in practice means that the implementation-defined size is 
sufficiently big on all OSs we care about.




Comment at: lldb/tools/driver/Driver.cpp:492-496
 #ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
+  _close(fd);
 #else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
+  close(fd);
 #endif

Do you think it would be possible to use 
`llvm::Process::SafelyCloseFileDescriptor` here? It looks like that still uses 
close (without the `_`) on windows, which we are trying hard to avoid here, but 
I'm not sure why. I know close is technically deprecated on windows, but AFAIK 
the only thing deprecated about it is the name, and otherwise it works 
correctly.

(If that works, I'd even consider removing this function entirely, as the only 
thing it does extra is clear the fd, but that does not seem necessary since 
we're now calling it immediately before we return (and the fds go out of scope 
anyway)).


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

https://reviews.llvm.org/D60152



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

[Lldb-commits] [PATCH] D60153: Re-enable most lldb-vscode tests on Linux.

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

I think we can give this a shot. We can always re-disable tests that are still 
flaky and iterate. Thank you for working on this.




Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py:95-97
+# Wait for a bit to ensure the process is launched, but not for so long
+# that the process has already finished by the time we attach.
+time.sleep(3)

I am not sure this will be enough. I've seen tests fail sporadically (like 1 
time out of a 1000 or so) even with bigger margins than this. I'd increase the 
wait in the test to at least 10 seconds. If you don't want to wait that long in 
the common case when things finish quickly, you can implement a `debugger_flag` 
pattern like some of the other tests do: 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60153



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

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thank you for taking the time to rename this function. I think this has been 
long overdue. However, I believe that the name `AddModule` might cause 
confusion coming from the other direction, as now one may think that it 
unconditionally adds a new module to the target (which also isn't true, as it 
will just reuse the existing one if it's already there). Maybe something like 
`GetOrCreateModule` would be better (and also in line with how llvm names these 
kinds of things)?

As for testing, doesn't this cause a eBroadcastBitModulesLoaded event to be 
sent through some SB API. You could subscribe to that and check that it comes 
only ~once, and contains the right modules. This won't test exactly the thing 
you're interested in, but it might be close enough.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60172



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

[Lldb-commits] [PATCH] D60178: [Reproducers] Capture return values of functions returning by ptr/ref

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I agree that these functions should be recorded. I think the reason that the 
infrastructure already mostly supports that is that we were modelling 
constructors as functions that return pointers (and you definitely need to 
record constructors).




Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:678-716
+  template  const Result &RecordResult(const Result &r) {
+UpdateBoundary();
+if (ShouldCapture()) {
+  assert(!m_result_recorded);
+  m_serializer.SerializeAll(r);
+  m_result_recorded = true;
+}

I would expect that all of these could be replaced with one function using 
universal references. I.e., something like:
```
template Result &&RecordResult(Result &&r) {
...
m_serializer.SerializeAll(r); // No std::forward, as you don't want to move r 
into this function
...
return std::forward(r);
}
```

Can you try that out?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60178



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

[Lldb-commits] [PATCH] D59826: [expression] Explicitly build the lookup table of any TagDecl's context

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

In D59826#1451536 , @clayborg wrote:

> Any way to dump the AST in a test to verify we don't create multiple?


I think I might be able to use the `log` object to dump the AST and then from 
python it would be possible to check for duplicates. Is that a good direction?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59826



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

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The reasoning makes sense to me, but maybe one of the standalone folks could 
confirm that.


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

https://reviews.llvm.org/D60180



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

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

[Alexey Lapshin via Phabricator via lldb-commits]

avl added a comment.

@rocallahan Would you mind to share the performance results of that patch, 
please ?  Similar to above table for rusoto test, but with timings ...


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54747



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

[Lldb-commits] [PATCH] D60119: modify-python-lldb.py: clean up __iter__ and __len__ support

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB357572: modify-python-lldb.py: clean up __iter__ and 
__len__ support (authored by labath, committed by ).
Herald added a subscriber: abidh.
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D60119?vs=193253&id=193468#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60119

Files:
  packages/Python/lldbsuite/test/python_api/debugger/TestDebuggerAPI.py
  
packages/Python/lldbsuite/test/python_api/default-constructor/sb_compileunit.py
  packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py
  packages/Python/lldbsuite/test/python_api/default-constructor/sb_section.py
  packages/Python/lldbsuite/test/python_api/default-constructor/sb_thread.py
  packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
  scripts/Python/modify-python-lldb.py
  scripts/interface/SBBreakpoint.i
  scripts/interface/SBCompileUnit.i
  scripts/interface/SBDebugger.i
  scripts/interface/SBInstructionList.i
  scripts/interface/SBProcess.i
  scripts/interface/SBSection.i
  scripts/interface/SBStringList.i
  scripts/interface/SBSymbolContextList.i
  scripts/interface/SBTarget.i
  scripts/interface/SBThread.i
  scripts/interface/SBType.i
  scripts/interface/SBValue.i
  scripts/interface/SBValueList.i

Index: scripts/Python/modify-python-lldb.py
===
--- scripts/Python/modify-python-lldb.py
+++ scripts/Python/modify-python-lldb.py
@@ -77,9 +77,6 @@
 
 # This supports the iteration protocol.
 iter_def = "def __iter__(self): return lldb_iter(self, '%s', '%s')"
-module_iter = "def module_iter(self): return lldb_iter(self, '%s', '%s')"
-breakpoint_iter = "def breakpoint_iter(self): return lldb_iter(self, '%s', '%s')"
-watchpoint_iter = "def watchpoint_iter(self): return lldb_iter(self, '%s', '%s')"
 section_iter = "def section_iter(self): return lldb_iter(self, '%s', '%s')"
 compile_unit_iter = "def compile_unit_iter(self): return lldb_iter(self, '%s', '%s')"
 
@@ -100,28 +97,7 @@
 #
 # This dictionary defines a mapping from classname to (getsize, getelem) tuple.
 #
-d = {'SBBreakpoint': ('GetNumLocations', 'GetLocationAtIndex'),
- 'SBCompileUnit': ('GetNumLineEntries', 'GetLineEntryAtIndex'),
- 'SBDebugger': ('GetNumTargets', 'GetTargetAtIndex'),
- 'SBModule': ('GetNumSymbols', 'GetSymbolAtIndex'),
- 'SBProcess': ('GetNumThreads', 'GetThreadAtIndex'),
- 'SBSection': ('GetNumSubSections', 'GetSubSectionAtIndex'),
- 'SBThread': ('GetNumFrames', 'GetFrameAtIndex'),
-
- 'SBInstructionList': ('GetSize', 'GetInstructionAtIndex'),
- 'SBStringList': ('GetSize', 'GetStringAtIndex',),
- 'SBSymbolContextList': ('GetSize', 'GetContextAtIndex'),
- 'SBTypeList': ('GetSize', 'GetTypeAtIndex'),
- 'SBValueList': ('GetSize', 'GetValueAtIndex'),
-
- 'SBType': ('GetNumberChildren', 'GetChildAtIndex'),
- 'SBValue': ('GetNumChildren', 'GetChildAtIndex'),
-
- # SBTarget needs special processing, see below.
- 'SBTarget': {'module': ('GetNumModules', 'GetModuleAtIndex'),
-  'breakpoint': ('GetNumBreakpoints', 'GetBreakpointAtIndex'),
-  'watchpoint': ('GetNumWatchpoints', 'GetWatchpointAtIndex')
-  },
+d = {'SBModule': ('GetNumSymbols', 'GetSymbolAtIndex'),
 
  # SBModule has an additional section_iter(), see below.
  'SBModule-section': ('GetNumSections', 'GetSectionAtIndex'),
@@ -238,16 +214,9 @@
 if match:
 # We found the beginning of the __init__ method definition.
 # This is a good spot to insert the iter support.
-#
-# But note that SBTarget has three types of iterations.
-if cls == "SBTarget":
-new_content.add_line(module_iter % (d[cls]['module']))
-new_content.add_line(breakpoint_iter % (d[cls]['breakpoint']))
-new_content.add_line(watchpoint_iter % (d[cls]['watchpoint']))
-else:
-if (state & DEFINING_ITERATOR):
-new_content.add_line(iter_def % d[cls])
-new_content.add_line(len_def % d[cls][0])
+
+new_content.add_line(iter_def % d[cls])
+new_content.add_line(len_def % d[cls][0])
 
 # SBModule has extra SBSection, SBCompileUnit iterators and
 # symbol_in_section_iter()!
Index: scripts/interface/SBTarget.i
===
--- scripts/interface/SBTarget.i
+++ scripts/interface/SBTarget.i
@@ -1103,6 +1103,21 @@
 modules.append(self.GetModuleAtIndex(idx))
 return modules
 
+def module_iter(self):
+'''Returns an iterator over all modules in a lldb.SBTarget
+object.'''
+return l

[Lldb-commits] [lldb] r357572 - modify-python-lldb.py: clean up __iter__ and __len__ support

[Pavel Labath via lldb-commits]

Author: labath
Date: Wed Apr  3 04:48:38 2019
New Revision: 357572

URL: http://llvm.org/viewvc/llvm-project?rev=357572&view=rev
Log:
modify-python-lldb.py: clean up __iter__ and __len__ support

Summary:
Instead of modifying the swig-generated code, just add the appropriate
methods to the interface files in order to get the swig to do the
generation for us.

This is a straight-forward move from the python script to the interface
files. The single class which has nontrivial handling in the script
(SBModule) has been left for a separate patch.

For the cases where I did not find any tests exercising the
iteration/length methods (i.e., no tests failed after I stopped emitting
them), I tried to add basic tests for that functionality.

Reviewers: zturner, jingham, amccarth

Subscribers: jdoerfert, lldb-commits

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/debugger/TestDebuggerAPI.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_compileunit.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_section.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_thread.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
lldb/trunk/scripts/Python/modify-python-lldb.py
lldb/trunk/scripts/interface/SBBreakpoint.i
lldb/trunk/scripts/interface/SBCompileUnit.i
lldb/trunk/scripts/interface/SBDebugger.i
lldb/trunk/scripts/interface/SBInstructionList.i
lldb/trunk/scripts/interface/SBProcess.i
lldb/trunk/scripts/interface/SBSection.i
lldb/trunk/scripts/interface/SBStringList.i
lldb/trunk/scripts/interface/SBSymbolContextList.i
lldb/trunk/scripts/interface/SBTarget.i
lldb/trunk/scripts/interface/SBThread.i
lldb/trunk/scripts/interface/SBType.i
lldb/trunk/scripts/interface/SBValue.i
lldb/trunk/scripts/interface/SBValueList.i

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/debugger/TestDebuggerAPI.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/debugger/TestDebuggerAPI.py?rev=357572&r1=357571&r2=357572&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/debugger/TestDebuggerAPI.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/debugger/TestDebuggerAPI.py
 Wed Apr  3 04:48:38 2019
@@ -34,6 +34,9 @@ class DebuggerAPITestCase(TestBase):
 self.dbg.SetPrompt(None)
 self.dbg.SetCurrentPlatform(None)
 self.dbg.SetCurrentPlatformSDKRoot(None)
+
+fresh_dbg = lldb.SBDebugger()
+self.assertEquals(len(fresh_dbg), 0)
 
 @add_test_categories(['pyapi'])
 def test_debugger_delete_invalid_target(self):

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_compileunit.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_compileunit.py?rev=357572&r1=357571&r2=357572&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_compileunit.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_compileunit.py
 Wed Apr  3 04:48:38 2019
@@ -12,5 +12,6 @@ def fuzz_obj(obj):
 obj.GetLineEntryAtIndex(0x)
 obj.FindLineEntryIndex(0, 0x, None)
 obj.GetDescription(lldb.SBStream())
+len(obj)
 for line_entry in obj:
 s = str(line_entry)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py?rev=357572&r1=357571&r2=357572&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py
 Wed Apr  3 04:48:38 2019
@@ -48,3 +48,4 @@ def fuzz_obj(obj):
 obj.GetNumSupportedHardwareWatchpoints(error)
 for thread in obj:
 s = str(thread)
+len(obj)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_section.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_section.py?rev=357572&r1=357571&r2=357572&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_section.py
 (

[Lldb-commits] [PATCH] D60195: modify-python-lldb.py: (Re)move __len__ and __iter__ support

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: amccarth, clayborg, jingham.

This patch moves the modify-python-lldb code for adding new functions to
the SBModule class into the SBModule interface file. As this is the last
class using this functionality, I also remove all support for this kind
of modifications from modify-python-lldb.py.


https://reviews.llvm.org/D60195

Files:
  
packages/Python/lldbsuite/test/python_api/module_section/TestModuleAndSection.py
  scripts/Python/modify-python-lldb.py
  scripts/interface/SBModule.i
  scripts/lldb.swig

Index: scripts/lldb.swig
===
--- scripts/lldb.swig
+++ scripts/lldb.swig
@@ -91,12 +91,6 @@
 elem = getattr(obj, getelem)
 for i in range(size()):
 yield elem(i)
-
-# ==
-# The modify-python-lldb.py script is responsible for post-processing this SWIG-
-# generated lldb.py module.  It is responsible for adding support for: iteration
-# protocol: __iter__, and built-in function len(): __len__.
-# ==
 %}
 
 %include "./Python/python-typemaps.swig"
Index: scripts/interface/SBModule.i
===
--- scripts/interface/SBModule.i
+++ scripts/interface/SBModule.i
@@ -370,6 +370,29 @@
 operator != (const lldb::SBModule &rhs) const;
  
 %pythoncode %{
+def __len__(self):
+'''Return the number of symbols in a lldb.SBModule object.'''
+return self.GetNumSymbols()
+
+def __iter__(self):
+'''Iterate over all symbols in a lldb.SBModule object.'''
+return lldb_iter(self, 'GetNumSymbols', 'GetSymbolAtIndex')
+
+def section_iter(self):
+'''Iterate over all sections in a lldb.SBModule object.'''
+return lldb_iter(self, 'GetNumSections', 'GetSectionAtIndex')
+
+def compile_unit_iter(self):
+'''Iterate over all compile units in a lldb.SBModule object.'''
+return lldb_iter(self, 'GetNumCompileUnits', 'GetCompileUnitAtIndex')
+
+def symbol_in_section_iter(self, section):
+'''Given a module and its contained section, returns an iterator on the
+symbols within the section.'''
+for sym in self:
+if in_range(sym, section):
+yield sym
+
 class symbols_access(object):
 re_compile_type = type(re.compile('.'))
 '''A helper object that will lazily hand out lldb.SBSymbol objects for a module when supplied an index, name, or regular expression.'''
Index: scripts/Python/modify-python-lldb.py
===
--- scripts/Python/modify-python-lldb.py
+++ scripts/Python/modify-python-lldb.py
@@ -75,55 +75,6 @@
 '^(%s|%s)""".*"""$' %
 (TWO_SPACES, EIGHT_SPACES))
 
-# This supports the iteration protocol.
-iter_def = "def __iter__(self): return lldb_iter(self, '%s', '%s')"
-section_iter = "def section_iter(self): return lldb_iter(self, '%s', '%s')"
-compile_unit_iter = "def compile_unit_iter(self): return lldb_iter(self, '%s', '%s')"
-
-# Called to implement the built-in function len().
-# Eligible objects are those containers with unambiguous iteration support.
-len_def = "def __len__(self): return self.%s()"
-
-# A convenience iterator for SBSymbol!
-symbol_in_section_iter_def = '''
-def symbol_in_section_iter(self, section):
-"""Given a module and its contained section, returns an iterator on the
-symbols within the section."""
-for sym in self:
-if in_range(sym, section):
-yield sym
-'''
-
-#
-# This dictionary defines a mapping from classname to (getsize, getelem) tuple.
-#
-d = {'SBModule': ('GetNumSymbols', 'GetSymbolAtIndex'),
-
- # SBModule has an additional section_iter(), see below.
- 'SBModule-section': ('GetNumSections', 'GetSectionAtIndex'),
- # And compile_unit_iter().
- 'SBModule-compile-unit': ('GetNumCompileUnits', 'GetCompileUnitAtIndex'),
- # As well as symbol_in_section_iter().
- 'SBModule-symbol-in-section': symbol_in_section_iter_def
- }
-
-def list_to_frag(list):
-"""Transform a list to equality program fragment.
-
-For example, ['GetID'] is transformed to 'self.GetID() == other.GetID()',
-and ['GetFilename', 'GetDirectory'] to 'self.GetFilename() == other.GetFilename()
-and self.GetDirectory() == other.GetDirectory()'.
-"""
-if not list:
-raise Exception("list should be non-empty")
-frag = StringIO.StringIO()
-for i in range(len(list)):
-if i > 0:
-frag.write(" and ")
-frag.write("self.{0}() == other.{0}()".format(list[i]))
-return frag.getvalue()
-
-
 class NewContent(StringIO.StringIO):
 """Simple

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py:160
+self.verify_module(modules[0],
+   "libuuidmatch.so", 
+   "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116")

The second failure I had was due to the full module showing up when the module 
was found, so this check failed.

The interesting part (and the reason why I checked out this patch) was that the 
module was still being found even when I removed the 
`basename_module_spec.GetFileSpec().GetDirectory().Clear();` line, which was my 
last uncertainty about this patch. So now I even more strongly believe that the 
clearing of the directory file spec is unnecessary and should be removed.



Comment at: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py:176
+so_path = self.getBuildArtifact("libuuidmismatch.so")
+self.yaml2obj("libuuidmatch.yaml", so_path)
+self.dbg.CreateTarget(None)

I think this is one of the causes of failure. Changing this to 
libuuidmismatch.yaml made it work for me. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60001



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

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

Hi, currently we have a private build server that executes the test-suite on 
ARC. There are failures for now, mostly due to unimplemented features for ARC 
like expressions support.
I'll take care of a public build-bot, if it is required.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718



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

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Major concern from my side is that clang does something like this too:

  clang/CMakeLists.txt
  86:  set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
  125:  link_directories("${LLVM_LIBRARY_DIR}")

It looks like AddLLVM.cmake uses it in function `configure_lit_site_cfg`:

  # They below might not be the build tree but provided binary tree.
  set(LLVM_SOURCE_DIR ${LLVM_MAIN_SRC_DIR})
  set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR})
  string(REPLACE "${CMAKE_CFG_INTDIR}" "${LLVM_BUILD_MODE}" LLVM_TOOLS_DIR 
"${LLVM_TOOLS_BINARY_DIR}")
  string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLVM_LIBS_DIR  
"${LLVM_LIBRARY_DIR}")

So we should always have some value for `LLVM_LIBRARY_DIR`. Not sure about the 
`link_directories` line.
Did you build and run test suite with this change? How does this string Replace 
work then?

> This is an issue because if you built libc++, it will try to link against 
> that one instead of the one from the android NDK.

Looking in-tree first seems like a reasonable default. You are cross-compiling 
only lldb-server so it can run on Android right? Everything else is built for 
your host platform?


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

https://reviews.llvm.org/D60180



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

Re: [Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Adrian Prantl via lldb-commits]

At the moment no. In a hypothetical future where LLVM is hosted on github, we 
could set up a system similar to the swift-lldb pull request testing, but for 
now that is all we have. On the flip side, I don't mind trying things out by 
committing a speculative fix as long as it is quickly reverted after the bot 
turns red.

-- adrian

> On Apr 2, 2019, at 8:58 PM, Greg Clayton  wrote:
> 
> No worries. I tried to get things working with a few patches. Is there a way 
> to try out a patch on green dragon other than just submitting the changes? 
> 
>> On Apr 2, 2019, at 3:03 PM, Adrian Prantl via Phabricator 
>>  wrote:
>> 
>> aprantl added a comment.
>> 
>> The bots were broken for more than six hours so I took the liberty to revert 
>> the change in r357534. Sorry for the inconvenience!
>> 
>> 
>> Repository:
>> rL LLVM
>> 
>> CHANGES SINCE LAST ACTION
>> https://reviews.llvm.org/D60001/new/
>> 
>> https://reviews.llvm.org/D60001
>> 
>> 
>> 
> 

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

[Lldb-commits] [lldb] r357600 - [LLDB] - Update the test cases after yaml2obj change.

[George Rimar via lldb-commits]

Author: grimar
Date: Wed Apr  3 08:28:35 2019
New Revision: 357600

URL: http://llvm.org/viewvc/llvm-project?rev=357600&view=rev
Log:
[LLDB] - Update the test cases after yaml2obj change.

https://reviews.llvm.org/D60122 (r357595) changed the
symbols description format in yaml2obj.

This change updates the LLDB tests.

Modified:
lldb/trunk/lit/Breakpoint/Inputs/split-dwarf-5-addrbase.dwo.yaml

lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file1.dwo.yaml

lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file2.dwo.yaml
lldb/trunk/lit/Modules/ELF/build-id-case.yaml
lldb/trunk/lit/Modules/ELF/duplicate-section.yaml
lldb/trunk/unittests/Core/Inputs/mangled-function-names.yaml
lldb/trunk/unittests/ObjectFile/ELF/Inputs/debug-info-relocations.pcm.yaml

lldb/trunk/unittests/ObjectFile/ELF/Inputs/sections-resolve-consistently.yaml
lldb/trunk/unittests/Symbol/Inputs/basic-call-frame-info.yaml

Modified: lldb/trunk/lit/Breakpoint/Inputs/split-dwarf-5-addrbase.dwo.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Breakpoint/Inputs/split-dwarf-5-addrbase.dwo.yaml?rev=357600&r1=357599&r2=357600&view=diff
==
--- lldb/trunk/lit/Breakpoint/Inputs/split-dwarf-5-addrbase.dwo.yaml (original)
+++ lldb/trunk/lit/Breakpoint/Inputs/split-dwarf-5-addrbase.dwo.yaml Wed Apr  3 
08:28:35 2019
@@ -30,6 +30,4 @@ Sections:
 Flags:   [ SHF_EXCLUDE ]
 AddressAlign:0x0001
 Content: 
011101B04225252513050325022E00111B120640186E2503253A0B3B0B3F19032E00111B1206401803253A0B3B0B49133F1904240003253E0B0B0B00
-Symbols: {}
-DynamicSymbols:  {}
 ...

Modified: 
lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file1.dwo.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file1.dwo.yaml?rev=357600&r1=357599&r2=357600&view=diff
==
--- 
lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file1.dwo.yaml 
(original)
+++ 
lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file1.dwo.yaml 
Wed Apr  3 08:28:35 2019
@@ -35,6 +35,4 @@ Sections:
 Flags:   [ SHF_EXCLUDE ]
 AddressAlign:0x0001
 Content: 150005000800010004000101040C1F04253200
-Symbols: {}
-DynamicSymbols:  {}
 ...

Modified: 
lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file2.dwo.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file2.dwo.yaml?rev=357600&r1=357599&r2=357600&view=diff
==
--- 
lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file2.dwo.yaml 
(original)
+++ 
lldb/trunk/lit/Breakpoint/Inputs/split-dwarf5-debug-stroffsets-file2.dwo.yaml 
Wed Apr  3 08:28:35 2019
@@ -35,6 +35,4 @@ Sections:
 Flags:   [ SHF_EXCLUDE ]
 AddressAlign:0x0001
 Content: 150005000800010004000100040C1F04253200
-Symbols: {}
-DynamicSymbols:  {}
 ...

Modified: lldb/trunk/lit/Modules/ELF/build-id-case.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/ELF/build-id-case.yaml?rev=357600&r1=357599&r2=357600&view=diff
==
--- lldb/trunk/lit/Modules/ELF/build-id-case.yaml (original)
+++ lldb/trunk/lit/Modules/ELF/build-id-case.yaml Wed Apr  3 08:28:35 2019
@@ -32,10 +32,9 @@ Sections:
 AddressAlign:0x0008
 Content: DEADBEEFBAADF00D
 Symbols:
-  Local:
-- Name:main
-  Type:STT_FUNC
-  Section: .text
-  Value:   0x004003D0
-  Size:0x0008
+  - Name:main
+Type:STT_FUNC
+Section: .text
+Value:   0x004003D0
+Size:0x0008
 ...

Modified: lldb/trunk/lit/Modules/ELF/duplicate-section.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Modules/ELF/duplicate-section.yaml?rev=357600&r1=357599&r2=357600&view=diff
==
--- lldb/trunk/lit/Modules/ELF/duplicate-section.yaml (original)
+++ lldb/trunk/lit/Modules/ELF/duplicate-section.yaml Wed Apr  3 08:28:35 2019
@@ -33,10 +33,9 @@ Sections:
 AddressAlign:0x0008
 Content: DEADBEEFBAADF00D
 Symbols:
-  Local:
-- Name:main
-  Type:STT_FUNC
-  Section: .text
-  Value:   0x004003D0
-  Size:0x0008
+  - Name:main
+Type:STT_FUNC
+Section: .text
+Value:   0x000

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth marked an inline comment as done.
amccarth added a comment.

Thanks for the review.

In D60152#1452704 , @labath wrote:

> FTR, I believe using pipes here is wrong altogether because it can lead to 
> deadlock. The size of the pipe buffer is implementation-defined, and since 
> there's noone reading from it while we write it, we can block indefinitely if 
> we encounter a particularly long sequence of -o commands. I guess the fact 
> that we haven't hit this in practice means that the implementation-defined 
> size is sufficiently big on all OSs we care about.


Agreed, pipes probably aren't the right way, but this was just a drive-by 
cleanup.  On Windows, the necessary size is passed to the `_pipe` call, so if 
we have an unusually large buffer of commands it should either work or fail 
gracefully rather than deadlocking.




Comment at: lldb/tools/driver/Driver.cpp:492-496
 #ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
+  _close(fd);
 #else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
+  close(fd);
 #endif

labath wrote:
> Do you think it would be possible to use 
> `llvm::Process::SafelyCloseFileDescriptor` here? It looks like that still 
> uses close (without the `_`) on windows, which we are trying hard to avoid 
> here, but I'm not sure why. I know close is technically deprecated on 
> windows, but AFAIK the only thing deprecated about it is the name, and 
> otherwise it works correctly.
> 
> (If that works, I'd even consider removing this function entirely, as the 
> only thing it does extra is clear the fd, but that does not seem necessary 
> since we're now calling it immediately before we return (and the fds go out 
> of scope anyway)).
I'm happy to use `llvm::sys::Process::SafelyCloseFileDescriptor` and delete my 
little wrapper.  I agree that resetting the df to -1 right before it goes out 
of scope isn't useful.  I'll update the patch momentarily.

If we're not that worried about the deprecated names on Windows, should I also 
just use `fdopen` and forget about wrapping that as well?

Either way, I'm going to keep the OpenPipe wrapper since the Windows version 
takes additional parameters.


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

https://reviews.llvm.org/D60152



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

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Greg Clayton via Phabricator via lldb-commits]

clayborg marked 2 inline comments as done.
clayborg added inline comments.



Comment at: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py:160
+self.verify_module(modules[0],
+   "libuuidmatch.so", 
+   "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116")

labath wrote:
> The second failure I had was due to the full module showing up when the 
> module was found, so this check failed.
> 
> The interesting part (and the reason why I checked out this patch) was that 
> the module was still being found even when I removed the 
> `basename_module_spec.GetFileSpec().GetDirectory().Clear();` line, which was 
> my last uncertainty about this patch. So now I even more strongly believe 
> that the clearing of the directory file spec is unnecessary and should be 
> removed.
And that _didn't_ work for me. If I removed it, it doesn't work on macOS. I 
wonder if we might be setting some image search paths up as part of the test. I 
just tested manually and if I do the command in a fresh LLDB, I get the 
fullpath, even on mac. If I run through the test suite, then I just get the 
basename???



Comment at: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py:176
+so_path = self.getBuildArtifact("libuuidmismatch.so")
+self.yaml2obj("libuuidmatch.yaml", so_path)
+self.dbg.CreateTarget(None)

labath wrote:
> I think this is one of the causes of failure. Changing this to 
> libuuidmismatch.yaml made it work for me. 
Ok, I will fix this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60001



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

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth updated this revision to Diff 193515.
amccarth edited the summary of this revision.
amccarth added a comment.

Further simplification per labath's feedback.

Specifically:

1. Use a utility function from llvm to avoid making own wrapper for closing a 
file descriptor.
2. Don't bother resetting fd numbers right before they go out of scope.
3. Live with the Windows-deprecated spelling of fdopen in favor of more 
`#ifdefs`.
4. Updated summary to reflect these changes.


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

https://reviews.llvm.org/D60152

Files:
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -22,6 +22,7 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -469,85 +470,58 @@
   return error;
 }
 
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size, int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
-
-  ::FILE *commands_file = nullptr;
-  fds[0] = -1;
-  fds[1] = -1;
-  int err = 0;
+static inline int OpenPipe(int fds[2], std::size_t size) {
 #ifdef _WIN32
-  err = _pipe(fds, commands_size, O_BINARY);
+  return _pipe(fds, size, O_BINARY);
 #else
-  err = pipe(fds);
+  (void) size;
+  return pipe(fds);
 #endif
-  if (err == 0) {
-ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-if (nrwr < 0) {
-  WithColor::error()
-  << format(
- "write(%i, %p, %" PRIu64
- ") failed (errno = %i) when trying to open LLDB commands pipe",
- fds[WRITE], static_cast(commands_data),
- static_cast(commands_size), errno)
-  << '\n';
-} else if (static_cast(nrwr) == commands_size) {
-// Close the write end of the pipe so when we give the read end to
-// the debugger/command interpreter it will exit when it consumes all
-// of the data
-#ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
-#else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
-#endif
-  // Now open the read file descriptor in a FILE * that we can give to
-  // the debugger as an input handle
-  commands_file = fdopen(fds[READ], "r");
-  if (commands_file != nullptr) {
-fds[READ] = -1; // The FILE * 'commands_file' now owns the read
-// descriptor Hand ownership if the FILE * over to the
-// debugger for "commands_file".
-  } else {
-WithColor::error() << format("fdopen(%i, \"r\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-  }
-}
-  } else {
+}
+
+static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
+  size_t commands_size) {
+  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
+  int fds[2] = {-1, -1};
+
+  if (OpenPipe(fds, commands_size) != 0) {
 WithColor::error()
 << "can't create pipe file descriptors for LLDB commands\n";
+return nullptr;
   }
 
-  return commands_file;
-}
+  ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
+  if (nrwr != commands_size) {
+WithColor::error()
+<< format(
+"write(%i, %p, %" PRIu64
+") failed (errno = %i) when trying to open LLDB commands pipe",
+fds[WRITE], static_cast(commands_data),
+static_cast(commands_size), errno)
+<< '\n';
+llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
+llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
+return nullptr;
+  }
 
-void CleanupAfterCommandSourcing(int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
+  // Close the write end of the pipe, so that the command interpreter will exit
+  // when it consumes all the data.
+  llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
 
-  // Close any pipes that we still have ownership of
-  if (fds[WRITE] != -1) {
-#ifdef _WIN32
-_close(fds[WRITE]);
-fds[WRITE] = -1;
-#else
-close(fds[WRITE]);
-fds[WRITE] = -1;
-#endif
+  // Open the read file descriptor as a FILE * that we can return as an input
+  // handle.
+  ::FILE *commands_file = fdopen(fds[READ], "rb");
+  if (commands_file == nullptr) {
+WithColor::error() << format("fdopen(%i, \"rb\") failed (errno = %i) "
+ "when trying to open LLDB commands pipe",
+ fds[READ], errno)
+

[Lldb-commits] [PATCH] D60195: modify-python-lldb.py: (Re)move __len__ and __iter__ support

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth accepted this revision.
amccarth added a comment.
This revision is now accepted and ready to land.

Nice!  Thanks for cleaning up the affected comments as well.


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

https://reviews.llvm.org/D60195



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

[Lldb-commits] [lldb] r357603 - Attempt #2 to get this patch working. I will watch the build bots carefully today.

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Wed Apr  3 09:30:44 2019
New Revision: 357603

URL: http://llvm.org/viewvc/llvm-project?rev=357603&view=rev
Log:
Attempt #2 to get this patch working. I will watch the build bots carefully 
today.

Allow partial UUID matching in Minidump core file plug-in

Breakpad had bugs in earlier versions where it would take a 20 byte ELF build 
ID and put it into the minidump file as a 16 byte PDB70 UUID with an age of 
zero. This would make it impossible to do postmortem debugging with one of 
these older minidump files.

This fix allows partial matching of UUIDs. To do this we first try and match 
with the full UUID value, and then fall back to removing the original directory 
path from the module specification and we remove the UUID requirement, and then 
manually do the matching ourselves. This allows scripts to find symbols files 
using a symbol server, place them all in a directory, use the "setting set 
target.exec-search-paths" setting to specify the directory, and then load the 
core file. The Target::GetSharedModule() can then find the correct file without 
doing any other matching and load it.

Tests were added to cover a partial UUID match where the breakpad file has a 16 
byte UUID and the actual file on disk has a 20 byte UUID, both where the first 
16 bytes match, and don't match.

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


Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmatch.yaml

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/libuuidmismatch.yaml

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-match.dmp
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-partial-uuids-mismatch.dmp
   (with props)
Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=357603&r1=357602&r2=357603&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Wed Apr  3 09:30:44 2019
@@ -8,6 +8,7 @@ from six import iteritems
 import shutil
 
 import lldb
+import os
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -132,3 +133,55 @@ class MiniDumpUUIDTestCase(TestBase):
 self.assertEqual(2, len(modules))
 self.verify_module(modules[0], "/not/exist/a", None)
 self.verify_module(modules[1], "/not/exist/b", None)
+
+@expectedFailureAll(oslist=["windows"])
+def test_partial_uuid_match(self):
+"""
+Breakpad has been known to create minidump files using CvRecord in 
each
+module whose signature is set to PDB70 where the UUID only 
contains the
+first 16 bytes of a 20 byte ELF build ID. Code was added to 
+ProcessMinidump.cpp to deal with this and allows partial UUID 
matching. 
+
+This test verifies that if we have a minidump with a 16 byte UUID, 
that
+we are able to associate a symbol file with a 20 byte UUID only if 
the
+first 16 bytes match. In this case we will see the path from the 
file
+we found in the test directory and the 20 byte UUID from the actual
+file, not the 16 byte shortened UUID from the minidump.
+"""
+so_path = self.getBuildArtifact("libuuidmatch.so")
+self.yaml2obj("libuuidmatch.yaml", so_path)
+self.dbg.CreateTarget(None)
+self.target = self.dbg.GetSelectedTarget()
+cmd = 'settings set target.exec-search-paths "%s"' % 
(os.path.dirname(so_path))
+self.dbg.HandleCommand(cmd)
+self.process = 
self.target.LoadCore("linux-arm-partial-uuids-match.dmp")
+modules = self.target.modules
+self.assertEqual(1, len(modules))
+self.verify_module(modules[0], so_path, 
+   "7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116")
+
+@expectedFailureAll(oslist=["windows"])
+def test_partial_uuid_mismatch(self):
+"""
+Breakpad has been known to create minidump files using CvRecord in 
each
+module whose signature is set to PDB70 where the UUID only 
contains the
+first 16 bytes of a 20 byte ELF build ID. Code was added to 
+ProcessMinidump.cpp to deal with this a

[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

ok, so I think I figured out what was going on: I had the .so files still in my 
build packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new 
directory. They didn't show up in the "svn stat" command because they are 
ignored!!! arg! That is what was causing problems when testing on my machine.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60001



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

[Lldb-commits] [PATCH] D60018: [codeview] Remove Type member from CVRecord

[Reid Kleckner via Phabricator via lldb-commits]

rnk marked 2 inline comments as done.
rnk added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60018



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

[Lldb-commits] [PATCH] D60018: [codeview] Remove Type member from CVRecord

[Reid Kleckner via Phabricator via lldb-commits]

rnk updated this revision to Diff 193526.
rnk added a comment.

- final updates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60018

Files:
  lld/COFF/PDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
  llvm/include/llvm/DebugInfo/CodeView/RecordSerialization.h
  llvm/include/llvm/DebugInfo/CodeView/SymbolSerializer.h
  llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h
  llvm/lib/DebugInfo/CodeView/AppendingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp
  llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
  llvm/lib/DebugInfo/CodeView/ContinuationRecordBuilder.cpp
  llvm/lib/DebugInfo/CodeView/GlobalTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/SimpleTypeSerializer.cpp
  llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
  llvm/lib/DebugInfo/CodeView/TypeDumpVisitor.cpp
  llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp
  llvm/lib/DebugInfo/CodeView/TypeTableCollection.cpp
  llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
  llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
  llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
  llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
  llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

Index: llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
===
--- llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
+++ llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
@@ -42,8 +42,6 @@
 }
 
 inline bool operator==(const CVType &R1, const CVType &R2) {
-  if (R1.Type != R2.Type)
-return false;
   if (R1.RecordData != R2.RecordData)
 return false;
   return true;
@@ -107,7 +105,7 @@
   GlobalState->Records.push_back(AR);
   GlobalState->Indices.push_back(Builder.writeLeafType(AR));
 
-  CVType Type(TypeLeafKind::LF_ARRAY, Builder.records().back());
+  CVType Type(Builder.records().back());
   GlobalState->TypeVector.push_back(Type);
 
   GlobalState->AllOffsets.push_back(
@@ -369,11 +367,10 @@
   TypeIndex IndexOne = Builder.writeLeafType(Modifier);
 
   // set up a type stream that refers to the above two serialized records.
-  std::vector TypeArray;
-  TypeArray.push_back(
-  CVType(static_cast(Class.Kind), Builder.records()[0]));
-  TypeArray.push_back(
-  CVType(static_cast(Modifier.Kind), Builder.records()[1]));
+  std::vector TypeArray = {
+  {Builder.records()[0]},
+  {Builder.records()[1]},
+  };
   BinaryItemStream ItemStream(llvm::support::little);
   ItemStream.setItems(TypeArray);
   VarStreamArray TypeStream(ItemStream);
Index: llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
===
--- llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
+++ llvm/tools/llvm-pdbutil/MinimalTypeDumper.cpp
@@ -224,7 +224,7 @@
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}",
fmt_align(Index, AlignStyle::Right, Width),
-   formatTypeLeafKind(Record.Type), Record.length());
+   formatTypeLeafKind(Record.kind()), Record.length());
   if (Hashes) {
 std::string H;
 if (Index.toArrayIndex() >= HashValues.size()) {
Index: llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
===
--- llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
+++ llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp
@@ -331,7 +331,7 @@
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}]",
fmt_align(Offset, AlignStyle::Right, 6),
-   formatSymbolKind(Record.Type), Record.length());
+   formatSymbolKind(Record.kind()), Record.length());
   P.Indent();
   return Error::success();
 }
Index: llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp
@@ -98,7 +98,7 @@
 
   CVType toCodeViewRecord(AppendingTypeTableBuilder &TS) const override {
 TS.writeLeafType(Record);
-return CVType(Kind, TS.records().back());
+return CVType(TS.records().back());
   }
 
   mutable T Record;
@@ -496,7 +496,7 @@
 Member.Member->writeTo(CRB);
   }
   TS.insertRecord(CRB);
-  return CVType(Kind, TS.records().back());
+  return CVType(TS.records().back());
 }
 
 void MappingTraits::mapping(IO &io, OneMethodRecord &Record) {
Index: llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
===
--- llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
+++ llvm/lib/ObjectYAML/CodeViewYAMLSymbols.cpp
@@ -248,7 +248,7 @@
 uint8_t *Buffer = Allocator.Allocate(TotalLen);
 ::memcpy(Buffer, 

Re: [Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

[Greg Clayton via lldb-commits]

So I resubmitted with 357603 and green dragon seems happy. I will continue to 
watch for any issues. Let me know if there are other failures that I don't know 
about

> On Apr 2, 2019, at 11:57 AM, Adrian Prantl via Phabricator 
>  wrote:
> 
> aprantl added a comment.
> 
> Since this commit, TestMiniDumpUUID_py is failing on green dragon.
> 
> Could you please revert the change or commit a fix?
> 
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/lastCompletedBuild/testReport/lldb-Suite/functionalities_postmortem_minidump-new/TestMiniDumpUUID_py/
> 
> 
> Repository:
>  rL LLVM
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D60001/new/
> 
> https://reviews.llvm.org/D60001
> 
> 
> 

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

[Lldb-commits] [PATCH] D59826: [expression] Explicitly build the lookup table of any TagDecl's context

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.



In D59826#1452771 , @martong wrote:

> In D59826#1451536 , @clayborg wrote:
>
> > Any way to dump the AST in a test to verify we don't create multiple?
>
>
> I think I might be able to use the `log` object to dump the AST and then from 
> python it would be possible to check for duplicates. Is that a good direction?


yes that would be great!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59826



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

[Lldb-commits] [PATCH] D60153: Re-enable most lldb-vscode tests on Linux.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Thanks for getting this stuff reliably working. I debug using VS Code every day 
using lldb-vscode and it is my favorite LLDB based debugger! I look forward to 
seeing support for Windows and linux being tested and available.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60153



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

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D60180#1453106 , @sgraenitz wrote:

> Major concern from my side is that clang does something like this too:
>
>   clang/CMakeLists.txt
>   86:  set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
>   125:  link_directories("${LLVM_LIBRARY_DIR}")
>
>
> It looks like AddLLVM.cmake uses it in function `configure_lit_site_cfg`:
>
>   # They below might not be the build tree but provided binary tree.
>   set(LLVM_SOURCE_DIR ${LLVM_MAIN_SRC_DIR})
>   set(LLVM_BINARY_DIR ${LLVM_BINARY_DIR})
>   string(REPLACE "${CMAKE_CFG_INTDIR}" "${LLVM_BUILD_MODE}" LLVM_TOOLS_DIR 
> "${LLVM_TOOLS_BINARY_DIR}")
>   string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLVM_LIBS_DIR  
> "${LLVM_LIBRARY_DIR}")
>   
>
> So we should always have some value for `LLVM_LIBRARY_DIR`. Not sure about 
> the `link_directories` line.
>  Did you build and run test suite with this change? How does this string 
> Replace work then?


Having the definition is fine, but the `link_directories` line is what causes 
the issue. I definitely built, but from I looked at my tmux history from last 
night and it looks like I ran the test suite from a unified build directory 
instead of an lldb standalone build directory, so I'll run the test suite there 
and report back. Knowing what I know now, I expect it to fail, so I'll need to 
revise and test this more thoroughly.

>> This is an issue because if you built libc++, it will try to link against 
>> that one instead of the one from the android NDK.
> 
> Looking in-tree first seems like a reasonable default. You are 
> cross-compiling only lldb-server so it can run on Android right? Everything 
> else is built for your host platform?

No, everything is being built for android. Cross-compiling lldb-server involves 
cross-compiling llvm libraries, clang libraries, and if you've got swift in the 
picture, swift host libraries. LLVM and clang libraries are built against the 
libc++ from the android NDK, but in standalone builds, LLDB will try to link 
against the freshly built libc++ from LLVM. You get loads of undefined 
references to symbols from the NDK's libc++.


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

https://reviews.llvm.org/D60180



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

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.



Comment at: lldb/tools/driver/Driver.cpp:492-496
 #ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
+  _close(fd);
 #else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
+  close(fd);
 #endif

amccarth wrote:
> labath wrote:
> > Do you think it would be possible to use 
> > `llvm::Process::SafelyCloseFileDescriptor` here? It looks like that still 
> > uses close (without the `_`) on windows, which we are trying hard to avoid 
> > here, but I'm not sure why. I know close is technically deprecated on 
> > windows, but AFAIK the only thing deprecated about it is the name, and 
> > otherwise it works correctly.
> > 
> > (If that works, I'd even consider removing this function entirely, as the 
> > only thing it does extra is clear the fd, but that does not seem necessary 
> > since we're now calling it immediately before we return (and the fds go out 
> > of scope anyway)).
> I'm happy to use `llvm::sys::Process::SafelyCloseFileDescriptor` and delete 
> my little wrapper.  I agree that resetting the df to -1 right before it goes 
> out of scope isn't useful.  I'll update the patch momentarily.
> 
> If we're not that worried about the deprecated names on Windows, should I 
> also just use `fdopen` and forget about wrapping that as well?
> 
> Either way, I'm going to keep the OpenPipe wrapper since the Windows version 
> takes additional parameters.
I think using the non-underscore fdopen is fine, but I am not a windows guy. 
(The nice thing about SafelyCloseFileDescriptor is that it already provides an 
abstraction and so we can add the underscore there centrally, should it ever 
become necessary).


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

https://reviews.llvm.org/D60152



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

[Lldb-commits] [PATCH] D59775: Minidump: Add support for reading/writing strings

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: amccarth.
labath added a comment.

In D59775#1442987 , @clayborg wrote:

> Looks fine to me, but probably need a LLVM specific person for the final ok


Maybe @zturner or @amccarth could be the LLVM person ? :)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59775



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

[Lldb-commits] [lldb] r357626 - Fix and simplify PrepareCommandsForSourcing

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed Apr  3 12:49:14 2019
New Revision: 357626

URL: http://llvm.org/viewvc/llvm-project?rev=357626&view=rev
Log:
Fix and simplify PrepareCommandsForSourcing

Spotted some problems in the Driver's PrepareCommandsForSourcing while
helping a colleague track another problem.

1. One error case was not handled because there was no else clause.
Fixed by switching to llvm's early-out style instead of nested
`if (succes) { } else { }` cases.  This keeps error handling close
to the actual error.

2. One call-site failed to call the clean-up function.  I solved this
by simplifying the API.  PrepareCommandsForSourcing no longer requires
the caller to provide a buffer for the pipe's file descriptors and to
call a separate clean-up function later.  PrepareCommandsForSourcing
now ensures the file descriptors are handled before returning.
(The read end of the pipe is held open by the returned FILE * as
before.)

I also eliminated an unnecessary local, shorted the lifetime of another,
and tried to improve the comments.

I wrapped the call to open the pipe to get the `#ifdef`s out of the
mainline.  I replaced the `close`/`_close` calls with a platform-neutral
helper from `llvm::sys` for the same reason.  Per discussion on the
review, I'm leaving the `fdopen` call to use the spelling that Windows
has officially deprecated because it still works it avoids more `#ifdef`s.

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

Modified:
lldb/trunk/tools/driver/Driver.cpp

Modified: lldb/trunk/tools/driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/driver/Driver.cpp?rev=357626&r1=357625&r2=357626&view=diff
==
--- lldb/trunk/tools/driver/Driver.cpp (original)
+++ lldb/trunk/tools/driver/Driver.cpp Wed Apr  3 12:49:14 2019
@@ -22,6 +22,7 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -469,85 +470,58 @@ SBError Driver::ProcessArgs(const opt::I
   return error;
 }
 
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size, int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
-
-  ::FILE *commands_file = nullptr;
-  fds[0] = -1;
-  fds[1] = -1;
-  int err = 0;
+static inline int OpenPipe(int fds[2], std::size_t size) {
 #ifdef _WIN32
-  err = _pipe(fds, commands_size, O_BINARY);
+  return _pipe(fds, size, O_BINARY);
 #else
-  err = pipe(fds);
+  (void) size;
+  return pipe(fds);
 #endif
-  if (err == 0) {
-ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-if (nrwr < 0) {
-  WithColor::error()
-  << format(
- "write(%i, %p, %" PRIu64
- ") failed (errno = %i) when trying to open LLDB commands 
pipe",
- fds[WRITE], static_cast(commands_data),
- static_cast(commands_size), errno)
-  << '\n';
-} else if (static_cast(nrwr) == commands_size) {
-// Close the write end of the pipe so when we give the read end to
-// the debugger/command interpreter it will exit when it consumes all
-// of the data
-#ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
-#else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
-#endif
-  // Now open the read file descriptor in a FILE * that we can give to
-  // the debugger as an input handle
-  commands_file = fdopen(fds[READ], "r");
-  if (commands_file != nullptr) {
-fds[READ] = -1; // The FILE * 'commands_file' now owns the read
-// descriptor Hand ownership if the FILE * over to the
-// debugger for "commands_file".
-  } else {
-WithColor::error() << format("fdopen(%i, \"r\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-  }
-}
-  } else {
+}
+
+static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
+  size_t commands_size) {
+  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
+  int fds[2] = {-1, -1};
+
+  if (OpenPipe(fds, commands_size) != 0) {
 WithColor::error()
 << "can't create pipe file descriptors for LLDB commands\n";
+return nullptr;
   }
 
-  return commands_file;
-}
-
-void CleanupAfterCommandSourcing(int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
-
-  // Close any pipes that we still have ownership of
-  if (fds[WRITE] != -1) {
-#ifdef _WIN32
-_close(fds[WRITE]);
-fds[WRITE] = -1;
-#else
-close(fds[WRITE]);
-fds[WRITE] = -1;
-#endif
+  ssize_t nrwr =

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

[Adrian McCarthy via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357626: Fix and simplify PrepareCommandsForSourcing 
(authored by amccarth, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60152?vs=193515&id=193572#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60152

Files:
  lldb/trunk/tools/driver/Driver.cpp

Index: lldb/trunk/tools/driver/Driver.cpp
===
--- lldb/trunk/tools/driver/Driver.cpp
+++ lldb/trunk/tools/driver/Driver.cpp
@@ -22,6 +22,7 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -469,85 +470,58 @@
   return error;
 }
 
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size, int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
-
-  ::FILE *commands_file = nullptr;
-  fds[0] = -1;
-  fds[1] = -1;
-  int err = 0;
-#ifdef _WIN32
-  err = _pipe(fds, commands_size, O_BINARY);
-#else
-  err = pipe(fds);
-#endif
-  if (err == 0) {
-ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-if (nrwr < 0) {
-  WithColor::error()
-  << format(
- "write(%i, %p, %" PRIu64
- ") failed (errno = %i) when trying to open LLDB commands pipe",
- fds[WRITE], static_cast(commands_data),
- static_cast(commands_size), errno)
-  << '\n';
-} else if (static_cast(nrwr) == commands_size) {
-// Close the write end of the pipe so when we give the read end to
-// the debugger/command interpreter it will exit when it consumes all
-// of the data
+static inline int OpenPipe(int fds[2], std::size_t size) {
 #ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
+  return _pipe(fds, size, O_BINARY);
 #else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
+  (void) size;
+  return pipe(fds);
 #endif
-  // Now open the read file descriptor in a FILE * that we can give to
-  // the debugger as an input handle
-  commands_file = fdopen(fds[READ], "r");
-  if (commands_file != nullptr) {
-fds[READ] = -1; // The FILE * 'commands_file' now owns the read
-// descriptor Hand ownership if the FILE * over to the
-// debugger for "commands_file".
-  } else {
-WithColor::error() << format("fdopen(%i, \"r\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-  }
-}
-  } else {
+}
+
+static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
+  size_t commands_size) {
+  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
+  int fds[2] = {-1, -1};
+
+  if (OpenPipe(fds, commands_size) != 0) {
 WithColor::error()
 << "can't create pipe file descriptors for LLDB commands\n";
+return nullptr;
   }
 
-  return commands_file;
-}
-
-void CleanupAfterCommandSourcing(int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
-
-  // Close any pipes that we still have ownership of
-  if (fds[WRITE] != -1) {
-#ifdef _WIN32
-_close(fds[WRITE]);
-fds[WRITE] = -1;
-#else
-close(fds[WRITE]);
-fds[WRITE] = -1;
-#endif
+  ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
+  if (nrwr != commands_size) {
+WithColor::error()
+<< format(
+"write(%i, %p, %" PRIu64
+") failed (errno = %i) when trying to open LLDB commands pipe",
+fds[WRITE], static_cast(commands_data),
+static_cast(commands_size), errno)
+<< '\n';
+llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
+llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
+return nullptr;
   }
 
-  if (fds[READ] != -1) {
-#ifdef _WIN32
-_close(fds[READ]);
-fds[READ] = -1;
-#else
-close(fds[READ]);
-fds[READ] = -1;
-#endif
+  // Close the write end of the pipe, so that the command interpreter will exit
+  // when it consumes all the data.
+  llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
+
+  // Open the read file descriptor as a FILE * that we can return as an input
+  // handle.
+  ::FILE *commands_file = fdopen(fds[READ], "rb");
+  if (commands_file == nullptr) {
+WithColor::error() << format("fdopen(%i, \"rb\") failed (errno = %i) "
+ "when trying to open LLDB commands pipe",
+

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: lldb/tools/driver/Driver.cpp:492-496
 #ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
+  _close(fd);
 #else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
+  close(fd);
 #endif

labath wrote:
> amccarth wrote:
> > labath wrote:
> > > Do you think it would be possible to use 
> > > `llvm::Process::SafelyCloseFileDescriptor` here? It looks like that still 
> > > uses close (without the `_`) on windows, which we are trying hard to 
> > > avoid here, but I'm not sure why. I know close is technically deprecated 
> > > on windows, but AFAIK the only thing deprecated about it is the name, and 
> > > otherwise it works correctly.
> > > 
> > > (If that works, I'd even consider removing this function entirely, as the 
> > > only thing it does extra is clear the fd, but that does not seem 
> > > necessary since we're now calling it immediately before we return (and 
> > > the fds go out of scope anyway)).
> > I'm happy to use `llvm::sys::Process::SafelyCloseFileDescriptor` and delete 
> > my little wrapper.  I agree that resetting the df to -1 right before it 
> > goes out of scope isn't useful.  I'll update the patch momentarily.
> > 
> > If we're not that worried about the deprecated names on Windows, should I 
> > also just use `fdopen` and forget about wrapping that as well?
> > 
> > Either way, I'm going to keep the OpenPipe wrapper since the Windows 
> > version takes additional parameters.
> I think using the non-underscore fdopen is fine, but I am not a windows guy. 
> (The nice thing about SafelyCloseFileDescriptor is that it already provides 
> an abstraction and so we can add the underscore there centrally, should it 
> ever become necessary).
I dug into this a little bit to find out what the actual difference is, and 
there isn't a difference at all.  `close` and `_close`, `fdopen` and `_fdopen`, 
and all other pairs of underscore and non underscore names both resolve to the 
exact same symbol in the object file (it links in an object file that has a 
symbol for `close` that aliases it to `_close`.  So there is literally no 
difference.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60152



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

[Lldb-commits] [PATCH] D60178: [Reproducers] Capture return values of functions returning by ptr/ref

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 193581.
JDevlieghere added a comment.

- Add tests for lldb-instr
- Feedback Pavel


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

https://reviews.llvm.org/D60178

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/lit/tools/lldb-instr/Inputs/foo.cpp
  lldb/lit/tools/lldb-instr/Inputs/foo.h
  lldb/lit/tools/lldb-instr/TestInstrumentationRecord.test
  lldb/lit/tools/lldb-instr/TestInstrumentationRegister.test
  lldb/source/API/SBAddress.cpp
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBBlock.cpp
  lldb/source/API/SBBreakpoint.cpp
  lldb/source/API/SBBreakpointLocation.cpp
  lldb/source/API/SBBreakpointName.cpp
  lldb/source/API/SBBroadcaster.cpp
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/API/SBCompileUnit.cpp
  lldb/source/API/SBData.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBDeclaration.cpp
  lldb/source/API/SBError.cpp
  lldb/source/API/SBEvent.cpp
  lldb/source/API/SBExecutionContext.cpp
  lldb/source/API/SBExpressionOptions.cpp
  lldb/source/API/SBFileSpec.cpp
  lldb/source/API/SBFileSpecList.cpp
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBFunction.cpp
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/API/SBLineEntry.cpp
  lldb/source/API/SBListener.cpp
  lldb/source/API/SBMemoryRegionInfo.cpp
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBModuleSpec.cpp
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBProcessInfo.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/API/SBSection.cpp
  lldb/source/API/SBSourceManager.cpp
  lldb/source/API/SBStringList.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBSymbol.cpp
  lldb/source/API/SBSymbolContext.cpp
  lldb/source/API/SBSymbolContextList.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/API/SBThread.cpp
  lldb/source/API/SBThreadCollection.cpp
  lldb/source/API/SBThreadPlan.cpp
  lldb/source/API/SBType.cpp
  lldb/source/API/SBTypeCategory.cpp
  lldb/source/API/SBTypeEnumMember.cpp
  lldb/source/API/SBTypeFilter.cpp
  lldb/source/API/SBTypeFormat.cpp
  lldb/source/API/SBTypeNameSpecifier.cpp
  lldb/source/API/SBTypeSummary.cpp
  lldb/source/API/SBTypeSynthetic.cpp
  lldb/source/API/SBUnixSignals.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBValueList.cpp
  lldb/source/API/SBVariablesOptions.cpp
  lldb/source/API/SBWatchpoint.cpp
  lldb/source/Utility/ReproducerInstrumentation.cpp
  lldb/tools/lldb-instr/Instrument.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -89,6 +89,8 @@
   /// {
   InstrumentedBar();
   InstrumentedFoo GetInstrumentedFoo();
+  InstrumentedFoo &GetInstrumentedFooRef();
+  InstrumentedFoo *GetInstrumentedFooPtr();
   void SetInstrumentedFoo(InstrumentedFoo *foo);
   void SetInstrumentedFoo(InstrumentedFoo &foo);
   void Validate();
@@ -201,6 +203,22 @@
   return LLDB_RECORD_RESULT(InstrumentedFoo(0));
 }
 
+InstrumentedFoo &InstrumentedBar::GetInstrumentedFooRef() {
+  LLDB_RECORD_METHOD_NO_ARGS(InstrumentedFoo &, InstrumentedBar,
+ GetInstrumentedFooRef);
+  InstrumentedFoo *foo = new InstrumentedFoo(0);
+  m_get_instrumend_foo_called = true;
+  return LLDB_RECORD_RESULT(*foo);
+}
+
+InstrumentedFoo *InstrumentedBar::GetInstrumentedFooPtr() {
+  LLDB_RECORD_METHOD_NO_ARGS(InstrumentedFoo *, InstrumentedBar,
+ GetInstrumentedFooPtr);
+  InstrumentedFoo *foo = new InstrumentedFoo(0);
+  m_get_instrumend_foo_called = true;
+  return LLDB_RECORD_RESULT(foo);
+}
+
 void InstrumentedBar::SetInstrumentedFoo(InstrumentedFoo *foo) {
   LLDB_RECORD_METHOD(void, InstrumentedBar, SetInstrumentedFoo,
  (InstrumentedFoo *), foo);
@@ -239,6 +257,10 @@
   LLDB_REGISTER_CONSTRUCTOR(InstrumentedBar, ());
   LLDB_REGISTER_METHOD(InstrumentedFoo, InstrumentedBar, GetInstrumentedFoo,
());
+  LLDB_REGISTER_METHOD(InstrumentedFoo &, InstrumentedBar,
+   GetInstrumentedFooRef, ());
+  LLDB_REGISTER_METHOD(InstrumentedFoo *, InstrumentedBar,
+   GetInstrumentedFooPtr, ());
   LLDB_REGISTER_METHOD(void, InstrumentedBar, SetInstrumentedFoo,
(InstrumentedFoo *));
   LLDB_REGISTER_METHOD(void, InstrumentedBar, SetInstrumentedFoo,
@@ -487,6 +509,80 @@
   {
 InstrumentedBar bar;
 InstrumentedFoo foo = bar.GetInstrumentedFoo();
+#if 0
+InstrumentedFoo& foo_ref = bar.GetInstrumentedFooRef();
+InstrumentedFoo* foo_ptr = bar.GetInstrumentedFooPtr();
+#endif
+
+int b = 200;
+float c = 300.3;
+double e = 400.4;
+
+foo.A(100);
+foo.B(b);

[Lldb-commits] [PATCH] D60153: Re-enable most lldb-vscode tests on Linux.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe updated this revision to Diff 193585.
jgorbe added a comment.

Increased wait time in test binary for TestVSCode_attach.py from 5 to 10 
seconds.

If this is still unreliable, or it makes the test take too long, I'll switch it
to use the `debugger_flag` pattern as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60153

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py

Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -21,7 +21,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_default(self):
 '''
@@ -41,7 +40,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_stopOnEntry(self):
 '''
@@ -63,7 +61,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_cwd(self):
 '''
@@ -92,7 +89,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_debuggerRoot(self):
 '''
@@ -122,7 +118,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_sourcePath(self):
 '''
@@ -150,7 +145,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_disableSTDIO(self):
 '''
@@ -167,7 +161,7 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
+@skipIfLinux # shell argument expansion doesn't seem to work on Linux
 @expectedFailureNetBSD
 @no_debug_info_test
 def test_shellExpandArguments_enabled(self):
@@ -194,7 +188,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_shellExpandArguments_disabled(self):
 '''
@@ -222,7 +215,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_args(self):
 '''
@@ -250,7 +242,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_environment(self):
 '''
@@ -285,7 +276,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_commands(self):
 '''
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
@@ -21,7 +21,6 @@
 
 @skipIfWindows
   

[Lldb-commits] [lldb] r357633 - Re-enable most lldb-vscode tests on Linux.

[Jorge Gorbe Moya via lldb-commits]

Author: jgorbe
Date: Wed Apr  3 13:43:20 2019
New Revision: 357633

URL: http://llvm.org/viewvc/llvm-project?rev=357633&view=rev
Log:
Re-enable most lldb-vscode tests on Linux.

Summary:
After https://reviews.llvm.org/D59828 and https://reviews.llvm.org/D59849,
I believe the problems with these tests hanging have been solved.

I tried enabling all of them on my machine, and got two failures:

- One of them was spawning a child process that lives for 5 seconds, waited
  for 5 seconds to attach to the child, and failed because the child wasn't
  there.

- The other one was a legit failure because shell expansion of arguments doesn't
  work on Linux.

This tests enables all lldb-vscode tests on Linux except for "launch process
with shell expansion of args" (which doesn't work), and fixes the other broken
test by reducing the time it waits before attaching to its child process.

Reviewers: zturner, clayborg

Subscribers: lldb-commits

Tags: #lldb

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py?rev=357633&r1=357632&r2=357633&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
 Wed Apr  3 13:43:20 2019
@@ -47,7 +47,6 @@ class TestVSCode_attach(lldbvscode_testc
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings 
aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as 
Darwin
 @skipIfNetBSD # Hangs on NetBSD as well
 @no_debug_info_test
 def test_by_pid(self):
@@ -65,7 +64,6 @@ class TestVSCode_attach(lldbvscode_testc
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings 
aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as 
Darwin
 @skipIfNetBSD # Hangs on NetBSD as well
 @no_debug_info_test
 def test_by_name(self):
@@ -94,14 +92,14 @@ class TestVSCode_attach(lldbvscode_testc
 stdin=subprocess.PIPE,
 stdout=subprocess.PIPE,
 stderr=subprocess.PIPE)
-# Wait for a bit to ensure the process is launched
-time.sleep(5)
+# Wait for a bit to ensure the process is launched, but not for so long
+# that the process has already finished by the time we attach.
+time.sleep(3)
 self.attach(program=program)
 self.set_and_hit_breakpoint(continueToExit=True)
 
 @skipUnlessDarwin
 @skipIfDarwin # Skip this test for now until we can figure out why tings 
aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as 
Darwin
 @skipIfNetBSD # Hangs on NetBSD as well
 @no_debug_info_test
 def test_by_name_waitFor(self):
@@ -120,7 +118,6 @@ class TestVSCode_attach(lldbvscode_testc
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings 
aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as 
Darwin
 @skipIfNetBSD # Hangs on NetBSD as well
 @no_debug_info_test
 def test_commands(self):

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c?rev=357633&r1=357632&r2=357633&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c 
Wed Apr  3 13:43:20 2019
@@ -3,6 +3,6 @@
 
 int main(int argc, char const *argv[]) {
   printf("pid = %i\n", getpid());
-  sleep(5);
+  sleep(10);
   return 0; // breakpoint 1
 }

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py?rev=357633&r1=357632

[Lldb-commits] [PATCH] D60153: Re-enable most lldb-vscode tests on Linux.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
jgorbe marked 2 inline comments as done.
Closed by commit rL357633: Re-enable most lldb-vscode tests on Linux. (authored 
by jgorbe, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60153?vs=193585&id=193586#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60153

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
  lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/main.c
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
  
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py

Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
@@ -21,7 +21,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # test hangs on linux under heavy load
 @no_debug_info_test
 def test_set_and_clear(self):
 '''Tests setting and clearing function breakpoints.
@@ -114,7 +113,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # test hangs on linux under heavy load
 @no_debug_info_test
 def test_functionality(self):
 '''Tests hitting breakpoints and the functionality of a single
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -21,7 +21,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_default(self):
 '''
@@ -41,7 +40,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_stopOnEntry(self):
 '''
@@ -63,7 +61,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_cwd(self):
 '''
@@ -92,7 +89,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_debuggerRoot(self):
 '''
@@ -122,7 +118,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_sourcePath(self):
 '''
@@ -150,7 +145,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_disableSTDIO(self):
 '''
@@ -167,7 +161,7 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
+@skipIfLinux # shell argument expansion doesn't seem to work on Linux
 @expectedFailureNetBSD
 @no_debug_info_test
 def test_shellExpandArguments_enabled(self):
@@ -194,7 +188,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-@skipIfLinux # This test is timing out and/or failing on Linux as well as Darwin
 @no_debug_info_test
 def test_shellExpandArguments_disabled(self):
 '''
@@ -222,7 +215,6 @@
 
 @skipIfWindows
 @skipIfDarwin # Skip this test for now until we can figure out why tings aren't working on build bots
-

[Lldb-commits] [PATCH] D60180: [CMake] Don't explicitly use LLVM_LIBRARY_DIR in standalone builds

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

So I ran the lldb test suite from a standalone build tree, and this patch 
didn't change anything. I added some logging to the CMake and 
`LLVM_LIBRARY_DIR` is being set correctly. It appears to be getting it from the 
LLVMConfig we get from `find_package(LLVM)`. I think we could also get rid of 
`LLVM_BINARY_DIR` if that's the case, since it appears we only set it for lit.

From the looks of it, clang needs to set it manually because they rely on 
llvm-config instead of using `find_package`.


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

https://reviews.llvm.org/D60180



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

[Lldb-commits] [PATCH] D60178: [Reproducers] Capture return values of functions returning by ptr/ref

[Frederic Riss via Phabricator via lldb-commits]

friss accepted this revision.
friss added a comment.
This revision is now accepted and ready to land.

This seems super mechanical and we discussed it at length offline. LGTM


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

https://reviews.llvm.org/D60178



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

[Lldb-commits] [lldb] r357639 - [Reproducers] Capture return values of functions returning by ptr/ref

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Apr  3 14:31:22 2019
New Revision: 357639

URL: http://llvm.org/viewvc/llvm-project?rev=357639&view=rev
Log:
[Reproducers] Capture return values of functions returning by ptr/ref

For some reason I had convinced myself that functions returning by
pointer or reference do not require recording their result. However,
after further considering I don't see how that could work, at least not
with the current implementation. Interestingly enough, the reproducer
instrumentation already (mostly) accounts for this, though the
lldb-instr tool did not.

This patch adds the missing macros and updates the lldb-instr tool.

Differential revision: https://reviews.llvm.org/D60178

Modified:
lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
lldb/trunk/lit/tools/lldb-instr/Inputs/foo.cpp
lldb/trunk/lit/tools/lldb-instr/Inputs/foo.h
lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRecord.test
lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test
lldb/trunk/source/API/SBAddress.cpp
lldb/trunk/source/API/SBAttachInfo.cpp
lldb/trunk/source/API/SBBlock.cpp
lldb/trunk/source/API/SBBreakpoint.cpp
lldb/trunk/source/API/SBBreakpointLocation.cpp
lldb/trunk/source/API/SBBreakpointName.cpp
lldb/trunk/source/API/SBBroadcaster.cpp
lldb/trunk/source/API/SBCommandInterpreter.cpp
lldb/trunk/source/API/SBCommandReturnObject.cpp
lldb/trunk/source/API/SBCompileUnit.cpp
lldb/trunk/source/API/SBData.cpp
lldb/trunk/source/API/SBDebugger.cpp
lldb/trunk/source/API/SBDeclaration.cpp
lldb/trunk/source/API/SBError.cpp
lldb/trunk/source/API/SBEvent.cpp
lldb/trunk/source/API/SBExecutionContext.cpp
lldb/trunk/source/API/SBExpressionOptions.cpp
lldb/trunk/source/API/SBFileSpec.cpp
lldb/trunk/source/API/SBFileSpecList.cpp
lldb/trunk/source/API/SBFrame.cpp
lldb/trunk/source/API/SBFunction.cpp
lldb/trunk/source/API/SBInstruction.cpp
lldb/trunk/source/API/SBInstructionList.cpp
lldb/trunk/source/API/SBLineEntry.cpp
lldb/trunk/source/API/SBListener.cpp
lldb/trunk/source/API/SBMemoryRegionInfo.cpp
lldb/trunk/source/API/SBMemoryRegionInfoList.cpp
lldb/trunk/source/API/SBModule.cpp
lldb/trunk/source/API/SBModuleSpec.cpp
lldb/trunk/source/API/SBProcess.cpp
lldb/trunk/source/API/SBProcessInfo.cpp
lldb/trunk/source/API/SBQueue.cpp
lldb/trunk/source/API/SBSection.cpp
lldb/trunk/source/API/SBSourceManager.cpp
lldb/trunk/source/API/SBStringList.cpp
lldb/trunk/source/API/SBStructuredData.cpp
lldb/trunk/source/API/SBSymbol.cpp
lldb/trunk/source/API/SBSymbolContext.cpp
lldb/trunk/source/API/SBSymbolContextList.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/API/SBThread.cpp
lldb/trunk/source/API/SBThreadCollection.cpp
lldb/trunk/source/API/SBThreadPlan.cpp
lldb/trunk/source/API/SBType.cpp
lldb/trunk/source/API/SBTypeCategory.cpp
lldb/trunk/source/API/SBTypeEnumMember.cpp
lldb/trunk/source/API/SBTypeFilter.cpp
lldb/trunk/source/API/SBTypeFormat.cpp
lldb/trunk/source/API/SBTypeNameSpecifier.cpp
lldb/trunk/source/API/SBTypeSummary.cpp
lldb/trunk/source/API/SBTypeSynthetic.cpp
lldb/trunk/source/API/SBUnixSignals.cpp
lldb/trunk/source/API/SBValue.cpp
lldb/trunk/source/API/SBValueList.cpp
lldb/trunk/source/API/SBVariablesOptions.cpp
lldb/trunk/source/API/SBWatchpoint.cpp
lldb/trunk/source/Utility/ReproducerInstrumentation.cpp
lldb/trunk/tools/lldb-instr/Instrument.cpp
lldb/trunk/unittests/Utility/ReproducerInstrumentationTest.cpp

Modified: lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h?rev=357639&r1=357638&r2=357639&view=diff
==
--- lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h (original)
+++ lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h Wed Apr  3 
14:31:22 2019
@@ -185,7 +185,7 @@ template  inline std::st
   }
 
 #define LLDB_RECORD_RESULT(Result) 
\
-  sb_recorder ? sb_recorder->RecordResult(Result) : Result;
+  sb_recorder ? sb_recorder->RecordResult(Result) : (Result);
 
 /// The LLDB_RECORD_DUMMY macro is special because it doesn't actually record
 /// anything. It's used to track API boundaries when we cannot record for
@@ -643,13 +643,7 @@ public:
   return;
 
 unsigned id = m_registry.GetID(uintptr_t(f));
-
-#ifndef LLDB_REPRO_INSTR_TRACE
-LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Recording {0}: {1}",
- id, m_pretty_func);
-#else
-llvm::errs() << "Recording " << id << ": " << m_pretty_func << "\n";
-#endif
+Log(id);
 
 m_serializer.SerializeAll(id);
 m_serializer.SerializeAll(args...);
@@ -670,13 +664,7 @@ public:
   return;
 
 unsigned id = 

[Lldb-commits] [PATCH] D60178: [Reproducers] Capture return values of functions returning by ptr/ref

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357639: [Reproducers] Capture return values of functions 
returning by ptr/ref (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60178?vs=193581&id=193599#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60178

Files:
  lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/trunk/lit/tools/lldb-instr/Inputs/foo.cpp
  lldb/trunk/lit/tools/lldb-instr/Inputs/foo.h
  lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRecord.test
  lldb/trunk/lit/tools/lldb-instr/TestInstrumentationRegister.test
  lldb/trunk/source/API/SBAddress.cpp
  lldb/trunk/source/API/SBAttachInfo.cpp
  lldb/trunk/source/API/SBBlock.cpp
  lldb/trunk/source/API/SBBreakpoint.cpp
  lldb/trunk/source/API/SBBreakpointLocation.cpp
  lldb/trunk/source/API/SBBreakpointName.cpp
  lldb/trunk/source/API/SBBroadcaster.cpp
  lldb/trunk/source/API/SBCommandInterpreter.cpp
  lldb/trunk/source/API/SBCommandReturnObject.cpp
  lldb/trunk/source/API/SBCompileUnit.cpp
  lldb/trunk/source/API/SBData.cpp
  lldb/trunk/source/API/SBDebugger.cpp
  lldb/trunk/source/API/SBDeclaration.cpp
  lldb/trunk/source/API/SBError.cpp
  lldb/trunk/source/API/SBEvent.cpp
  lldb/trunk/source/API/SBExecutionContext.cpp
  lldb/trunk/source/API/SBExpressionOptions.cpp
  lldb/trunk/source/API/SBFileSpec.cpp
  lldb/trunk/source/API/SBFileSpecList.cpp
  lldb/trunk/source/API/SBFrame.cpp
  lldb/trunk/source/API/SBFunction.cpp
  lldb/trunk/source/API/SBInstruction.cpp
  lldb/trunk/source/API/SBInstructionList.cpp
  lldb/trunk/source/API/SBLineEntry.cpp
  lldb/trunk/source/API/SBListener.cpp
  lldb/trunk/source/API/SBMemoryRegionInfo.cpp
  lldb/trunk/source/API/SBMemoryRegionInfoList.cpp
  lldb/trunk/source/API/SBModule.cpp
  lldb/trunk/source/API/SBModuleSpec.cpp
  lldb/trunk/source/API/SBProcess.cpp
  lldb/trunk/source/API/SBProcessInfo.cpp
  lldb/trunk/source/API/SBQueue.cpp
  lldb/trunk/source/API/SBSection.cpp
  lldb/trunk/source/API/SBSourceManager.cpp
  lldb/trunk/source/API/SBStringList.cpp
  lldb/trunk/source/API/SBStructuredData.cpp
  lldb/trunk/source/API/SBSymbol.cpp
  lldb/trunk/source/API/SBSymbolContext.cpp
  lldb/trunk/source/API/SBSymbolContextList.cpp
  lldb/trunk/source/API/SBTarget.cpp
  lldb/trunk/source/API/SBThread.cpp
  lldb/trunk/source/API/SBThreadCollection.cpp
  lldb/trunk/source/API/SBThreadPlan.cpp
  lldb/trunk/source/API/SBType.cpp
  lldb/trunk/source/API/SBTypeCategory.cpp
  lldb/trunk/source/API/SBTypeEnumMember.cpp
  lldb/trunk/source/API/SBTypeFilter.cpp
  lldb/trunk/source/API/SBTypeFormat.cpp
  lldb/trunk/source/API/SBTypeNameSpecifier.cpp
  lldb/trunk/source/API/SBTypeSummary.cpp
  lldb/trunk/source/API/SBTypeSynthetic.cpp
  lldb/trunk/source/API/SBUnixSignals.cpp
  lldb/trunk/source/API/SBValue.cpp
  lldb/trunk/source/API/SBValueList.cpp
  lldb/trunk/source/API/SBVariablesOptions.cpp
  lldb/trunk/source/API/SBWatchpoint.cpp
  lldb/trunk/source/Utility/ReproducerInstrumentation.cpp
  lldb/trunk/tools/lldb-instr/Instrument.cpp
  lldb/trunk/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/trunk/tools/lldb-instr/Instrument.cpp
===
--- lldb/trunk/tools/lldb-instr/Instrument.cpp
+++ lldb/trunk/tools/lldb-instr/Instrument.cpp
@@ -245,7 +245,9 @@
 
 // If the function returns a class or struct, we need to wrap its return
 // statement(s).
-if (!ShouldInsertDummy && ReturnType->isStructureOrClassType()) {
+bool ShouldRecordResult = ReturnType->isStructureOrClassType() ||
+  ReturnType->getPointeeCXXRecordDecl();
+if (!ShouldInsertDummy && ShouldRecordResult) {
   SBReturnVisitor Visitor(MyRewriter);
   Visitor.TraverseDecl(Decl);
 }
Index: lldb/trunk/source/API/SBType.cpp
===
--- lldb/trunk/source/API/SBType.cpp
+++ lldb/trunk/source/API/SBType.cpp
@@ -86,7 +86,7 @@
   if (this != &rhs) {
 m_opaque_sp = rhs.m_opaque_sp;
   }
-  return *this;
+  return LLDB_RECORD_RESULT(*this);
 }
 
 SBType::~SBType() {}
@@ -592,7 +592,7 @@
  i < rhs_size; i++)
   Append(const_cast(rhs).GetTypeAtIndex(i));
   }
-  return *this;
+  return LLDB_RECORD_RESULT(*this);
 }
 
 void SBTypeList::Append(SBType type) {
@@ -642,7 +642,7 @@
 if (rhs.IsValid())
   m_opaque_up.reset(new TypeMemberImpl(rhs.ref()));
   }
-  return *this;
+  return LLDB_RECORD_RESULT(*this);
 }
 
 bool SBTypeMember::IsValid() const {
@@ -771,7 +771,7 @@
 
   if (this != &rhs)
 m_opaque_sp = rhs.m_opaque_sp;
-  return *this;
+  return LLDB_RECORD_RESULT(*this);
 }
 
 bool SBTypeMemberFunction::IsValid() const {
Index: lldb/trunk/source/API/SBValue.cpp
===

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

OK looked things over a bit.

I think there's a case for adopting llvm::ErrorOr here, and elsewhere across, 
but today the only place we're using that return type is when the error is 
encoded in a std::error_code (aka fancy errno).  We should have an ErrorOr that 
uses a Status object to contain the error, I think that's possible?  I've seen 
people use their own objects to return errors with llvm::Expected.  In any 
event, I think a change like this is a little out of scope for what I'm doing 
in this patch right now.

After spending time looking at all the error handling codepaths related to 
AddOrCreateModule, if we simply fail to find a binary we return an empty 
ModuleSP and the platform may set a Status with an error string (e.g. 
PlatformPOSIX will say no such file) if that's non-nullptr.  Or there may be a 
genuine error - a Target that doesn't have a Platform, we found a binary but it 
is an explicitly disallowed type (e.g. eTypeStubLibrary, a long-obsolete binary 
format on Darwin), we can return an error message explaining that to the caller 
if the Status ptr is non-nullptr.

The important wrinkle with AddOrCreateModule is that most of the callers are 
deep inside lldb internal mechanisms -- the DynamicLoaders.  And for most of 
those, we're going to handle file-not-found in custom ways, we're not going to 
relay the error messages.  For instance, the user-process DynamicLoaderDarwin, 
when it fails to find a file, it will call Process::ReadModuleFromMemory().  If 
the kernel debugging DynamicLoaderDarwinKernel fails to load a kext (a solib), 
it accumulates a list of kexts that it could not find, and at the end it will 
present a sorted list of them.   These callers have no interest in the returned 
error message.

On the other hand, a caller like CommandObjectTarget, which is directly driven 
by user interaction, we must print the error message ("file not found" and 
such).

So in this particularly API, because only some callers want the error strings, 
leaving this as an optional argument seems reasonable.  An llvm::ErrorOr which 
returns a ModuleSP or a Status would be a fine - and only the callers that want 
to print the error message would retrieve the Status object, the others would 
do their normal behavior when couldn't get the ModuleSP.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60172



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

[Lldb-commits] [lldb] r357641 - Un-xfail one of the TestMiniDumpUUID tests on Windows

[Stella Stamenova via lldb-commits]

Author: stella.stamenova
Date: Wed Apr  3 14:57:41 2019
New Revision: 357641

URL: http://llvm.org/viewvc/llvm-project?rev=357641&view=rev
Log:
Un-xfail one of the TestMiniDumpUUID tests on Windows

The test is passing on Windows and the windows bot is failing because of the 
unexpected pass

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=357641&r1=357640&r2=357641&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Wed Apr  3 14:57:41 2019
@@ -160,7 +160,6 @@ class MiniDumpUUIDTestCase(TestBase):
 self.verify_module(modules[0], so_path, 
"7295E17C-6668-9E05-CBB5-DEE5003865D5-5267C116")
 
-@expectedFailureAll(oslist=["windows"])
 def test_partial_uuid_mismatch(self):
 """
 Breakpad has been known to create minidump files using CvRecord in 
each


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

[Lldb-commits] [PATCH] D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 193609.
jasonmolenda added a comment.

Remove const bool notify's.  Rename method to Target::GetOrCreateModule.  
Refine the method headerdoc a bit.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60172

Files:
  include/lldb/Core/ModuleList.h
  include/lldb/Target/Target.h
  source/API/SBTarget.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Core/DynamicLoader.cpp
  source/Core/ModuleList.cpp
  source/Expression/FunctionCaller.cpp
  source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/Process/minidump/ProcessMinidump.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -1442,7 +1442,8 @@
"Target::SetExecutableModule (executable = '%s')",
executable_sp->GetFileSpec().GetPath().c_str());
 
-m_images.Append(executable_sp); // The first image is our executable file
+const bool notify = true;
+m_images.Append(executable_sp, notify); // The first image is our executable file
 
 // If we haven't set an architecture yet, reset our architecture based on
 // what we found in the executable module.
@@ -1482,7 +1483,8 @@
   platform_dependent_file_spec = dependent_file_spec;
 
 ModuleSpec module_spec(platform_dependent_file_spec, m_arch.GetSpec());
-ModuleSP image_module_sp(GetSharedModule(module_spec));
+ModuleSP image_module_sp(GetOrCreateModule(module_spec, 
+   true /* notify */));
 if (image_module_sp) {
   ObjectFile *objfile = image_module_sp->GetObjectFile();
   if (objfile)
@@ -1984,8 +1986,8 @@
   return false;
 }
 
-ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec,
- Status *error_ptr) {
+ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
+   Status *error_ptr) {
   ModuleSP module_sp;
 
   Status error;
@@ -2118,8 +2120,9 @@
   Module *old_module_ptr = old_module_sp.get();
   old_module_sp.reset();
   ModuleList::RemoveSharedModuleIfOrphaned(old_module_ptr);
-} else
-  m_images.Append(module_sp);
+} else {
+  m_images.Append(module_sp, notify);
+}
   } else
 module_sp.reset();
 }
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -388,7 +388,8 @@
 Status error;
 // Try and find a module with a full UUID that matches. This function will
 // add the module to the target if it finds one.
-lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
+lldb::ModuleSP module_sp = GetTarget().GetOrCreateModule(module_spec, 
+ true /* notify */, &error);
 if (!module_sp) {
   // Try and find a module without specifying the UUID and only looking for
   // the file given a basename. We then will look for a partial UUID match
@@ -400,7 +401,8 @@
   ModuleSpec basename_module_spec(module_spec);
   basename_module_spec.GetUUID().Clear();
   basename_module_spec.GetFileSpec().GetDirectory().Clear();
-  module_sp = GetTarget().GetSharedModule(basename_module_spec, &error);
+  module_sp = GetTarget().GetOrCreateModule(basename_module_spec, 
+ true /* notify */, &error);
   if (module_sp) {
 // We consider the module to be a match if the minidump UUID is a
 // prefix of the actual UUID, or if either of the UUIDs are empty.
@@ -430,7 +432,7 @@
 
   module_sp = Module::CreateModuleFromObjectFile(
   module_spec, module->base_of_image, module->size_of_image);
-  GetTarget().GetImages().Append(module_sp);
+  GetTarget().GetImages().Append(module_sp, true /* notify */);
 }
 
 bool load_addr_changed = false;
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -244,7 +244,8 @@
   exe_module_spec.GetFileSpec()

[Lldb-commits] [lldb] r357658 - [codeview] Remove Type member from CVRecord

[Reid Kleckner via lldb-commits]

Author: rnk
Date: Wed Apr  3 17:28:48 2019
New Revision: 357658

URL: http://llvm.org/viewvc/llvm-project?rev=357658&view=rev
Log:
[codeview] Remove Type member from CVRecord

Summary:
Now CVType and CVSymbol are effectively type-safe wrappers around
ArrayRef. Make the kind() accessor load it from the
RecordPrefix, which is the same for types and symbols.

Reviewers: zturner, aganea

Subscribers: hiraditya, llvm-commits

Tags: #llvm

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp?rev=357658&r1=357657&r2=357658&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp Wed 
Apr  3 17:28:48 2019
@@ -188,7 +188,7 @@ Error UdtRecordCompleter::visitKnownMemb
   TypeIndex method_list_idx = overloaded.MethodList;
 
   CVType method_list_type = m_tpi.getType(method_list_idx);
-  assert(method_list_type.Type == LF_METHODLIST);
+  assert(method_list_type.kind() == LF_METHODLIST);
 
   MethodOverloadListRecord method_list;
   llvm::cantFail(TypeDeserializer::deserializeAs(


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

[Lldb-commits] [PATCH] D60018: [codeview] Remove Type member from CVRecord

[Reid Kleckner via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLD357658: [codeview] Remove Type member from CVRecord 
(authored by rnk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60018?vs=193526&id=193633#toc

Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D60018

Files:
  COFF/PDB.cpp


Index: COFF/PDB.cpp
===
--- COFF/PDB.cpp
+++ COFF/PDB.cpp
@@ -759,7 +759,7 @@
   if (Kind == SymbolKind::S_GPROC32_ID || Kind == SymbolKind::S_LPROC32_ID) {
 SmallVector Refs;
 auto Content = RecordData.drop_front(sizeof(RecordPrefix));
-CVSymbol Sym(Kind, RecordData);
+CVSymbol Sym(RecordData);
 discoverTypeIndicesInSymbol(Sym, Refs);
 assert(Refs.size() == 1);
 assert(Refs.front().Count == 1);
@@ -959,7 +959,7 @@
 MutableArrayRef RecordBytes;
 if (NeedsRealignment) {
   RecordBytes = copyAndAlignSymbol(Sym, AlignedSymbolMem);
-  Sym = CVSymbol(Sym.kind(), RecordBytes);
+  Sym = CVSymbol(RecordBytes);
 } else {
   // Otherwise, we can actually mutate the symbol directly, since we
   // copied it to apply relocations.
@@ -983,7 +983,7 @@
 // An object file may have S_xxx_ID symbols, but these get converted to
 // "real" symbols in a PDB.
 translateIdSymbols(RecordBytes, TMerger.getIDTable());
-Sym = CVSymbol(symbolKind(RecordBytes), RecordBytes);
+Sym = CVSymbol(RecordBytes);
 
 // If this record refers to an offset in the object file's string 
table,
 // add that item to the global PDB string table and re-write the index.


Index: COFF/PDB.cpp
===
--- COFF/PDB.cpp
+++ COFF/PDB.cpp
@@ -759,7 +759,7 @@
   if (Kind == SymbolKind::S_GPROC32_ID || Kind == SymbolKind::S_LPROC32_ID) {
 SmallVector Refs;
 auto Content = RecordData.drop_front(sizeof(RecordPrefix));
-CVSymbol Sym(Kind, RecordData);
+CVSymbol Sym(RecordData);
 discoverTypeIndicesInSymbol(Sym, Refs);
 assert(Refs.size() == 1);
 assert(Refs.front().Count == 1);
@@ -959,7 +959,7 @@
 MutableArrayRef RecordBytes;
 if (NeedsRealignment) {
   RecordBytes = copyAndAlignSymbol(Sym, AlignedSymbolMem);
-  Sym = CVSymbol(Sym.kind(), RecordBytes);
+  Sym = CVSymbol(RecordBytes);
 } else {
   // Otherwise, we can actually mutate the symbol directly, since we
   // copied it to apply relocations.
@@ -983,7 +983,7 @@
 // An object file may have S_xxx_ID symbols, but these get converted to
 // "real" symbols in a PDB.
 translateIdSymbols(RecordBytes, TMerger.getIDTable());
-Sym = CVSymbol(symbolKind(RecordBytes), RecordBytes);
+Sym = CVSymbol(RecordBytes);
 
 // If this record refers to an offset in the object file's string table,
 // add that item to the global PDB string table and re-write the index.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits