[Lldb-commits] [PATCH] D73661: [lldb] Move clang-based files out of Symbol

2020-01-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I think this is a big milestone. Thanks for working on this.

The main question  I have is about the new location of this code. This patch 
puts it under ExpressionParser/Clang, which is not completely unreasonable, as 
that's where most of the clang stuff is. However, it does create some 
awkward-looking (to me) dependencies, or even loops 
(SymbolFileDWARF<->ExpressionParserClang). Also the dep from data formatters in 
Language/CPLusPlus on ExpressionParser/Clang is odd, because the data 
formatters don't actually use/need expressions do to their work.

So, as much as I hate proliferating plugins, I have to ask this question: 
Should this be a new plugin kind (TypeSystem/Clang) instead ? The name of the 
class, and the presence of the Initialize function already seem to indicate 
that. And it seems to me like this would create a reasonable dependency graph 
between the various plugins. TypeSystemClang would be at the bottom of this. 
The various SymbolFile plugins would depend on it, because they generate clang 
ASTs. ExpressionParserClang would depend on it because it pulls the generated 
ASTs that way. The same goes for the data formatters.  But there would be no 
dependency between SymbolFiles and expression parsers or data formatters, 
because the former should just provide the ast (and not care about who the 
consumer is) and the latter should consume it (regardless of the source).

I think this ExpressionParser vs. TypeSystem would also make sense in terms of 
the outgoing dependencies. The type system should depend +/- only on clangAST, 
whereas the expression parser would need pretty much the whole clang.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73661



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


[Lldb-commits] [PATCH] D73665: Stop emitting a breakpoint for each location in a breakpoint when responding to breakpoint commands.

2020-01-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

This is not ideal, but if that's what vscode wants, then I guess we just have 
to do it.

However, are you sure that this is all that needs to be done here. It seems 
like also the handling of eBreakpointEventTypeLocationsAdded/Removed events in 
EventThreadFunction should be changed too. Otherwise, I think the whole 
breakpoint will disappear whenever a single location goes away...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73665



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


[Lldb-commits] [PATCH] D73594: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

2020-01-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4103
+// this point.
+// TODO:  Is this part worthwhile?  `foo.exe` will never match `foo.pdb`
+if (matching_modules.IsEmpty())

amccarth wrote:
> labath wrote:
> > This is not unreasonable in the non-pdb world. You can have a stripped 
> > version of a file somewhere prepared for deployment to some device (or 
> > downloaded from the device), and then you'll also have an unstripped 
> > version of that file (with the same name) in some build folder.
> Sure, if you've got two files with the same name in two directories that's 
> fine.
> 
> But the loop tries peeling the extension off of the supplied file name and 
> comparing it to the target's file name.  I guess it's trying to match 
> something like `foo.unstripped` to `foo`.  Is that a typical thing?
Ah, sorry, I missed that part.

Yes, this is also thing that's used sometimes (probably even more often than 
the first thing). One of the conventions for automatically finding [[ 
https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html | separate 
debug files ]] involves appending `.debug` to your file and then placing it in 
a special folder. I guess this was added to enable one to do "target symbols 
add /whereever/foo.debug" and have it match "foo".

That said, stripping the extension in a loop does seem like overkill.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4175-4178
+if (object_file->GetFileSpec() != symbol_fspec) {
+  result.AppendWarning("there is a discrepancy between the module file "
+   "spec and the symbol file spec\n");
+}

amccarth wrote:
> clayborg wrote:
> > amccarth wrote:
> > > amccarth wrote:
> > > > clayborg wrote:
> > > > > labath wrote:
> > > > > > This part is not good. Everywhere else except pdbs this means that 
> > > > > > we were in fact unable to load the symbol file. What happens is 
> > > > > > that if we cannot load the object file representing the symbol file 
> > > > > > (at [[ 
> > > > > > https://github.com/llvm/llvm-project/blob/master/lldb/source/Symbol/SymbolVendor.cpp#L48
> > > > > >  | SymbolVendor.cpp:48 ]]), we fall back to creating a SymbolFile 
> > > > > > using the object file of the original module (line 52).
> > > > > > 
> > > > > > The effect of that is that the symbol file creation will always 
> > > > > > succeed, the previous checks are more-or-less useless, and the only 
> > > > > > way to check if we are really using the symbols from the file the 
> > > > > > user specified is to compare the file specs.
> > > > > > 
> > > > > > Now, PDB symbol files are abusing this fallback, and reading the 
> > > > > > symbols from the pdb files even though they were in fact asked to 
> > > > > > read them from the executable file. This is why this may sound like 
> > > > > > a "discrepancy" coming from the pdb world, but everywhere else this 
> > > > > > just means that the symbol file was not actually loaded.
> > > > > This could also fail if the symbol file spec was a bundle which got 
> > > > > resolved when the module was loaded. If the user specified 
> > > > > "/path/to/foo.dSYM" and the underlying code was able to resolve the 
> > > > > symbol file in the dSYM bundle to 
> > > > > "/path/to/foo.dSYM/Contents/Resources/DWARF/foo".
> > > > Interesting.  I did not expect that fallback behavior from the API.
> > > > 
> > > > > PDB symbol files are abusing this fallback
> > > > 
> > > > I'm not sure there's abuse going on here.  I'm not understanding 
> > > > something about how lldb models this situation.  Consider the case 
> > > > where the user explicitly asks lldb to load symbols from a PDB:
> > > > 
> > > > (lldb) target add symbols D:\my_symbols\foo.pdb
> > > > 
> > > > Then `symbol_file` is the PDB and `object_file` is the executable or 
> > > > DLL.  I would expect those file specs to match only in the case where 
> > > > the symbols are in the binary (in other words, when symbol file and 
> > > > object file are the same file).  Even ignoring the PDB case, it seems 
> > > > this wouldn't even work in the case you mentioned above where the 
> > > > object file is stripped and the symbol file is an unstripped copy of 
> > > > the object file (possibly with a different name).  You can also get 
> > > > here if the symbols were matched by UUID rather than file spec.  Or 
> > > > when the symbols were matched by the basename minus some extension.
> > > > 
> > > > Given that dsyms work, I must be misunderstanding something here.  Can 
> > > > you help me understand?
> > > > 
> > > > What's the right thing to do here?  If I just undo this refactor, then 
> > > > we're back to silent failures when the symbols exist in a different 
> > > > file than the object.
> > > In your example, the file specs do not match.
> > > 
> > > 1. The old code would therefore reject the symbol file a

[Lldb-commits] [PATCH] D73661: [lldb] Move clang-based files out of Symbol

2020-01-30 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Pretty much what Pavel said, otherwise this LGTM. Maybe also rename/move the 
unit test files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73661



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


[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

2020-01-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D72751#1847617 , @clayborg wrote:

> Yes the current approach allows anyone to load any section at any address. On 
> Darwin systems, the DYLD shared cache will move __TEXT, __DATA, and other 
> sections around such that all __TEXT sections from all shared libraries in 
> the shared cache are all in the one contiguous range. The slide is different 
> for each section, so we have some nice flexibility with being able to set the 
> section load address individually. They will even invert the memory order 
> sometimes where in the file we have __TEXT followed by __DATA, but in the 
> shared cache __DATA appears at a lower address than __TEXT.


Sorry about the off-topic, but I found this bit very interesting. Greg, how 
does this work with code referencing the variables in the data section (e.g. 
`static int x; int *f() { return &x; }`). This code on elf, even when linked in 
fully position-independent mode will not contain any relocations because it is 
assumed that the dynamic loader will not change the relative layout of code and 
data (and so the code can use pc-relative addressing). This obviously does not 
work if data can be moved around independently. Does that mean that darwin will 
include some additional relocations which need to be resolved at load time?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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


[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

2020-01-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D72751#1848384 , @paolosev wrote:

> Regarding:
>
> >> - make the base DynamicLoader class instantiatable, and use it whenever we 
> >> fail to find a specialized plugin
> >> - same as above, but only do that for ProcessGDBRemote instances
> >> - make ProcessGDBRemote call LoadModules() itself if no dynamic loader 
> >> instance is available WDYT?
> > 
> > I am fine with 1 as long as we document the DynamicLoader class to say that 
> > it will call Process::LoadModules() and will be used if no specialized 
> > loader is needed for your platform. I would like to a see a solution that 
> > will work for any process plug-in and not just ProcessGDBRemote. If we 
> > change solution 3 above to say "Make lldb_private::Process call 
> > LoadModules() itself if no dynamic loader instance is available" then 
> > solution 3 is also fine.
>
> there is a problem: if I remove DynamicLoaderWasmDYLD what happens is that 
> `DynamicLoaderStatic` is found as a valid loader for a triple like 
> **"wasm32-unknown-unknown-wasm"** because the Triple::OS is 
> llvm::Triple::UnknownOS (I found out the hard way when I was registering 
> DynamicLoaderWasmDYLD after DynamicLoaderStatic :-)).
>
>   ...
>   
>
> call stack:
>  DynamicLoaderStatic::CreateInstance(lldb_private::Process * process, bool 
> force) Line 29
>  DynamicLoader::FindPlugin(lldb_private::Process * process, const char * 
> plugin_name) Line 52
>  lldb_private::process_gdb_remote::ProcessGDBRemote::GetDynamicLoader() Line 
> 3993
>  lldb_private::Process::CompleteAttach() Line 2931
>  lldb_private::Process::ConnectRemote(lldb_private::Stream * strm, 
> llvm::StringRef remote_url) Line 3022
>
>   Could ProcessGDBRemote::GetDynamicLoader behave differently just when the 
> architecture is wasm32, maybe? But then it is probably cleaner to add this 
> plugin class, what do you think?


Well.. I think DynamicLoaderStatic is being too grabby. :)

However, when I though about that idea further, I realized that any default 
plugin we could implement this way could not be complete, as we would be 
missing the part which (un)loads modules when a new shared library 
(dis)appears. This is something that cannot be done in a generic way, as that 
usually requires setting breakpoint on some special symbol, or getting 
notifications about shared library events in some other way. I don't know 
whether this is something that you will also need/plan to implement for wasm, 
or if one can assume that all modules are loaded from the get-go, but this is 
what convinced me that putting this in a separate plugin is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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


[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

2020-01-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

lgtm, per the previous comment. A couple of additional inline comments in the 
test.




Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py:78
+file_content = bytearray(file.read())
+addr_from = addr - self.load_address
+addr_to = addr_from + min(length, len(file_content) - 
addr_from)

maybe also check that `addr >= self.load_address`. I wouldn't be surprised if 
lldb (now or in the future) decides it wants to try reading some other parts of 
memory too... (and we should return an error instead of falling over in that 
case).



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWasm.py:107-123
+self.assertEquals(0x4000a, code_section.GetLoadAddress(target))
+
+debug_info_section = module.GetSectionAtIndex(1)
+self.assertEquals(".debug_info", debug_info_section.GetName())
+self.assertEquals(0x40050, 
debug_info_section.GetLoadAddress(target))
+
+debug_abbrev_section = module.GetSectionAtIndex(2)

I'm not sure how much we can rely on yaml2obj offsets not changing. Hard-coding 
the order of sections is fine, but maybe if would be better to check something 
like `section.GetFileOffset() == 0x400...0 | section.GetFileOffset()`. Since 
section parsing is checked elsewhere, the main thing we want to ensure here is 
that the `0x400...0` thingy is plumbed through correctly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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


[Lldb-commits] [PATCH] D73661: [lldb] Move clang-based files out of Symbol

2020-01-30 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Also this doesn't compile on Linux:

  FAILED: 
tools/lldb/source/Plugins/ABI/SysV-ppc64/CMakeFiles/lldbPluginABISysV_ppc64.dir/ABISysV_ppc64.cpp.o

  
  /usr/lib/ccache/bin/clang++  -DGTEST_HAS_RTTI=0 -DHAVE_ROUND 
-DLLDB_CONFIGURATION_RELEASE -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS 
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/lldb/source/Plugins/ABI/SysV-ppc64 
-I/home/teemperor/work/ci/llvm/lldb/source/Plugins/ABI/SysV-ppc64 
-Itools/lldb/source -I/home/teemperor/work/ci/llvm/lldb/include 
-Itools/lldb/include -I/usr/include/libxml2 -Iinclude 
-I/home/teemperor/work/ci/llvm/llvm/include -I/usr/include/python3.8 
-I/home/teemperor/work/ci/llvm/llvm/../clang/include 
-Itools/lldb/../clang/include -I/home/teemperor/work/ci/llvm/lldb/source/. 
-fPIC -fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default 
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
-Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections 
-Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing 
-Wno-deprecated-register -Wno-vla-extension -O3-UNDEBUG  -fno-exceptions 
-fno-rtti -std=c++14 -MD -MT 
tools/lldb/source/Plugins/ABI/SysV-ppc64/CMakeFiles/lldbPluginABISysV_ppc64.dir/ABISysV_ppc64.cpp.o
 -MF 
tools/lldb/source/Plugins/ABI/SysV-ppc64/CMakeFiles/lldbPluginABISysV_ppc64.dir/ABISysV_ppc64.cpp.o.d
 -o 
tools/lldb/source/Plugins/ABI/SysV-ppc64/CMakeFiles/lldbPluginABISysV_ppc64.dir/ABISysV_ppc64.cpp.o
 -c 
/home/teemperor/work/ci/llvm/lldb/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp

 
  
/home/teemperor/work/ci/llvm/lldb/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:22:10:
 fatal error: 'lldb/Symbol/TypeSystemClang.h' file not found
 
  #include "lldb/Symbol/TypeSystemClang.h"
   ^~~
  1 error generated.  
  ninja: build stopped: cannot make progress due to previous errors.
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73661



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


[Lldb-commits] [lldb] df8a986 - [lldb][NFC] Remove TypeSystemClang::GetASTContext calls in IRForTarget

2020-01-30 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-01-30T12:07:19+01:00
New Revision: df8a986f533d6b1726e79acfa53ba854943704c3

URL: 
https://github.com/llvm/llvm-project/commit/df8a986f533d6b1726e79acfa53ba854943704c3
DIFF: 
https://github.com/llvm/llvm-project/commit/df8a986f533d6b1726e79acfa53ba854943704c3.diff

LOG: [lldb][NFC] Remove TypeSystemClang::GetASTContext calls in IRForTarget

Similar to previous commits, this just replaces the lookup in the
global map with the reference to the TypeSystemClang instance we already
have in this context.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
index 52fb0d1f4eae..2b711db7a5a5 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -356,6 +356,10 @@ class ClangASTSource : public clang::ExternalASTSource,
   /// True if lookup succeeded; false otherwise.
   ClangASTImporter::DeclOrigin GetDeclOrigin(const clang::Decl *decl);
 
+  /// Returns the TypeSystem that uses this ClangASTSource instance as it's
+  /// ExternalASTSource.
+  TypeSystemClang *GetTypeSystem() const { return m_clang_ast_context; }
+
 protected:
   bool FindObjCMethodDeclsWithOrigin(
   unsigned int current_id, NameSearchContext &context,

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 0a0314dd4b75..bcf13e5c877c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -283,17 +283,13 @@ bool IRForTarget::CreateResultVariable(llvm::Function 
&llvm_function) {
   clang::QualType element_qual_type = 
pointer_pointertype->getPointeeType();
 
   m_result_type = lldb_private::TypeFromParser(
-  element_qual_type.getAsOpaquePtr(),
-  lldb_private::TypeSystemClang::GetASTContext(
-  &result_decl->getASTContext()));
+  m_decl_map->GetTypeSystem()->GetType(element_qual_type));
 } else if (pointer_objcobjpointertype) {
   clang::QualType element_qual_type =
   clang::QualType(pointer_objcobjpointertype->getObjectType(), 0);
 
   m_result_type = lldb_private::TypeFromParser(
-  element_qual_type.getAsOpaquePtr(),
-  lldb_private::TypeSystemClang::GetASTContext(
-  &result_decl->getASTContext()));
+  m_decl_map->GetTypeSystem()->GetType(element_qual_type));
 } else {
   LLDB_LOG(log, "Expected result to have pointer type, but it did not");
 
@@ -305,9 +301,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function 
&llvm_function) {
 }
   } else {
 m_result_type = lldb_private::TypeFromParser(
-result_var->getType().getAsOpaquePtr(),
-lldb_private::TypeSystemClang::GetASTContext(
-&result_decl->getASTContext()));
+m_decl_map->GetTypeSystem()->GetType(result_var->getType()));
   }
 
   lldb::TargetSP target_sp(m_execution_unit.GetTarget());
