[Lldb-commits] [lldb] ba6f25d - [lldb][NFC] Make clang-format happy by removing trailing space in ArchSpec.cpp

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T09:13:48+01:00
New Revision: ba6f25d7d3671f8ff1d072a43a292950dbbf899e

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

LOG: [lldb][NFC] Make clang-format happy by removing trailing space in 
ArchSpec.cpp

Added: 


Modified: 
lldb/source/Utility/ArchSpec.cpp

Removed: 




diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index b638a6138cfe..3dae25ceacd6 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -444,7 +444,7 @@ static const ArchDefinitionEntry g_elf_arch_entries[] = {
 {ArchSpec::eCore_hexagon_generic, llvm::ELF::EM_HEXAGON,
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
- 0xu, 0xu }, // ARC
+ 0xu, 0xu}, // ARC
 };
 
 static const ArchDefinition g_elf_arch_def = {



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I think this is really nice. I have some minor remarks here and there but 
otherwise this LGTM.




Comment at: 
lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp:88
+
+  if ((module_sp = modules.FindFirstModule(module_spec))) {
+UpdateLoadedSections(module_sp, link_map_addr, base_addr,

what about making this if and the one blow a `if (ModuleSP module_sp = ...) { 
...; return module_sp; }`. Then you don't need to do the double-parentheses 
trick and the end of this function can just be `return ModuleSP();` so it is 
obvious that the end of this function is the error code path.



Comment at: 
lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp:171
+  auto arch = m_process->GetTarget().GetArchitecture();
+  if (arch.GetMachine() != llvm::Triple::wasm32) {
+return ThreadPlanSP();

no brackets



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:32
+const char *magic = reinterpret_cast(data_sp->GetBytes());
+if (strncmp(magic, llvm::wasm::WasmMagic, sizeof(llvm::wasm::WasmMagic)) !=
+0) {

aprantl wrote:
> Is a StringRef comparison easier to read here?
(IMHO it is)



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:65
+data_sp = MapFileData(*file, length, file_offset);
+if (!data_sp) {
+  return nullptr;

no brackets



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:79
+data_sp = MapFileData(*file, length, file_offset);
+if (!data_sp) {
+  return nullptr;

No brackets



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:88
+  ArchSpec spec = objfile_up->GetArchitecture();
+  if (spec && objfile_up->SetModulesArchitecture(spec)) {
+return objfile_up.release();

no brackets



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:110
+
+// static
+bool ObjectFileWasm::GetVaruint7(DataExtractor §ion_header_data,

I don't think we do these `static` comments usually in LLVM?



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:128
+  uint64_t value = section_header_data.GetULEB128(offset_ptr);
+  if (*offset_ptr == initial_offset || value > uint64_t(1) << 32) {
+return false;

Single line if -> no brackets needed.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:172
+}
+std::string sect_name(reinterpret_cast(name_bytes),
+  name_len);

Should also be switched to ConstString if you make the member of the section 
info a ConstString.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:191
+}
+m_symbols_url =
+ConstString(reinterpret_cast(url_bytes), 
url_len);

This member is only created here and only used below from what I can see? Also 
you never compare it against any other strings so it should be a `std::string`.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:292
+  for (SectionInfoCollConstIter it = m_sect_infos.begin();
+   it != m_sect_infos.end(); ++it) {
+const section_info §_info = *it;

range-based loop



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:299
+  section_type = eSectionTypeCode;
+else if (g_sect_name_dwarf_debug_abbrev == sect_info.name.c_str())
+  section_type = eSectionTypeDWARFDebugAbbrev;

Your `sect_info.name` is already a std::string so comparing here against a 
ConstString is just a slower and less readable.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:447
+  ModuleSP module_sp(GetModule());
+  if (module_sp) {
+std::lock_guard guard(module_sp->GetMutex());

early-exit



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:449
+std::lock_guard guard(module_sp->GetMutex());
+s->Printf("%p: ", static_cast(this));
+s->Indent();

Bonus: You can write this code by directly using `llvm::raw_ostream` by just 
calling `s->AsRawOstream()` to get the equivalent `raw_ostream`. I migrate all 
code to LLVM's stream classes so not having more code using lldb::Stream would 
be nice (but not required to get this patch in).

(same for the Stream code below)



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h:116
+uint32_t id;
+std::string name;
+  } section_info_t;

This should be a `ConstString`. OR you keep this as a `std::string` and then 
you move all other ConstString variables that compare against your section 
names to be just plain `std::st

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D69309#1787297 , @jarin wrote:

> In D69309#1752738 , @friss wrote:
>
> > Basically, today the debug info will describe an entity named "Foo". 
> > The accelerator tables all reference this name. So when Clang asks us if we 
> > know "Foo" (which is what happens when instantiating), we fail to find the 
> > right instantiations. The consensus of the above discussion was that we 
> > should change the debug info to have "Foo" as the name of any 
> > instantiation, with a child DIE describing the template arguments. Just 
> > doing this in the compiler causes test failures in LLDB, so there's some 
> > work to do in LLDB to support this.
>
>
> Frederic, you say that "doing this in the compiler causes test failures in 
> LLDB", which implies you have tried adding the template in the compiler. Do 
> you have that compiler patch lying around so that we could have a look at 
> what can be done on the lldb side?
>
> I agree that a good long term fix is to have "Foo" as an entity in DWARF, 
> although for backwards compatibility it might be better if the "Foo" template 
> just contained references to the instantiations rather than having them as 
> children.


I am afraid you're overestimating the scope of that idea. I *think* that Fred 
was referring to simply changing the string that gets put into the DW_AT_name 
field of the /instantation/ (and, by extension, the accelerator table). The 
debug info would still describe instantiations only.

I don't believe anyone here was proposing to have DWARF actually describe 
templates themselves -- that might be possible, but you'd have to get pretty 
creative with dwarf attributes (and convince a bunch of people that this is 
actually a good idea).


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

https://reviews.llvm.org/D69309



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

[Lldb-commits] [PATCH] D71487: [LLDB] Fix address computation for inline function

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks. I think we're getting closer, and I think we can start talking about a 
more direct way to test this behavior.

This is now definitely, as Adrian called it, a DWARF peculiarity, and so an 
assembly file with hard-coded dwarf would be much better. Can you take a look 
at the files in `test/Shell/SymbolFile/DWARF` and create something similar. It 
don't think you'll need very complicated dwarf -- it should be sufficient to 
have two compile units with one function each, and set one of the function's 
DW_AT_low_pc to zero. The test command could be something like running 
`lldb-test symbols %t -find=function -name=` and checking that it returns 
just a single entry...




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2358-2360
+if (die.GetAttributeValueAsAddress(DW_AT_low_pc, /*fail_value=*/1) == 0 &&
+!GetObjectFile()->GetModule()->HaveSectionAtAddressZero())
+  continue;

I think this check should go into the `ResolveFunction` call, as that function 
already does some validation of the function address. 

And I still believe that we shouldn't special case address zero, but rather 
just reject any address that does not map onto a known section (it should be 
sufficient to replace the `addr.IsValid()` check in `ResolveFunction` with 
`addr.IsSectionOffset()`). Functions not mapping to any sections are going to 
be mostly useless anyway, because the first thing that lldb does when resolving 
an address, is to resolve the module it should search in. It uses the section 
list to do that, and so if the function does not belong to any section, lldb 
will never find it, as it will not even get around to asking the Module about 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71487



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

[Lldb-commits] [lldb] d5b54bb - [lldb] Add support for calling objc_direct methods from LLDB's expression evaluator.

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T10:28:40+01:00
New Revision: d5b54bbfaf19a8527ebf70fbf23cb8d2937f15ef

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

LOG: [lldb] Add support for calling objc_direct methods from LLDB's expression 
evaluator.

Summary:
D69991 introduced `__attribute__((objc_direct))` that allows directly calling 
methods without message passing.
This patch adds support for calling methods with this attribute to LLDB's 
expression evaluator.

The patch can be summarised in that LLDB just adds the same attribute to our 
module AST when we find a
method with `__attribute__((objc_direct))` in our debug information.

Reviewers: aprantl, shafik

Reviewed By: shafik

Subscribers: JDevlieghere, lldb-commits

Tags: #lldb

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

Added: 
lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/Makefile

lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/main.m

Modified: 
lldb/include/lldb/Symbol/ClangASTContext.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/source/Symbol/ClangASTContext.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTContext.h 
b/lldb/include/lldb/Symbol/ClangASTContext.h
index 9537f33b3386..5d228e7bdad7 100644
--- a/lldb/include/lldb/Symbol/ClangASTContext.h
+++ b/lldb/include/lldb/Symbol/ClangASTContext.h
@@ -845,7 +845,7 @@ class ClangASTContext : public TypeSystem {
 // (lldb::opaque_compiler_type_t type, "-[NString
 // stringWithCString:]")
   const CompilerType &method_compiler_type, lldb::AccessType access,
-  bool is_artificial, bool is_variadic);
+  bool is_artificial, bool is_variadic, bool is_objc_direct_call);
 
   static bool SetHasExternalStorage(lldb::opaque_compiler_type_t type,
 bool has_extern);

diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/Makefile 
b/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/Makefile
new file mode 100644
index ..afecbf969483
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS := -lobjc -framework Foundation
+
+include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
 
b/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
new file mode 100644
index ..f0152de1ac33
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -0,0 +1,5 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(
+__file__, globals(), [decorators.skipUnlessDarwin])

diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/main.m 
b/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/main.m
new file mode 100644
index ..1a199acdda45
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/main.m
@@ -0,0 +1,79 @@
+#import 
+
+int side_effect = 0;
+
+NSString *str = @"some string";
+
+const char *directCallConflictingName() {
+  return "wrong function";
+}
+
+@interface Foo : NSObject {
+  int instance_var;
+}
+-(int) entryPoint;
+@end
+
+@implementation Foo
+-(int) entryPoint
+{
+  // Try calling directly with self. Same as in the main method otherwise.
+  return 0; //%self.expect("expr [self directCallNoArgs]", substrs=["called 
directCallNoArgs"])
+//%self.expect("expr [self directCallArgs: ]", substrs=["= 
2345"])
+//%self.expect("expr side_effect = 0; [self directCallVoidReturn]; 
side_effect", substrs=["= 4321"])
+//%self.expect("expr [self directCallNSStringArg: str]", 
substrs=['@"some string"'])
+//%self.expect("expr [self directCallIdArg: (id)str]", 
substrs=['@"some string appendix"'])
+//%self.expect("expr [self directCallConflictingName]", 
substrs=["correct function"])
+}
+
+// Declare several objc_direct functions we can test.
+-(const char *) directCallNoArgs __attribute__((objc_direct))
+{
+  return "called directCallNoArgs";
+}
+
+-(void) directCallVoidReturn __attribute__((objc_direct))
+{
+  side_effect = 4321;
+}
+
+-(int) directCallArgs:(int)i __attribute__((objc_direct))
+{
+  // Use the arg in some way to make sure that gets passed correctly.
+  return i + 1234;
+}
+
+-(NSString *) directCallNSStringArg:(NSSt

[Lldb-commits] [PATCH] D71196: [lldb] Add support for calling objc_direct methods from LLDB's expression evaluator.

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd5b54bbfaf19: [lldb] Add support for calling objc_direct 
methods from LLDB's expression… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71196

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/Makefile
  
lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
  lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/main.m
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Symbol/ClangASTContext.cpp

Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -7769,7 +7769,7 @@
   // (lldb::opaque_compiler_type_t type, "-[NString
   // stringWithCString:]")
 const CompilerType &method_clang_type, lldb::AccessType access,
-bool is_artificial, bool is_variadic) {
+bool is_artificial, bool is_variadic, bool is_objc_direct_call) {
   if (!type || !method_clang_type.IsValid())
 return nullptr;
 
@@ -7875,6 +7875,18 @@
 llvm::ArrayRef());
   }
 
+  if (is_objc_direct_call) {
+// Add a the objc_direct attribute to the declaration we generate that
+// we generate a direct method call for this ObjCMethodDecl.
+objc_method_decl->addAttr(
+clang::ObjCDirectAttr::CreateImplicit(*ast, SourceLocation()));
+// Usually Sema is creating implicit parameters (e.g., self) when it
+// parses the method. We don't have a parsing Sema when we build our own
+// AST here so we manually need to create these implicit parameters to
+// make the direct call code generation happy.
+objc_method_decl->createImplicitParams(*ast, class_interface_decl);
+  }
+
   class_interface_decl->addDecl(objc_method_decl);
 
 #ifdef LLDB_CONFIGURATION_DEBUG
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -244,6 +244,7 @@
   bool is_scoped_enum = false;
   bool is_vector = false;
   bool is_virtual = false;
+  bool is_objc_direct_call = false;
   bool exports_symbols = false;
   clang::StorageClass storage = clang::SC_None;
   const char *mangled_name = nullptr;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -390,6 +390,10 @@
   is_complete_objc_class = form_value.Signed();
   break;
 
+case DW_AT_APPLE_objc_direct:
+  is_objc_direct_call = true;
+  break;
+
 case DW_AT_APPLE_runtime_class:
   class_language = (LanguageType)form_value.Signed();
   break;
@@ -958,7 +962,8 @@
   clang::ObjCMethodDecl *objc_method_decl =
   m_ast.AddMethodToObjCObjectType(
   class_opaque_type, attrs.name.GetCString(), clang_type,
-  attrs.accessibility, attrs.is_artificial, is_variadic);
+  attrs.accessibility, attrs.is_artificial, is_variadic,
+  attrs.is_objc_direct_call);
   type_handled = objc_method_decl != NULL;
   if (type_handled) {
 LinkDeclContextToDIE(objc_method_decl, die);
Index: lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/main.m
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/objc/objc_direct-methods/main.m
@@ -0,0 +1,79 @@
+#import 
+
+int side_effect = 0;
+
+NSString *str = @"some string";
+
+const char *directCallConflictingName() {
+  return "wrong function";
+}
+
+@interface Foo : NSObject {
+  int instance_var;
+}
+-(int) entryPoint;
+@end
+
+@implementation Foo
+-(int) entryPoint
+{
+  // Try calling directly with self. Same as in the main method otherwise.
+  return 0; //%self.expect("expr [self directCallNoArgs]", substrs=["called directCallNoArgs"])
+//%self.expect("expr [self directCallArgs: ]", substrs=["= 2345"])
+//%self.expect("expr side_effect = 0; [self directCallVoidReturn]; side_effect", substrs=["= 4321"])
+//%self.expect("expr [self directCallNSStringArg: str]", substrs=['@"some string"'])
+//%self.expect("expr [self directCallIdArg: (id)str]", substrs=['@"some string appendix"'])
+//%self.expect("expr [self directCallConflictingName]", substrs=["correct function"])

[Lldb-commits] [lldb] 6e1fe49 - [lldb][NFC] Remove implementation of GetOriginalDecl and just call GetDeclOrigin instead

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T10:42:09+01:00
New Revision: 6e1fe4966c402d17a253b38192cfd5e8b919ab8e

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

LOG: [lldb][NFC] Remove implementation of GetOriginalDecl and just call 
GetDeclOrigin instead

Those functions have the same semantics beside some small optimization of not 
creating
a new empty ASTContextMetadataSP value in the metadata map. We never actually 
hit this
optimization according to test coverage so let's just call GetDeclOrigin 
instead.

Added: 


Modified: 
lldb/source/Symbol/ClangASTImporter.cpp

Removed: 




diff  --git a/lldb/source/Symbol/ClangASTImporter.cpp 
b/lldb/source/Symbol/ClangASTImporter.cpp
index 1cfec2531d8b..3c11c3250af9 100644
--- a/lldb/source/Symbol/ClangASTImporter.cpp
+++ b/lldb/source/Symbol/ClangASTImporter.cpp
@@ -1139,16 +1139,5 @@ void 
ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from,
 
 clang::Decl *
 ClangASTImporter::ASTImporterDelegate::GetOriginalDecl(clang::Decl *To) {
-  ASTContextMetadataSP to_context_md =
-  m_master.GetContextMetadata(&To->getASTContext());
-
-  if (!to_context_md)
-return nullptr;
-
-  OriginMap::iterator iter = to_context_md->m_origins.find(To);
-
-  if (iter == to_context_md->m_origins.end())
-return nullptr;
-
-  return const_cast(iter->second.decl);
+  return m_master.GetDeclOrigin(To).decl;
 }



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this is pretty good for a first pass, but I would like to see this 
split up into (at least) three patches (one for each plugin). That way, we can 
properly focus on each plugin. For instance, I'm pretty sure that the object 
file and symbol vendor changes are fully testable. The dynamic loader stuff may 
or may not be, but I don't want to discuss that yet to avoid too many parallel 
threads going on.




Comment at: lldb/source/API/SystemInitializerFull.cpp:179
   ObjectFilePECOFF::Initialize();
+  wasm::ObjectFileWasm::Initialize();
 

sbc100 wrote:
> Why is the namespace needed here for wasm but not the other three above.. 
> seems inconsistent.
Some of the older code puts "plugins" into the default namespace, but lately 
we've started to put new plugins into their own namespaces. However, most of 
the old plugins have not been migrated yet.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:299
+  section_type = eSectionTypeCode;
+else if (g_sect_name_dwarf_debug_abbrev == sect_info.name.c_str())
+  section_type = eSectionTypeDWARFDebugAbbrev;

teemperor wrote:
> Your `sect_info.name` is already a std::string so comparing here against a 
> ConstString is just a slower and less readable.
ELF and PECOFF code has been already converted to use StringSwitch for this 
stuff. I'd do the same here.



Comment at: llvm/include/llvm/BinaryFormat/ELF.h:314
   EM_BPF = 247,   // Linux kernel bpf virtual machine
+  EM_WASM = 248,  // WebAssembly
 };

sbc100 wrote:
> This seems like an odd place to add this, given that are not using or relying 
> on ELF anywhere.  Does this make sense?
Indeed, that looks very unexpected, and begs an explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

In D69309#1787409 , @labath wrote:

> In D69309#1787297 , @jarin wrote:
>
> > In D69309#1752738 , @friss wrote:
> >
> > > Basically, today the debug info will describe an entity named "Foo". 
> > > The accelerator tables all reference this name. So when Clang asks us if 
> > > we know "Foo" (which is what happens when instantiating), we fail to find 
> > > the right instantiations. The consensus of the above discussion was that 
> > > we should change the debug info to have "Foo" as the name of any 
> > > instantiation, with a child DIE describing the template arguments. Just 
> > > doing this in the compiler causes test failures in LLDB, so there's some 
> > > work to do in LLDB to support this.
> >
> >
> > Frederic, you say that "doing this in the compiler causes test failures in 
> > LLDB", which implies you have tried adding the template in the compiler. Do 
> > you have that compiler patch lying around so that we could have a look at 
> > what can be done on the lldb side?
> >
> > I agree that a good long term fix is to have "Foo" as an entity in DWARF, 
> > although for backwards compatibility it might be better if the "Foo" 
> > template just contained references to the instantiations rather than having 
> > them as children.
>
>
> I am afraid you're overestimating the scope of that idea. I *think* that Fred 
> was referring to simply changing the string that gets put into the DW_AT_name 
> field of the /instantation/ (and, by extension, the accelerator table). The 
> debug info would still describe instantiations only.
>
> I don't believe anyone here was proposing to have DWARF actually describe 
> templates themselves -- that might be possible, but you'd have to get pretty 
> creative with dwarf attributes (and convince a bunch of people that this is 
> actually a good idea).


You are right, I was indeed imagining something else. What Fred proposes makes 
a lot of sense to me, although the lookup of instantiations might be slow 
because there would be no accelerator for that (but we could build one).


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

https://reviews.llvm.org/D69309



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

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D69309#1787468 , @jarin wrote:

> What Fred proposes makes a lot of sense to me, although the lookup of 
> instantiations might be slow because there would be no accelerator for that 
> (but we could build one).


Do we ever lookup instantiations through this API? Seems pretty fragile, due to 
all the template brackets and all kinds of whitespace one could insert between 
them...


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

https://reviews.llvm.org/D69309



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

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

In D69309#1787481 , @labath wrote:

> In D69309#1787468 , @jarin wrote:
>
> > What Fred proposes makes a lot of sense to me, although the lookup of 
> > instantiations might be slow because there would be no accelerator for that 
> > (but we could build one).
>
>
> Do we ever lookup instantiations through this API? Seems pretty fragile, due 
> to all the template brackets and all kinds of whitespace one could insert 
> between them...


I did it in this patch and it worked, but I agree it is brittle. Other than 
that, I think it is not used.


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

https://reviews.llvm.org/D69309



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

[Lldb-commits] [lldb] dcd1432 - [lldb-vscode] Centrally skip debug info variants for vscode tests

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-17T11:08:52+01:00
New Revision: dcd14324dced7d91aa8bf78a607055ca093b27bf

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

LOG: [lldb-vscode] Centrally skip debug info variants for vscode tests

Previously each test was annotated manually. This does the same thing.

Added: 


Modified: 

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

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

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

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

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

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

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/stackTrace/TestVSCode_stackTrace.py

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/step/TestVSCode_step.py

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/variables/TestVSCode_variables.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
index cb2ac355df53..835bd0b86ef2 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
@@ -46,7 +46,6 @@ def set_and_hit_breakpoint(self, continueToExit=True):
 
 @skipIfWindows
 @skipIfNetBSD # Hangs on NetBSD as well
-@no_debug_info_test
 def test_by_pid(self):
 '''
 Tests attaching to a process by process ID.
@@ -62,7 +61,6 @@ def test_by_pid(self):
 
 @skipIfWindows
 @skipIfNetBSD # Hangs on NetBSD as well
-@no_debug_info_test
 def test_by_name(self):
 '''
 Tests attaching to a process by process name.
@@ -101,7 +99,6 @@ def cleanup():
 @skipUnlessDarwin
 @skipIfDarwin
 @skipIfNetBSD # Hangs on NetBSD as well
-@no_debug_info_test
 def test_by_name_waitFor(self):
 '''
 Tests attaching to a process by process name and waiting for the
@@ -119,7 +116,6 @@ def test_by_name_waitFor(self):
 @skipIfWindows
 @skipIfDarwin
 @skipIfNetBSD # Hangs on NetBSD as well
-@no_debug_info_test
 def test_commands(self):
 '''
 Tests the "initCommands", "preRunCommands", "stopCommands",

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
index f5438b2e817c..8b13b9b161f2 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
@@ -17,7 +17,6 @@ class 
TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @skipIfWindows
-@no_debug_info_test
 def test_set_and_clear(self):
 '''Tests setting and clearing source file and line breakpoints.
This packet is a bit tricky on the debug adaptor side since there
@@ -149,7 +148,6 @@ def test_set_and_clear(self):
 "expect breakpoint still verified")
 
 @skipIfWindows
-@no_debug_info_test
 def test_functionality(self):
 '''Tests hitting breakpoints and the functionality of a single
breakpoint, like 'conditions' and 'hitCondition' settings.'''

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setExceptionBreakpoints.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setExceptionBreakpoints.py
index 3e28b5782295..d20a3434188e 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setExceptionBreakpoints.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/TestVSCode_setExceptionBreakpoints.py
@@ -18,7 +18,6 @@ class TestVSCode_setExceptionBreakpoints(
 
 @skipIfWindows
 @expectedFailureNetBSD
-@no_debug_info_test
 def test_functionality(self):
 '''Tests setting and clearing exception breakpoints.
This packet is a bit tricky on the debug adaptor side since there

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/breakpoint/T

[Lldb-commits] [lldb] d9ca412 - [lldb][NFC] Remove all unnecessary includes for ClangASTSourceCommon.h

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T11:21:11+01:00
New Revision: d9ca412a8a75ab64af33a9e95d1319c4dd2004d2

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

LOG: [lldb][NFC] Remove all unnecessary includes for ClangASTSourceCommon.h

These files only need the definition of ClangASTMetadata (which was
previously in the ClangASTSourceCommon.h) or don't need the include at all.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 0496e9e87b9f..6ef56ced21ce 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -38,7 +38,6 @@
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/Block.h"
 #include "lldb/Symbol/ClangASTContext.h"
-#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/ObjectFile.h"

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index bd97c68bff79..d7b216c99feb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/Value.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Symbol/ClangASTImporter.h"
-#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
+#include "lldb/Symbol/ClangASTMetadata.h"
 #include "lldb/Symbol/ClangUtil.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 986b0b785d87..1f7366f5e184 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -17,7 +17,7 @@
 #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ClangASTContext.h"
-#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
+#include "lldb/Symbol/ClangASTMetadata.h"
 #include "lldb/Symbol/ClangUtil.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 22d1b08ea9e7..370c339fb74b 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -20,7 +20,6 @@
 #include "lldb/Core/StreamBuffer.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Symbol/ClangASTContext.h"
-#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Symbol/ClangUtil.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/LineTable.h"

diff  --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index ccb5b6373ee4..245d99c8c05b 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -16,7 +16,7 @@
 
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ClangASTContext.h"
-#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
+#include "lldb/Symbol/ClangASTMetadata.h"
 #include "lldb/Symbol/ClangUtil.h"
 #include "lldb/Symbol/Declaration.h"
 #include "lldb/Symbol/SymbolFile.h"



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

[Lldb-commits] [PATCH] D71440: Extending step-over range past calls was causing deadlocks, fix that.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the explanation (and for updating the test). This actually explains 
why it sometimes seemed that I lose control over other threads while stepping 
one of them. I'll be sure to use the --run-mode argument next time. This seems 
fine given how the current stepping logic works, though I personally would 
expect that the other also threads stay stopped when i do something called 
"thread step-over", but I get the feeling I expect a different kind of 
threading behavior from my debugger than most people...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71440



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

[Lldb-commits] [lldb] ff0102b - [lldb] Remove modern-type-lookup

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T12:24:31+01:00
New Revision: ff0102b32cfe506dfc16a86e38e70b0940697aa2

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

LOG: [lldb] Remove modern-type-lookup

Summary:
As discussed on the mailing list [1] we have to make a decision for how to 
proceed with the modern-type-lookup.

This patch removes modern-type-lookup from LLDB. This just removes all the code 
behind the modern-type-lookup
setting but it does *not* remove any code from Clang (i.e., the 
ExternalASTMerger and the clang-import-test stay around
for now).

The motivation for this is that I don't think that the current approach of 
implementing modern-type-lookup
will work out. Especially creating a completely new lookup system behind some 
setting that is never turned on by anyone
and then one day make one big switch to the new system seems wrong. It doesn't 
fit into the way LLVM is developed and has
so far made the transition work much more complicated than it has to be.

A lot of the benefits that were supposed to come with the modern-type-lookup 
are related to having a better organization
in the way types move across LLDB and having less dependencies on unrelated 
LLDB code. By just looking at the current code (mostly
the ClangASTImporter) I think we can reach the same goals by just incrementally 
cleaning up, documenting, refactoring
and actually testing the existing code we have.

[1] http://lists.llvm.org/pipermail/lldb-dev/2019-December/015831.html

Reviewers: shafik, martong

Subscribers: rnkovacs, christof, arphaman, JDevlieghere, usaxena95, 
lldb-commits, friss

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTContext.h
lldb/include/lldb/Target/Target.h
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
lldb/source/Symbol/ClangASTContext.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td

Removed: 

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/Makefile

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/TestBasicObjcModernTypeLookup.py

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/main.m

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/Makefile

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/TestBasicModernTypeLookup.py

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/main.cpp

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/Makefile

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/TestObjModulesModernTypeLookup.py

lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/main.m



diff  --git a/lldb/include/lldb/Symbol/ClangASTContext.h 
b/lldb/include/lldb/Symbol/ClangASTContext.h
index 5d228e7bdad7..9c37b94219f0 100644
--- a/lldb/include/lldb/Symbol/ClangASTContext.h
+++ b/lldb/include/lldb/Symbol/ClangASTContext.h
@@ -21,7 +21,6 @@
 #include 
 
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/ExternalASTMerger.h"
 #include "clang/AST/TemplateBase.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
@@ -965,10 +964,7 @@ class ClangASTContext : public TypeSystem {
 
   clang::DeclarationName
   GetDeclarationName(const char *name, const CompilerType 
&function_clang_type);
-  
-  virtual const clang::ExternalASTMerger::OriginMap &GetOriginMap() {
-return m_origins;
-  }
+
 protected:
   const clang::ClassTemplateSpecializationDecl *
   GetAsTemplateSpecialization(lldb::opaque_compiler_type_t type);
@@ -993,7 +989,6 @@ class ClangASTContext : public TypeSystem {
   CompleteTagDeclCallback m_callback_tag_decl = nullptr;
   CompleteObjCInterfaceDeclCallback m_callback_objc_decl = nullptr;
   void *m_callback_baton = nullptr;
-  clang::ExternalASTMerger::OriginMap m

[Lldb-commits] [PATCH] D71562: [lldb] Remove modern-type-lookup

[Raphael Isemann via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGff0102b32cfe: [lldb] Remove modern-type-lookup (authored by 
teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71562

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/include/lldb/Target/Target.h
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/TestBasicObjcModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic-objc/main.m
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/TestBasicModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/basic/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/TestObjModulesModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/objc-modules/main.m
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -4,9 +4,6 @@
   def InjectLocalVars : Property<"inject-local-vars", "Boolean">,
 Global, DefaultTrue,
 Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
-  def UseModernTypeLookup : Property<"use-modern-type-lookup", "Boolean">,
-Global, DefaultFalse,
-Desc<"If true, use Clang's modern type lookup infrastructure.">;
 }
 
 let Definition = "target" in {
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3545,18 +3545,6 @@
 true);
 }
 
-bool TargetProperties::GetUseModernTypeLookup() const {
-  const Property *exp_property = m_collection_sp->GetPropertyAtIndex(
-  nullptr, false, ePropertyExperimental);
-  OptionValueProperties *exp_values =
-  exp_property->GetValue()->GetAsProperties();
-  if (exp_values)
-return exp_values->GetPropertyAtIndexAsBoolean(
-nullptr, ePropertyUseModernTypeLookup, true);
-  else
-return true;
-}
-
 ArchSpec TargetProperties::GetDefaultArchitecture() const {
   OptionValueArch *value = m_collection_sp->GetPropertyAtIndexAsOptionValueArch(
   nullptr, ePropertyDefaultArch);
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -9517,9 +9517,3 @@
 ClangASTContextForExpressions::GetPersistentExpressionState() {
   return m_persistent_variables.get();
 }
-
-clang::ExternalASTMerger &
-ClangASTContextForExpressions::GetMergerUnchecked() {
-  lldbassert(m_scratch_ast_source_up != nullptr);
-  return m_scratch_ast_source_up->GetMergerUnchecked();
-}
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
@@ -30,8 +30,6 @@
   uint32_t FindDecls(ConstString name, bool append, uint32_t max_matches,
  std::vector &decls) override;
 
-  clang::ExternalASTMerger::ImporterSource GetImporterSource() override;
-
   friend class AppleObjCExternal

[Lldb-commits] [PATCH] D71562: [lldb] Remove modern-type-lookup

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I'm landing this now as I started the cleanup work for the current lookup code 
and the modern-type-lookup code is blocking that refactoring.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71562



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

[Lldb-commits] [PATCH] D71562: [lldb] Remove modern-type-lookup

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

> I'm curious what you think should happen to the clang-import-test. We could 
> either rewrite the tests as unit tests in the ASTImporterTest you guys are 
> already using or we move the necessary parts of the ExternalASTMerger into 
> the clang-import-test

Raphael, I think there is no point to keep the clang-import-test and the 
ExternalASTMerger in Clang if we remove the only user of these things.
So, in my opinion the best would be on a long-term if we could cover these 
tests with unit tests in ASTImporterTest.
On the other hand, I understand that rewriting these tests could be quite a 
work, so perhaps we should gradually add the new unit tests and once we are 
ready then we could remove entirely the clang-import-test and the 
ExternalASTMerger.
Also, some of the tests are already covered by the existing unit tests, e.g. 
`switch-stmt` with `ImportSwitch`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71562



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

[Lldb-commits] [lldb] 4aee81c - [lldb][NFC] Allow creating ClangExpressionDeclMap and ClangASTSource without a Target and add basic unit test

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T14:04:12+01:00
New Revision: 4aee81c4f73359230e358108bc428e3b0cc59566

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

LOG: [lldb][NFC] Allow creating ClangExpressionDeclMap and ClangASTSource 
without a Target and add basic unit test

The ClangExpressionDeclMap should be testable from a unit test. This is 
currently
impossible as they have both dependencies on Target/ExecutionContext from their
constructor. This patch allows constructing these classes without an active 
Target
and adds the missing tests for running without a target that we can do at least
a basic lookup test without crashing.

Added: 
lldb/unittests/Expression/ClangExpressionDeclMapTest.cpp

Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
lldb/source/Symbol/ClangASTContext.cpp
lldb/unittests/Expression/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 94a23c8069a1..f37fe21b5545 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -49,10 +49,11 @@ class ScopedLexicalDeclEraser {
 };
 }
 
-ClangASTSource::ClangASTSource(const lldb::TargetSP &target)
+ClangASTSource::ClangASTSource(const lldb::TargetSP &target,
+   const lldb::ClangASTImporterSP &importer)
 : m_import_in_progress(false), m_lookups_enabled(false), m_target(target),
   m_ast_context(nullptr), m_active_lexical_decls(), m_active_lookups() {
-  m_ast_importer_sp = m_target->GetClangASTImporter();
+  m_ast_importer_sp = importer;
 }
 
 void ClangASTSource::InstallASTContext(ClangASTContext &clang_ast_context,
@@ -65,9 +66,13 @@ void ClangASTSource::InstallASTContext(ClangASTContext 
&clang_ast_context,
 }
 
 ClangASTSource::~ClangASTSource() {
-  if (m_ast_importer_sp)
-m_ast_importer_sp->ForgetDestination(m_ast_context);
+  if (!m_ast_importer_sp)
+return;
+
+  m_ast_importer_sp->ForgetDestination(m_ast_context);
 
+  if (!m_target)
+return;
   // We are in the process of destruction, don't create clang ast context on
   // demand by passing false to
   // Target::GetScratchClangASTContext(create_on_demand).
@@ -683,6 +688,9 @@ void ClangASTSource::FindExternalVisibleDecls(
   if (IgnoreName(name, true))
 return;
 
+  if (!m_target)
+return;
+
   if (module_sp && namespace_decl) {
 CompilerDeclContext found_namespace_decl;
 

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
index 0ae65e526e7e..e0442aeca326 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -38,7 +38,11 @@ class ClangASTSource : public ClangExternalASTSourceCommon,
   ///
   /// \param[in] target
   /// A reference to the target containing debug information to use.
-  ClangASTSource(const lldb::TargetSP &target);
+  ///
+  /// \param[in] importer
+  /// The ClangASTImporter to use.
+  ClangASTSource(const lldb::TargetSP &target,
+ const lldb::ClangASTImporterSP &importer);
 
   /// Destructor
   ~ClangASTSource() override;

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index 3a6b7018e48f..33867fb4d45a 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -65,9 +65,10 @@ const char *g_lldb_local_vars_namespace_cstr = 
"$__lldb_local_vars";
 ClangExpressionDeclMap::ClangExpressionDeclMap(
 bool keep_result_in_memory,
 Materializer::PersistentVariableDelegate *result_delegate,
-ExecutionContext &exe_ctx, ValueObject *ctx_obj)
-: ClangASTSource(exe_ctx.GetTargetSP()), m_found_entities(),
-  m_struct_members(), m_keep_result_in_memory(keep_result_in_memory),
+const lldb::TargetSP &target, const lldb::ClangASTImporterSP &importer,
+ValueObject *ctx_obj)
+: ClangASTSource(target, importer), m_found_entities(), m_struct_members(),
+  m_keep_result_in_memory(keep_result_in_memory),
   m_result_delegate(result_delegate), m_ctx_obj(ctx_obj), m_pars

[Lldb-commits] [PATCH] D71603: [lldb] Make ClangASTImporter::LayoutRecordType always return the same layout

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added a reviewer: shafik.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.
teemperor removed reviewers: martong, a.sidorin.
Herald added a reviewer: a.sidorin.

ClangASTImporter::LayoutRecordType provides record layouts for Clang to use 
during CodeGen.

Currently this method only provides a layout once and then deletes the layout 
from the internal
storage. The next time we get a request for the same record layout we pretend 
that we don't know
the record and leave CodeGen on its own to produce the layout. This system 
currently seems to work as we
usually request a layout over an ASTContext which caches the result, so the 
ClangASTImporter
will most likely never be asked twice to layout a record unless the ASTContext 
changes at some
point.

I think it makes sense that we do not rely on the caching mechanism in the 
ASTContext to save
us from doing the wrong thing and instead always return the same layout from 
ClangASTContext.
Also marks the method as `const` because it is essentially just a getter now.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D71603

Files:
  lldb/include/lldb/Symbol/ClangASTImporter.h
  lldb/source/Symbol/ClangASTImporter.cpp
  lldb/unittests/Symbol/TestClangASTImporter.cpp


Index: lldb/unittests/Symbol/TestClangASTImporter.cpp
===
--- lldb/unittests/Symbol/TestClangASTImporter.cpp
+++ lldb/unittests/Symbol/TestClangASTImporter.cpp
@@ -203,18 +203,25 @@
   layout_info.field_offsets[field] = 1;
   importer.InsertRecordDecl(source_record, layout_info);
 
-  uint64_t bit_size;
-  uint64_t alignment;
-  llvm::DenseMap field_offsets;
-  llvm::DenseMap base_offsets;
-  llvm::DenseMap vbase_offsets;
-  importer.LayoutRecordType(source_record, bit_size, alignment, field_offsets,
-base_offsets, vbase_offsets);
-
-  EXPECT_EQ(15U, bit_size);
-  EXPECT_EQ(2U, alignment);
-  EXPECT_EQ(1U, field_offsets.size());
-  EXPECT_EQ(1U, field_offsets[field]);
-  EXPECT_EQ(0U, base_offsets.size());
-  EXPECT_EQ(0U, vbase_offsets.size());
+  // Check that asking for the layout multiple times will always return the
+  // same result. Three times should be enough to find any changing
+  for (unsigned i = 0; i < 3U; ++i) {
+SCOPED_TRACE("Iteration: " + std::to_string(i));
+
+uint64_t bit_size;
+uint64_t alignment;
+llvm::DenseMap field_offsets;
+llvm::DenseMap 
base_offsets;
+llvm::DenseMap
+vbase_offsets;
+importer.LayoutRecordType(source_record, bit_size, alignment, 
field_offsets,
+  base_offsets, vbase_offsets);
+
+EXPECT_EQ(15U, bit_size);
+EXPECT_EQ(2U, alignment);
+EXPECT_EQ(1U, field_offsets.size());
+EXPECT_EQ(1U, field_offsets[field]);
+EXPECT_EQ(0U, base_offsets.size());
+EXPECT_EQ(0U, vbase_offsets.size());
+  }
 }
Index: lldb/source/Symbol/ClangASTImporter.cpp
===
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -525,26 +525,23 @@
 llvm::DenseMap
 &base_offsets,
 llvm::DenseMap
-&vbase_offsets) {
-  RecordDeclToLayoutMap::iterator pos =
+&vbase_offsets) const {
+  RecordDeclToLayoutMap::const_iterator pos =
   m_record_decl_to_layout_map.find(record_decl);
-  bool success = false;
   base_offsets.clear();
   vbase_offsets.clear();
   if (pos != m_record_decl_to_layout_map.end()) {
 bit_size = pos->second.bit_size;
 alignment = pos->second.alignment;
-field_offsets.swap(pos->second.field_offsets);
-base_offsets.swap(pos->second.base_offsets);
-vbase_offsets.swap(pos->second.vbase_offsets);
-m_record_decl_to_layout_map.erase(pos);
-success = true;
-  } else {
-bit_size = 0;
-alignment = 0;
-field_offsets.clear();
+field_offsets = pos->second.field_offsets;
+base_offsets = pos->second.base_offsets;
+vbase_offsets = pos->second.vbase_offsets;
+return true;
   }
-  return success;
+  bit_size = 0;
+  alignment = 0;
+  field_offsets.clear();
+  return false;
 }
 
 void ClangASTImporter::InsertRecordDecl(clang::RecordDecl *decl,
Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -67,7 +67,7 @@
   llvm::DenseMap
   &base_offsets,
   llvm::DenseMap
-  &vbase_offsets);
+  &vbase_offsets) const;
 
   bool CanImport(const CompilerType &type);
 


Index: lldb/unittests/Symbol/TestClangASTImporter.cpp
===
--- lldb/unittests/Symbol/TestClangASTImporter.cpp
+++ lldb/unittests/Symbol/TestClangASTImporter.cpp
@@ -203,18 +203,25 

[Lldb-commits] [lldb] b852b3c - [lldb][NFC] Rename ClangASTImporter::InsertRecordDecl to SetRecordLayout and document it

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T15:56:07+01:00
New Revision: b852b3c982d2e8ad3f13c626b3e3655e5b3c399e

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

LOG: [lldb][NFC] Rename ClangASTImporter::InsertRecordDecl to SetRecordLayout 
and document it

This function is just setting the layout for the given RecordDecl so
the current name is not very descriptive. Also add some documentation for it.

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTImporter.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/source/Symbol/ClangASTImporter.cpp
lldb/unittests/Symbol/TestClangASTImporter.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTImporter.h 
b/lldb/include/lldb/Symbol/ClangASTImporter.h
index 58e068658ad8..e0448249eba8 100644
--- a/lldb/include/lldb/Symbol/ClangASTImporter.h
+++ b/lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -58,7 +58,14 @@ class ClangASTImporter {
   clang::Decl *DeportDecl(clang::ASTContext *dst_ctx,
   clang::ASTContext *src_ctx, clang::Decl *decl);
 
-  void InsertRecordDecl(clang::RecordDecl *decl, const LayoutInfo &layout);
+  /// Sets the layout for the given RecordDecl. The layout will later be
+  /// used by Clang's during code generation. Not calling this function for
+  /// a RecordDecl will cause that Clang's codegen tries to layout the
+  /// record by itself.
+  ///
+  /// \param decl The RecordDecl to set the layout for.
+  /// \param layout The layout for the record.
+  void SetRecordLayout(clang::RecordDecl *decl, const LayoutInfo &layout);
 
   bool LayoutRecordType(
   const clang::RecordDecl *record_decl, uint64_t &bit_size,

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index d7b216c99feb..136027ee4d55 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1720,7 +1720,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext &sc,
 ClangASTContext::GetAsRecordDecl(clang_type);
 
 if (record_decl) {
-  GetClangASTImporter().InsertRecordDecl(
+  GetClangASTImporter().SetRecordLayout(
   record_decl, ClangASTImporter::LayoutInfo());
 }
   }
@@ -2134,7 +2134,7 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 clang::CXXRecordDecl *record_decl =
 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
 if (record_decl)
-  GetClangASTImporter().InsertRecordDecl(record_decl, layout_info);
+  GetClangASTImporter().SetRecordLayout(record_decl, layout_info);
   }
 
   return (bool)clang_type;

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 3c494dc83986..7221144407c1 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -231,6 +231,6 @@ void UdtRecordCompleter::complete() {
   ClangASTContext::CompleteTagDeclarationDefinition(m_derived_ct);
 
   if (auto *record_decl = llvm::dyn_cast(&m_tag_decl)) {
-m_ast_builder.importer().InsertRecordDecl(record_decl, m_layout);
+m_ast_builder.importer().SetRecordLayout(record_decl, m_layout);
   }
 }

diff  --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index 245d99c8c05b..7bf94c64aa4f 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1207,7 +1207,7 @@ bool PDBASTParser::CompleteTypeFromUDT(
   if (!record_decl)
 return static_cast(compiler_type);
 
-  GetClangASTImporter().InsertRecordDecl(record_decl, layout_info);
+  GetClangASTImporter().SetRecordLayout(record_decl, layout_info);
 
   return static_cast(compiler_type);
 }

diff  --git a/lldb/source/Symbol/ClangASTImporter.cpp 
b/lldb/source/Symbol/ClangASTImporter.cpp
index 3c11c3250af9..de5448d4317d 100644
--- a/lldb/source/Symbol/ClangASTImporter.cpp
+++ b/lldb/source/Symbol/ClangASTImporter.cpp
@@ -547,7 +547,7 @@ bool ClangASTImporter::LayoutRecordType(
   return success;
 }
 
-void ClangASTImporter::InsertRecordDecl(clang::RecordDecl *decl,
+void ClangASTImporter::SetRecordLayout(clang::RecordDecl *decl,
 const LayoutInfo &layout) {
   m_record_decl_to_layout_map.insert(std::make_pair(decl, layout));
 }

diff  --git a/lldb/unittests/Symbol/TestClangASTImporter.cpp 
b

[Lldb-commits] [lldb] 268f37d - [lldb][NFC] Use StringRef in CreateRecordType and CreateObjCClass

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-17T16:10:34+01:00
New Revision: 268f37df6e45c4b0603bd4d964483a0d84da44c1

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

LOG: [lldb][NFC] Use StringRef in CreateRecordType and CreateObjCClass

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTContext.h

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/source/Symbol/ClangASTContext.cpp
lldb/unittests/Symbol/TestClangASTImporter.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTContext.h 
b/lldb/include/lldb/Symbol/ClangASTContext.h
index 9c37b94219f0..6cebd6f3b62a 100644
--- a/lldb/include/lldb/Symbol/ClangASTContext.h
+++ b/lldb/include/lldb/Symbol/ClangASTContext.h
@@ -263,8 +263,9 @@ class ClangASTContext : public TypeSystem {
 bool omit_empty_base_classes);
 
   CompilerType CreateRecordType(clang::DeclContext *decl_ctx,
-lldb::AccessType access_type, const char *name,
-int kind, lldb::LanguageType language,
+lldb::AccessType access_type,
+llvm::StringRef name, int kind,
+lldb::LanguageType language,
 ClangASTMetadata *metadata = nullptr,
 bool exports_symbols = false);
 
@@ -322,8 +323,9 @@ class ClangASTContext : public TypeSystem {
 
   static bool RecordHasFields(const clang::RecordDecl *record_decl);
 
-  CompilerType CreateObjCClass(const char *name, clang::DeclContext *decl_ctx,
-   bool isForwardDecl, bool isInternal,
+  CompilerType CreateObjCClass(llvm::StringRef name,
+   clang::DeclContext *decl_ctx, bool 
isForwardDecl,
+   bool isInternal,
ClangASTMetadata *metadata = nullptr);
 
   bool SetTagTypeKind(clang::QualType type, int kind) const;

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
index 6402e80d6f98..76375e22ad6a 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -128,7 +128,7 @@ clang::QualType AppleObjCTypeEncodingParser::BuildAggregate(
   if (!lldb_ctx)
 return clang::QualType();
   CompilerType union_type(lldb_ctx->CreateRecordType(
-  nullptr, lldb::eAccessPublic, name.c_str(), kind, lldb::eLanguageTypeC));
+  nullptr, lldb::eAccessPublic, name, kind, lldb::eLanguageTypeC));
   if (union_type) {
 ClangASTContext::StartTagDeclarationDefinition(union_type);
 

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 1f7366f5e184..b58550beb04a 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -778,9 +778,8 @@ clang::QualType 
PdbAstBuilder::CreateRecordType(PdbTypeSymId id,
   metadata.SetUserID(toOpaqueUid(id));
   metadata.SetIsDynamicCXXType(false);
 
-  CompilerType ct =
-  m_clang.CreateRecordType(context, access, uname.c_str(), ttk,
-   lldb::eLanguageTypeC_plus_plus, &metadata);
+  CompilerType ct = m_clang.CreateRecordType(
+  context, access, uname, ttk, lldb::eLanguageTypeC_plus_plus, &metadata);
 
   lldbassert(ct.IsValid());
 

diff  --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index 7bf94c64aa4f..740b39016862 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -417,9 +417,9 @@ lldb::TypeSP PDBASTParser::CreateLLDBTypeFromPDBType(const 
PDBSymbol &type) {
   metadata.SetUserID(type.getSymIndexId());
   metadata.SetIsDynamicCXXType(false);
 
-  clang_type = m_ast.CreateRecordType(
-  decl_context, access, name.c_str(), tag_type_kind,
-  lldb::eLanguageTypeC_plus_plus, &metadata);
+  clang_type =
+  m_ast.CreateRecordType(decl_context, access, name, tag_type_kind,
+ lldb::eLanguageTypeC_plus_plus, &metadata);
   assert(clang_type.IsValid());
 
   auto record_decl =

diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTCo

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Frederic Riss via Phabricator via lldb-commits]

friss added a comment.

In D69309#1787409 , @labath wrote:

> In D69309#1787297 , @jarin wrote:
>
> > In D69309#1752738 , @friss wrote:
> >
> > > Basically, today the debug info will describe an entity named "Foo". 
> > > The accelerator tables all reference this name. So when Clang asks us if 
> > > we know "Foo" (which is what happens when instantiating), we fail to find 
> > > the right instantiations. The consensus of the above discussion was that 
> > > we should change the debug info to have "Foo" as the name of any 
> > > instantiation, with a child DIE describing the template arguments. Just 
> > > doing this in the compiler causes test failures in LLDB, so there's some 
> > > work to do in LLDB to support this.
> >
> >
> > Frederic, you say that "doing this in the compiler causes test failures in 
> > LLDB", which implies you have tried adding the template in the compiler. Do 
> > you have that compiler patch lying around so that we could have a look at 
> > what can be done on the lldb side?
> >
> > I agree that a good long term fix is to have "Foo" as an entity in DWARF, 
> > although for backwards compatibility it might be better if the "Foo" 
> > template just contained references to the instantiations rather than having 
> > them as children.
>
>
> I am afraid you're overestimating the scope of that idea. I *think* that Fred 
> was referring to simply changing the string that gets put into the DW_AT_name 
> field of the /instantation/ (and, by extension, the accelerator table). The 
> debug info would still describe instantiations only.


Just confirming that this is indeed what I meant. I don't have the patch handy 
anymore (it was extremely small, about 5 lines IIRC).


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

https://reviews.llvm.org/D69309



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

Re: [Lldb-commits] [PATCH] D71440: Extending step-over range past calls was causing deadlocks, fix that.

[Jim Ingham via lldb-commits]

> On Dec 17, 2019, at 2:41 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> Thanks for the explanation (and for updating the test). This actually 
> explains why it sometimes seemed that I lose control over other threads while 
> stepping one of them. I'll be sure to use the --run-mode argument next time. 
> This seems fine given how the current stepping logic works, though I 
> personally would expect that the other also threads stay stopped when i do 
> something called "thread step-over", but I get the feeling I expect a 
> different kind of threading behavior from my debugger than most people...

For folks that know how debuggers work, and are aware that "thread step-over" 
might cause deadlocks because it is only running one thread, and are willing to 
manage that themselves, sticking to one thread can make debugging 
multi-threaded apps more accurate.  Since you can suspend & resume individual 
threads, you can even move the blocking thread a just past releasing the lock 
before continuing on the first thread.  Some day lldb will also know about all 
the locking primitives, and can know "Thread A is blocked on a lock held by 
Thread B" and can tell when Thread B releases the lock.  At that point we could 
do thread stepping in a much more fine grained way.

For now, my experience supporting lldb is that for most people the debugger is 
a black box, and if they see this sort of behavior they will think the debugger 
has hung and not know (or want to know) that they can control this behavior.  
So I'm pretty sure the current default mode is appropriate for the majority of 
our users.

BTW, if you use the "step", "next" and "fin" aliases rather than "thread 
step-in", etc. you could just switch the aliases to set the run mode to 
this-thread  for your uses.  It would also be appropriate to add a setting for 
the default run mode as well, as that would make it easier to switch behaviors 
on the fly.  I don't think I omitted this for a reason other than time...

Jim


> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D71440/new/
> 
> https://reviews.llvm.org/D71440
> 
> 
> 

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

[Lldb-commits] [PATCH] D71630: [lldb] Add a SubsystemRAII that takes care of calling Initialize and Terminate in the unit tests

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: labath, JDevlieghere.
Herald added subscribers: lldb-commits, abidh, MaskRay, emaste.
Herald added a reviewer: martong.
Herald added a reviewer: espindola.
Herald added a reviewer: shafik.
Herald added a project: LLDB.
teemperor added a comment.
Herald added a subscriber: rnkovacs.

I only converted like 40% of our tests to the new system as I first wanted to 
get some feedback before I do all the refactoring. Also I'm open to ideas how 
to name the member (currently I just use `subsystems` without the `m_` prefix 
as it's not really a real member with any contents).


Many of our tests need to initialize certain subsystems/plugins of LLDB such as
`FileSystem` or `HostInfo` by calling their static `Initialize` functions 
before the
test starts and then calling `::Terminate` after the test is done (in reverse 
order).
This adds a lot of error-prone boilerplate code to our testing code.

This patch adds a RAII called SubsystemRAII that ensures that we always call
::Initialize and then call ::Terminate after the test is done (and that the 
Terminate
calls are always in the reverse order of the ::Initialize calls). It also gets 
rid of
all of the boilerplate that we had for these calls.

Per-fixture initialization is still not very nice with this approach as it would
require some kind of static unique_ptr that gets manually assigned/reseted
from the gtest SetUpTestCase/TearDownTestCase functions. Because of that
I changed all per-fixture setup to now do per-test setup which can be done
by just having the SubsystemRAII as a member of the test fixture. This change 
doesn't
influence our normal test runtime as LIT anyway runs each test case separately
(and the Initialize/Terminate calls are anyway not very expensive). It will 
however
make running all tests in a single executable slightly slower.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D71630

Files:
  lldb/unittests/Core/MangledTest.cpp
  lldb/unittests/Editline/EditlineTest.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/Host/HostInfoTest.cpp
  lldb/unittests/Interpreter/TestCompletion.cpp
  lldb/unittests/Language/Highlighting/HighlighterTest.cpp
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
  lldb/unittests/ObjectFile/PECOFF/TestPECallFrameInfo.cpp
  lldb/unittests/Symbol/TestClangASTImporter.cpp
  lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
  lldb/unittests/Target/ModuleCacheTest.cpp
  lldb/unittests/TestingSupport/SubsystemRAII.h

Index: lldb/unittests/TestingSupport/SubsystemRAII.h
===
--- /dev/null
+++ lldb/unittests/TestingSupport/SubsystemRAII.h
@@ -0,0 +1,59 @@
+//===- SubsystemRAII.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_SUBSYSTEMRAII_H
+#define LLDB_UNITTESTS_TESTINGSUPPORT_SUBSYSTEMRAII_H
+
+namespace lldb_private {
+
+template  class SubsystemRAII {};
+
+/// RAII for initializing and deinitializing LLDB subsystems.
+///
+/// This RAII takes care of calling the Initialize and Terminate functions for
+/// the subsystems specified by its template arguments. The ::Initialize
+/// functions are called on construction for each subsystem template parameter
+/// in the order in which they are passed as template parameters.
+/// The ::Terminate functions are called in the reverse order at destruction
+/// time.
+///
+/// Constructing this RAII in a scope like this:
+///
+///   @code{.cpp}
+///   {
+/// SubsystemRAII Subsystems;
+/// DoingTestWork();
+///   }
+///   @endcode
+///
+/// is equivalent to the following code:
+///
+///   @code{.cpp}
+///   {
+/// FileSystem::Initialize();
+/// HostInfo::Initialize();
+///
+/// DoingTestWork();
+///
+/// FileSystem::Terminate();
+/// HostInfo::Terminate();
+///   }
+///   @endcode
+template  class SubsystemRAII {
+  /// RAII for a single subsystem.
+  template  struct SubsystemRAIICase {
+SubsystemRAIICase() { SubT::Initialize(); }
+~SubsystemRAIICase() { SubT::Terminate(); }
+  };
+
+  SubsystemRAIICase CurrentSubsystem;
+  SubsystemRAII RemainingSubsystems;
+};
+} // namespace lldb_private
+
+#endif // LLDB_UNITTESTS_TESTINGSUPPORT_SUBSYSTEMRAII_H
Index: lldb/unittests/Target/ModuleCacheTest.cpp
===
--- lldb/unittests/Target/Mod

[Lldb-commits] [PATCH] D71630: [lldb] Add a SubsystemRAII that takes care of calling Initialize and Terminate in the unit tests

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.
Herald added a subscriber: rnkovacs.

I only converted like 40% of our tests to the new system as I first wanted to 
get some feedback before I do all the refactoring. Also I'm open to ideas how 
to name the member (currently I just use `subsystems` without the `m_` prefix 
as it's not really a real member with any contents).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71630



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

[Lldb-commits] [PATCH] D71633: [lldb-vscode] Only close the debuggers in/out when DAP is over stdin/out

[Nathan Lanza via Phabricator via lldb-commits]

lanza created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The DAP is usually communicated over STDIN and STDOUT and thus lldb's
Debugger instance printing and reading from these files can cause
conflicts. However, if the DAP communication was set up to be done via
a socket then we can leave these files open as they can provide
valueable logging and feedback to the user.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71633

Files:
  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
@@ -1200,11 +1200,13 @@
   // executable when attaching to a process by process ID in a "attach"
   // request.
   FILE *out = llvm::sys::RetryAfterSignal(nullptr, fopen, dev_null_path, "w");
-  if (out) {
-// Set the output and error file handles to redirect into nothing otherwise
+  if (out && !g_vsc.input.descriptor.m_is_socket) {
+// If the input and output descriptors are STDIN and STDOUT then we need to
+// set the output and error file handles to redirect into nothing otherwise
 // if any code in LLDB prints to the debugger file handles, the output and
 // error file handles are initialized to STDOUT and STDERR and any output
-// will kill our debug session.
+// will kill our debug session. However, if the communication is via 
sockets
+// then we can leave these open.
 g_vsc.debugger.SetOutputFileHandle(out, true);
 g_vsc.debugger.SetErrorFileHandle(out, false);
   }


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1200,11 +1200,13 @@
   // executable when attaching to a process by process ID in a "attach"
   // request.
   FILE *out = llvm::sys::RetryAfterSignal(nullptr, fopen, dev_null_path, "w");
-  if (out) {
-// Set the output and error file handles to redirect into nothing otherwise
+  if (out && !g_vsc.input.descriptor.m_is_socket) {
+// If the input and output descriptors are STDIN and STDOUT then we need to
+// set the output and error file handles to redirect into nothing otherwise
 // if any code in LLDB prints to the debugger file handles, the output and
 // error file handles are initialized to STDOUT and STDERR and any output
-// will kill our debug session.
+// will kill our debug session. However, if the communication is via sockets
+// then we can leave these open.
 g_vsc.debugger.SetOutputFileHandle(out, true);
 g_vsc.debugger.SetErrorFileHandle(out, false);
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D71633: [lldb-vscode] Only close the debuggers in/out when DAP is over stdin/out

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1202-1212
   FILE *out = llvm::sys::RetryAfterSignal(nullptr, fopen, dev_null_path, "w");
-  if (out) {
-// Set the output and error file handles to redirect into nothing otherwise
+  if (out && !g_vsc.input.descriptor.m_is_socket) {
+// If the input and output descriptors are STDIN and STDOUT then we need to
+// set the output and error file handles to redirect into nothing otherwise
 // if any code in LLDB prints to the debugger file handles, the output and
 // error file handles are initialized to STDOUT and STDERR and any output
+// will kill our debug session. However, if the communication is via 
sockets

Test if this is a socket first so we don't open dev null is we don't need to:

```
if (!g_vsc.input.descriptor.m_is_socket) {
  FILE *out = llvm::sys::RetryAfterSignal(nullptr, fopen, dev_null_path, "w");
if (out) {
  // If the input and output descriptors are STDIN and STDOUT then we need 
to
  // set the output and error file handles to redirect into nothing 
otherwise
  // if any code in LLDB prints to the debugger file handles, the output and
  // error file handles are initialized to STDOUT and STDERR and any output
  // will kill our debug session. However, if the communication is via 
sockets
  // then we can leave these open.
  g_vsc.debugger.SetOutputFileHandle(out, true);
  g_vsc.debugger.SetErrorFileHandle(out, false);
}
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71633



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

[Lldb-commits] [PATCH] D71633: [lldb-vscode] Only close the debuggers in/out when DAP is over stdin/out

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

minor nit and this is good


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71633



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev updated this revision to Diff 234463.
paolosev marked 2 inline comments as done.
paolosev removed a project: LLVM.
paolosev added a comment.

Addressed first review comments.


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

https://reviews.llvm.org/D71575

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/DynamicLoader/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp
  lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/ObjectFile/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/WASM/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp
  lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/WASM/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/WASM/SymbolVendorWasm.cpp
  lldb/source/Plugins/SymbolVendor/WASM/SymbolVendorWasm.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/unittests/ObjectFile/CMakeLists.txt
  lldb/unittests/ObjectFile/WASM/CMakeLists.txt
  lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp

Index: lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
===
--- /dev/null
+++ lldb/unittests/ObjectFile/WASM/TestObjectFileWasm.cpp
@@ -0,0 +1,281 @@
+//===-- TestObjectFileWasm.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 "gtest/gtest.h"
+
+#include "Plugins/ObjectFile/WASM/ObjectFileWasm.h"
+#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Platform.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Reproducer.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+class ObjectFileWasmTest : public testing::Test {
+public:
+  void SetUp() override {
+llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+FileSystem::Initialize();
+HostInfo::Initialize();
+wasm::ObjectFileWasm::Initialize();
+  }
+
+  void TearDown() override {
+wasm::ObjectFileWasm::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+Reproducer::Terminate();
+  }
+};
+
+TEST_F(ObjectFileWasmTest, SectionsResolveConsistently) {
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !WASM
+FileHeader:
+  Version: 0x0001
+Sections:
+  - Type:TYPE
+Signatures:
+  - Index:   0
+ParamTypes:
+  - I32
+ReturnTypes:
+  - I32
+  - Type:FUNCTION
+FunctionTypes:   [ 0 ]
+  - Type:TABLE
+Tables:
+  - ElemType:FUNCREF
+Limits:
+  Flags:   [ HAS_MAX ]
+  Initial: 0x0001
+  Maximum: 0x0001
+  - Type:MEMORY
+Memories:
+  - Initial: 0x0002
+  - Type:GLOBAL
+Globals:
+  - Index:   0
+Type:I32
+Mutable: true
+InitExpr:
+  Opcode:  I32_CONST
+  Value:   66560
+  - Type:EXPORT
+Exports:
+  - Name:memory
+Kind:MEMORY
+Index:   0
+  - Name:square
+Kind:FUNCTION
+Index:   0
+  - Type:CODE
+Functions:
+  - Index:   0
+Locals:
+  - Type:I32
+Count:   6
+Body:238080808000210141102102200120026B21032003200036020C200328020C2104200328020C2105200420056C210620060F0B
+  - Type:CUSTOM
+Name:name
+FunctionNames:
+  - Index:   0
+Name:square
+  - Type:CUSTOM
+Name:producers
+Languages:
+  - Name:C99
+Version: ''
+Tools:
+  - Name:clang
+Version: '10.0.0'
+  - Type:CUSTOM
+Name:external_debug_info
+Payload: 0F7371756172655F73796D2E7761736D
+...
+)");
+
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ArchSpec arch("wasm32-unknown-unknown-wasm");
+  Platform::SetHostPlatfor

[Lldb-commits] [PATCH] D71575: [LLDB] Add initial support for WebAssembly debugging

[Paolo Severini via Phabricator via lldb-commits]

paolosev marked 43 inline comments as done.
paolosev added a comment.

Thanks for all the comments! I am updating the code following your suggestions.
Next step will be to split this into three distinct patches, as suggested by 
Pavel.




Comment at: 
lldb/source/Plugins/DynamicLoader/WASM-DYLD/DynamicLoaderWasmDYLD.cpp:1
+//===-- DynamicLoaderWasmDYLD.cpp *- C++
+//-*-===//

aprantl wrote:
> 1. This hangs over the line
> 2. a -*- C++ -*- is only necessary for .h files where C vs. C++ is ambiguous
> 
Fixed, but I see the -*- C++ -*- in all files.



Comment at: lldb/source/Plugins/ObjectFile/WASM/ObjectFileWasm.cpp:1
+//===-- ObjectFileWasm.cpp  -*- C++ -*-===//
+//

aprantl wrote:
> ditto
But all the C++ files seem to have "-*- C++ -*-===//" in the first line.



Comment at: lldb/source/Utility/ArchSpec.cpp:107
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arm, ArchSpec::eCore_arm_armv8l,
+ "armv8l"},
 {eByteOrderLittle, 4, 4, 4, llvm::Triple::aarch64_32,

sbc100 wrote:
> Is this just clang format being greedy?
Yes, it was edited by clang format.



Comment at: llvm/include/llvm/BinaryFormat/ELF.h:314
   EM_BPF = 247,   // Linux kernel bpf virtual machine
+  EM_WASM = 248,  // WebAssembly
 };

labath wrote:
> sbc100 wrote:
> > This seems like an odd place to add this, given that are not using or 
> > relying on ELF anywhere.  Does this make sense?
> Indeed, that looks very unexpected, and begs an explanation.
Oops... my mistake. This is a relic from an old version where I expected Wasm 
stripped debug symbol might also come in an ELF file. Removed.


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

https://reviews.llvm.org/D71575



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