@@ -1094,8 +1088,7 @@ bool 
IRForTarget::RewritePersistentAlloc(llvm::Instruction *persistent_alloc) {
   clang::VarDecl *decl = reinterpret_cast(ptr);
 
   lldb_private::TypeFromParser result_decl_type(
-  decl->getType().getAsOpaquePtr(),
-  lldb_private::TypeSystemClang::GetASTContext(&decl->getASTContext()));
+  m_decl_map->GetTypeSystem()->GetType(decl->getType()));
 
   StringRef decl_name(decl->getName());
   lldb_private::ConstString persistent_variable_name(decl_name.data(),
@@ -1225,10 +1218,8 @@ bool IRForTarget::MaybeHandleVariable(Value 
*llvm_value_ptr) {
 if (value_decl == nullptr)
   return false;
 
-lldb_private::CompilerType compiler_type(
-lldb_private::TypeSystemClang::GetASTContext(
-&value_decl->getASTContext()),
-value_decl->getType().getAsOpaquePtr());
+lldb_private::CompilerType compiler_type =
+m_decl_map->GetTypeSystem()->GetType(value_decl->getType());
 
 const Type *value_type = nullptr;
 



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


[Lldb-commits] [lldb] 727ed11 - [AVR] Recognize the AVR architecture in lldb

2020-01-30 Thread Ayke van Laethem via lldb-commits

Author: Ayke van Laethem
Date: 2020-01-30T13:40:31+01:00
New Revision: 727ed11b24c08c84a608886a1716b81c93640d80

URL: 
https://github.com/llvm/llvm-project/commit/727ed11b24c08c84a608886a1716b81c93640d80
DIFF: 
https://github.com/llvm/llvm-project/commit/727ed11b24c08c84a608886a1716b81c93640d80.diff

LOG: [AVR] Recognize the AVR architecture in lldb

This commit adds AVR support to lldb. With this change, it can load a
binary and do basic things like dump a line table.

Not much else has been implemented, that should be done in later
changes.

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

Added: 
lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml

Modified: 
lldb/include/lldb/Utility/ArchSpec.h
lldb/source/Utility/ArchSpec.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ArchSpec.h 
b/lldb/include/lldb/Utility/ArchSpec.h
index 6e209dfd2e46..d1a257bedd36 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -188,6 +188,8 @@ class ArchSpec {
 
 eCore_arc, // little endian ARC
 
+eCore_avr,
+
 eCore_wasm32,
 
 kNumCores,

diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index 08931b58cd2c..bb4771c6488c 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -218,6 +218,8 @@ static const CoreDefinition g_core_definitions[] = {
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
 {eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"},
 
+{eByteOrderLittle, 2, 2, 4, llvm::Triple::avr, ArchSpec::eCore_avr, "avr"},
+
 {eByteOrderLittle, 4, 1, 4, llvm::Triple::wasm32, ArchSpec::eCore_wasm32,
  "wasm32"},
 };
@@ -448,6 +450,8 @@ static const ArchDefinitionEntry g_elf_arch_entries[] = {
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // AVR
 };
 
 static const ArchDefinition g_elf_arch_def = {

diff  --git a/lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml 
b/lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
new file mode 100644
index ..3326ce66fa5d
--- /dev/null
+++ b/lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
@@ -0,0 +1,30 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Plugin name: elf
+# CHECK: Architecture: avr--
+# CHECK: Executable: true
+# CHECK: Stripped: false
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK:   Name: .text
+# CHECK-NEXT:   Type: code
+# CHECK-NEXT:   Permissions: r-x
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x0
+# CHECK-NEXT:   VM size: 4
+# CHECK-NEXT:   File size: 4
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_AVR
+  Flags:   [ EF_AVR_ARCH_AVR1, EF_AVR_ARCH_AVR4 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+AddressAlign:0x0001
+Content: 'FECF'
+...



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


[Lldb-commits] [PATCH] D73539: [AVR] Recognize the AVR architecture in lldb

2020-01-30 Thread Ayke via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG727ed11b24c0: [AVR] Recognize the AVR architecture in lldb 
(authored by aykevl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73539

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml


Index: lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
@@ -0,0 +1,30 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Plugin name: elf
+# CHECK: Architecture: avr--
+# CHECK: Executable: true
+# CHECK: Stripped: false
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK:   Name: .text
+# CHECK-NEXT:   Type: code
+# CHECK-NEXT:   Permissions: r-x
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x0
+# CHECK-NEXT:   VM size: 4
+# CHECK-NEXT:   File size: 4
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_AVR
+  Flags:   [ EF_AVR_ARCH_AVR1, EF_AVR_ARCH_AVR4 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+AddressAlign:0x0001
+Content: 'FECF'
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -218,6 +218,8 @@
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
 {eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"},
 
+{eByteOrderLittle, 2, 2, 4, llvm::Triple::avr, ArchSpec::eCore_avr, "avr"},
+
 {eByteOrderLittle, 4, 1, 4, llvm::Triple::wasm32, ArchSpec::eCore_wasm32,
  "wasm32"},
 };
@@ -448,6 +450,8 @@
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // AVR
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -188,6 +188,8 @@
 
 eCore_arc, // little endian ARC
 
+eCore_avr,
+
 eCore_wasm32,
 
 kNumCores,


Index: lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
@@ -0,0 +1,30 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Plugin name: elf
+# CHECK: Architecture: avr--
+# CHECK: Executable: true
+# CHECK: Stripped: false
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK:   Name: .text
+# CHECK-NEXT:   Type: code
+# CHECK-NEXT:   Permissions: r-x
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x0
+# CHECK-NEXT:   VM size: 4
+# CHECK-NEXT:   File size: 4
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_AVR
+  Flags:   [ EF_AVR_ARCH_AVR1, EF_AVR_ARCH_AVR4 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+AddressAlign:0x0001
+Content: 'FECF'
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -218,6 +218,8 @@
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
 {eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"},
 
+{eByteOrderLittle, 2, 2, 4, llvm::Triple::avr, ArchSpec::eCore_avr, "avr"},
+
 {eByteOrderLittle, 4, 1, 4, llvm::Triple::wasm32, ArchSpec::eCore_wasm32,
  "wasm32"},
 };
@@ -448,6 +450,8 @@
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // AVR
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -188,6 +188,8 @@
 
 eCore_arc, // little endian ARC
 
+eCore_avr,
+
 eCore_wasm32,
 
 kNumCores,
___

[Lldb-commits] [PATCH] D73384: [lldb/Lit] Change the lldbtest format to behave more like shell test.

2020-01-30 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

I suppose you need to make the timeout branch entirely separate from the 
non-timeout branch, as the variable assignment does not take place then.




Comment at: lldb/test/API/lldbtest.py:100
+output = """Script:\n--\n%s\n--\nExit Code: %d\n""" % (
+' '.join(cmd), exitCode)
+if timeoutInfo is not None:

This crashes on timeout since `exitCode` is not declared:

```
UNRESOLVED: lldb-api :: tools/lldb-vscode/variables/TestVSCode_variables.py 
(1906 of 1907)
 TEST 'lldb-api :: 
tools/lldb-vscode/variables/TestVSCode_variables.py' FAILED 
Exception during script execution:
Traceback (most recent call last):
  File "/home/mgorny/llvm-project/llvm/utils/lit/lit/worker.py", line 91, in 
_execute_test_handle_errors
return _adapt_result(test.config.test_format.execute(test, lit_config))
  File "/home/mgorny/llvm-project/llvm/tools/lldb/test/API/lldbtest.py", line 
100, in execute
' '.join(cmd), exitCode)
UnboundLocalError: local variable 'exitCode' referenced before assignment
```



Comment at: lldb/test/API/lldbtest.py:105
+
+if out:
+output += """Command Output (stdout):\n--\n%s\n--\n""" % (out,)

I suppose this will crash as well, and below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73384



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


[Lldb-commits] [PATCH] D73665: Stop emitting a breakpoint for each location in a breakpoint when responding to breakpoint commands.

2020-01-30 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D73665#1848755 , @labath wrote:

> This is not ideal, but if that's what vscode wants, then I guess we just have 
> to do it.
>
> However, are you sure that this is all that needs to be done here. It seems 
> like also the handling of eBreakpointEventTypeLocationsAdded/Removed events 
> in EventThreadFunction should be changed too. Otherwise, I think the whole 
> breakpoint will disappear whenever a single location goes away...


That is the nice thing about this fix: anyone that reports a breakpoint uses:

  llvm::json::Value CreateBreakpoint(lldb::SBBreakpointLocation &bp_loc)

And this is the function I fixed!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73665



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


[Lldb-commits] [PATCH] D73594: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

2020-01-30 Thread Adrian McCarthy via Phabricator via lldb-commits
amccarth marked 3 inline comments as done.
amccarth added a comment.

I backed out the TODO and the warn-and-continue behavior to expedite this patch 
which is just supposed to be a refactor for readability.

We can revisit the issue of debug symbol file names not matching the object 
file names in the future (if necessary).




Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4103
+// this point.
+// TODO:  Is this part worthwhile?  `foo.exe` will never match `foo.pdb`
+if (matching_modules.IsEmpty())

labath wrote:
> amccarth wrote:
> > labath wrote:
> > > This is not unreasonable in the non-pdb world. You can have a stripped 
> > > version of a file somewhere prepared for deployment to some device (or 
> > > downloaded from the device), and then you'll also have an unstripped 
> > > version of that file (with the same name) in some build folder.
> > Sure, if you've got two files with the same name in two directories that's 
> > fine.
> > 
> > But the loop tries peeling the extension off of the supplied file name and 
> > comparing it to the target's file name.  I guess it's trying to match 
> > something like `foo.unstripped` to `foo`.  Is that a typical thing?
> Ah, sorry, I missed that part.
> 
> Yes, this is also thing that's used sometimes (probably even more often than 
> the first thing). One of the conventions for automatically finding [[ 
> https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html | 
> separate debug files ]] involves appending `.debug` to your file and then 
> placing it in a special folder. I guess this was added to enable one to do 
> "target symbols add /whereever/foo.debug" and have it match "foo".
> 
> That said, stripping the extension in a loop does seem like overkill.
OK, I'm removing the TODO in favor of a comment with the `foo` and `foo.debug` 
you gave.  Whether it should be a loop or not can be addressed later.  Whether 
we should do something that would also work for Windows (e.g., `foo.exe` 
matching `foo.pdb`) can also be considered later.

This patch is just supposed to be a refactor for readability/debuggability.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4175-4178
+if (object_file->GetFileSpec() != symbol_fspec) {
+  result.AppendWarning("there is a discrepancy between the module file "
+   "spec and the symbol file spec\n");
+}

labath wrote:
> amccarth wrote:
> > clayborg wrote:
> > > amccarth wrote:
> > > > amccarth wrote:
> > > > > clayborg wrote:
> > > > > > labath wrote:
> > > > > > > This part is not good. Everywhere else except pdbs this means 
> > > > > > > that we were in fact unable to load the symbol file. What happens 
> > > > > > > is that if we cannot load the object file representing the symbol 
> > > > > > > file (at [[ 
> > > > > > > https://github.com/llvm/llvm-project/blob/master/lldb/source/Symbol/SymbolVendor.cpp#L48
> > > > > > >  | SymbolVendor.cpp:48 ]]), we fall back to creating a SymbolFile 
> > > > > > > using the object file of the original module (line 52).
> > > > > > > 
> > > > > > > The effect of that is that the symbol file creation will always 
> > > > > > > succeed, the previous checks are more-or-less useless, and the 
> > > > > > > only way to check if we are really using the symbols from the 
> > > > > > > file the user specified is to compare the file specs.
> > > > > > > 
> > > > > > > Now, PDB symbol files are abusing this fallback, and reading the 
> > > > > > > symbols from the pdb files even though they were in fact asked to 
> > > > > > > read them from the executable file. This is why this may sound 
> > > > > > > like a "discrepancy" coming from the pdb world, but everywhere 
> > > > > > > else this just means that the symbol file was not actually loaded.
> > > > > > This could also fail if the symbol file spec was a bundle which got 
> > > > > > resolved when the module was loaded. If the user specified 
> > > > > > "/path/to/foo.dSYM" and the underlying code was able to resolve the 
> > > > > > symbol file in the dSYM bundle to 
> > > > > > "/path/to/foo.dSYM/Contents/Resources/DWARF/foo".
> > > > > Interesting.  I did not expect that fallback behavior from the API.
> > > > > 
> > > > > > PDB symbol files are abusing this fallback
> > > > > 
> > > > > I'm not sure there's abuse going on here.  I'm not understanding 
> > > > > something about how lldb models this situation.  Consider the case 
> > > > > where the user explicitly asks lldb to load symbols from a PDB:
> > > > > 
> > > > > (lldb) target add symbols D:\my_symbols\foo.pdb
> > > > > 
> > > > > Then `symbol_file` is the PDB and `object_file` is the executable or 
> > > > > DLL.  I would expect those file specs to match only in the case where 
> > > > > the symbols are in the binary (in other words, when symbol file and 
> > > > > object file are the same file).  Even ignoring the PDB case, it seems 
> > 

[Lldb-commits] [PATCH] D73594: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

2020-01-30 Thread Adrian McCarthy via Phabricator via lldb-commits
amccarth updated this revision to Diff 241510.
amccarth marked an inline comment as done.
amccarth added a comment.

- Backed out the warn-and-continue behavior change.  It should be addressed 
later.
- Changed lldb_assert to assert.
- Improved comment in place of the proposed TODO.


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

https://reviews.llvm.org/D73594

Files:
  lldb/source/Commands/CommandObjectTarget.cpp

Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -4054,12 +4054,10 @@
 module_spec.GetFileSpec().GetFilename() = symbol_fspec.GetFilename();
 }
 
-// We now have a module that represents a symbol file that can be used
-// for a module that might exist in the current target, so we need to
-// find that module in the target
-ModuleList matching_module_list;
+// Now module_spec represents a symbol file for a module that might exist
+// in the current target.  Let's find possible matches.
+ModuleList matching_modules;
 
-size_t num_matches = 0;
 // First extract all module specs from the symbol file
 lldb_private::ModuleSpecList symfile_module_specs;
 if (ObjectFile::GetModuleSpecifications(module_spec.GetSymbolFileSpec(),
@@ -4070,34 +4068,30 @@
   target_arch_module_spec.GetArchitecture() = target->GetArchitecture();
   if (symfile_module_specs.FindMatchingModuleSpec(target_arch_module_spec,
   symfile_module_spec)) {
-// See if it has a UUID?
 if (symfile_module_spec.GetUUID().IsValid()) {
   // It has a UUID, look for this UUID in the target modules
   ModuleSpec symfile_uuid_module_spec;
   symfile_uuid_module_spec.GetUUID() = symfile_module_spec.GetUUID();
   target->GetImages().FindModules(symfile_uuid_module_spec,
-  matching_module_list);
-  num_matches = matching_module_list.GetSize();
+  matching_modules);
 }
   }
 
-  if (num_matches == 0) {
-// No matches yet, iterate through the module specs to find a UUID
-// value that we can match up to an image in our target
-const size_t num_symfile_module_specs =
-symfile_module_specs.GetSize();
-for (size_t i = 0; i < num_symfile_module_specs && num_matches == 0;
-  ++i) {
+  if (matching_modules.IsEmpty()) {
+// No matches yet.  Iterate through the module specs to find a UUID
+// value that we can match up to an image in our target.
+const size_t num_symfile_module_specs = symfile_module_specs.GetSize();
+for (size_t i = 0;
+ i < num_symfile_module_specs && matching_modules.IsEmpty(); ++i) {
   if (symfile_module_specs.GetModuleSpecAtIndex(
   i, symfile_module_spec)) {
 if (symfile_module_spec.GetUUID().IsValid()) {
-  // It has a UUID, look for this UUID in the target modules
+  // It has a UUID.  Look for this UUID in the target modules.
   ModuleSpec symfile_uuid_module_spec;
   symfile_uuid_module_spec.GetUUID() =
   symfile_module_spec.GetUUID();
   target->GetImages().FindModules(symfile_uuid_module_spec,
-  matching_module_list);
-  num_matches = matching_module_list.GetSize();
+  matching_modules);
 }
   }
 }
@@ -4105,13 +4099,11 @@
 }
 
 // Just try to match up the file by basename if we have no matches at
-// this point
-if (num_matches == 0) {
-  target->GetImages().FindModules(module_spec, matching_module_list);
-  num_matches = matching_module_list.GetSize();
-}
+// this point.  For example, module foo might have symbols in foo.debug.
+if (matching_modules.IsEmpty())
+  target->GetImages().FindModules(module_spec, matching_modules);
 
-while (num_matches == 0) {
+while (matching_modules.IsEmpty()) {
   ConstString filename_no_extension(
   module_spec.GetFileSpec().GetFileNameStrippingExtension());
   // Empty string returned, let's bail
@@ -4124,82 +4116,93 @@
 
   // Replace basename with one fewer extension
   module_spec.GetFileSpec().GetFilename() = filename_no_extension;
-  target->GetImages().FindModules(module_spec, matching_module_list);
-  num_matches = matching_module_list.GetSize();
+  target->GetImages().FindModules(module_spec, matching_modules);
+}
+
+if (matching_modules.IsEmpty()) {
+  StreamString ss_symfile_uuid;
+  if (module_spec.GetUUID().IsValid()) {
+ss_symfile_uuid << " (";
+module_spec.GetUUID().Dump(&

[Lldb-commits] [lldb] 05badc6 - [lldb/Reproducers] Fix API boundary tracking bug

2020-01-30 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-01-30T11:22:12-08:00
New Revision: 05badc60b7f4dff3c1b9efd5d7eea13979e255db

URL: 
https://github.com/llvm/llvm-project/commit/05badc60b7f4dff3c1b9efd5d7eea13979e255db
DIFF: 
https://github.com/llvm/llvm-project/commit/05badc60b7f4dff3c1b9efd5d7eea13979e255db.diff

LOG: [lldb/Reproducers] Fix API boundary tracking bug

When recording the result from the LLDB_RECORD_RESULT macro, we need to
update the boundary so we capture the copy constructor. However, when
called to record the this pointer of the (copy) constructor itself, the
boundary should not be toggled, because it is called from the
LLDB_RECORD_CONSTRUCTOR macro, which might be followed by other API
calls.

This manifested itself as an object encountered during replay that we
hadn't seen before. The index-to-object mapping would return a nullptr
and lldb would crash.

Added: 


Modified: 
lldb/include/lldb/Utility/ReproducerInstrumentation.h

Removed: 




diff  --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h 
b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index 71b85a266592..e91eb7f5922c 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -98,7 +98,7 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
 sb_recorder.Record(data.GetSerializer(), data.GetRegistry(),   
\
&lldb_private::repro::construct::doit, 
\
__VA_ARGS__);   
\
-sb_recorder.RecordResult(this);
\
+sb_recorder.RecordResult(this, false); 
\
   }
 
 #define LLDB_RECORD_CONSTRUCTOR_NO_ARGS(Class) 
\
@@ -107,7 +107,7 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
 sb_recorder.Record(data.GetSerializer(), data.GetRegistry(),   
\
&lldb_private::repro::construct::doit);
\
-sb_recorder.RecordResult(this);
\
+sb_recorder.RecordResult(this, false); 
\
   }
 
 #define LLDB_RECORD_METHOD(Result, Class, Method, Signature, ...)  
\
@@ -175,7 +175,7 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
static_cast(&Class::Method)); 
\
   }
 
-#define LLDB_RECORD_RESULT(Result) sb_recorder.RecordResult(Result);
+#define LLDB_RECORD_RESULT(Result) sb_recorder.RecordResult(Result, true);
 
 /// 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
@@ -716,8 +716,16 @@ class Recorder {
   }
 
   /// Record the result of a function call.
-  template  Result RecordResult(Result &&r) {
-UpdateBoundary();
+  template 
+  Result RecordResult(Result &&r, bool update_boundary) {
+// When recording the result from the LLDB_RECORD_RESULT macro, we need to
+// update the boundary so we capture the copy constructor. However, when
+// called to record the this pointer of the (copy) constructor, the
+// boundary should not be toggled, because it is called from the
+// LLDB_RECORD_CONSTRUCTOR macro, which might be followed by other API
+// calls.
+if (update_boundary)
+  UpdateBoundary();
 if (m_serializer && ShouldCapture()) {
   assert(!m_result_recorded);
   m_serializer->SerializeAll(r);



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


[Lldb-commits] [PATCH] D73665: Stop emitting a breakpoint for each location in a breakpoint when responding to breakpoint commands.

2020-01-30 Thread Greg Clayton via Phabricator via lldb-commits
clayborg updated this revision to Diff 241543.
clayborg added a comment.

Handle breakpoint events in the same way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73665

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/LLDBUtils.cpp
  lldb/tools/lldb-vscode/LLDBUtils.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -379,27 +379,20 @@
 if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
   auto event_type =
   lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event);
-  const auto num_locs =
-  lldb::SBBreakpoint::GetNumBreakpointLocationsFromEvent(event);
   auto bp = lldb::SBBreakpoint::GetBreakpointFromEvent(event);
   bool added = event_type & lldb::eBreakpointEventTypeLocationsAdded;
   bool removed =
   event_type & lldb::eBreakpointEventTypeLocationsRemoved;
   if (added || removed) {
-for (size_t i = 0; i < num_locs; ++i) {
-  auto bp_loc =
-  lldb::SBBreakpoint::GetBreakpointLocationAtIndexFromEvent(
-  event, i);
-  auto bp_event = CreateEventObject("breakpoint");
-  llvm::json::Object body;
-  body.try_emplace("breakpoint", CreateBreakpoint(bp_loc));
-  if (added)
-body.try_emplace("reason", "new");
-  else
-body.try_emplace("reason", "removed");
-  bp_event.try_emplace("body", std::move(body));
-  g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
-}
+auto bp_event = CreateEventObject("breakpoint");
+llvm::json::Object body;
+body.try_emplace("breakpoint", CreateBreakpoint(bp));
+if (added)
+  body.try_emplace("reason", "new");
+else
+  body.try_emplace("reason", "removed");
+bp_event.try_emplace("body", std::move(body));
+g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
   }
 }
   } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
Index: lldb/tools/lldb-vscode/LLDBUtils.h
===
--- lldb/tools/lldb-vscode/LLDBUtils.h
+++ lldb/tools/lldb-vscode/LLDBUtils.h
@@ -106,46 +106,6 @@
 /// The LLDB frame index ID.
 uint32_t GetLLDBFrameID(uint64_t dap_frame_id);
 
-/// Given a LLDB breakpoint, make a breakpoint ID that is unique to a
-/// specific breakpoint and breakpoint location.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] bp_loc
-/// The LLDB break point location.
-///
-/// \return
-/// A unique integer that allows us to easily find the right
-/// stack frame within a thread on subsequent VS code requests.
-int64_t MakeVSCodeBreakpointID(lldb::SBBreakpointLocation &bp_loc);
-
-/// Given a VSCode breakpoint ID, convert to a LLDB breakpoint ID.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] dap_breakpoint_id
-/// The VSCode breakpoint ID to convert to an LLDB breakpoint ID.
-///
-/// \return
-/// The LLDB breakpoint ID.
-uint32_t GetLLDBBreakpointID(uint64_t dap_breakpoint_id);
-
-/// Given a VSCode breakpoint ID, convert to a LLDB breakpoint location ID.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] dap_breakpoint_id
-/// The VSCode frame ID to convert to a breakpoint location ID.
-///
-/// \return
-/// The LLDB breakpoint location ID.
-uint32_t GetLLDBBreakpointLocationID(uint64_t dap_breakpoint_id);
 } // namespace lldb_vscode
 
 #endif
Index: lldb/tools/lldb-vscode/LLDBUtils.cpp
===
--- lldb/tools/lldb-vscode/LLDBUtils.cpp
+++ lldb/tools/lldb-vscode/LLDBUtils.cpp
@@ -79,19 +79,4 @@
frame.GetFrameID());
 }
 
-static uint32_t constexpr BREAKPOINT_ID_SHIFT = 22;
-
-uint32_t GetLLDBBreakpointID(uint64_t dap_breakpoint_id) {
-  return dap_breakpoint_id >> BREAKPOINT_ID_SHIFT;
-}
-
-uint32_t GetLLDBBreakpointLocationID(uint64_t dap_breakpoint_id) {
-  return dap_breakpoint_id & ((1u << BREAKPOINT_ID_SHIFT) - 1);
-}
-
-int64_t M

[Lldb-commits] [PATCH] D73279: testsuite: generalize `DWARFASTParserClangTests` based on `DWARFExpressionTest`'s YAML

2020-01-30 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 241545.
jankratochvil added a comment.
Herald added a subscriber: mgorny.

By making the new `.cpp` file I get now this error:

  In file included from 
/home/jkratoch/redhat/llvm-monorepo/lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp:9:
  In file included from 
/home/jkratoch/redhat/llvm-monorepo/lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h:14:
  In file included from 
/home/jkratoch/redhat/llvm-monorepo/lldb/unittests/TestingSupport/SubsystemRAII.h:13:
  In file included from 
/home/jkratoch/redhat/llvm-monorepo/llvm/include/llvm/Testing/Support/Error.h:14:
  
/home/jkratoch/redhat/llvm-monorepo/llvm/include/llvm/Testing/Support/SupportHelpers.h:16:10:
 fatal error: 'gmock/gmock-matchers.h' file not found
  #include "gmock/gmock-matchers.h"
   ^~~~

If anyone has an idea... Originally the header file was being built by: 
`lldb/unittests/Expression/CMakeLists.txt`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73279

Files:
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/TestingSupport/CMakeLists.txt
  lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp
  lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h
  lldb/unittests/TestingSupport/module.modulemap

Index: lldb/unittests/TestingSupport/module.modulemap
===
--- lldb/unittests/TestingSupport/module.modulemap
+++ lldb/unittests/TestingSupport/module.modulemap
@@ -13,4 +13,5 @@
 module lldb_TestingSupport_Symbol {
   requires cplusplus
   module ClangTestUtils { header "Symbol/ClangTestUtils.h" export * }
+  module YAMLModuleTester { header "Symbol/YAMLModuleTester.h" export * }
 }
Index: lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h
===
--- /dev/null
+++ lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h
@@ -0,0 +1,89 @@
+//===- YAMLModuleTester.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UNITTESTS_TESTINGSUPPORT_SYMBOL_YAMLMODULETESTER_H
+#define LLDB_UNITTESTS_TESTINGSUPPORT_SYMBOL_YAMLMODULETESTER_H
+
+#include "Plugins/SymbolFile/DWARF/DWARFUnit.h"
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/TypeSystemClang.h"
+#include "llvm/ObjectYAML/DWARFEmitter.h"
+
+namespace lldb_private {
+
+/// A mock module holding an object file parsed from YAML.
+class YAMLModule : public lldb_private::Module {
+public:
+  YAMLModule(ArchSpec &arch) : Module(FileSpec("test"), arch) {}
+  void SetObjectFile(lldb::ObjectFileSP obj_file) { m_objfile_sp = obj_file; }
+  ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
+};
+
+/// A mock object file that can be parsed from YAML.
+class YAMLObjectFile : public lldb_private::ObjectFile {
+  const lldb::ModuleSP m_module_sp;
+  llvm::StringMap> &m_section_map;
+  /// Because there is only one DataExtractor in the ObjectFile
+  /// interface, all sections are copied into a contiguous buffer.
+  std::vector m_buffer;
+
+public:
+  YAMLObjectFile(const lldb::ModuleSP &module_sp,
+ llvm::StringMap> &map);
+
+  /// Callback for initializing the module's list of sections.
+  void CreateSections(SectionList &unified_section_list) override;
+
+  /// \{
+  /// Stub methods that aren't needed here.
+  ConstString GetPluginName() override { return ConstString("YAMLObjectFile"); }
+  uint32_t GetPluginVersion() override { return 0; }
+  void Dump(Stream *s) override {}
+  uint32_t GetAddressByteSize() const override { return 8; }
+  uint32_t GetDependentModules(FileSpecList &file_list) override { return 0; }
+  bool IsExecutable() const override { return 0; }
+  ArchSpec GetArchitecture() override { return {}; }
+  Symtab *GetSymtab() override { return nullptr; }
+  bool IsStripped() override { return false; }
+  UUID GetUUID() override { return {}; }
+  lldb::ByteOrder GetByteOrder() const override {
+return lldb::eByteOrderLittle;
+  }
+  bool ParseHeader() override { return false; }
+  Type CalculateType() override { return {}; }
+  Strata CalculateStrata() override { return {}; }
+  /// \}
+};
+
+/// Helper class that can construct a module from YAML and evaluate
+/// DWARF expressions on it.
+class YAMLModuleTester {
+  //  SubsystemRAII subsystems;
+  SubsystemRAII subsystems;
+  llvm::StringMap> m_s

[Lldb-commits] [PATCH] D73661: [lldb] Move clang-based files out of Symbol

2020-01-30 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

I vote to make this a plug-in as well!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73661



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


[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

2020-01-30 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D72751#1848880 , @labath wrote:

> In D72751#1847617 , @clayborg wrote:
>
> > Yes the current approach allows anyone to load any section at any address. 
> > On Darwin systems, the DYLD shared cache will move __TEXT, __DATA, and 
> > other sections around such that all __TEXT sections from all shared 
> > libraries in the shared cache are all in the one contiguous range. The 
> > slide is different for each section, so we have some nice flexibility with 
> > being able to set the section load address individually. They will even 
> > invert the memory order sometimes where in the file we have __TEXT followed 
> > by __DATA, but in the shared cache __DATA appears at a lower address than 
> > __TEXT.
>
>
> Sorry about the off-topic, but I found this bit very interesting. Greg, how 
> does this work with code referencing the variables in the data section (e.g. 
> `static int x; int *f() { return &x; }`). This code on elf, even when linked 
> in fully position-independent mode will not contain any relocations because 
> it is assumed that the dynamic loader will not change the relative layout of 
> code and data (and so the code can use pc-relative addressing). This 
> obviously does not work if data can be moved around independently. Does that 
> mean that darwin will include some additional relocations which need to be 
> resolved at load time?


No, the relocations are performed when the shared cache is made and they are 
removed! This also works for PLT calls from one shared library to another. If 
two shared libraries have PLT entries to each other's functions, those are 
resolved and don't require a call to the dynamic loader! There are two shared 
caches: one for running only and one for development. The development shared 
caches leave PLT entries alone so that interposing can happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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


[Lldb-commits] [PATCH] D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging

2020-01-30 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.



> there is a problem: if I remove DynamicLoaderWasmDYLD what happens is that 
> `DynamicLoaderStatic` is found as a valid loader for a triple like 
> **"wasm32-unknown-unknown-wasm"** because the Triple::OS is 
> llvm::Triple::UnknownOS (I found out the hard way when I was registering 
> DynamicLoaderWasmDYLD after DynamicLoaderStatic :-)).
>  There is an explicit check for UnknownOS:
> 
>   DynamicLoader *DynamicLoaderStatic::CreateInstance(Process *process,
>  bool force) {
> bool create = force;
> if (!create) {
>   const llvm::Triple &triple_ref =
>   process->GetTarget().GetArchitecture().GetTriple();
>   const llvm::Triple::OSType os_type = triple_ref.getOS();
>   if ((os_type == llvm::Triple::UnknownOS))
> create = true;
> }
> ...
>   
>   call stack:
>   DynamicLoaderStatic::CreateInstance(lldb_private::Process * process, bool 
> force) Line 29
>   DynamicLoader::FindPlugin(lldb_private::Process * process, const char * 
> plugin_name) Line 52
>   lldb_private::process_gdb_remote::ProcessGDBRemote::GetDynamicLoader() Line 
> 3993
>   lldb_private::Process::CompleteAttach() Line 2931
>   lldb_private::Process::ConnectRemote(lldb_private::Stream * strm, 
> llvm::StringRef remote_url) Line 3022
> 
> 
> Could ProcessGDBRemote::GetDynamicLoader behave differently just when the 
> architecture is wasm32, maybe? But then it is probably cleaner to add this 
> plugin class, what do you think?

So if you really have no OS, and no vendor, then this plug-in does get used. 
Why? Because it assumes that things will be loaded where they are (file_addr == 
load_addr). Could you create your WASM stuff in a way such that all addresses 
that are in a memory loaded object file have the file address the same as the 
load address? Do you ever actually have files on disk that we load for WASM? Or 
is it always loaded from memory?

It **is** ok to add your plug-in that recognizes WASM with no OS and no vendor, 
you will just need to make sure that the plug-in is registered before 
DynamicLoaderStatic. Since the plug-in manager will run through each plug-in 
linearly when looking for a match. This is also why any "auto registration of 
plug-ins" will cause problems as if we don't control the plug-in registration 
order, we will have issues that will arise. That is a different topic though, 
but I thought I remembered seeing a patch that tried to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72751



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


[Lldb-commits] [PATCH] D73661: [lldb] Move clang-based files out of Symbol

2020-01-30 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In D73661#1848750 , @labath wrote:

> I think this is a big milestone. Thanks for working on this.
>
> The main question  I have is about the new location of this code. This patch 
> puts it under ExpressionParser/Clang, which is not completely unreasonable, 
> as that's where most of the clang stuff is. However, it does create some 
> awkward-looking (to me) dependencies, or even loops 
> (SymbolFileDWARF<->ExpressionParserClang). Also the dep from data formatters 
> in Language/CPLusPlus on ExpressionParser/Clang is odd, because the data 
> formatters don't actually use/need expressions do to their work.
>
> So, as much as I hate proliferating plugins, I have to ask this question: 
> Should this be a new plugin kind (TypeSystem/Clang) instead ? The name of the 
> class, and the presence of the Initialize function already seem to indicate 
> that. And it seems to me like this would create a reasonable dependency graph 
> between the various plugins. TypeSystemClang would be at the bottom of this. 
> The various SymbolFile plugins would depend on it, because they generate 
> clang ASTs. ExpressionParserClang would depend on it because it pulls the 
> generated ASTs that way. The same goes for the data formatters.  But there 
> would be no dependency between SymbolFiles and expression parsers or data 
> formatters, because the former should just provide the ast (and not care 
> about who the consumer is) and the latter should consume it (regardless of 
> the source).
>
> I think this ExpressionParser vs. TypeSystem would also make sense in terms 
> of the outgoing dependencies. The type system should depend +/- only on 
> clangAST, whereas the expression parser would need pretty much the whole 
> clang.
>
> WDYT?


I think that creating a TypeSystem plugin is probably the better option here. 
After looking more closely at the dependency graph I think that what you 
pointed out is enough to justify a new plugin. I'll update this patch.

In D73661#1848981 , @teemperor wrote:

> Also this doesn't compile on Linux:


Thanks for checking this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73661



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


[Lldb-commits] [lldb] 92a42b6 - [lldb][NFC] LLDB_LOGF to LLDB_LOG conversion in ClangASTImporter

2020-01-30 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-01-30T22:10:31+01:00
New Revision: 92a42b6a4d1544acb96f334369ea6c1c948634e3

URL: 
https://github.com/llvm/llvm-project/commit/92a42b6a4d1544acb96f334369ea6c1c948634e3
DIFF: 
https://github.com/llvm/llvm-project/commit/92a42b6a4d1544acb96f334369ea6c1c948634e3.diff

LOG: [lldb][NFC] LLDB_LOGF to LLDB_LOG conversion in ClangASTImporter

Added: 


Modified: 
lldb/source/Symbol/ClangASTImporter.cpp

Removed: 




diff  --git a/lldb/source/Symbol/ClangASTImporter.cpp 
b/lldb/source/Symbol/ClangASTImporter.cpp
index 847fa5cb091d..10e9868769d7 100644
--- a/lldb/source/Symbol/ClangASTImporter.cpp
+++ b/lldb/source/Symbol/ClangASTImporter.cpp
@@ -83,16 +83,16 @@ clang::Decl *ClangASTImporter::CopyDecl(clang::ASTContext 
*dst_ast,
 user_id = metadata->GetUserID();
 
   if (NamedDecl *named_decl = dyn_cast(decl))
-LLDB_LOGF(log,
-  "  [ClangASTImporter] WARNING: Failed to import a %s "
-  "'%s', metadata 0x%" PRIx64,
-  decl->getDeclKindName(),
-  named_decl->getNameAsString().c_str(), user_id);
+LLDB_LOG(log,
+ "  [ClangASTImporter] WARNING: Failed to import a {0} "
+ "'{1}', metadata {2}",
+ decl->getDeclKindName(), named_decl->getNameAsString(),
+ user_id);
   else
-LLDB_LOGF(log,
-  "  [ClangASTImporter] WARNING: Failed to import a %s, "
-  "metadata 0x%" PRIx64,
-  decl->getDeclKindName(), user_id);
+LLDB_LOG(log,
+ "  [ClangASTImporter] WARNING: Failed to import a {0}, "
+ "metadata {1}",
+ decl->getDeclKindName(), user_id);
 }
 return nullptr;
   }
@@ -169,12 +169,11 @@ class DeclContextOverride {
 if (clang::Decl *escaped_child = GetEscapedChild(decl)) {
   Log 
*log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
-  LLDB_LOGF(log,
-"[ClangASTImporter] DeclContextOverride couldn't "
-"override (%sDecl*)%p - its child (%sDecl*)%p escapes",
-decl->getDeclKindName(), static_cast(decl),
-escaped_child->getDeclKindName(),
-static_cast(escaped_child));
+  LLDB_LOG(log,
+   "[ClangASTImporter] DeclContextOverride couldn't "
+   "override ({0}Decl*){1} - its child ({2}Decl*){3} escapes",
+   decl->getDeclKindName(), decl, escaped_child->getDeclKindName(),
+   escaped_child);
   lldbassert(0 && "Couldn't override!");
 }
 
@@ -298,8 +297,8 @@ CompilerType ClangASTImporter::DeportType(TypeSystemClang 
&dst,
   llvm::cast(src_type.GetTypeSystem());
 
   LLDB_LOG(log,
-   "[ClangASTImporter] DeportType called on ({0}Type*){1:x} "
-   "from (ASTContext*){2:x} to (ASTContext*){3:x}",
+   "[ClangASTImporter] DeportType called on ({0}Type*){1} "
+   "from (ASTContext*){2} to (ASTContext*){3}",
src_type.GetTypeName(), src_type.GetOpaqueQualType(),
&src_ctxt->getASTContext(), &dst.getASTContext());
 
@@ -318,11 +317,10 @@ clang::Decl 
*ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx,
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   clang::ASTContext *src_ctx = &decl->getASTContext();
-  LLDB_LOGF(log,
-"[ClangASTImporter] DeportDecl called on (%sDecl*)%p from "
-"(ASTContext*)%p to (ASTContext*)%p",
-decl->getDeclKindName(), static_cast(decl),
-static_cast(src_ctx), static_cast(dst_ctx));
+  LLDB_LOG(log,
+   "[ClangASTImporter] DeportDecl called on ({0}Decl*){1} from "
+   "(ASTContext*){2} to (ASTContext*){3}",
+   decl->getDeclKindName(), decl, src_ctx, dst_ctx);
 
   DeclContextOverride decl_context_override;
 
@@ -337,11 +335,10 @@ clang::Decl 
*ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx,
   if (!result)
 return nullptr;
 
-  LLDB_LOGF(
-  log,
-  "[ClangASTImporter] DeportDecl deported (%sDecl*)%p to (%sDecl*)%p",
-  decl->getDeclKindName(), static_cast(decl),
-  result->getDeclKindName(), static_cast(result));
+  LLDB_LOG(log,
+   "[ClangASTImporter] DeportDecl deported ({0}Decl*){1} to "
+   "({2}Decl*){3}",
+   decl->getDeclKindName(), decl, result->getDeclKindName(), result);
 
   return result;
 }
@@ -795,9 +792,9 @@ void ClangASTImporter::BuildNamespaceMap(const 
clang::NamespaceDecl *decl) {
 void ClangASTImporter::ForgetDestination(clang::ASTContext *dst_ast) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
-  LLDB_LOGF(log,
-"[ClangASTImporter] Forgetting destination (ASTContext*)%p",
-static_cast(dst_ast));
+  LLDB_LOG(log,
+

Re: [Lldb-commits] [lldb] e3a7c77 - [lldb/Lit] Change the lldbtest format to behave more like shell test.

2020-01-30 Thread Vedant Kumar via lldb-commits


> On Jan 24, 2020, at 4:18 PM, Jonas Devlieghere via lldb-commits 
>  wrote:
> 
> 
> Author: Jonas Devlieghere
> Date: 2020-01-24T16:17:55-08:00
> New Revision: e3a7c7713cd87e37a95a544373cd21f6f06ab94e
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/e3a7c7713cd87e37a95a544373cd21f6f06ab94e
> DIFF: 
> https://github.com/llvm/llvm-project/commit/e3a7c7713cd87e37a95a544373cd21f6f06ab94e.diff
> 
> LOG: [lldb/Lit] Change the lldbtest format to behave more like shell test.
> 
> The current lldbtest format has a number of shortcomings, all related to
> how we omit information based on why the test fails. For example, a
> successful test would print nothing, even when `-a` is passed to lit.
> It's not up to the test format to decide whether to print something or
> not, that's handled by lit itself. For other test results we would
> sometimes print stdout & stderr, but not always, such as when a timeout
> was reached or we couldn't parse the dotest output.
> 
> This patch changes the lldbtest format and makes it behave more like
> lit. We now always print the dotest invocation, the exit code, the
> output to stdout & stderr. If you're used to dealing with ShTests in
> lit, this will feel all very familiar.
> 
> Differential revision: https://reviews.llvm.org/D73384
> 
> Added: 
> 
> 
> Modified: 
>lldb/test/API/lldbtest.py
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/test/API/lldbtest.py b/lldb/test/API/lldbtest.py
> index 349a67f22fb3..864d5ea1df03 100644
> --- a/lldb/test/API/lldbtest.py
> +++ b/lldb/test/API/lldbtest.py
> @@ -86,33 +86,46 @@ def execute(self, test, litConfig):
> shutil.copy(python, copied_python)
> cmd[0] = copied_python
> 
> +timeoutInfo = None
> try:
> out, err, exitCode = lit.util.executeCommand(
> cmd,
> env=test.config.environment,
> timeout=litConfig.maxIndividualTestTime)
> except lit.util.ExecuteCommandTimeoutException:
> -return (lit.Test.TIMEOUT, 'Reached timeout of {} seconds'.format(
> -litConfig.maxIndividualTestTime))
> +timeoutInfo = 'Reached timeout of {} seconds'.format(
> +litConfig.maxIndividualTestTime)
> +
> +output = """Script:\n--\n%s\n--\nExit Code: %d\n""" % (
> +' '.join(cmd), exitCode)

Heads up, when a test times out this prints:

  File "/Users/vsk/src/llvm-project-master/lldb/test/API/lldbtest.py", line 
100, in execute
' '.join(cmd), exitCode)
UnboundLocalError: local variable 'exitCode' referenced before assignment


> +if timeoutInfo is not None:
> +output += """Timeout: %s\n""" % (timeoutInfo,)
> +output += "\n"
> +
> +if out:
> +output += """Command Output (stdout):\n--\n%s\n--\n""" % (out,)
> +if err:
> +output += """Command Output (stderr):\n--\n%s\n--\n""" % (err,)
> +
> +if timeoutInfo:
> +return lit.Test.TIMEOUT, output
> 
> if exitCode:
> # Match FAIL but not XFAIL.
> for line in out.splitlines() + err.splitlines():
> if line.startswith('FAIL:'):
> -return lit.Test.FAIL, out + err
> +return lit.Test.FAIL, output
> 
> if 'XPASS:' in out or 'XPASS:' in err:
> -return lit.Test.XPASS, out + err
> +return lit.Test.XPASS, output
> 
> has_unsupported_tests = 'UNSUPPORTED:' in out or 'UNSUPPORTED:' in err
> has_passing_tests = 'PASS:' in out or 'PASS:' in err
> if has_unsupported_tests and not has_passing_tests:
> -return lit.Test.UNSUPPORTED, out + err
> +return lit.Test.UNSUPPORTED, output
> 
> passing_test_line = 'RESULT: PASSED'
> if passing_test_line not in out and passing_test_line not in err:
> -msg = ('Unable to find %r in dotest output (exit code 
> %d):\n\n%s%s'
> -   % (passing_test_line, exitCode, out, err))
> -return lit.Test.UNRESOLVED, msg
> +return lit.Test.UNRESOLVED, output
> 
> -return lit.Test.PASS, ''
> +return lit.Test.PASS, output
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


Re: [Lldb-commits] [lldb] e3a7c77 - [lldb/Lit] Change the lldbtest format to behave more like shell test.

2020-01-30 Thread Jonas Devlieghere via lldb-commits
Yep, Michał also reported this here: https://reviews.llvm.org/D73384

I hope to take a look at that today.

On Thu, Jan 30, 2020 at 1:43 PM Vedant Kumar  wrote:
>
>
>
> > On Jan 24, 2020, at 4:18 PM, Jonas Devlieghere via lldb-commits 
> >  wrote:
> >
> >
> > Author: Jonas Devlieghere
> > Date: 2020-01-24T16:17:55-08:00
> > New Revision: e3a7c7713cd87e37a95a544373cd21f6f06ab94e
> >
> > URL: 
> > https://github.com/llvm/llvm-project/commit/e3a7c7713cd87e37a95a544373cd21f6f06ab94e
> > DIFF: 
> > https://github.com/llvm/llvm-project/commit/e3a7c7713cd87e37a95a544373cd21f6f06ab94e.diff
> >
> > LOG: [lldb/Lit] Change the lldbtest format to behave more like shell test.
> >
> > The current lldbtest format has a number of shortcomings, all related to
> > how we omit information based on why the test fails. For example, a
> > successful test would print nothing, even when `-a` is passed to lit.
> > It's not up to the test format to decide whether to print something or
> > not, that's handled by lit itself. For other test results we would
> > sometimes print stdout & stderr, but not always, such as when a timeout
> > was reached or we couldn't parse the dotest output.
> >
> > This patch changes the lldbtest format and makes it behave more like
> > lit. We now always print the dotest invocation, the exit code, the
> > output to stdout & stderr. If you're used to dealing with ShTests in
> > lit, this will feel all very familiar.
> >
> > Differential revision: https://reviews.llvm.org/D73384
> >
> > Added:
> >
> >
> > Modified:
> >lldb/test/API/lldbtest.py
> >
> > Removed:
> >
> >
> >
> > 
> > diff  --git a/lldb/test/API/lldbtest.py b/lldb/test/API/lldbtest.py
> > index 349a67f22fb3..864d5ea1df03 100644
> > --- a/lldb/test/API/lldbtest.py
> > +++ b/lldb/test/API/lldbtest.py
> > @@ -86,33 +86,46 @@ def execute(self, test, litConfig):
> > shutil.copy(python, copied_python)
> > cmd[0] = copied_python
> >
> > +timeoutInfo = None
> > try:
> > out, err, exitCode = lit.util.executeCommand(
> > cmd,
> > env=test.config.environment,
> > timeout=litConfig.maxIndividualTestTime)
> > except lit.util.ExecuteCommandTimeoutException:
> > -return (lit.Test.TIMEOUT, 'Reached timeout of {} 
> > seconds'.format(
> > -litConfig.maxIndividualTestTime))
> > +timeoutInfo = 'Reached timeout of {} seconds'.format(
> > +litConfig.maxIndividualTestTime)
> > +
> > +output = """Script:\n--\n%s\n--\nExit Code: %d\n""" % (
> > +' '.join(cmd), exitCode)
>
> Heads up, when a test times out this prints:
>
>   File "/Users/vsk/src/llvm-project-master/lldb/test/API/lldbtest.py", line 
> 100, in execute
> ' '.join(cmd), exitCode)
> UnboundLocalError: local variable 'exitCode' referenced before assignment
>
>
> > +if timeoutInfo is not None:
> > +output += """Timeout: %s\n""" % (timeoutInfo,)
> > +output += "\n"
> > +
> > +if out:
> > +output += """Command Output (stdout):\n--\n%s\n--\n""" % (out,)
> > +if err:
> > +output += """Command Output (stderr):\n--\n%s\n--\n""" % (err,)
> > +
> > +if timeoutInfo:
> > +return lit.Test.TIMEOUT, output
> >
> > if exitCode:
> > # Match FAIL but not XFAIL.
> > for line in out.splitlines() + err.splitlines():
> > if line.startswith('FAIL:'):
> > -return lit.Test.FAIL, out + err
> > +return lit.Test.FAIL, output
> >
> > if 'XPASS:' in out or 'XPASS:' in err:
> > -return lit.Test.XPASS, out + err
> > +return lit.Test.XPASS, output
> >
> > has_unsupported_tests = 'UNSUPPORTED:' in out or 'UNSUPPORTED:' in 
> > err
> > has_passing_tests = 'PASS:' in out or 'PASS:' in err
> > if has_unsupported_tests and not has_passing_tests:
> > -return lit.Test.UNSUPPORTED, out + err
> > +return lit.Test.UNSUPPORTED, output
> >
> > passing_test_line = 'RESULT: PASSED'
> > if passing_test_line not in out and passing_test_line not in err:
> > -msg = ('Unable to find %r in dotest output (exit code 
> > %d):\n\n%s%s'
> > -   % (passing_test_line, exitCode, out, err))
> > -return lit.Test.UNRESOLVED, msg
> > +return lit.Test.UNRESOLVED, output
> >
> > -return lit.Test.PASS, ''
> > +return lit.Test.PASS, output
> >
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-b

[Lldb-commits] [PATCH] D73148: [lldb/Value] Avoid reading more data than the host has available

2020-01-30 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 241602.
vsk retitled this revision from "[lldb/Value] Report size of Value as size of 
underlying data buffer" to "[lldb/Value] Avoid reading more data than the host 
has available".
vsk edited the summary of this revision.
vsk added a comment.

Address review feedback:

- Add a test that doesn't depend on the compiler.
- Convert the old test into a simpler regression test.
- Based on feedback from Jim, don't try to change the definition of a Value's 
size, as the expectation that the type is correct is baked in deeply.


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

https://reviews.llvm.org/D73148

Files:
  lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/TestNoASanExceptionAfterEvalOP_piece.py
  lldb/packages/Python/lldbsuite/test/functionalities/optimized_code/main.cpp
  lldb/source/Core/Value.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-smaller-than-struct.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-smaller-than-struct.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-smaller-than-struct.s
@@ -0,0 +1,110 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "target variable reset" -b | FileCheck %s
+
+# CHECK: (lldb) target variable reset
+# CHECK: (auto_reset) reset = {
+# CHECK:   ptr = 0x{{0+$}}
+# Note: We need to find some way to represent "prev" as unknown/undefined.
+# CHECK:   prev = false
+# CHECK: }
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   2   # Abbreviation Code
+.byte   52  # DW_TAG_variable
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   2   # DW_AT_location
+.byte   24  # DW_FORM_exprloc
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   3   # Abbreviation Code
+.byte   36  # DW_TAG_base_type
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   62  # DW_AT_encoding
+.byte   11  # DW_FORM_data1
+.byte   11  # DW_AT_byte_size
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   4   # Abbreviation Code
+.byte   19  # DW_TAG_structure_type
+.byte   1   # DW_CHILDREN_yes
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   11  # DW_AT_byte_size
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   5   # Abbreviation Code
+.byte   13  # DW_TAG_member
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   56  # DW_AT_data_member_location
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   6   # Abbreviation Code
+.byte   15  # DW_TAG_pointer_type
+.byte   0   # DW_CHILDREN_no
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   0   # EOM(3)
+
+.section.debug_info,"",@progbits
+.Lcu_begin0:
+.long   .Lcu_end-.Lcu_start # Length of Unit
+.Lcu_start:
+.short  4   # DWARF version number
+.long   .debug_

[Lldb-commits] [PATCH] D73148: [lldb/Value] Avoid reading more data than the host has available

2020-01-30 Thread Vedant Kumar via Phabricator via lldb-commits
vsk marked an inline comment as done.
vsk added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/dw_op_piece/main.cpp:34
+  //% self.filecheck("image lookup -va $pc", "main.cpp", 
"-check-prefix=INFO-V1")
+  // INFO-V1: name = "v1", type = "S1", location = DW_OP_reg0 RAX, DW_OP_piece 
0x4, DW_OP_piece 0x4, DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x8, decl
+  sink++;

aprantl wrote:
> It seems unlikely that this lowering survives very long. Could you move this 
> to unit tests instead?
I'm not sure how to write this as a unit test. I expect that the added 
regression test + assembly test covers what we want to cover, though.


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

https://reviews.llvm.org/D73148



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


[Lldb-commits] [lldb] 457a6d4 - [lldb/Reproducers] Fix typo in CMake so we actually replay.

2020-01-30 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-01-30T15:51:29-08:00
New Revision: 457a6d49d565075ae99f0e5a931bbed6512dce2f

URL: 
https://github.com/llvm/llvm-project/commit/457a6d49d565075ae99f0e5a931bbed6512dce2f
DIFF: 
https://github.com/llvm/llvm-project/commit/457a6d49d565075ae99f0e5a931bbed6512dce2f.diff

LOG: [lldb/Reproducers] Fix typo in CMake so we actually replay.

The CMakeLists.txt had a typo which meant that check-lldb-repro was
capturing twice instead of capturing and then replaying. This also
uncovered a missing import in lldb-repro.py. This patch fixes both
issues.

Added: 


Modified: 
lldb/test/Shell/CMakeLists.txt
lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
lldb/utils/lldb-repro/lldb-repro.py

Removed: 




diff  --git a/lldb/test/Shell/CMakeLists.txt b/lldb/test/Shell/CMakeLists.txt
index c9490d276a4c..e02adb389db2 100644
--- a/lldb/test/Shell/CMakeLists.txt
+++ b/lldb/test/Shell/CMakeLists.txt
@@ -27,6 +27,6 @@ add_lit_testsuite(check-lldb-repro-capture
 add_lit_testsuite(check-lldb-repro
   "Running lldb shell test suite with reproducer replay"
   ${CMAKE_CURRENT_BINARY_DIR}
-  PARAMS "lldb-run-with-repro=capture"
+  PARAMS "lldb-run-with-repro=replay"
   DEPENDS lldb-test-deps)
 add_dependencies(check-lldb-repro check-lldb-repro-capture)

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/debug_loc.s 
b/lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
index 0892b5f0d2d5..1410ff2674ff 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
@@ -3,6 +3,7 @@
 # "reasonable".
 
 # REQUIRES: x86
+# UNSUPPORTED: lldb-repro
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOC=0 > %t
 # RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \

diff  --git a/lldb/utils/lldb-repro/lldb-repro.py 
b/lldb/utils/lldb-repro/lldb-repro.py
index f7bcd3baa7a4..fddd65ee093c 100755
--- a/lldb/utils/lldb-repro/lldb-repro.py
+++ b/lldb/utils/lldb-repro/lldb-repro.py
@@ -14,11 +14,12 @@
 recorded session.
 """
 
-import sys
+import hashlib
 import os
-import tempfile
+import shutil
 import subprocess
-import hashlib
+import sys
+import tempfile
 
 
 def help():



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


[Lldb-commits] [lldb] 58c4fa2 - [lldb/Reproducers] Use LLDB_RECORD_DUMMY for GetStopDescription

2020-01-30 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-01-30T15:40:58-08:00
New Revision: 58c4fa2c538a73527aeeb4c7535016d9b9a1df18

URL: 
https://github.com/llvm/llvm-project/commit/58c4fa2c538a73527aeeb4c7535016d9b9a1df18
DIFF: 
https://github.com/llvm/llvm-project/commit/58c4fa2c538a73527aeeb4c7535016d9b9a1df18.diff

LOG: [lldb/Reproducers] Use LLDB_RECORD_DUMMY for GetStopDescription

GetStopDescription writes to a const char* with a given length. However,
the reproducer instrumentation serialized the char pointer and length
separately.

To serialize the string, we naively look for the first null byte to
determine its length. This can lead to the method overwriting the input
buffer when the assumed string length is smaller than the actual number
of bytes written by GetStopDescription.

The real solution is to have a custom serializer that takes both
arguments into account. However, given that these are output parameters,
they don't affect replay. If the string is passed as input later, it's
is recorded as such. Therefore I've replaced the instrumentation macro
with LLDB_RECORD_DUMMY which skips the serialization.

Added: 


Modified: 
lldb/source/API/SBThread.cpp

Removed: 




diff  --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index f90e93130960..6fe4f66763cb 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -313,7 +313,7 @@ 
SBThread::GetStopReasonExtendedBacktraces(InstrumentationRuntimeType type) {
 }
 
 size_t SBThread::GetStopDescription(char *dst, size_t dst_len) {
-  LLDB_RECORD_METHOD(size_t, SBThread, GetStopDescription, (char *, size_t),
+  LLDB_RECORD_DUMMY(size_t, SBThread, GetStopDescription, (char *, size_t),
  dst, dst_len);
 
   std::unique_lock lock;



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


[Lldb-commits] [lldb] 45e3f66 - Auto-completion bug fix for dot operator

2020-01-30 Thread Walter Erquinigo via lldb-commits

Author: Hector Diaz
Date: 2020-01-30T16:02:58-08:00
New Revision: 45e3f6660cf4503a8f63ce0a22e574f6d0997914

URL: 
https://github.com/llvm/llvm-project/commit/45e3f6660cf4503a8f63ce0a22e574f6d0997914
DIFF: 
https://github.com/llvm/llvm-project/commit/45e3f6660cf4503a8f63ce0a22e574f6d0997914.diff

LOG: Auto-completion bug fix for dot operator

Summary:
There was a bug on LLDB VSCode where there was the following behavior:

//Code

```
struct foo {
int bar:
};
...
foo my_foo = {10};
```

Trying to auto-complete my_foo.b with my_foo.bar resulted instead with 
my_foo.my_foo.bar

This diff fixes this bug and adds some tests to check correct behavior.

It also fixes the same bug using the arrow operator (->) when user manually 
requests completions.
TODO: Fix bug where no recommended completions are automatically shown with 
arrow operator

{F11249959}

{F11249958}

Reviewers: wallace

Reviewed By: wallace

Subscribers: teemperor, labath, lldb-commits

Tags: #lldb

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

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
index d1d92346e436..155d476bb58a 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
@@ -113,3 +113,76 @@ def test_completions(self):
 }
 ],
 )
+
+self.verify_completions(
+self.vscode.get_completions("foo1.v"),
+[
+{
+"text": "var1",
+"label": "foo1.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.my_bar_object.v"),
+[
+{
+"text": "var1",
+"label": "foo1.my_bar_object.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.var1 + foo1.v"),
+[
+{
+"text": "var1",
+"label": "foo1.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.var1 + v"),
+[
+{
+"text": "var1",
+"label": "var1 -- int &"
+}
+]
+)
+
+#should correctly handle spaces between objects and member operators
+self.verify_completions(
+self.vscode.get_completions("foo1 .v"),
+[
+{
+"text": "var1",
+"label": ".var1 -- int"
+}
+],
+[
+{
+"text": "var2",
+"label": ".var2 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1 . v"),
+[
+{
+"text": "var1",
+"label": "var1 -- int"
+}
+], 
+[
+{
+"text": "var2",
+"label": "var2 -- int"
+}
+]
+)

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
index 14a8815a5244..f77dba592f55 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
@@ -1,6 +1,17 @@
 #include 
 #include 
 
+struct bar {
+  int var1;
+};
+
+struct foo {
+  int var1;
+  bar* my_bar_pointer;
+  bar my_bar_object;
+  foo* next_foo;
+};
+
 int fun(std::vector var) {
   return var.size(); // breakpoint 1
 }
@@ -12,5 +23,8 @@ int main(int argc, char const *argv[]) {
   std::string str2 = "b";
   std::vector vec;
   fun(vec);
+  bar bar1 = {2};
+  bar* bar2 = &bar1; 
+  foo foo1 = {3,&bar1, bar1, NULL};
   return 0; // breakpoint 2
 }

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 8fad2a501773..83ea23653ff0 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -951,9 +951,17 @@ void request_completions(const llvm::json::Object 
&request) {

[Lldb-commits] [PATCH] D73506: Auto-completion bug fix for dot operator

2020-01-30 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG45e3f6660cf4: Auto-completion bug fix for dot operator 
(authored by Hector Diaz , committed by Walter Erquinigo 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73506

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -951,9 +951,17 @@
   for (size_t i = 0; i < count; i++) {
 std::string match = matches.GetStringAtIndex(i);
 std::string description = descriptions.GetStringAtIndex(i);
-
+
 llvm::json::Object item;
-EmplaceSafeString(item, "text", match);
+
+llvm::StringRef match_ref = match;
+for(llvm::StringRef commit_point: {".", "->"}) {
+  if (match_ref.contains(commit_point)){
+match_ref = match_ref.rsplit(commit_point).second;
+  }
+}
+EmplaceSafeString(item, "text", match_ref);
+
 if (description.empty())
   EmplaceSafeString(item, "label", match);
 else
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
@@ -1,6 +1,17 @@
 #include 
 #include 
 
+struct bar {
+  int var1;
+};
+
+struct foo {
+  int var1;
+  bar* my_bar_pointer;
+  bar my_bar_object;
+  foo* next_foo;
+};
+
 int fun(std::vector var) {
   return var.size(); // breakpoint 1
 }
@@ -12,5 +23,8 @@
   std::string str2 = "b";
   std::vector vec;
   fun(vec);
+  bar bar1 = {2};
+  bar* bar2 = &bar1; 
+  foo foo1 = {3,&bar1, bar1, NULL};
   return 0; // breakpoint 2
 }
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
@@ -113,3 +113,76 @@
 }
 ],
 )
+
+self.verify_completions(
+self.vscode.get_completions("foo1.v"),
+[
+{
+"text": "var1",
+"label": "foo1.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.my_bar_object.v"),
+[
+{
+"text": "var1",
+"label": "foo1.my_bar_object.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.var1 + foo1.v"),
+[
+{
+"text": "var1",
+"label": "foo1.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.var1 + v"),
+[
+{
+"text": "var1",
+"label": "var1 -- int &"
+}
+]
+)
+
+#should correctly handle spaces between objects and member operators
+self.verify_completions(
+self.vscode.get_completions("foo1 .v"),
+[
+{
+"text": "var1",
+"label": ".var1 -- int"
+}
+],
+[
+{
+"text": "var2",
+"label": ".var2 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1 . v"),
+[
+{
+"text": "var1",
+"label": "var1 -- int"
+}
+], 
+[
+{
+"text": "var2",
+"label": "var2 -- int"
+}
+]
+)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D73517: [lldb] Delete ValueObject::GetBaseClassPath

2020-01-30 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:2041
 
 void ValueObject::GetExpressionPath(Stream &s, bool qualify_cxx_base_classes,
 GetExpressionPathFormat epformat) {

labath wrote:
> Should we remove the `qualify_cxx_base_classes` argument as well, given that 
> it no longer does anything ?
It seems that this is a part of the SBValue interface. I think we can remove it 
from ValueObject but SBValue will have to stick with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73517



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


[Lldb-commits] [lldb] e1451a7 - [lldb][NFCI] Rename variable in ValueObject

2020-01-30 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2020-01-30T16:53:01-08:00
New Revision: e1451a724de743f1634c864205b77196edbd527e

URL: 
https://github.com/llvm/llvm-project/commit/e1451a724de743f1634c864205b77196edbd527e
DIFF: 
https://github.com/llvm/llvm-project/commit/e1451a724de743f1634c864205b77196edbd527e.diff

LOG: [lldb][NFCI] Rename variable in ValueObject

Added: 


Modified: 
lldb/source/Core/ValueObject.cpp

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 283ab309e314..99def5512088 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -3278,12 +3278,12 @@ ValueObjectSP ValueObject::Persist() {
   ValueObjectSP const_result_sp =
   ValueObjectConstResult::Create(target_sp.get(), GetValue(), name);
 
-  ExpressionVariableSP clang_var_sp =
+  ExpressionVariableSP persistent_var_sp =
   persistent_state->CreatePersistentVariable(const_result_sp);
-  clang_var_sp->m_live_sp = clang_var_sp->m_frozen_sp;
-  clang_var_sp->m_flags |= ExpressionVariable::EVIsProgramReference;
+  persistent_var_sp->m_live_sp = persistent_var_sp->m_frozen_sp;
+  persistent_var_sp->m_flags |= ExpressionVariable::EVIsProgramReference;
 
-  return clang_var_sp->GetValueObject();
+  return persistent_var_sp->GetValueObject();
 }
 
 bool ValueObject::IsSyntheticChildrenGenerated() {



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


[Lldb-commits] [PATCH] D73148: [lldb/Value] Avoid reading more data than the host has available

2020-01-30 Thread Vedant Kumar via Phabricator via lldb-commits
vsk planned changes to this revision.
vsk added a comment.

Yikes, this version seems to break TestClassTemplateParameterPack.py (and 
probably other tests as well). Maybe we expect to be able to read past the end 
of the buffer inside of a ValueObjectChild, because the buffer for the next 
ValueObjectChild is supposed to be there?


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

https://reviews.llvm.org/D73148



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


[Lldb-commits] [lldb] 31905c2 - [lldb][NFCI] Delete commented out code

2020-01-30 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2020-01-30T18:02:36-08:00
New Revision: 31905c2bbb80f79366d92fc6426818fb20896659

URL: 
https://github.com/llvm/llvm-project/commit/31905c2bbb80f79366d92fc6426818fb20896659
DIFF: 
https://github.com/llvm/llvm-project/commit/31905c2bbb80f79366d92fc6426818fb20896659.diff

LOG: [lldb][NFCI] Delete commented out code

Added: 


Modified: 
lldb/source/Core/ModuleList.cpp

Removed: 




diff  --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index cde6d13e3d23..51d4d5a463a6 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -590,10 +590,6 @@ size_t ModuleList::GetSize() const {
 }
 
 void ModuleList::Dump(Stream *s) const {
-  //  s.Printf("%.*p: ", (int)sizeof(void*) * 2, this);
-  //  s.Indent();
-  //  s << "ModuleList\n";
-
   std::lock_guard guard(m_modules_mutex);
   collection::const_iterator pos, end = m_modules.end();
   for (pos = m_modules.begin(); pos != end; ++pos) {



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


[Lldb-commits] [PATCH] D73661: [lldb] Move clang-based files out of Symbol

2020-01-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Awesome, thanks for doing this Alex!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73661



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


[Lldb-commits] [PATCH] D73594: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

2020-01-30 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Looks reasonable to me. Any objections from anyone else? We do have tests for 
this right?


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

https://reviews.llvm.org/D73594



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


[Lldb-commits] [PATCH] D73279: testsuite: generalize `DWARFASTParserClangTests` based on `DWARFExpressionTest`'s YAML

2020-01-30 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 241630.
jankratochvil added a comment.

It needed to add the line:

  include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googlemock/include)

But then maybe it also needs:

  # Our current version of gtest does not properly recognize C++11 support
  # with MSVC, so it falls back to tr1 / experimental classes.  Since LLVM
  # itself requires C++11, we can safely force it on unconditionally so that
  # we don't have to fight with the buggy gtest check.
  add_definitions(-DGTEST_LANG_CXX11=1)
  add_definitions(-DGTEST_HAS_TR1_TUPLE=0)

as being used in such cases in cmakefiles in llvm/ but on Linux I do not see if 
it is really needed here or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73279

Files:
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/TestingSupport/CMakeLists.txt
  lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp
  lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h
  lldb/unittests/TestingSupport/module.modulemap

Index: lldb/unittests/TestingSupport/module.modulemap
===
--- lldb/unittests/TestingSupport/module.modulemap
+++ lldb/unittests/TestingSupport/module.modulemap
@@ -13,4 +13,5 @@
 module lldb_TestingSupport_Symbol {
   requires cplusplus
   module ClangTestUtils { header "Symbol/ClangTestUtils.h" export * }
+  module YAMLModuleTester { header "Symbol/YAMLModuleTester.h" export * }
 }
Index: lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h
===
--- /dev/null
+++ lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.h
@@ -0,0 +1,43 @@
+//===- YAMLModuleTester.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UNITTESTS_TESTINGSUPPORT_SYMBOL_YAMLMODULETESTER_H
+#define LLDB_UNITTESTS_TESTINGSUPPORT_SYMBOL_YAMLMODULETESTER_H
+
+#include "Plugins/SymbolFile/DWARF/DWARFUnit.h"
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/TypeSystemClang.h"
+
+namespace lldb_private {
+
+/// Helper class that can construct a module from YAML and evaluate
+/// DWARF expressions on it.
+class YAMLModuleTester {
+  //  SubsystemRAII subsystems;
+  SubsystemRAII subsystems;
+  llvm::StringMap> m_sections_map;
+  lldb::ModuleSP m_module_sp;
+  lldb::ObjectFileSP m_objfile_sp;
+  DWARFUnitSP m_dwarf_unit;
+  std::unique_ptr m_symfile_dwarf;
+
+public:
+  /// Parse the debug info sections from the YAML description.
+  YAMLModuleTester(llvm::StringRef yaml_data, llvm::StringRef triple);
+  DWARFUnitSP GetDwarfUnit() { return m_dwarf_unit; }
+
+  // Evaluate a raw DWARF expression.
+  llvm::Expected Eval(llvm::ArrayRef expr);
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_UNITTESTS_TESTINGSUPPORT_SYMBOL_YAMLMODULETESTER_H
Index: lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp
===
--- /dev/null
+++ lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp
@@ -0,0 +1,116 @@
+//===- YAMLModuleTester.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestingSupport/Symbol/YAMLModuleTester.h"
+#include "lldb/Core/Section.h"
+#include "llvm/ObjectYAML/DWARFEmitter.h"
+
+using namespace lldb_private;
+
+/// A mock module holding an object file parsed from YAML.
+class YAMLModule : public lldb_private::Module {
+public:
+  YAMLModule(ArchSpec &arch) : Module(FileSpec("test"), arch) {}
+  void SetObjectFile(lldb::ObjectFileSP obj_file) { m_objfile_sp = obj_file; }
+  ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
+};
+
+/// A mock object file that can be parsed from YAML.
+class YAMLObjectFile : public lldb_private::ObjectFile {
+  const lldb::ModuleSP m_module_sp;
+  llvm::StringMap> &m_section_map;
+  /// Because there is only one DataExtractor in the ObjectFile
+  /// interface, all sections are copied into a contiguous buffer.
+  std::vector m_buffer;
+
+public:
+  YAMLObjectFile(const lldb::ModuleSP &module_sp,
+   llvm::StringMap> &map)
+: ObjectFile(module_sp, &module_sp->GetFi

[Lldb-commits] [PATCH] D70646: Move non-DWARF code: `DWARFUnit` -> `SymbolFileDWARF`

2020-01-30 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 241634.
jankratochvil marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70646

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -305,6 +305,26 @@
 
   lldb_private::FileSpec GetFile(DWARFUnit &unit, size_t file_idx);
 
+  static llvm::Expected
+  GetTypeSystem(DWARFUnit &unit);
+
+  static DWARFASTParser *GetDWARFParser(DWARFUnit &unit);
+
+  // CompilerDecl related functions
+
+  static lldb_private::CompilerDecl GetDecl(const DWARFDIE &die);
+
+  static lldb_private::CompilerDeclContext GetDeclContext(const DWARFDIE &die);
+
+  static lldb_private::CompilerDeclContext
+  GetContainingDeclContext(const DWARFDIE &die);
+
+  static DWARFDeclContext GetDWARFDeclContext(const DWARFDIE &die);
+
+  static lldb::LanguageType LanguageTypeFromDWARF(uint64_t val);
+
+  static lldb::LanguageType GetLanguage(DWARFUnit &unit);
+
 protected:
   typedef llvm::DenseMap
   DIEToTypePtr;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -688,7 +688,7 @@
   cu_file_spec.SetFile(remapped_file, FileSpec::Style::native);
   }
 
-  LanguageType cu_language = DWARFUnit::LanguageTypeFromDWARF(
+  LanguageType cu_language = SymbolFileDWARF::LanguageTypeFromDWARF(
   cu_die.GetAttributeValueAsUnsigned(DW_AT_language, 0));
 
   bool is_optimized = dwarf_cu.GetNonSkeletonUnit().GetIsOptimized();
@@ -766,8 +766,7 @@
   if (!die.IsValid())
 return nullptr;
 
-  auto type_system_or_err =
-  GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
+  auto type_system_or_err = GetTypeSystemForLanguage(GetLanguage(*die.GetCU()));
   if (auto err = type_system_or_err.takeError()) {
 LLDB_LOG_ERROR(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_SYMBOLS),
std::move(err), "Unable to parse function");
@@ -799,7 +798,7 @@
   std::lock_guard guard(GetModuleMutex());
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
   if (dwarf_cu)
-return dwarf_cu->GetLanguageType();
+return GetLanguage(*dwarf_cu);
   else
 return eLanguageTypeUnknown;
 }
@@ -1290,7 +1289,7 @@
   // SymbolFileDWARF::GetDIE(). See comments inside the
   // SymbolFileDWARF::GetDIE() for details.
   if (DWARFDIE die = GetDIE(type_uid))
-return die.GetDecl();
+return GetDecl(die);
   return CompilerDecl();
 }
 
@@ -1303,7 +1302,7 @@
   // SymbolFileDWARF::GetDIE(). See comments inside the
   // SymbolFileDWARF::GetDIE() for details.
   if (DWARFDIE die = GetDIE(type_uid))
-return die.GetDeclContext();
+return GetDeclContext(die);
   return CompilerDeclContext();
 }
 
@@ -1314,7 +1313,7 @@
   // SymbolFileDWARF::GetDIE(). See comments inside the
   // SymbolFileDWARF::GetDIE() for details.
   if (DWARFDIE die = GetDIE(type_uid))
-return die.GetContainingDeclContext();
+return GetContainingDeclContext(die);
   return CompilerDeclContext();
 }
 
@@ -1445,7 +1444,7 @@
   dwarf_die.GetID(), dwarf_die.GetTagAsCString(),
   type->GetName().AsCString());
 assert(compiler_type);
-DWARFASTParser *dwarf_ast = dwarf_die.GetDWARFParser();
+DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU());
 if (dwarf_ast)
   return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type);
   }
@@ -2117,7 +2116,7 @@
   sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
   if (parent_decl_ctx) {
-DWARFASTParser *dwarf_ast = die.GetDWARFParser();
+DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU());
 if (dwarf_ast) {
   CompilerDeclContext actual_parent_decl_ctx =
   dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
@@ -2276,7 +2275,7 @@
 return true;
 
   if (die) {
-DWARFASTParser *dwar

[Lldb-commits] [PATCH] D70646: Move non-DWARF code: `DWARFUnit` -> `SymbolFileDWARF`

2020-01-30 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked 2 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4011
+void SymbolFileDWARF::GetDWARFDeclContext(const DWARFDIE &die,
+  DWARFDeclContext &dwarf_decl_ctx) {
+  if (!die.IsValid()) {

labath wrote:
> please return the DWARFDeclContext by value here
I did the same also for `DWARFDebugInfoEntry::GetDWARFDeclContext`. It is a 
part of the updated patch but [[ 
https://people.redhat.com/jkratoch/GetDWARFDeclContext.patch | you can see it 
separately ]].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70646



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


Re: [Lldb-commits] [lldb] e3a7c77 - [lldb/Lit] Change the lldbtest format to behave more like shell test.

2020-01-30 Thread Jonas Devlieghere via lldb-commits
commit 196b31f9f19d743f55fa70744ddfd2f85d6ad117 (HEAD -> master, origin/master)
Author: Jonas Devlieghere 
Date:   Thu Jan 30 21:23:58 2020 -0800

[lldb/Lit] Fix UnboundLocalError when reaching a timeout.

Fixes the UnboundLocalError for the local variables out, err and
exitCode when a timeout is hit.

On Thu, Jan 30, 2020 at 1:53 PM Jonas Devlieghere  wrote:
>
> Yep, Michał also reported this here: https://reviews.llvm.org/D73384
>
> I hope to take a look at that today.
>
> On Thu, Jan 30, 2020 at 1:43 PM Vedant Kumar  wrote:
> >
> >
> >
> > > On Jan 24, 2020, at 4:18 PM, Jonas Devlieghere via lldb-commits 
> > >  wrote:
> > >
> > >
> > > Author: Jonas Devlieghere
> > > Date: 2020-01-24T16:17:55-08:00
> > > New Revision: e3a7c7713cd87e37a95a544373cd21f6f06ab94e
> > >
> > > URL: 
> > > https://github.com/llvm/llvm-project/commit/e3a7c7713cd87e37a95a544373cd21f6f06ab94e
> > > DIFF: 
> > > https://github.com/llvm/llvm-project/commit/e3a7c7713cd87e37a95a544373cd21f6f06ab94e.diff
> > >
> > > LOG: [lldb/Lit] Change the lldbtest format to behave more like shell test.
> > >
> > > The current lldbtest format has a number of shortcomings, all related to
> > > how we omit information based on why the test fails. For example, a
> > > successful test would print nothing, even when `-a` is passed to lit.
> > > It's not up to the test format to decide whether to print something or
> > > not, that's handled by lit itself. For other test results we would
> > > sometimes print stdout & stderr, but not always, such as when a timeout
> > > was reached or we couldn't parse the dotest output.
> > >
> > > This patch changes the lldbtest format and makes it behave more like
> > > lit. We now always print the dotest invocation, the exit code, the
> > > output to stdout & stderr. If you're used to dealing with ShTests in
> > > lit, this will feel all very familiar.
> > >
> > > Differential revision: https://reviews.llvm.org/D73384
> > >
> > > Added:
> > >
> > >
> > > Modified:
> > >lldb/test/API/lldbtest.py
> > >
> > > Removed:
> > >
> > >
> > >
> > > 
> > > diff  --git a/lldb/test/API/lldbtest.py b/lldb/test/API/lldbtest.py
> > > index 349a67f22fb3..864d5ea1df03 100644
> > > --- a/lldb/test/API/lldbtest.py
> > > +++ b/lldb/test/API/lldbtest.py
> > > @@ -86,33 +86,46 @@ def execute(self, test, litConfig):
> > > shutil.copy(python, copied_python)
> > > cmd[0] = copied_python
> > >
> > > +timeoutInfo = None
> > > try:
> > > out, err, exitCode = lit.util.executeCommand(
> > > cmd,
> > > env=test.config.environment,
> > > timeout=litConfig.maxIndividualTestTime)
> > > except lit.util.ExecuteCommandTimeoutException:
> > > -return (lit.Test.TIMEOUT, 'Reached timeout of {} 
> > > seconds'.format(
> > > -litConfig.maxIndividualTestTime))
> > > +timeoutInfo = 'Reached timeout of {} seconds'.format(
> > > +litConfig.maxIndividualTestTime)
> > > +
> > > +output = """Script:\n--\n%s\n--\nExit Code: %d\n""" % (
> > > +' '.join(cmd), exitCode)
> >
> > Heads up, when a test times out this prints:
> >
> >   File "/Users/vsk/src/llvm-project-master/lldb/test/API/lldbtest.py", line 
> > 100, in execute
> > ' '.join(cmd), exitCode)
> > UnboundLocalError: local variable 'exitCode' referenced before assignment
> >
> >
> > > +if timeoutInfo is not None:
> > > +output += """Timeout: %s\n""" % (timeoutInfo,)
> > > +output += "\n"
> > > +
> > > +if out:
> > > +output += """Command Output (stdout):\n--\n%s\n--\n""" % 
> > > (out,)
> > > +if err:
> > > +output += """Command Output (stderr):\n--\n%s\n--\n""" % 
> > > (err,)
> > > +
> > > +if timeoutInfo:
> > > +return lit.Test.TIMEOUT, output
> > >
> > > if exitCode:
> > > # Match FAIL but not XFAIL.
> > > for line in out.splitlines() + err.splitlines():
> > > if line.startswith('FAIL:'):
> > > -return lit.Test.FAIL, out + err
> > > +return lit.Test.FAIL, output
> > >
> > > if 'XPASS:' in out or 'XPASS:' in err:
> > > -return lit.Test.XPASS, out + err
> > > +return lit.Test.XPASS, output
> > >
> > > has_unsupported_tests = 'UNSUPPORTED:' in out or 'UNSUPPORTED:' 
> > > in err
> > > has_passing_tests = 'PASS:' in out or 'PASS:' in err
> > > if has_unsupported_tests and not has_passing_tests:
> > > -return lit.Test.UNSUPPORTED, out + err
> > > +return lit.Test.UNSUPPORTED, output
> > >
> > > passing_test_line = 'RESULT: PASSED'
> > > if passing_test_line not in out and passing_test_line not in err:
> > > -msg = ('Unable to find %r in dot

[Lldb-commits] [lldb] 196b31f - [lldb/Lit] Fix UnboundLocalError when reaching a timeout.

2020-01-30 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-01-30T21:23:58-08:00
New Revision: 196b31f9f19d743f55fa70744ddfd2f85d6ad117

URL: 
https://github.com/llvm/llvm-project/commit/196b31f9f19d743f55fa70744ddfd2f85d6ad117
DIFF: 
https://github.com/llvm/llvm-project/commit/196b31f9f19d743f55fa70744ddfd2f85d6ad117.diff

LOG: [lldb/Lit] Fix UnboundLocalError when reaching a timeout.

Fixes the UnboundLocalError for the local variables out, err and
exitCode when a timeout is hit.

Added: 


Modified: 
lldb/test/API/lldbtest.py

Removed: 




diff  --git a/lldb/test/API/lldbtest.py b/lldb/test/API/lldbtest.py
index 864d5ea1df03..65aafab5c617 100644
--- a/lldb/test/API/lldbtest.py
+++ b/lldb/test/API/lldbtest.py
@@ -92,7 +92,10 @@ def execute(self, test, litConfig):
 cmd,
 env=test.config.environment,
 timeout=litConfig.maxIndividualTestTime)
-except lit.util.ExecuteCommandTimeoutException:
+except lit.util.ExecuteCommandTimeoutException as e:
+out = e.out
+err = e.err
+exitCode = e.exitCode
 timeoutInfo = 'Reached timeout of {} seconds'.format(
 litConfig.maxIndividualTestTime)
 



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


Re: [Lldb-commits] [lldb] e3a7c77 - [lldb/Lit] Change the lldbtest format to behave more like shell test.

2020-01-30 Thread Michał Górny via lldb-commits
On Thu, 2020-01-30 at 21:24 -0800, Jonas Devlieghere wrote:
> commit 196b31f9f19d743f55fa70744ddfd2f85d6ad117 (HEAD -> master, 
> origin/master)
> Author: Jonas Devlieghere 
> Date:   Thu Jan 30 21:23:58 2020 -0800
> 
> [lldb/Lit] Fix UnboundLocalError when reaching a timeout.
> 
> Fixes the UnboundLocalError for the local variables out, err and
> exitCode when a timeout is hit.
> 

That looks nice, thank you!

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 22b0448 - [lldb][NFCI] Remove unused LanguageType parameters

2020-01-30 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2020-01-30T21:57:23-08:00
New Revision: 22b044877d239c40c9a932d1ea47d489c507000f

URL: 
https://github.com/llvm/llvm-project/commit/22b044877d239c40c9a932d1ea47d489c507000f
DIFF: 
https://github.com/llvm/llvm-project/commit/22b044877d239c40c9a932d1ea47d489c507000f.diff

LOG: [lldb][NFCI] Remove unused LanguageType parameters

These parameters are unused in these methods, and some of them only had a
LanguageType parameter to pipe to other methods that don't use it
either.

Added: 


Modified: 
lldb/include/lldb/Core/Mangled.h
lldb/include/lldb/Symbol/Function.h
lldb/source/API/SBBlock.cpp
lldb/source/API/SBFrame.cpp
lldb/source/API/SBFunction.cpp
lldb/source/API/SBSymbol.cpp
lldb/source/API/SBType.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/Mangled.cpp
lldb/source/Expression/ExpressionVariable.cpp
lldb/source/Expression/IRExecutionUnit.cpp
lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
lldb/source/Plugins/Language/ObjC/Cocoa.cpp

lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Symbol/Symbol.cpp
lldb/source/Symbol/SymbolContext.cpp
lldb/source/Symbol/Symtab.cpp
lldb/source/Symbol/Variable.cpp
lldb/source/Target/ThreadPlanStepOverRange.cpp
lldb/unittests/Core/MangledTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Mangled.h 
b/lldb/include/lldb/Core/Mangled.h
index 6af68c3f5857..d8cadc340a83 100644
--- a/lldb/include/lldb/Core/Mangled.h
+++ b/lldb/include/lldb/Core/Mangled.h
@@ -132,13 +132,13 @@ class Mangled {
   ///
   /// \return
   /// A const reference to the demangled name string object.
-  ConstString GetDemangledName(lldb::LanguageType language) const;
+  ConstString GetDemangledName() const;
 
   /// Display demangled name get accessor.
   ///
   /// \return
   /// A const reference to the display demangled name string object.
-  ConstString GetDisplayDemangledName(lldb::LanguageType language) const;
+  ConstString GetDisplayDemangledName() const;
 
   void SetDemangledName(ConstString name) { m_demangled = name; }
 
@@ -165,8 +165,7 @@ class Mangled {
   /// A const reference to the preferred name string object if this
   /// object has a valid name of that kind, else a const reference to the
   /// other name is returned.
-  ConstString GetName(lldb::LanguageType language,
-  NamePreference preference = ePreferDemangled) const;
+  ConstString GetName(NamePreference preference = ePreferDemangled) const;
 
   /// Check if "name" matches either the mangled or demangled name.
   ///
@@ -175,13 +174,12 @@ class Mangled {
   ///
   /// \return
   /// \b True if \a name matches either name, \b false otherwise.
-  bool NameMatches(ConstString name, lldb::LanguageType language) const {
+  bool NameMatches(ConstString name) const {
 if (m_mangled == name)
   return true;
-return GetDemangledName(language) == name;
+return GetDemangledName() == name;
   }
-  bool NameMatches(const RegularExpression ®ex,
-   lldb::LanguageType language) const;
+  bool NameMatches(const RegularExpression ®ex) const;
 
   /// Get the memory cost of this object.
   ///

diff  --git a/lldb/include/lldb/Symbol/Function.h 
b/lldb/include/lldb/Symbol/Function.h
index c8f888c3bdd1..d3c32165dc9d 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -199,11 +199,11 @@ class InlineFunctionInfo : public FunctionInfo {
   /// The stream to which to dump the object description.
   void Dump(Stream *s, bool show_fullpaths) const;
 
-  void DumpStopContext(Stream *s, lldb::LanguageType language) const;
+  void DumpStopContext(Stream *s) const;
 
-  ConstString GetName(lldb::LanguageType language) const;
+  ConstString GetName() const;
 
-  ConstString GetDisplayName(lldb::LanguageType language) const;
+  ConstString GetDisplayName() const;
 
   /// Get accessor for the call site declaration information.
   ///

diff  --git a/lldb/source/API/SBBlock.cpp b/lldb/source/API/SBBlock.cpp
index 857b906549d7..a5fee445d5c6 100644
--- a/lldb/source/API/SBBlock.cpp
+++ b/lldb/source/API/SBBlock.cpp
@@ -71,13 +71,7 @@ const char *SBBlock::GetInlinedName() const {
 const InlineFunctionInfo *inlined_info =
 m_opaque_ptr->GetInlinedFunctionInfo();
 if (inlined_info) {
-  Function *function = m_

[Lldb-commits] [PATCH] D73766: [RFC] Make substrs argument to self.expect ordered by default.

2020-01-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a project: LLDB.

This patch is more of an RFC to change the default behavior of the `substrs` 
argument to `self.expect`. Currently, the elements of `substrs` are unordered 
and as long as the string appears in the output, the assertion passes. I think 
we can be more precise by requiring that the substrings be ordered in the way 
they appear. My hope is that this will make it harder to accidentally pass a 
check because a string appears out of order.

A possible alternative is to keep the option but have it off by default. 
Currently there's about 50 tests failing because of this. If we like to move 
forward with making this the default I plan to incrementally update tests 
before flipping the switch.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D73766

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2261,6 +2261,7 @@
 substrs=None,
 trace=False,
 error=False,
+ordered=True,
 matching=True,
 exe=True,
 inHistory=False):
@@ -2341,8 +2342,12 @@
 # Look for sub strings, if specified.
 keepgoing = matched if matching else not matched
 if substrs and keepgoing:
+start = 0
 for substr in substrs:
-matched = output.find(substr) != -1
+index = output[start:].find(substr)
+print("Looking for %s in %s" % (substr, output[start:]))
+start = start + index if ordered and matching else 0
+matched = index != -1
 with recording(self, trace) as sbuf:
 print("%s sub string: %s" % (heading, substr), file=sbuf)
 print("Matched" if matched else "Not matched", file=sbuf)
Index: 
lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
@@ -46,7 +46,7 @@
 
 self.expect(
 "thread info -s",
-substrs=["instrumentation_class", "api_name", "class_name", 
"selector", "description"])
+substrs=["api_name", "class_name", "description", 
"instrumentation_class", "selector"])
 self.assertEqual(thread.GetStopReason(), 
lldb.eStopReasonInstrumentation)
 output_lines = self.res.GetOutput().split('\n')
 json_line = '\n'.join(output_lines[2:])


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2261,6 +2261,7 @@
 substrs=None,
 trace=False,
 error=False,
+ordered=True,
 matching=True,
 exe=True,
 inHistory=False):
@@ -2341,8 +2342,12 @@
 # Look for sub strings, if specified.
 keepgoing = matched if matching else not matched
 if substrs and keepgoing:
+start = 0
 for substr in substrs:
-matched = output.find(substr) != -1
+index = output[start:].find(substr)
+print("Looking for %s in %s" % (substr, output[start:]))
+start = start + index if ordered and matching else 0
+matched = index != -1
 with recording(self, trace) as sbuf:
 print("%s sub string: %s" % (heading, substr), file=sbuf)
 print("Matched" if matched else "Not matched", file=sbuf)
Index: lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
@@ -46,7 +46,7 @@
 
 self.expect(
 "thread info -s",
-substrs=["instrumentation_class", "api_name", "class_name", "selector", "description"])
+substrs=["api_name", "class_name", "description", "instrumentation_class", "selector"])
 self.assertEqual(thread.GetStopReason(), lldb.eStopReasonInstrumentation)
 output_lines = self.res.GetOutput().split('\n')
 json_line = '\n'.join(output_lines[2:])
___
lldb-commits mailing list
lldb-commits@lists.ll

[Lldb-commits] [PATCH] D73766: [RFC] Make substrs argument to self.expect ordered by default.

2020-01-30 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 241637.
JDevlieghere added a comment.

Remove debug print


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

https://reviews.llvm.org/D73766

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2261,6 +2261,7 @@
 substrs=None,
 trace=False,
 error=False,
+ordered=True,
 matching=True,
 exe=True,
 inHistory=False):
@@ -2341,8 +2342,11 @@
 # Look for sub strings, if specified.
 keepgoing = matched if matching else not matched
 if substrs and keepgoing:
+start = 0
 for substr in substrs:
-matched = output.find(substr) != -1
+index = output[start:].find(substr)
+start = start + index if ordered and matching else 0
+matched = index != -1
 with recording(self, trace) as sbuf:
 print("%s sub string: %s" % (heading, substr), file=sbuf)
 print("Matched" if matched else "Not matched", file=sbuf)
Index: 
lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
===
--- 
lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
+++ 
lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
@@ -46,7 +46,7 @@
 
 self.expect(
 "thread info -s",
-substrs=["instrumentation_class", "api_name", "class_name", 
"selector", "description"])
+substrs=["api_name", "class_name", "description", 
"instrumentation_class", "selector"])
 self.assertEqual(thread.GetStopReason(), 
lldb.eStopReasonInstrumentation)
 output_lines = self.res.GetOutput().split('\n')
 json_line = '\n'.join(output_lines[2:])


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2261,6 +2261,7 @@
 substrs=None,
 trace=False,
 error=False,
+ordered=True,
 matching=True,
 exe=True,
 inHistory=False):
@@ -2341,8 +2342,11 @@
 # Look for sub strings, if specified.
 keepgoing = matched if matching else not matched
 if substrs and keepgoing:
+start = 0
 for substr in substrs:
-matched = output.find(substr) != -1
+index = output[start:].find(substr)
+start = start + index if ordered and matching else 0
+matched = index != -1
 with recording(self, trace) as sbuf:
 print("%s sub string: %s" % (heading, substr), file=sbuf)
 print("Matched" if matched else "Not matched", file=sbuf)
Index: lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/mtc/simple/TestMTCSimple.py
@@ -46,7 +46,7 @@
 
 self.expect(
 "thread info -s",
-substrs=["instrumentation_class", "api_name", "class_name", "selector", "description"])
+substrs=["api_name", "class_name", "description", "instrumentation_class", "selector"])
 self.assertEqual(thread.GetStopReason(), lldb.eStopReasonInstrumentation)
 output_lines = self.res.GetOutput().split('\n')
 json_line = '\n'.join(output_lines[2:])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 381e81a - [lldb][NFCI] Remove UserExpression::GetJITModule

2020-01-30 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2020-01-30T23:20:19-08:00
New Revision: 381e81a048f8cd6aab6a41c87ad1b1658f052192

URL: 
https://github.com/llvm/llvm-project/commit/381e81a048f8cd6aab6a41c87ad1b1658f052192
DIFF: 
https://github.com/llvm/llvm-project/commit/381e81a048f8cd6aab6a41c87ad1b1658f052192.diff

LOG: [lldb][NFCI] Remove UserExpression::GetJITModule

UserExpression::GetJITModule was used to support an option in
UserExpression::Evaluate that let you hold onto the JIT Module used during
the expression evaluation. This was only actually used in one spot --
REPL::IOHandlerInputComplete. That method didn't actually take use the
JIT module it got back, so this feature was not used in practice.
This means that we can delete the support in UserExpression::Evaluate
and delete the UserExpression::GetJITModule method entirely.

Added: 


Modified: 
lldb/include/lldb/Expression/LLVMUserExpression.h
lldb/include/lldb/Expression/UserExpression.h
lldb/source/Expression/LLVMUserExpression.cpp
lldb/source/Expression/REPL.cpp
lldb/source/Expression/UserExpression.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Expression/LLVMUserExpression.h 
b/lldb/include/lldb/Expression/LLVMUserExpression.h
index 2679c01a4e00..70ef12f15ced 100644
--- a/lldb/include/lldb/Expression/LLVMUserExpression.h
+++ b/lldb/include/lldb/Expression/LLVMUserExpression.h
@@ -71,8 +71,6 @@ class LLVMUserExpression : public UserExpression {
   /// translation unit.
   const char *Text() override { return m_transformed_text.c_str(); }
 
-  lldb::ModuleSP GetJITModule() override;
-
 protected:
   lldb::ExpressionResults
   DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,

diff  --git a/lldb/include/lldb/Expression/UserExpression.h 
b/lldb/include/lldb/Expression/UserExpression.h
index 83122d8ba518..4ac20b2c2535 100644
--- a/lldb/include/lldb/Expression/UserExpression.h
+++ b/lldb/include/lldb/Expression/UserExpression.h
@@ -213,8 +213,6 @@ class UserExpression : public Expression {
 return lldb::ExpressionVariableSP();
   }
 
-  virtual lldb::ModuleSP GetJITModule() { return lldb::ModuleSP(); }
-
   /// Evaluate one expression in the scratch context of the target passed in
   /// the exe_ctx and return its result.
   ///
@@ -244,9 +242,6 @@ class UserExpression : public Expression {
   /// If non-nullptr, the fixed expression is copied into the provided
   /// string.
   ///
-  /// \param[out] jit_module_sp_ptr
-  /// If non-nullptr, used to persist the generated IR module.
-  ///
   /// \param[in] ctx_obj
   /// If specified, then the expression will be evaluated in the context of
   /// this object. It means that the context object's address will be
@@ -265,7 +260,6 @@ class UserExpression : public Expression {
llvm::StringRef expr_cstr, llvm::StringRef expr_prefix,
lldb::ValueObjectSP &result_valobj_sp, Status &error,
std::string *fixed_expression = nullptr,
-   lldb::ModuleSP *jit_module_sp_ptr = nullptr,
ValueObject *ctx_obj = nullptr);
 
   static const Status::ValueType kNoResult =

diff  --git a/lldb/source/Expression/LLVMUserExpression.cpp 
b/lldb/source/Expression/LLVMUserExpression.cpp
index 72b2c5d0ee09..d7958a50f9bb 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -357,8 +357,3 @@ bool LLVMUserExpression::PrepareToExecuteJITExpression(
   return true;
 }
 
-lldb::ModuleSP LLVMUserExpression::GetJITModule() {
-  if (m_execution_unit_sp)
-return m_execution_unit_sp->GetJITModule();
-  return lldb::ModuleSP();
-}

diff  --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp
index d49b7644d7f6..8a1eb5d4e5c9 100644
--- a/lldb/source/Expression/REPL.cpp
+++ b/lldb/source/Expression/REPL.cpp
@@ -291,12 +291,10 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, 
std::string &code) {
   const char *expr_prefix = nullptr;
   lldb::ValueObjectSP result_valobj_sp;
   Status error;
-  lldb::ModuleSP jit_module_sp;
   lldb::ExpressionResults execution_results =
   UserExpression::Evaluate(exe_ctx, expr_options, code.c_str(),
expr_prefix, result_valobj_sp, error,
-   nullptr, // Fixed Expression
-   &jit_module_sp);
+   nullptr); // fixed expression
 
   // CommandInterpreter &ci = debugger.GetCommandInterpreter();
 

diff  --git a/lldb/source/Expression/UserExpression.cpp 
b/lldb/source/Expression/UserExpression.cpp
index b5af59729130..0cbcbe8e5107 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -139,12 +139,12 @@ lldb::addr_t 
UserExpression::GetObjectPointer(lldb::StackFrameSP frame_sp,
   ret

[Lldb-commits] [PATCH] D73766: [RFC] Make substrs argument to self.expect ordered by default.

2020-01-30 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

I'm very much in favor of making this more strict by default. My only problem 
is that the current error message is really cryptic:

  AssertionError: False is not True : 'expr --show-types -- *(StgClosure*)$r14' 
returns expected result, got '(StgClosure) $3 = {
(StgClosure *) &$3 = 0x00215eb0
(int) addr = 2186928
(int) load_address = 2186928
  }'

I originally did something like this for `expect_expr` and having something 
like `self.assertIn(subsets, output[:start], "Untruncated output: " + output)` 
in there helped a lot with debugging.


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

https://reviews.llvm.org/D73766



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