[Lldb-commits] [lldb] 9eaf0ab - Revert "[lldb/Utility] Provide a stringify_append overload for function pointers."

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-16T17:52:09-07:00
New Revision: 9eaf0abebff9c61fa01c6ca69cbc74b1464efe14

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

LOG: Revert "[lldb/Utility] Provide a stringify_append overload for function 
pointers."

Temporarily reverts commit d10386e1779599d217b5b849a079f29dfbe17024
because it breaks the Windows build. MSVC complains about an ambiguous
call to an overloaded function.

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 75f38929e362..3b5dde3d2e2a 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -34,17 +34,12 @@ inline void stringify_append(llvm::raw_string_ostream &ss, 
const T &t) {
 
 template 
 inline void stringify_append(llvm::raw_string_ostream &ss, T *t) {
-  ss << static_cast(t);
+  ss << reinterpret_cast(t);
 }
 
 template 
 inline void stringify_append(llvm::raw_string_ostream &ss, const T *t) {
-  ss << static_cast(t);
-}
-
-template 
-inline void stringify_append(llvm::raw_string_ostream &ss, T (*t)(Args...)) {
-  ss << "function pointer";
+  ss << reinterpret_cast(t);
 }
 
 template <>



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

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Vedant Kumar via Phabricator via lldb-commits]

vsk planned changes to this revision.
vsk added a comment.

Sorry for the delay, I plan to have an update for this soon.

In D77843#1978964 , @JDevlieghere 
wrote:

> Have you asked the foundation people about the ASCII support in NSString?


No, I'm not sure what to ask. Having thought things over, I think my 'FIXME' 
about 'In practice, do we ever prefer ASCII-only formatting over UTF8 
formatting?' was premature, so I'll delete it.

I did some experiments with forcing invalid data into an NSString and seeing 
what comes out of NSLog. Both with `dataUsingEncoding:NSASCIIStringEncoding`, 
or with just typing in invalid bytes, NSLog just prints an empty string (in the 
latter case -WCFString-literal even warns that input conversion is stopped 
after an invalid byte is seen). I don't think it'd be helpful for lldb to match 
that behavior? At least, we don't match that behavior today, we try to escape 
and print out the invalid bytes.

If we want to continue to not match NSLog's behavior, then I think we should we 
treat invalid bytes under ASCII vs. utf differently (we can print the actual 
character in the utf case, and a sequence of '\x..\x..' otherwise).




Comment at: lldb/source/DataFormatters/StringPrinter.cpp:259
+  case StringPrinter::EscapeStyle::Swift:
+// Prints up to 14 characters, then a \0 terminator.
+escaped_len = sprintf((char *)data, "\\u{%x}", (unsigned)codepoint);

shafik wrote:
> Maybe I am missing something but I don't think the `sprintf` below can go up 
> to 14 chars.
You're right, I got %x mixed up with %u.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843



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

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added subscribers: jfb, arphaman, mgorny.

No need to roll our own implementation for something that's covered by llvm.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D78337

Files:
  lldb/include/lldb/Host/TaskPool.h
  lldb/include/lldb/module.modulemap
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/TaskPool.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/TaskPoolTest.cpp

Index: lldb/unittests/Host/TaskPoolTest.cpp
===
--- lldb/unittests/Host/TaskPoolTest.cpp
+++ /dev/null
@@ -1,45 +0,0 @@
-#include "gtest/gtest.h"
-
-#include "lldb/Host/TaskPool.h"
-
-using namespace lldb_private;
-
-TEST(TaskPoolTest, AddTask) {
-  auto fn = [](int x) { return x * x + 1; };
-
-  auto f1 = TaskPool::AddTask(fn, 1);
-  auto f2 = TaskPool::AddTask(fn, 2);
-  auto f3 = TaskPool::AddTask(fn, 3);
-  auto f4 = TaskPool::AddTask(fn, 4);
-
-  ASSERT_EQ(10, f3.get());
-  ASSERT_EQ(2, f1.get());
-  ASSERT_EQ(17, f4.get());
-  ASSERT_EQ(5, f2.get());
-}
-
-TEST(TaskPoolTest, RunTasks) {
-  std::vector r(4);
-
-  auto fn = [](int x, int &y) { y = x * x + 1; };
-
-  TaskPool::RunTasks([fn, &r]() { fn(1, r[0]); }, [fn, &r]() { fn(2, r[1]); },
- [fn, &r]() { fn(3, r[2]); }, [fn, &r]() { fn(4, r[3]); });
-
-  ASSERT_EQ(2, r[0]);
-  ASSERT_EQ(5, r[1]);
-  ASSERT_EQ(10, r[2]);
-  ASSERT_EQ(17, r[3]);
-}
-
-TEST(TaskPoolTest, TaskMap) {
-  int data[4];
-  auto fn = [&data](int x) { data[x] = x * x; };
-
-  TaskMapOverInt(0, 4, fn);
-
-  ASSERT_EQ(data[0], 0);
-  ASSERT_EQ(data[1], 1);
-  ASSERT_EQ(data[2], 4);
-  ASSERT_EQ(data[3], 9);
-}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -11,7 +11,6 @@
   SocketAddressTest.cpp
   SocketTest.cpp
   SocketTestUtilities.cpp
-  TaskPoolTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -13,14 +13,22 @@
 #include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
 #include "lldb/Core/Module.h"
-#include "lldb/Host/TaskPool.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/Support/ThreadPool.h"
 
 using namespace lldb_private;
 using namespace lldb;
 
+static void TaskMapOverInt(size_t end,
+   const llvm::function_ref &func) {
+  llvm::ThreadPool pool;
+  for (size_t i = 0; i < end; i++)
+pool.async(func, i);
+  pool.wait();
+}
+
 void ManualDWARFIndex::Index() {
   if (!m_dwarf)
 return;
@@ -79,12 +87,12 @@
   // done indexing to make sure we don't pull in all DWARF dies, but we need
   // to wait until all units have been indexed in case a DIE in one
   // unit refers to another and the indexes accesses those DIEs.
-  TaskMapOverInt(0, units_to_index.size(), extract_fn);
+  TaskMapOverInt(units_to_index.size(), extract_fn);
 
   // Now create a task runner that can index each DWARF unit in a
   // separate thread so we can index quickly.
 
-  TaskMapOverInt(0, units_to_index.size(), parser_fn);
+  TaskMapOverInt(units_to_index.size(), parser_fn);
 
   auto finalize_fn = [this, &sets](NameToDIE(IndexSet::*index)) {
 NameToDIE &result = m_set.*index;
@@ -93,14 +101,16 @@
 result.Finalize();
   };
 
-  TaskPool::RunTasks([&]() { finalize_fn(&IndexSet::function_basenames); },
- [&]() { finalize_fn(&IndexSet::function_fullnames); },
- [&]() { finalize_fn(&IndexSet::function_methods); },
- [&]() { finalize_fn(&IndexSet::function_selectors); },
- [&]() { finalize_fn(&IndexSet::objc_class_selectors); },
- [&]() { finalize_fn(&IndexSet::globals); },
- [&]() { finalize_fn(&IndexSet::types); },
- [&]() { finalize_fn(&IndexSet::namespaces); });
+  llvm::ThreadPool pool;
+  pool.async(finalize_fn, &IndexSet::function_basenames);
+  pool.async(finalize_fn, &IndexSet::function_fullnames);
+  pool.async(finalize_fn, &IndexSet::function_methods);
+  pool.async(finalize_fn, &IndexSet::function_selectors);
+  pool.async(finalize_fn, &IndexSet::objc_class_selectors);
+  pool.async(finalize_fn, &IndexSet::globals);
+  pool.async(finalize_fn, &IndexSet::types);
+  pool.async(finalize_fn, &IndexSet::namespaces);
+  pool.wait();
 }
 
 void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, SymbolFileDWARFDwo *dwp,
Index: lldb/source/Host/common/TaskPool.cpp
===

[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:8823
 
 case clang::Type::Typedef: {
   const clang::TypedefType *typedef_type =

shafik wrote:
> Does it make sense to apply these change to non-objective-C cases?
It does absolutely! I must admit I sort of forgot to update the other cases, 
because they weren't on my critical path. Done now.



Comment at: lldb/tools/lldb-test/lldb-test.cpp:726
   if (DumpClangAST) {
-if (Find != FindType::None)
-  return make_string_error("Cannot both search and dump clang AST.");
-if (Regex || !Context.empty() || !File.empty() || Line != 0)
-  return make_string_error(
-  "-regex, -context, -name, -file and -line options are not "
-  "applicable for dumping clang AST.");
-return dumpClangAST;
+if (Find == FindType::None) {
+  if (Regex || !Context.empty() || !File.empty() || Line != 0)

shafik wrote:
> The relationship between `DumpClangAST` and `Find` is not obvious. Basically 
> we are falling through here in the `FindType::Type` case.
> 
> Perhaps a comment here and maybe on the ` Map.Dump(&Stream, false, 
> GetDescriptionLevel());` line might help clarify how these options are 
> related.
I added the explanation to the documentation of the -dump-clang-ast option.


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

https://reviews.llvm.org/D78329



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

[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 258197.
aprantl marked 2 inline comments as done.
aprantl added a comment.

Address feedback from Shafik. Thanks!


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

https://reviews.llvm.org/D78329

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/Type.h
  lldb/include/lldb/Symbol/TypeMap.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Symbol/TypeMap.cpp
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h
  lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -169,10 +169,13 @@
 static cl::opt DumpAST("dump-ast",
  cl::desc("Dump AST restored from symbols."),
  cl::sub(SymbolsSubcommand));
-static cl::opt
-DumpClangAST("dump-clang-ast",
- cl::desc("Dump clang AST restored from symbols."),
- cl::sub(SymbolsSubcommand));
+static cl::opt DumpClangAST(
+"dump-clang-ast",
+cl::desc("Dump clang AST restored from symbols. When used on its own this "
+ "will dump the entire AST of all loaded symbols. When combined "
+ "with -find, it changes the presentation of the search results "
+ "from pretty-printing the types to an AST dump."),
+cl::sub(SymbolsSubcommand));
 
 static cl::opt Verify("verify", cl::desc("Verify symbol information."),
 cl::sub(SymbolsSubcommand));
@@ -192,7 +195,7 @@
 static Error findVariables(lldb_private::Module &Module);
 static Error dumpModule(lldb_private::Module &Module);
 static Error dumpAST(lldb_private::Module &Module);
-static Error dumpClangAST(lldb_private::Module &Module);
+static Error dumpEntireClangAST(lldb_private::Module &Module);
 static Error verify(lldb_private::Module &Module);
 
 static Expected getAction();
@@ -404,6 +407,10 @@
   return List.GetVariableAtIndex(0)->GetDeclContext();
 }
 
+static lldb::DescriptionLevel GetDescriptionLevel() {
+  return opts::symbols::DumpClangAST ? eDescriptionLevelVerbose : eDescriptionLevelFull;
+}
+
 Error opts::symbols::findFunctions(lldb_private::Module &Module) {
   SymbolFile &Symfile = *Module.GetSymbolFile();
   SymbolContextList List;
@@ -534,7 +541,12 @@
 
   outs() << formatv("Found {0} types:\n", Map.GetSize());
   StreamString Stream;
-  Map.Dump(&Stream, false);
+  // Resolve types to force-materialize typedef types.
+  Map.ForEach([&](TypeSP &type) {
+type->GetFullCompilerType();
+return false;
+  });
+  Map.Dump(&Stream, false, GetDescriptionLevel());
   outs() << Stream.GetData() << "\n";
   return Error::success();
 }
@@ -615,7 +627,7 @@
   return Error::success();
 }
 
-Error opts::symbols::dumpClangAST(lldb_private::Module &Module) {
+Error opts::symbols::dumpEntireClangAST(lldb_private::Module &Module) {
   Module.ParseAllDebugSymbols();
 
   SymbolFile *symfile = Module.GetSymbolFile();
@@ -719,13 +731,17 @@
   }
 
   if (DumpClangAST) {
-if (Find != FindType::None)
-  return make_string_error("Cannot both search and dump clang AST.");
-if (Regex || !Context.empty() || !File.empty() || Line != 0)
-  return make_string_error(
-  "-regex, -context, -name, -file and -line options are not "
-  "applicable for dumping clang AST.");
-return dumpClangAST;
+if (Find == FindType::None) {
+  if (Regex || !Context.empty() || !File.empty() || Line != 0)
+return make_string_error(
+"-regex, -context, -name, -file and -line options are not "
+"applicable for dumping the entire clang AST. Either combine with "
+"-find, or use -dump-clang-ast as a standalone option.");
+  return dumpEntireClangAST;
+}
+if (Find != FindType::Type)
+  return make_string_error("This combination of -dump-clang-ast and -find "
+   " is not yet implemented.");
   }
 
   if (Regex && !Context.empty())
Index: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
===
--- lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
+++ lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
@@ -1,17 +1,22 @@
 // RUN: %clang --target=x86_64-apple-macosx -g -gmodules \
 // RUN:-fmodules -fmodules-cache-path=%t.cache \
 // RUN:-c -o %t.o %s -I%S/Inputs
-// RUN: lldb-test symbols -dump-clang-ast %t.o | FileCheck %s
 // Verify that the owning module information from DWARF is preserved in the AST.
 
 @import A;
 
 Typedef t1;
-// CHECK-DAG: TypedefDecl {{.*}} imported in A Typedef
+// RUN: lldb-test

Re: [Lldb-commits] [lldb] 9f6a308 - [lldb/Utility] Fix a bug in stringify_append for printing addresses.

[Shafik Yaghmour via lldb-commits]

> On Apr 16, 2020, at 3:17 PM, Davidino Italiano  wrote:
> 
> 
> 
>> On Apr 16, 2020, at 2:37 PM, Shafik Yaghmour via lldb-commits 
>> mailto:lldb-commits@lists.llvm.org>> wrote:
>> 
>>> 
>>> On Apr 16, 2020, at 2:04 PM, Jonas Devlieghere via lldb-commits 
>>> mailto:lldb-commits@lists.llvm.org>> wrote:
>>> 
>>> 
>>> Author: Jonas Devlieghere
>>> Date: 2020-04-16T14:03:55-07:00
>>> New Revision: 9f6a308457d1bfebf1cee94b0306e738f270e512
>>> 
>>> URL: 
>>> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512
>>>  
>>> 
>>> DIFF: 
>>> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512.diff
>>>  
>>> 
>>> 
>>> LOG: [lldb/Utility] Fix a bug in stringify_append for printing addresses.
>>> 
>>> The recent change in the API macros revealed that we were not printing
>>> the pointer address for a bunch of methods, but rather the address of
>>> the pointer. It's something I had already noticed while looking at some
>>> reproducer traces, but hadn't made it to the top of my list yet. This
>>> fixes the issue by providing a more specific overload.
>>> 
>>> 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 3728e19386d1..3b5dde3d2e2a 100644
>>> --- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
>>> +++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
>>> @@ -32,6 +32,11 @@ inline void stringify_append(llvm::raw_string_ostream 
>>> &ss, const T &t) {
>>>  ss << &t;
>>> }
>>> 
>> 
>> 
>> Note you can use the narrower static_cast
>> 
> 
> That would be wrong.

In the function pointer case, it would be ill-formed but otherwise this is a 
standard conversion and those are valid via static_cast, see [conv.ptr]p2 
http://eel.is/c++draft/conv.ptr#2

> 
>>> +template 
>>> +inline void stringify_append(llvm::raw_string_ostream &ss, T *t) {
>>> +  ss << reinterpret_cast(t);
>>> +}
>>> +
>>> template 
>>> inline void stringify_append(llvm::raw_string_ostream &ss, const T *t) {
>>>  ss << reinterpret_cast(t);
>>> @@ -115,7 +120,7 @@ template  inline std::string 
>>> stringify_args(const Ts &... ts) {
>>> 
>>> #define LLDB_CONSTRUCT_(T, ...) 
>>>\
>>>  lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,  
>>>   \
>>> -  stringify_args(__VA_ARGS__));
>>> \
>>> +  stringify_args(this, 
>>> __VA_ARGS__));  \
>>>  if (lldb_private::repro::InstrumentationData _data =   
>>>   \
>>>  LLDB_GET_INSTRUMENTATION_DATA()) { 
>>>   \
>>>_recorder.Record(_data.GetSerializer(), _data.GetRegistry(), 
>>>   \
>>> 
>>> 
>>> 
>>> ___
>>> 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 
>> 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:8823
 
 case clang::Type::Typedef: {
   const clang::TypedefType *typedef_type =

Does it make sense to apply these change to non-objective-C cases?



Comment at: lldb/tools/lldb-test/lldb-test.cpp:726
   if (DumpClangAST) {
-if (Find != FindType::None)
-  return make_string_error("Cannot both search and dump clang AST.");
-if (Regex || !Context.empty() || !File.empty() || Line != 0)
-  return make_string_error(
-  "-regex, -context, -name, -file and -line options are not "
-  "applicable for dumping clang AST.");
-return dumpClangAST;
+if (Find == FindType::None) {
+  if (Regex || !Context.empty() || !File.empty() || Line != 0)

The relationship between `DumpClangAST` and `Find` is not obvious. Basically we 
are falling through here in the `FindType::Type` case.

Perhaps a comment here and maybe on the ` Map.Dump(&Stream, false, 
GetDescriptionLevel());` line might help clarify how these options are related.


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

https://reviews.llvm.org/D78329



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

[Lldb-commits] [lldb] ce77900 - [DWARF] Rename a function and comment it for clarity.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-04-16T15:37:09-07:00
New Revision: ce7790044faa48a1ec49b6339797180e05520cef

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

LOG: [DWARF] Rename a function and comment it for clarity.

Pointed out by Adrian.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index b089c4e1f04a..1c1acd9dd61a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -1016,7 +1016,7 @@ DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(const 
DWARFUnit *cu) const {
   return nullptr;
 }
 
-bool DWARFDebugInfoEntry::IsGlobalOrStaticVariable() const {
+bool DWARFDebugInfoEntry::IsGlobalOrStaticScopeVariable() const {
   if (Tag() != DW_TAG_variable)
 return false;
   const DWARFDebugInfoEntry *parent_die = GetParent();

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index c05d79c01817..e12a19c13d1c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -167,7 +167,10 @@ class DWARFDebugInfoEntry {
   void SetSiblingIndex(uint32_t idx) { m_sibling_idx = idx; }
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
-  bool IsGlobalOrStaticVariable() const;
+  // This function returns true if the variable scope is either
+  // global or (file-static). It will return false for static variables
+  // that are local to a function, as they have local scope.
+  bool IsGlobalOrStaticScopeVariable() const;
 
 protected:
   static DWARFDeclContext

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 535e79a7ecc7..3fe38e75e612 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -204,7 +204,7 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit,
 case DW_AT_location:
 case DW_AT_const_value:
   has_location_or_const_value = true;
-  is_global_or_static_variable = die.IsGlobalOrStaticVariable();
+  is_global_or_static_variable = die.IsGlobalOrStaticScopeVariable();
 
   break;
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 400ba6e1f443..b13331c4852e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3390,7 +3390,8 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const 
SymbolContext &sc,
 }
   }
 } else {
-  if (location_is_const_value_data && 
die.GetDIE()->IsGlobalOrStaticVariable())
+  if (location_is_const_value_data &&
+  die.GetDIE()->IsGlobalOrStaticScopeVariable())
 scope = eValueTypeVariableStatic;
   else {
 scope = eValueTypeVariableLocal;



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

Re: [Lldb-commits] [lldb] 9f6a308 - [lldb/Utility] Fix a bug in stringify_append for printing addresses.

[Jonas Devlieghere via lldb-commits]

I thought it was just undefined behavior, but apparently it's illegal
because there's no guarantee pointers to function and pointers to objects
share the same address space. TIL.

I've pushed an overload in d10386e1779599d217b5b849a079f29dfbe17024

On Thu, Apr 16, 2020 at 3:17 PM Davidino Italiano 
wrote:

>
>
> On Apr 16, 2020, at 2:37 PM, Shafik Yaghmour via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>
> On Apr 16, 2020, at 2:04 PM, Jonas Devlieghere via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>
> Author: Jonas Devlieghere
> Date: 2020-04-16T14:03:55-07:00
> New Revision: 9f6a308457d1bfebf1cee94b0306e738f270e512
>
> URL:
> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512
> DIFF:
> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512.diff
>
> LOG: [lldb/Utility] Fix a bug in stringify_append for printing addresses.
>
> The recent change in the API macros revealed that we were not printing
> the pointer address for a bunch of methods, but rather the address of
> the pointer. It's something I had already noticed while looking at some
> reproducer traces, but hadn't made it to the top of my list yet. This
> fixes the issue by providing a more specific overload.
>
> 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 3728e19386d1..3b5dde3d2e2a 100644
> --- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
> +++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
> @@ -32,6 +32,11 @@ inline void stringify_append(llvm::raw_string_ostream
> &ss, const T &t) {
>  ss << &t;
> }
>
>
>
> Note you can use the narrower static_cast
>
>
> That would be wrong.
>
> +template 
> +inline void stringify_append(llvm::raw_string_ostream &ss, T *t) {
> +  ss << reinterpret_cast(t);
> +}
> +
> template 
> inline void stringify_append(llvm::raw_string_ostream &ss, const T *t) {
>  ss << reinterpret_cast(t);
> @@ -115,7 +120,7 @@ template  inline std::string
> stringify_args(const Ts &... ts) {
>
> #define LLDB_CONSTRUCT_(T, ...)
>\
>  lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,
>\
> -  stringify_args(__VA_ARGS__));
>\
> +  stringify_args(this,
> __VA_ARGS__));  \
>  if (lldb_private::repro::InstrumentationData _data =
> \
>  LLDB_GET_INSTRUMENTATION_DATA()) {
>   \
>_recorder.Record(_data.GetSerializer(), _data.GetRegistry(),
>   \
>
>
>
> ___
> 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
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] d10386e - [lldb/Utility] Provide a stringify_append overload for function pointers.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-16T15:35:51-07:00
New Revision: d10386e1779599d217b5b849a079f29dfbe17024

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

LOG: [lldb/Utility] Provide a stringify_append overload for function pointers.

Converting a function pointer to an object pointer is illegal as nothing
requires it to be in the same address space. Add an overload for
function pointers so we don't convert do this illegal conversion, and
simply print out "function pointer".

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 3b5dde3d2e2a..75f38929e362 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -34,12 +34,17 @@ inline void stringify_append(llvm::raw_string_ostream &ss, 
const T &t) {
 
 template 
 inline void stringify_append(llvm::raw_string_ostream &ss, T *t) {
-  ss << reinterpret_cast(t);
+  ss << static_cast(t);
 }
 
 template 
 inline void stringify_append(llvm::raw_string_ostream &ss, const T *t) {
-  ss << reinterpret_cast(t);
+  ss << static_cast(t);
+}
+
+template 
+inline void stringify_append(llvm::raw_string_ostream &ss, T (*t)(Args...)) {
+  ss << "function pointer";
 }
 
 template <>



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

[Lldb-commits] [lldb] 7fa342b - Remove attach-failed-due-to-SIP checks which were not working

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2020-04-16T15:22:14-07:00
New Revision: 7fa342bd2a6be51998c399f145143d8f45da1f4d

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

LOG: Remove attach-failed-due-to-SIP checks which were not working

The SIP debugserver was calling in attach_failed_due_to_sip
haven't worked for a while; remove them.  To check this
properly we'd need debugsever to call out to codesign(1) to
inspect the entitlements, or the equivalant API,
and I'm not interested in adding that at this point.  SIP
is has been the default on macOS for a couple of releases
and it's expected behavior now.



Added: 


Modified: 
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/RNBRemote.cpp 
b/lldb/tools/debugserver/source/RNBRemote.cpp
index 8eed06381d3a..df358065f877 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3663,30 +3663,6 @@ static bool process_does_not_exist (nub_process_t pid) {
   return true; // process does not exist
 }
 
-static bool attach_failed_due_to_sip (nub_process_t pid) {
-  bool retval = false;
-#if defined(__APPLE__) &&  
\
-  (__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 101000)
-
-  // csr_check(CSR_ALLOW_TASK_FOR_PID) will be nonzero if System Integrity
-  // Protection is in effect.
-  if (csr_check(CSR_ALLOW_TASK_FOR_PID) == 0) 
-return false;
-
-  if (rootless_allows_task_for_pid(pid) == 0)
-retval = true;
-
-  int csops_flags = 0;
-  int csops_ret = ::csops(pid, CS_OPS_STATUS, &csops_flags,
-   sizeof(csops_flags));
-  if (csops_ret != -1 && (csops_flags & CS_RESTRICT)) {
-retval = true;
-  }
-#endif
-
-  return retval;
-}
-
 // my_uid and process_uid are only initialized if this function
 // returns true -- that there was a uid mismatch -- and those
 // id's may want to be used in the error message.
@@ -4065,13 +4041,6 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
"processes.");
   return SendPacket(return_message.c_str());
 }
-if (attach_failed_due_to_sip (pid_attaching_to)) {
-  DNBLogError("Attach failed because of SIP protection.");
-  std::string return_message = "E96;";
-  return_message += cstring_to_asciihex_string("cannot attach "
-"to process due to System Integrity Protection");
-  return SendPacket(return_message.c_str());
-}
   }
 
   std::string error_explainer = "attach failed";



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

Re: [Lldb-commits] [lldb] 9f6a308 - [lldb/Utility] Fix a bug in stringify_append for printing addresses.

[Davidino Italiano via lldb-commits]

> On Apr 16, 2020, at 2:37 PM, Shafik Yaghmour via lldb-commits 
>  wrote:
> 
>> 
>> On Apr 16, 2020, at 2:04 PM, Jonas Devlieghere via lldb-commits 
>>  wrote:
>> 
>> 
>> Author: Jonas Devlieghere
>> Date: 2020-04-16T14:03:55-07:00
>> New Revision: 9f6a308457d1bfebf1cee94b0306e738f270e512
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512.diff
>> 
>> LOG: [lldb/Utility] Fix a bug in stringify_append for printing addresses.
>> 
>> The recent change in the API macros revealed that we were not printing
>> the pointer address for a bunch of methods, but rather the address of
>> the pointer. It's something I had already noticed while looking at some
>> reproducer traces, but hadn't made it to the top of my list yet. This
>> fixes the issue by providing a more specific overload.
>> 
>> 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 3728e19386d1..3b5dde3d2e2a 100644
>> --- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
>> +++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
>> @@ -32,6 +32,11 @@ inline void stringify_append(llvm::raw_string_ostream 
>> &ss, const T &t) {
>>  ss << &t;
>> }
>> 
> 
> 
> Note you can use the narrower static_cast
> 

That would be wrong.

>> +template 
>> +inline void stringify_append(llvm::raw_string_ostream &ss, T *t) {
>> +  ss << reinterpret_cast(t);
>> +}
>> +
>> template 
>> inline void stringify_append(llvm::raw_string_ostream &ss, const T *t) {
>>  ss << reinterpret_cast(t);
>> @@ -115,7 +120,7 @@ template  inline std::string 
>> stringify_args(const Ts &... ts) {
>> 
>> #define LLDB_CONSTRUCT_(T, ...)  
>>   \
>>  lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,   
>>  \
>> -  stringify_args(__VA_ARGS__)); 
>>\
>> +  stringify_args(this, 
>> __VA_ARGS__));  \
>>  if (lldb_private::repro::InstrumentationData _data =
>>  \
>>  LLDB_GET_INSTRUMENTATION_DATA()) {  
>>  \
>>_recorder.Record(_data.GetSerializer(), _data.GetRegistry(),  
>>  \
>> 
>> 
>> 
>> ___
>> 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 
> 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] 9f6a308 - [lldb/Utility] Fix a bug in stringify_append for printing addresses.

[Jonas Devlieghere via lldb-commits]

On Thu, Apr 16, 2020 at 2:37 PM Shafik Yaghmour  wrote:

>
> > On Apr 16, 2020, at 2:04 PM, Jonas Devlieghere via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> >
> > Author: Jonas Devlieghere
> > Date: 2020-04-16T14:03:55-07:00
> > New Revision: 9f6a308457d1bfebf1cee94b0306e738f270e512
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512
> > DIFF:
> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512.diff
> >
> > LOG: [lldb/Utility] Fix a bug in stringify_append for printing addresses.
> >
> > The recent change in the API macros revealed that we were not printing
> > the pointer address for a bunch of methods, but rather the address of
> > the pointer. It's something I had already noticed while looking at some
> > reproducer traces, but hadn't made it to the top of my list yet. This
> > fixes the issue by providing a more specific overload.
> >
> > 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 3728e19386d1..3b5dde3d2e2a 100644
> > --- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
> > +++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
> > @@ -32,6 +32,11 @@ inline void stringify_append(llvm::raw_string_ostream
> &ss, const T &t) {
> >   ss << &t;
> > }
> >
>
>
> Note you can use the narrower static_cast
>

That's not true here because this function is also used for function
pointers.


>
> > +template 
> > +inline void stringify_append(llvm::raw_string_ostream &ss, T *t) {
> > +  ss << reinterpret_cast(t);
> > +}
> > +
> > template 
> > inline void stringify_append(llvm::raw_string_ostream &ss, const T *t) {
> >   ss << reinterpret_cast(t);
> > @@ -115,7 +120,7 @@ template  inline std::string
> stringify_args(const Ts &... ts) {
> >
> > #define LLDB_CONSTRUCT_(T, ...)
>   \
> >   lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,
>   \
> > -
> stringify_args(__VA_ARGS__));\
> > +  stringify_args(this,
> __VA_ARGS__));  \
> >   if (lldb_private::repro::InstrumentationData _data =
>\
> >   LLDB_GET_INSTRUMENTATION_DATA()) {
>\
> > _recorder.Record(_data.GetSerializer(), _data.GetRegistry(),
>\
> >
> >
> >
> > ___
> > 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

[Lldb-commits] [PATCH] D78333: Add Objective-C property accessors loaded from Clang module DWARF to lookup

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: teemperor, shafik.
aprantl added a parent revision: D78329: Allow lldb-test to combine -find with 
-dump-clang-ast.

This patch fixes a bug when synthesizing an ObjC property from -gmodules debug 
info. Because the method declaration that is injected via the non-modular 
property implementation is not added to the ObjCInterfaceDecl's lookup pointer, 
a second copy of the accessor would be generated when processing the 
ObjCPropertyDecl. This can be avoided by finding the existing method decl in 
ClangExternalASTSourceCallbacks::FindExternalVisibleDeclsByName() and adding it 
to the LookupPtr.


https://reviews.llvm.org/D78333

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm


Index: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
===
--- lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
+++ lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
@@ -27,14 +27,21 @@
 // CHECK-DAG: EnumDecl {{.*}} imported in A {{.*}} Enum_e
 // FIXME: -EnumConstantDecl {{.*}} imported in A a
 
+@implementation SomeClass {
+  int private_ivar;
+}
+@synthesize number = private_ivar;
+@end
+
 SomeClass *obj1;
 // RUN: lldb-test symbols -dump-clang-ast -find type --language=ObjC++ \
 // RUN:   -compiler-context 'Module:A,Struct:SomeClass' %t.o \
 // RUN:   | FileCheck %s --check-prefix=CHECK-OBJC
 // CHECK-OBJC: ObjCInterfaceDecl {{.*}} imported in A SomeClass
-// CHECK-OBJC: |-ObjCPropertyDecl {{.*}} imported in A number 'int' readonly
-// CHECK-OBJC: | `-getter ObjCMethod {{.*}} 'number'
-// CHECK-OBJC: `-ObjCMethodDecl {{.*}} imported in A implicit - number 'int'
+// CHECK-OBJC-NEXT: |-ObjCIvarDecl
+// CHECK-OBJC-NEXT: |-ObjCMethodDecl 0x[[NUMBER:[0-9a-f]+]]{{.*}} imported in A
+// CHECK-OBJC-NEXT: `-ObjCPropertyDecl {{.*}} imported in A number 'int' 
readonly
+// CHECK-OBJC-NEXT:   `-getter ObjCMethod 0x[[NUMBER]] 'number'
 
 // Template specializations are not yet supported, so they lack the ownership 
info:
 Template t2;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -344,6 +344,13 @@
   member->setFromASTFile();
   member->setOwningModuleID(id.GetValue());
   member->setModuleOwnershipKind(clang::Decl::ModuleOwnershipKind::Visible);
+  if (auto *nd = llvm::dyn_cast(member))
+if (auto *dc = llvm::dyn_cast(parent)) {
+  // This triggers ExternalASTSource::FindExternalVisibleDeclsByName() to 
be
+  // called when searching for members.
+  dc->setHasExternalLexicalStorage(true);
+  dc->setHasExternalVisibleStorage(true);
+}
 }
 
 char TypeSystemClang::ID;
Index: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
@@ -30,6 +30,9 @@
   llvm::function_ref IsKindWeWant,
   llvm::SmallVectorImpl &Result) override;
 
+  bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC,
+  clang::DeclarationName Name) override;
+
   void CompleteType(clang::TagDecl *tag_decl) override;
 
   void CompleteType(clang::ObjCInterfaceDecl *objc_decl) override;
Index: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
===
--- 
lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
+++ 
lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
@@ -10,6 +10,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
 
 using namespace lldb_private;
 
@@ -46,6 +47,19 @@
   }
 }
 
+bool ClangExternalASTSourceCallbacks::FindExternalVisibleDeclsByName(
+const clang::DeclContext *DC, clang::DeclarationName Name) {
+  llvm::SmallVector decls;
+  // Objective-C methods are not added into the LookupPtr when they originate
+  // from an external source. SetExternalVisibleDeclsForName() adds them.
+  if (auto *oid = llvm::dyn_cast(DC)) {
+for (auto *omd : oid->methods())
+  if (omd->getDeclName() == Name)
+decls.push_back(omd);
+  }
+  return !SetExternalVisibleDeclsForName(DC, Name, decls).empty();
+}
+
 OptionalClangModuleID
 ClangExternalASTSourceCallbacks::RegisterModule(clang::Module *module) {
   m_modules.push_back(module);


Index: lldb/test/Shell/Sym

Re: [Lldb-commits] [lldb] 9f6a308 - [lldb/Utility] Fix a bug in stringify_append for printing addresses.

[Shafik Yaghmour via lldb-commits]

> On Apr 16, 2020, at 2:04 PM, Jonas Devlieghere via lldb-commits 
>  wrote:
> 
> 
> Author: Jonas Devlieghere
> Date: 2020-04-16T14:03:55-07:00
> New Revision: 9f6a308457d1bfebf1cee94b0306e738f270e512
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512
> DIFF: 
> https://github.com/llvm/llvm-project/commit/9f6a308457d1bfebf1cee94b0306e738f270e512.diff
> 
> LOG: [lldb/Utility] Fix a bug in stringify_append for printing addresses.
> 
> The recent change in the API macros revealed that we were not printing
> the pointer address for a bunch of methods, but rather the address of
> the pointer. It's something I had already noticed while looking at some
> reproducer traces, but hadn't made it to the top of my list yet. This
> fixes the issue by providing a more specific overload.
> 
> 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 3728e19386d1..3b5dde3d2e2a 100644
> --- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
> +++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
> @@ -32,6 +32,11 @@ inline void stringify_append(llvm::raw_string_ostream &ss, 
> const T &t) {
>   ss << &t;
> }
> 


Note you can use the narrower static_cast

> +template 
> +inline void stringify_append(llvm::raw_string_ostream &ss, T *t) {
> +  ss << reinterpret_cast(t);
> +}
> +
> template 
> inline void stringify_append(llvm::raw_string_ostream &ss, const T *t) {
>   ss << reinterpret_cast(t);
> @@ -115,7 +120,7 @@ template  inline std::string 
> stringify_args(const Ts &... ts) {
> 
> #define LLDB_CONSTRUCT_(T, ...)   
>  \
>   lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,   
>  \
> -  stringify_args(__VA_ARGS__));  
>   \
> +  stringify_args(this, 
> __VA_ARGS__));  \
>   if (lldb_private::repro::InstrumentationData _data =
>  \
>   LLDB_GET_INSTRUMENTATION_DATA()) {  
>  \
> _recorder.Record(_data.GetSerializer(), _data.GetRegistry(),  
>  \
> 
> 
> 
> ___
> 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

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Remove duplication similar to D77602 .

Honestly, at this point I'm not convinced that the difference between active 
and passive replay warrants special handling. Other than the slightly different 
macros, which are unfortunate but OTOH also a good indicator that something 
special is happening,  there's not that much more to support active/passive 
replay than there is for the generic case.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77759

Files:
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBFileSpec.cpp
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBThread.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -131,7 +131,7 @@
   int C(float *c);
   float GetC();
   int D(const char *d) const;
-  void GetD(char *buffer, size_t length);
+  size_t GetD(char *buffer, size_t length);
   static void E(double e);
   double GetE();
   static int F();
@@ -254,10 +254,11 @@
   return 2;
 }
 
-void InstrumentedFoo::GetD(char *buffer, size_t length) {
-  LLDB_RECORD_METHOD(void, InstrumentedFoo, GetD, (char *, size_t), buffer,
- length);
+size_t InstrumentedFoo::GetD(char *buffer, size_t length) {
+  LLDB_RECORD_CHAR_PTR_METHOD(size_t, InstrumentedFoo, GetD, (char *, size_t),
+  buffer, "", length);
   ::snprintf(buffer, length, "%s", m_d.c_str());
+  return m_d.size();
 }
 
 void InstrumentedFoo::E(double e) {
@@ -371,7 +372,7 @@
   LLDB_REGISTER_METHOD(int, InstrumentedFoo, GetA, ());
   LLDB_REGISTER_METHOD(int &, InstrumentedFoo, GetB, ());
   LLDB_REGISTER_METHOD(float, InstrumentedFoo, GetC, ());
-  LLDB_REGISTER_METHOD(void, InstrumentedFoo, GetD, (char *, size_t));
+  LLDB_REGISTER_METHOD(size_t, InstrumentedFoo, GetD, (char *, size_t));
   LLDB_REGISTER_METHOD(double, InstrumentedFoo, GetE, ());
   LLDB_REGISTER_METHOD(bool, InstrumentedFoo, GetF, ());
 }
@@ -811,6 +812,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
   }
@@ -836,6 +840,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
   }
@@ -917,6 +924,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -948,6 +958,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -982,6 +995,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -1013,6 +1029,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -1047,6 +1066,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
@@ -1078,6 +1100,9 @@
 EXPECT_EQ(foo.GetA(), 100);
 EXPECT_EQ(foo.GetB(), 200);
 EXPECT_NEAR(foo.GetC(), 300.3, 0.01);
+char buffer[100];
+foo.GetD(buffer, 100);
+EXPECT_STREQ(buffer, "bar");
 EXPECT_NEAR(foo.GetE(), 400.4, 0.01);
 EXPECT_EQ(foo.GetF(), true);
 
Index: lldb/source/API/SBThread.cpp
===
--- lldb/source/API/SBThread.cpp
+++ lldb/source/API/SBThread.cpp
@@ -312,8 +312,8 @@
 }
 
 size_t SBThread::GetStopDescription(char *dst, 

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:305
 if (is_trivially_serializable::value)
-  return;
+  return const_cast(t);
 // We need to make a copy as the original object might go out of scope.

labath wrote:
> JDevlieghere wrote:
> > labath wrote:
> > > What's up with the `const_cast` ? Should this maybe take a `T &t` 
> > > argument and let `T` be deduced as `const U` when needed?
> > I need to decode the template instantiation error for the details, but we 
> > need to return a non-const T. 
> It would be good to at least know the reason for that, because const_casts 
> smell..
The problem is that you cannot cannot bind a non-const lvalue reference of type 
`T&` to an rvalue of type `T`.



Comment at: lldb/source/API/SBReproducerPrivate.h:80-85
+  FileSpec file = l->GetFile();
+  static auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
+  if (!error_or_file)
+return {};
+  static ReplayData r((*error_or_file)->getBuffer());
+  return {r.GetDeserializer(), r.GetRegistry()};

labath wrote:
> Could this be done in the initialization code somewhere (inside 
> `Reproducer::Initialize`, I guess)? We could avoid static variables and get 
> better error handling that way...
Not if we want to keep the `SBProvider` in API instead of Utility. 


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

https://reviews.llvm.org/D77602



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

[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: shafik, teemperor, jingham, labath.

This patch threads an lldb::DescriptionLevel through the typesystem to allow 
dumping the full Clang AST (level=verbose) of any lldb::Type in addition to the 
human-readable source description (default level=full). This type dumping 
interface is currently not exposed through the SBAPI.

The application is to let lldb-test dump the clang AST of search results. I 
need this to test lazy type completion of clang types in subsequent patches.


https://reviews.llvm.org/D78329

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/Type.h
  lldb/include/lldb/Symbol/TypeMap.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Symbol/TypeMap.cpp
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h
  lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -192,7 +192,7 @@
 static Error findVariables(lldb_private::Module &Module);
 static Error dumpModule(lldb_private::Module &Module);
 static Error dumpAST(lldb_private::Module &Module);
-static Error dumpClangAST(lldb_private::Module &Module);
+static Error dumpEntireClangAST(lldb_private::Module &Module);
 static Error verify(lldb_private::Module &Module);
 
 static Expected getAction();
@@ -404,6 +404,10 @@
   return List.GetVariableAtIndex(0)->GetDeclContext();
 }
 
+static lldb::DescriptionLevel GetDescriptionLevel() {
+  return opts::symbols::DumpClangAST ? eDescriptionLevelVerbose : eDescriptionLevelFull;
+}
+
 Error opts::symbols::findFunctions(lldb_private::Module &Module) {
   SymbolFile &Symfile = *Module.GetSymbolFile();
   SymbolContextList List;
@@ -534,7 +538,7 @@
 
   outs() << formatv("Found {0} types:\n", Map.GetSize());
   StreamString Stream;
-  Map.Dump(&Stream, false);
+  Map.Dump(&Stream, false, GetDescriptionLevel());
   outs() << Stream.GetData() << "\n";
   return Error::success();
 }
@@ -615,7 +619,7 @@
   return Error::success();
 }
 
-Error opts::symbols::dumpClangAST(lldb_private::Module &Module) {
+Error opts::symbols::dumpEntireClangAST(lldb_private::Module &Module) {
   Module.ParseAllDebugSymbols();
 
   SymbolFile *symfile = Module.GetSymbolFile();
@@ -719,13 +723,17 @@
   }
 
   if (DumpClangAST) {
-if (Find != FindType::None)
-  return make_string_error("Cannot both search and dump clang AST.");
-if (Regex || !Context.empty() || !File.empty() || Line != 0)
-  return make_string_error(
-  "-regex, -context, -name, -file and -line options are not "
-  "applicable for dumping clang AST.");
-return dumpClangAST;
+if (Find == FindType::None) {
+  if (Regex || !Context.empty() || !File.empty() || Line != 0)
+return make_string_error(
+"-regex, -context, -name, -file and -line options are not "
+"applicable for dumping the entire clang AST. Either combine with "
+"-find, or use -dump-clang-ast as a standalone option.");
+  return dumpEntireClangAST;
+}
+if (Find != FindType::Type)
+  return make_string_error("This combination of -dump-clang-ast and -find "
+   " is not yet implemented.");
   }
 
   if (Regex && !Context.empty())
Index: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
===
--- lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
+++ lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
@@ -1,7 +1,6 @@
 // RUN: %clang --target=x86_64-apple-macosx -g -gmodules \
 // RUN:-fmodules -fmodules-cache-path=%t.cache \
 // RUN:-c -o %t.o %s -I%S/Inputs
-// RUN: lldb-test symbols -dump-clang-ast %t.o | FileCheck %s
 // Verify that the owning module information from DWARF is preserved in the AST.
 
 @import A;
@@ -29,7 +28,13 @@
 // FIXME: -EnumConstantDecl {{.*}} imported in A a
 
 SomeClass *obj1;
-// CHECK-DAG: ObjCInterfaceDecl {{.*}} imported in A {{.*}} SomeClass
+// RUN: lldb-test symbols -dump-clang-ast -find type --language=ObjC++ \
+// RUN:   -compiler-context 'Module:A,Struct:SomeClass' %t.o \
+// RUN:   | FileCheck %s --check-prefix=CHECK-OBJC
+// CHECK-OBJC: ObjCInterfaceDecl {{.*}} imported in A SomeClass
+// CHECK-OBJC: |-ObjCPropertyDecl {{.*}} imported in A number 'int' readonly
+// CHECK-OBJC: | `-getter ObjCMethod {{.*}} 'number'
+// CHECK-OBJC: `-ObjCMethodDecl {{.*}} imported in A implicit - number 'int'
 
 // Template specializations are not yet supported, so they lack the ownership info:
 Template t2;
Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h
==

Re: [Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to regenerate this test, if needed.

[Davidino Italiano via lldb-commits]

> On Apr 16, 2020, at 2:01 PM, Robinson, Paul  wrote:
> 
> 
> 
>> -Original Message-
>> From: lldb-commits > > On Behalf Of
>> Davide Italiano via lldb-commits
>> Sent: Thursday, April 16, 2020 4:32 PM
>> To: lldb-commits@lists.llvm.org 
>> Subject: [Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to
>> regenerate this test, if needed.
>> 
>> 
>> Author: Davide Italiano
>> Date: 2020-04-16T13:31:32-07:00
>> New Revision: 1fae85a8534ec51ca893899314bd244b3e9684c7
>> 
>> URL: https://github.com/llvm/llvm-
>> project/commit/1fae85a8534ec51ca893899314bd244b3e9684c7
>> DIFF: https://github.com/llvm/llvm-
>> project/commit/1fae85a8534ec51ca893899314bd244b3e9684c7.diff
>> 
>> LOG: [DWARF] Add instructions to regenerate this test, if needed.
>> 
>> Added:
>> 
>> 
>> Modified:
>>lldb/test/Shell/SymbolFile/DWARF/static_scope.s
>> 
>> Removed:
>> 
>> 
>> 
>> ##
>> ##
>> diff  --git a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
>> b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
>> index 84a69e08ecfc..02d497ac9ccb 100644
>> --- a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
>> +++ b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
>> @@ -3,6 +3,15 @@
>> 
>> # REQUIRES: x86
>> 
>> +# Original test case (for future reference), compiled with:
>> +# $ clang-10 -g -Og test.c -o test
>> +# $ cat test.c
>> +# volatile int a;
>> +# main() {
>> +#   int b = 3;
> 
> Did you mean this to be `static`?
> --paulr
> 

No. Read the the review that introduced this test.

>> +#   a;
>> +# }
>> +
>> # RUN: llvm-mc -triple=x86_64-apple-macosx10.15.0 -filetype=obj %s > %t.o
>> # RUN: lldb-test symbols %t.o | FileCheck %s
>> 
>> 
>> 
>> 
>> ___
>> 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

[Lldb-commits] [lldb] 9f6a308 - [lldb/Utility] Fix a bug in stringify_append for printing addresses.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-16T14:03:55-07:00
New Revision: 9f6a308457d1bfebf1cee94b0306e738f270e512

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

LOG: [lldb/Utility] Fix a bug in stringify_append for printing addresses.

The recent change in the API macros revealed that we were not printing
the pointer address for a bunch of methods, but rather the address of
the pointer. It's something I had already noticed while looking at some
reproducer traces, but hadn't made it to the top of my list yet. This
fixes the issue by providing a more specific overload.

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 3728e19386d1..3b5dde3d2e2a 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -32,6 +32,11 @@ inline void stringify_append(llvm::raw_string_ostream &ss, 
const T &t) {
   ss << &t;
 }
 
+template 
+inline void stringify_append(llvm::raw_string_ostream &ss, T *t) {
+  ss << reinterpret_cast(t);
+}
+
 template 
 inline void stringify_append(llvm::raw_string_ostream &ss, const T *t) {
   ss << reinterpret_cast(t);
@@ -115,7 +120,7 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
 
 #define LLDB_CONSTRUCT_(T, ...)
\
   lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,
\
-  stringify_args(__VA_ARGS__));
\
+  stringify_args(this, __VA_ARGS__));  
\
   if (lldb_private::repro::InstrumentationData _data = 
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
 _recorder.Record(_data.GetSerializer(), _data.GetRegistry(),   
\



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

Re: [Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to regenerate this test, if needed.

[Robinson, Paul via lldb-commits]

> -Original Message-
> From: lldb-commits  On Behalf Of
> Davide Italiano via lldb-commits
> Sent: Thursday, April 16, 2020 4:32 PM
> To: lldb-commits@lists.llvm.org
> Subject: [Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to
> regenerate this test, if needed.
> 
> 
> Author: Davide Italiano
> Date: 2020-04-16T13:31:32-07:00
> New Revision: 1fae85a8534ec51ca893899314bd244b3e9684c7
> 
> URL: https://github.com/llvm/llvm-
> project/commit/1fae85a8534ec51ca893899314bd244b3e9684c7
> DIFF: https://github.com/llvm/llvm-
> project/commit/1fae85a8534ec51ca893899314bd244b3e9684c7.diff
> 
> LOG: [DWARF] Add instructions to regenerate this test, if needed.
> 
> Added:
> 
> 
> Modified:
> lldb/test/Shell/SymbolFile/DWARF/static_scope.s
> 
> Removed:
> 
> 
> 
> ##
> ##
> diff  --git a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
> b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
> index 84a69e08ecfc..02d497ac9ccb 100644
> --- a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
> +++ b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
> @@ -3,6 +3,15 @@
> 
>  # REQUIRES: x86
> 
> +# Original test case (for future reference), compiled with:
> +# $ clang-10 -g -Og test.c -o test
> +# $ cat test.c
> +# volatile int a;
> +# main() {
> +#   int b = 3;

Did you mean this to be `static`?
--paulr

> +#   a;
> +# }
> +
>  # RUN: llvm-mc -triple=x86_64-apple-macosx10.15.0 -filetype=obj %s > %t.o
>  # RUN: lldb-test symbols %t.o | FileCheck %s
> 
> 
> 
> 
> ___
> 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

[Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to regenerate this test, if needed.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-04-16T13:31:32-07:00
New Revision: 1fae85a8534ec51ca893899314bd244b3e9684c7

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

LOG: [DWARF] Add instructions to regenerate this test, if needed.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/static_scope.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s 
b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
index 84a69e08ecfc..02d497ac9ccb 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
@@ -3,6 +3,15 @@
 
 # REQUIRES: x86
 
+# Original test case (for future reference), compiled with:
+# $ clang-10 -g -Og test.c -o test
+# $ cat test.c
+# volatile int a;
+# main() {
+#   int b = 3;
+#   a;
+# }
+
 # RUN: llvm-mc -triple=x86_64-apple-macosx10.15.0 -filetype=obj %s > %t.o
 # RUN: lldb-test symbols %t.o | FileCheck %s
 



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

[Lldb-commits] [lldb] 8cac6d1 - [Shell] Remove incorrectly cargo-culted UNSUPPORTED.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-04-16T13:22:46-07:00
New Revision: 8cac6d1875e094f2b78621f3ff12e61553cd12ec

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

LOG: [Shell] Remove incorrectly cargo-culted UNSUPPORTED.

Let's see if this sticks on the bots.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/static_scope.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s 
b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
index 17b248579849..84a69e08ecfc 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
@@ -2,7 +2,6 @@
 # variable `b`, which is `local` and not `static`.
 
 # REQUIRES: x86
-# UNSUPPORTED: lldb-repro
 
 # RUN: llvm-mc -triple=x86_64-apple-macosx10.15.0 -filetype=obj %s > %t.o
 # RUN: lldb-test symbols %t.o | FileCheck %s



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

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 258123.
JDevlieghere marked 8 inline comments as done.

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

https://reviews.llvm.org/D77602

Files:
  lldb/include/lldb/Utility/Reproducer.h
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBReproducerPrivate.h
  lldb/source/Utility/Reproducer.cpp
  lldb/source/Utility/ReproducerInstrumentation.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -52,23 +52,53 @@
 
 static llvm::Optional g_registry;
 static llvm::Optional g_serializer;
+static llvm::Optional g_deserializer;
 
 inline InstrumentationData GetTestInstrumentationData() {
+  assert(!(g_serializer && g_deserializer));
   if (g_serializer)
 return InstrumentationData(*g_serializer, *g_registry);
+  if (g_deserializer)
+return InstrumentationData(*g_deserializer, *g_registry);
   return InstrumentationData();
 }
 
 class TestInstrumentationDataRAII {
-public:
+private:
   TestInstrumentationDataRAII(llvm::raw_string_ostream &os) {
 g_registry.emplace();
 g_serializer.emplace(os);
+g_deserializer.reset();
+  }
+
+  TestInstrumentationDataRAII(llvm::StringRef buffer) {
+g_registry.emplace();
+g_serializer.reset();
+g_deserializer.emplace(buffer);
   }
 
-  ~TestInstrumentationDataRAII() {
+  friend std::unique_ptr
+  std::make_unique(llvm::raw_string_ostream &os);
+  friend std::unique_ptr
+  std::make_unique(llvm::StringRef &buffer);
+
+public:
+  ~TestInstrumentationDataRAII() { Reset(); }
+
+  void Reset() {
 g_registry.reset();
 g_serializer.reset();
+g_deserializer.reset();
+  }
+
+  static std::unique_ptr
+  GetRecordingData(llvm::raw_string_ostream &os) {
+return std::make_unique(os);
+  }
+
+  static std::unique_ptr
+  GetReplayData(llvm::StringRef buffer) {
+return std::make_unique(buffer);
   }
 };
 
@@ -95,11 +125,17 @@
   InstrumentedFoo(const InstrumentedFoo &foo);
   InstrumentedFoo &operator=(const InstrumentedFoo &foo);
   void A(int a);
+  int GetA();
   void B(int &b) const;
+  int &GetB();
   int C(float *c);
+  float GetC();
   int D(const char *d) const;
+  void GetD(char *buffer, size_t length);
   static void E(double e);
+  double GetE();
   static int F();
+  bool GetF();
   void Validate() override;
    }
   virtual bool IsA(Class c) override { return c == Class::Foo; }
@@ -182,35 +218,71 @@
   m_a = a;
 }
 
+int InstrumentedFoo::GetA() {
+  LLDB_RECORD_METHOD_NO_ARGS(int, InstrumentedFoo, GetA);
+
+  return m_a;
+}
+
 void InstrumentedFoo::B(int &b) const {
   LLDB_RECORD_METHOD_CONST(void, InstrumentedFoo, B, (int &), b);
   m_called++;
   m_b = b;
 }
 
+int &InstrumentedFoo::GetB() {
+  LLDB_RECORD_METHOD_NO_ARGS(int &, InstrumentedFoo, GetB);
+
+  return m_b;
+}
+
 int InstrumentedFoo::C(float *c) {
   LLDB_RECORD_METHOD(int, InstrumentedFoo, C, (float *), c);
   m_c = *c;
   return 1;
 }
 
+float InstrumentedFoo::GetC() {
+  LLDB_RECORD_METHOD_NO_ARGS(float, InstrumentedFoo, GetC);
+
+  return m_c;
+}
+
 int InstrumentedFoo::D(const char *d) const {
   LLDB_RECORD_METHOD_CONST(int, InstrumentedFoo, D, (const char *), d);
   m_d = std::string(d);
   return 2;
 }
 
+void InstrumentedFoo::GetD(char *buffer, size_t length) {
+  LLDB_RECORD_METHOD(void, InstrumentedFoo, GetD, (char *, size_t), buffer,
+ length);
+  ::snprintf(buffer, length, "%s", m_d.c_str());
+}
+
 void InstrumentedFoo::E(double e) {
   LLDB_RECORD_STATIC_METHOD(void, InstrumentedFoo, E, (double), e);
   g_e = e;
 }
 
+double InstrumentedFoo::GetE() {
+  LLDB_RECORD_METHOD_NO_ARGS(double, InstrumentedFoo, GetE);
+
+  return g_e;
+}
+
 int InstrumentedFoo::F() {
   LLDB_RECORD_STATIC_METHOD_NO_ARGS(int, InstrumentedFoo, F);
   g_f = true;
   return 3;
 }
 
+bool InstrumentedFoo::GetF() {
+  LLDB_RECORD_METHOD_NO_ARGS(bool, InstrumentedFoo, GetF);
+
+  return g_f;
+}
+
 void InstrumentedFoo::Validate() {
   LLDB_RECORD_METHOD_NO_ARGS(void, InstrumentedFoo, Validate);
   EXPECT_EQ(m_a, 100);
@@ -296,6 +368,12 @@
   LLDB_REGISTER_METHOD(void, InstrumentedBar, SetInstrumentedFoo,
(InstrumentedFoo &));
   LLDB_REGISTER_METHOD(void, InstrumentedBar, Validate, ());
+  LLDB_REGISTER_METHOD(int, InstrumentedFoo, GetA, ());
+  LLDB_REGISTER_METHOD(int &, InstrumentedFoo, GetB, ());
+  LLDB_REGISTER_METHOD(float, InstrumentedFoo, GetC, ());
+  LLDB_REGISTER_METHOD(void, InstrumentedFoo, GetD, (char *, size_t));
+  LLDB_REGISTER_METHOD(double, InstrumentedFoo, GetE, ());
+  LLDB_REGISTER_METHOD(bool, InstrumentedFoo, GetF, ());
 }
 
 static const Pod p;
@@ -534,7 +612,7 @@
   llvm::raw_string_ostream os(str);
 

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere closed this revision.
JDevlieghere added a comment.

Closed by 3a6b60fa623da6e311b61c812932917085067cd3 



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

https://reviews.llvm.org/D78242



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

[Lldb-commits] [PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked an inline comment as done.
shafik added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:8173
+  // If a RecordDecl has base classes they won't be imported and we will
+  // be missing anything that we inherit from those bases.
+  if (FromRecord->hasExternalLexicalStorage() &&

teemperor wrote:
> I'm not sure how it can be that ASTImporter::CompleteDecl starts but never 
> finishes the decl as it does both things at once unconditionally?
> ```
> lang=c++
>   TD->startDefinition();
>   TD->setCompleteDefinition(true);
> ```
> 
> FWIW, this whole comment could just be `Ask the external source if this is 
> actually a complete record that just hasn't been completed yet` or something 
> like this. I think if we reach this point then it makes a complete sense to 
> ask the external source for more info. The explanation about the base classes 
> seems to be just a smaller detail of the whole picture here.
`TD->setCompleteDefinition(true);` just sets a flag but it does not import the 
bases, which it can not do since From is not defined. See 
`ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To, 
ImportDefinitionKind Kind)` which does this after it does 
`To->startDefinition();`. So we can't really consider the definition finished 
if we don't do this. 

Do you have a better way of describing this situation?

I think it is important to describe why we don't use `CompleteDecl` since the 
other cases do.



Comment at: clang/lib/AST/ASTImporter.cpp:8173
+  // If a RecordDecl has base classes they won't be imported and we will
+  // be missing anything that we inherit from those bases.
+  if (FromRecord->hasExternalLexicalStorage() &&

martong wrote:
> shafik wrote:
> > teemperor wrote:
> > > I'm not sure how it can be that ASTImporter::CompleteDecl starts but 
> > > never finishes the decl as it does both things at once unconditionally?
> > > ```
> > > lang=c++
> > >   TD->startDefinition();
> > >   TD->setCompleteDefinition(true);
> > > ```
> > > 
> > > FWIW, this whole comment could just be `Ask the external source if this 
> > > is actually a complete record that just hasn't been completed yet` or 
> > > something like this. I think if we reach this point then it makes a 
> > > complete sense to ask the external source for more info. The explanation 
> > > about the base classes seems to be just a smaller detail of the whole 
> > > picture here.
> > `TD->setCompleteDefinition(true);` just sets a flag but it does not import 
> > the bases, which it can not do since From is not defined. See 
> > `ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To, 
> > ImportDefinitionKind Kind)` which does this after it does 
> > `To->startDefinition();`. So we can't really consider the definition 
> > finished if we don't do this. 
> > 
> > Do you have a better way of describing this situation?
> > 
> > I think it is important to describe why we don't use `CompleteDecl` since 
> > the other cases do.
> > Ask the external source if this is actually a complete record that just 
> > hasn't been completed yet
> 
> FWIW this seems to be a recurring pattern, so maybe it would be worth to do 
> this not just with `RecordDecl`s but with other kind of decls. E.g. an 
> `EnumDecl` could have an external source and not  completed, couldn't it?
This is definitely more costly, for the `EnumDecl` case I don't see how we 
could get in a similar situation since we don't have inheritance. I looked over 
the `EnumDecl` members and I don't see anything that would require it be 
defined.


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

https://reviews.llvm.org/D78000



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

[Lldb-commits] [PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 258084.
shafik added a comment.

- Added unit test
- Modified condition for defining `FromRecord` to  whether it has an 
`ExternalSource`


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

https://reviews.llvm.org/D78000

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py
  
lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp

Index: lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp
@@ -0,0 +1,35 @@
+struct B {
+  int dump() const;
+};
+
+int B::dump() const { return 42; }
+
+// Derived class D obtains dump() method from base class B
+struct D : public B {
+  // Introduce a TypedefNameDecl
+  using Iterator = D *;
+};
+
+struct C {
+  // This will cause use to invoke VisitTypedefNameDecl(...) when Importing
+  // the DeclContext of C.
+  // We will invoke ImportContext(...) which should force the From Decl to
+  // be defined if it not already defined. We will then Import the definition
+  // to the To Decl.
+  // This will force processing of the base class of D which allows us to see
+  // base class methods such as dump().
+  D::Iterator iter;
+
+  bool f(D *DD) {
+return true; //%self.expect_expr("DD->dump()", result_type="int", result_value="42")
+  }
+};
+
+int main() {
+  C c;
+  D d;
+
+  c.f(&d);
+
+  return 0;
+}
Index: lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals())
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5922,6 +5922,72 @@
   EXPECT_TRUE(ToA);
 }
 
+struct ImportWithExternalSource : ASTImporterOptionSpecificTestBase {
+  ImportWithExternalSource() {
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport,
+ const std::shared_ptr &SharedState) {
+  return new ASTImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, MinimalImport,
+ // We use the regular lookup.
+ /*SharedState=*/nullptr);
+};
+  }
+};
+
+/// An ExternalASTSource that keeps track of the tags is completed.
+struct SourceWithCompletedTagList : clang::ExternalASTSource {
+  std::vector &CompletedTags;
+  SourceWithCompletedTagList(std::vector &CompletedTags)
+  : CompletedTags(CompletedTags) {}
+  void CompleteType(TagDecl *Tag) override {
+auto *Record = cast(Tag);
+Record->startDefinition();
+Record->completeDefinition();
+CompletedTags.push_back(Tag);
+  }
+  void
+  FindExternalLexicalDecls(const DeclContext *DC,
+   llvm::function_ref IsKindWeWant,
+   SmallVectorImpl &Result) override {
+DC->setHasExternalLexicalStorage(true);
+  }
+};
+
+TEST_P(ImportWithExternalSource, CompleteRecordBeforeImporting) {
+  // Create an empty TU.
+  TranslationUnitDecl *FromTU = getTuDecl("", Lang_CXX, "input.cpp");
+
+  // Create and add the test ExternalASTSource.
+  std::vector CompletedTags;
+  IntrusiveRefCntPtr source =
+  new SourceWithCompletedTagList(CompletedTags);
+  clang::ASTContext &context = FromTU->getASTContext();
+  context.setExternalSource(std::move(source));
+
+  // Create a dummy class by hand with external lexical storage.
+  IdentifierInfo &Ident = context.Idents.get("test_class");
+  auto *Record = CXXRecordDecl::Create(
+  context, TTK_Class, FromTU, SourceLocation(), SourceLocation(), &Ident);
+  Record->setHasExternalLexicalStorage();
+  FromTU->addDecl(Record);
+
+  // Do a minimal import of the created class.
+  EXPECT_EQ(0U, CompletedTags.size());
+  Decl *Imported = Import(Record, Lang_CXX);
+  EXPECT_EQ(0U, CompletedTags.size());
+
+  // Import the definition of the created class.
+  llvm::Error err = findFromTU(Record)->Importer->ImportDefinition(Record);
+  EXPECT_FALSE((bool)err);
+  consumeError(std::move(err));
+
+  // Make sure the class was completed once.
+  EXPECT_EQ(1U, CompletedTags.size());
+  EXPECT_EQ(Record, Compl

[Lldb-commits] [lldb] 3a6b60f - [lldb/Docs] Add some more info about the test suite structure

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-16T10:17:59-07:00
New Revision: 3a6b60fa623da6e311b61c812932917085067cd3

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

LOG: [lldb/Docs] Add some more info about the test suite structure

Expand on the structure of the LLDB test suite. So far this information
has been mostly "tribal knowledge". By writing it down I hope to make it
easier to understand our test suite for anyone that's new to the
project.

Added: 


Modified: 
lldb/docs/resources/test.rst

Removed: 




diff  --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index dd40a1e51549..6f39a45d4b72 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -1,27 +1,200 @@
 Testing
 ===
 
+.. contents::
+   :local:
+
+Test Suite Structure
+
+
 The LLDB test suite consists of three 
diff erent kinds of test:
 
-* Unit test. These are located under ``lldb/unittests`` and are written in C++
-  using googletest.
-* Integration tests that test the debugger through the SB API. These are
-  located under ``lldb/packages/Python/lldbsuite`` and are written in Python
-  using ``dotest`` (LLDB's custom testing framework on top of unittest2).
-* Integration tests that test the debugger through the command line. These are
-  located under `lldb/test/Shell` and are written in a shell-style format
-  using FileCheck to verify its output.
+* **Unit tests**: written in C++ using the googletest unit testing library.
+* **Shell tests**: Integration tests that test the debugger through the command
+  line. These tests interact with the debugger either through the command line
+  driver or through ``lldb-test`` which is a tool that exposes the internal
+  data structures in an easy-to-parse way for testing. Most people will know
+  these as *lit tests* in LLVM, although lit is the test driver and ShellTest
+  is the test format that uses ``RUN:`` lines. `FileCheck
+  `_ is used to verify
+  the output.
+* **API tests**: Integration tests that interact with the debugger through the
+  SB API. These are written in Python and use LLDB's ``dotest.py`` testing
+  framework on top of Python's `unittest2
+  `_.
+
+All three test suites use ``lit`` (`LLVM Integrated Tester
+`_ ) as the test driver. The test
+suites can be run as a whole or separately.
+
+
+Unit Tests
+``
+
+Unit tests are located under ``lldb/unittests``. If it's possible to test
+something in isolation or as a single unit, you should make it a unit test.
+
+Often you need instances of the core objects such as a debugger, target or
+process, in order to test something meaningful. We already have a handful of
+tests that have the necessary boiler plate, but this is something we could
+abstract away and make it more user friendly.
+
+Shell Tests
+```
+
+Shell tests are located under ``lldb/test/Shell``. These tests are generally
+built around checking the output of ``lldb`` (the command line driver) or
+``lldb-test`` using ``FileCheck``. Shell tests are generally small and fast to
+write because they require little boilerplate.
+
+``lldb-test`` is a relatively new addition to the test suite. It was the first
+tool that was added that is designed for testing. Since then it has been
+continuously extended with new subcommands, improving our test coverage. Among
+other things you can use it to query lldb for symbol files, for object files
+and breakpoints.
+
+Obviously shell tests are great for testing the command line driver itself or
+the subcomponents already exposed by lldb-test. But when it comes to LLDB's
+vast functionality, most things can be tested both through the driver as well
+as the Python API. For example, to test setting a breakpoint, you could do it
+from the command line driver with ``b main`` or you could use the SB API and do
+something like ``target.BreakpointCreateByName`` [#]_.
+
+A good rule of thumb is to prefer shell tests when what is being tested is
+relatively simple. Expressivity is limited compared to the API tests, which
+means that you have to have a well-defined test scenario that you can easily
+match with ``FileCheck``.
+
+Another thing to consider are the binaries being debugged, which we call
+inferiors. For shell tests, they have to be relatively simple. The
+``dotest.py`` test framework has extensive support for complex build scenarios
+and 
diff erent variants, which is described in more detail below, while shell
+tests are limited to single lines of shell commands with compiler and linker
+invocations.
+
+On the same topic, another interesting aspect of the shell tests is that there
+y

[Lldb-commits] [PATCH] D78141: [lldb/Reproducers] Simplify LLDB_RECORD macros

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3237f861cc2: [lldb/Reproducers] Simplify LLDB_RECORD macros 
(authored by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78141

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

Index: lldb/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -84,21 +84,20 @@
   #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_METHOD_CONST(Result, Class, Method, Signature)   \
-  R.Register(&invoke::method_const<( \
- &Class::Method)>::doit,   \
+  R.Register(&invoke::method<(&Class::Method)>::doit, \
  #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_STATIC_METHOD(Result, Class, Method, Signature)  \
-  R.Register(  \
-  &invoke::method_static<(&Class::Method)>::doit, \
-  #Result, #Class, #Method, #Signature)
+  R.Register(&invoke::method<(&Class::Method)>::doit, \
+ #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_CHAR_PTR_REDIRECT_STATIC(Result, Class, Method)  \
-  R.Register(&invoke::method_static<(  \
- &Class::Method)>::doit,   \
- &char_ptr_redirect::method_static<(   \
- &Class::Method)>::doit,   \
- #Result, #Class, #Method, "(char*, size_t");
+  R.Register(  \
+  &invoke::method<(&Class::Method)>::doit, \
+  &char_ptr_redirect::method<(&Class::Method)>::doit,  \
+  #Result, #Class, #Method, "(char*, size_t");
 
 #define LLDB_REGISTER_CHAR_PTR_REDIRECT(Result, Class, Method) \
   R.Register(&invoke::method<(  \
@@ -109,97 +108,55 @@
 
 #define LLDB_REGISTER_CHAR_PTR_REDIRECT_CONST(Result, Class, Method)   \
   R.Register(&invoke::method_const<(&Class::Method)>::doit, \
- &char_ptr_redirect::method_const<(&Class::Method)>::doit, \
+ const>::method<(&Class::Method)>::doit,   \
+ &char_ptr_redirect::method<(&Class::Method)>::doit,\
  #Result, #Class, #Method, "(char*, size_t");
 
-#define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...) \
+#define LLDB_CONSTRUCT_(T, ...)\
   lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,\
   stringify_args(__VA_ARGS__));\
   if (lldb_private::repro::InstrumentationData _data = \
   LLDB_GET_INSTRUMENTATION_DATA()) {   \
 _recorder.Record(_data.GetSerializer(), _data.GetRegistry(),   \
- &lldb_private::repro::construct::doit,   \
- __VA_ARGS__); \
+ &lldb_private::repro::construct::doit, __VA_ARGS__);   \
 _recorder.RecordResult(this, false);   \
   }
 
+#define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...) \
+  LLDB_CONSTRUCT_(Class Signature, __VA_ARGS__)
+
 #define LLDB_RECORD_CONSTRUCTOR_NO_ARGS(Class) \
-  lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION);   \
-  if (lldb_private::repro::InstrumentationData _data = \
-  LLDB_GET_INSTRUMENTATION_DATA()) {   \
-_recorder.Record(_data.GetSerializer(), _data.GetRegistry(),   \
- &lldb_private::repro::construct::doit);  \
-_recorder.RecordResult(this, false);   \
-  }
+  LLDB_CONSTRUCT_(Class(), lldb_private::repro::EmptyArg())
 
-#define LLDB_RECORD_METHOD(Result, Class, Method, Signature, ...)  \
+#define LLDB_RECORD_(T1, T2, ...)  \
   lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,\
-  stringify_args(*this, __VA_ARGS__)); \
+  stringify_args(__VA_ARGS__));\
   if (lldb_private::repro::InstrumentationData _data = \
   LLDB_GET_INSTRUMENTATION_DATA()) {   \
 _recorder.Record(_data.GetSerializer(), _data.GetRegistry(),   \
-  

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 11 inline comments as done.
JDevlieghere added a comment.

Thanks a lot for the feedback everyone!


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

https://reviews.llvm.org/D78242



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

[Lldb-commits] [lldb] a3237f8 - [lldb/Reproducers] Simplify LLDB_RECORD macros

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-16T09:55:28-07:00
New Revision: a3237f861cc2b4c3cd29d86f0a0212dfd4d38d56

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

LOG: [lldb/Reproducers] Simplify LLDB_RECORD macros

Redefine the LLDB_RECORD macros in terms of a common uber-macro to
reduce code duplication across them.

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

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 8e02f4f8278d..3728e19386d1 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -84,21 +84,20 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
   #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_METHOD_CONST(Result, Class, Method, Signature)   
\
-  R.Register(&invoke::method_const<( 
\
- &Class::Method)>::doit,   
\
+  R.Register(&invoke::method<(&Class::Method)>::doit, 
\
  #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_STATIC_METHOD(Result, Class, Method, Signature)  
\
-  R.Register(  
\
-  &invoke::method_static<(&Class::Method)>::doit, 
\
-  #Result, #Class, #Method, #Signature)
+  R.Register(&invoke::method<(&Class::Method)>::doit, 
\
+ #Result, #Class, #Method, #Signature)
 
 #define LLDB_REGISTER_CHAR_PTR_REDIRECT_STATIC(Result, Class, Method)  
\
-  R.Register(&invoke::method_static<(  
\
- &Class::Method)>::doit,   
\
- &char_ptr_redirect::method_static<(   
\
- &Class::Method)>::doit,   
\
- #Result, #Class, #Method, "(char*, size_t");
+  R.Register(  
\
+  &invoke::method<(&Class::Method)>::doit, 
\
+  &char_ptr_redirect::method<(&Class::Method)>::doit,  
\
+  #Result, #Class, #Method, "(char*, size_t");
 
 #define LLDB_REGISTER_CHAR_PTR_REDIRECT(Result, Class, Method) 
\
   R.Register(&invoke::method<(  
\
@@ -109,97 +108,55 @@ template  inline std::string 
stringify_args(const Ts &... ts) {
 
 #define LLDB_REGISTER_CHAR_PTR_REDIRECT_CONST(Result, Class, Method)   
\
   R.Register(&invoke::method_const<(&Class::Method)>::doit, 
\
- &char_ptr_redirect::method_const<(&Class::Method)>::doit, 
\
+ const>::method<(&Class::Method)>::doit,   
\
+ &char_ptr_redirect::method<(&Class::Method)>::doit,
\
  #Result, #Class, #Method, "(char*, size_t");
 
-#define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...) 
\
+#define LLDB_CONSTRUCT_(T, ...)
\
   lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,
\
   stringify_args(__VA_ARGS__));
\
   if (lldb_private::repro::InstrumentationData _data = 
\
   LLDB_GET_INSTRUMENTATION_DATA()) {   
\
 _recorder.Record(_data.GetSerializer(), _data.GetRegistry(),   
\
- &lldb_private::repro::construct::doit,   
\
- __VA_ARGS__); 
\
+ &lldb_private::repro::construct::doit, __VA_ARGS__);   
\
 _recorder.RecordResult(this, false);   
\
   }
 
+#define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...) 
\
+  LLDB_CONSTRUCT_(Class Signature, __VA_ARGS__)
+
 #define LLDB_RECORD_CONSTRUCTOR_NO_ARGS(Class) 
\
-  lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION);   
\
-  if (lldb_private::repro::InstrumentationData _data = 
\
-  LLDB_GET_INSTRUMENTATION_DATA()) {   
\
-_recorder.Record(_data.GetSerializer(), _data.GetRegistry(),   
\
- &lldb_private::repro::construct::doit);  
\
-_recorder.RecordResult(this, false);   
\
-  }
+  LLDB_CONSTRUCT_(Class(), lldb_private::repro::EmptyArg())
 
-#define LLDB_RECORD_METHOD(Result, Class, Method, Signature, ...)  
\
+#define LLDB_RECORD_(T1, T2, ...)

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

[Yuri Per via Phabricator via lldb-commits]

yuri updated this revision to Diff 258034.
yuri added a comment.

Added @skipIfWindows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529

Files:
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,44 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Copy wrong executable to the location outside of sysroot
+exe_outside = os.path.join(self.getBuildDir(), "bin", "a.out")
+lldbutil.mkdir_p(os.path.dirname(exe_outside))
+shutil.copyfile("altmain.out", exe_outside)
+
+# Copy correct executable to the location inside sysroot
+tmp_sysroot = os.path.join(self.getBuildDir(), "mock_sysroot")
+exe_inside = os.path.join(tmp_sysroot, os.path.relpath(exe_outside, 
"/"))
+lldbutil.mkdir_p(os.path.dirname(exe_inside))
+shutil.copyfile("linux-x86_64.out", exe_inside)
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), "patched.core")
+with open("linux-x86_64.core", "rb") as f:
+core = f.read()
+core = replace_path(core, "/test" * 817 + "/a.out", exe_outside)
+with open(core_file, "wb") as f:
+f.write(core)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % 
tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore(core_file)
+
+# Check that we found executable from the sysroot
+mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+self.assertEqual(mod_path, exe_inside)
+self.check_all(process, self._x86_64_pid, self._x86_64_regions, 
"a.out")
+
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
@@ -369,3 +407,9 @@
 self.check_all(process, pid, region_count, thread_name)
 
 self.dbg.DeleteTarget(target)
+
+def replace_path(binary, replace_from, replace_to):
+src = replace_from.encode()
+dst = replace_to.encode()
+dst += b"\0" * (len(src) - len(dst))
+return binary.replace(src, dst)
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -21,7 +21,7 @@
 return m_remote_platform_sp->GetModuleSpec(module_file_spec, arch,
module_spec);
 
-  return Platform::GetModuleSpec(module_file_spec, arch, module_spec);
+  return false;
 }
 
 Status RemoteAwarePlatform::RunShellCommand(


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,44 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+@skipIfWindows
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Copy wrong executable to the location outside of sysroot
+exe_outside = os.path.join(self.getBuildDir(), "bin", "a.out")
+lldbutil.mkdir_p(os.path.dirname(exe_outside))
+shutil.copyfile("altmain.out", exe_outside)
+
+# Copy correct executable to the location inside sysroot
+tmp_sysroot = os.path.join(self.getBuildDir(), "mock_sysroot")
+exe_inside = os.path.join(tmp_sysroot, os.path.relpath(exe_outside, "/"))
+lldbutil.mkdir_p(os.path.dirname(exe_inside))
+shutil.copyfile("linux-x86_64.out", exe_inside)
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), "patched.core")
+with open("linux-x86_64.core", "rb") as f:
+core = f.read()
+core = replace_path(core, "/test" * 817 + "/a.out", exe_outside)
+with open(core_file, "wb") as f:
+f.write(core)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % tmp_sysroot)
+target = self.dbg.CreateTarget(N

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

[Yuri Per via Phabricator via lldb-commits]

yuri added a comment.

> I don't believe this test will pass on windows, because of the different path 
> styles. I don't think it's possible to meaningfully test this on windows, as 
> it requires the path in the core file to match a host path. So, I think you 
> can just slap `@skipIfWindows` on the test.

Added

> Do you have commit access?

No, I don't. Please commit this change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529



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

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

This looks good overall. I would add a section describing which test suite to 
use when you're interested in a particular DWARF feature for example. I heard 
from my GDB colleagues that the don't use a compiler, because that might change 
and produce a different DWARF. Instead they use a DWARF assembler (IIRC). I 
guess the analogy to LLDB would be yaml2obj which I see not mentioned here. 
Does it make sense to at least reference it in a section?

I forgot to mention that I'd love to have this at hand when I began developing 
for LLDB. So thank you very much for writing this up!!!




Comment at: lldb/docs/resources/test.rst:86
+
+API tests are located under ``lldb/test/API``. Thy are run with the
+``dotest.py``. Tests are written in Python and test binaries (inferiors) are

s/Thy/They



Comment at: lldb/docs/resources/test.rst:88
+``dotest.py``. Tests are written in Python and test binaries (inferiors) are
+compiled with Make. The majority of API tests are end-to-end tests that compile
+programs from source, run them, and debug the processes.

I personally struggled with the way inferiors are being build. The Makefile 
includes another Makefile and was more like a framework than simple make. You 
had to set special variables in order for the included Makefile to pick it up. 
That level of indirection made it quite complicated for me to get what I 
wanted. To put it differently, it would be nice if you could describe what the 
Makefile should look like or what is expected.



Comment at: lldb/docs/resources/test.rst:94
+The test directory will always contain a python file, starting with ``Test``.
+Most of the tests are structured as a binary begin debugged, so there will be
+one or more sources file and a Makefile.

s/begin/being



Comment at: lldb/docs/resources/test.rst:95
+Most of the tests are structured as a binary begin debugged, so there will be
+one or more sources file and a Makefile.
 

s/sources/source
s/file/files



Comment at: lldb/docs/resources/test.rst:122
+
+It's possible to skip or XFAIL tests using decorators. You'll see them a lot.
+The debugger can be sensitive to things like the architecture, the host and

Maybe link XFAIL to 
https://ftp.gnu.org/old-gnu/Manuals/dejagnu-1.3/html_node/dejagnu_6.html ? 



Comment at: lldb/docs/resources/test.rst:142
+building inferiors. Every test has its own Makefile, most of them only a few
+lines long. A shared Makefile (``Makefile.rules``) with about a thousand lines
+of rules takes care of most if not all of the boiler plate, while individual

Ah, there you're mentioning it.


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

https://reviews.llvm.org/D78242



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

[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 258004.
kwk added a comment.

- Remove not needed include


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.cpp
  lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
  lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
  lldb/test/Shell/Breakpoint/search-support-files.test

Index: lldb/test/Shell/Breakpoint/search-support-files.test
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/search-support-files.test
@@ -0,0 +1,46 @@
+# In these tests we will set breakpoints on a function by name. That function
+# is defined in a header file (search-support-files.h) and will therefore be
+# inlined into the file that includes it (search-support-files.cpp).
+#
+# TODO(kwk): Check that we can also do the same with C++ methods in files?
+#(See https://lldb.llvm.org/use/tutorial.html and look for --method.)
+
+# RUN: %build %p/Inputs/search-support-files.cpp -o dummy.out
+# RUN: %lldb -b -s %s dummy.out | FileCheck --color --dump-input=fail %s 
+
+#Set breakpoint by function name.
+
+breakpoint set -n inlined_42
+# CHECK: (lldb) breakpoint set -n inlined_42
+# CHECK-NEXT: Breakpoint 1: where = dummy.out`inlined_42() + 4 at search-support-files.h:1:20, address = 0x0{{.*}}
+
+breakpoint set -n main
+# CHECK: (lldb) breakpoint set -n main
+# CHECK-NEXT: Breakpoint 2: where = dummy.out`main + 22 at search-support-files.cpp:4:11, address = 0x0{{.*}}
+
+#   Set breakpoint by function name and filename (the one in which the function
+#   is inlined, aka the compilation unit).
+
+breakpoint set -n inlined_42 -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n inlined_42 -f search-support-files.cpp
+# CHECK-NEXT: Breakpoint 3: no locations (pending).
+
+#   Set breakpoint by function name and source filename (the file in which the
+#   function is defined).
+#
+#   NOTE: This test is the really interesting one as it shows that we can
+# search by source files that are themselves no compulation units.
+
+breakpoint set -n inlined_42 -f search-support-files.h
+# CHECK: (lldb) breakpoint set -n inlined_42 -f search-support-files.h
+# CHECK-NEXT: Breakpoint 4: where = dummy.out`inlined_42() + 4 at search-support-files.h:1:20, address = 0x0{{.*}}
+
+#   Set breakpoint by function name and source filename. This time the file
+#   doesn't exist to prove that we haven't widen the search space too much. When
+#   we search for a function in a file that doesn't exist, we should get no
+#   results.
+
+breakpoint set -n inlined_42 -f file-not-existing.h
+# CHECK: (lldb) breakpoint set -n inlined_42 -f file-not-existing.h
+# CHECK-NEXT: Breakpoint 5: no locations (pending).
+# CHECK-NEXT: WARNING: Unable to resolve breakpoint to any actual locations.
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
@@ -0,0 +1 @@
+int inlined_42() { return 42; }
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
@@ -0,0 +1,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,12 +712,7 @@
 if (m_cu_spec_list.GetSize() != 0)
   return false; // Has no comp_unit so can't pass the file check.
   }
-  FileSpec cu_spec;
-  if (sym_ctx.comp_unit)
-cu_spec = sym_ctx.comp_unit->GetPrimaryFile();
-  if (m_cu_spec_list.FindFileIndex(0, cu_spec, false) == UINT32_MAX)
-return false; // Fails the file check
-  return SearchFilterByModuleList::ModulePasses(sym_ctx.module_sp); 
+  return SearchFilterByModuleList::ModulePasses(sym_ctx.module_sp);
 }
 
 bool SearchFilterByModuleListAndCU::CompUnitPasses(FileSpec &fileSpec) {
@@ -811,7 +806,7 @@
 }
 
 uint32_t SearchFilterByModuleListAndCU::GetFilterRequiredItems() {
-  return eSymbolContextModule | eSymbolContextCompUnit;
+  return eSymbolContextModule | eSymbolContextCompUnit | eSymbolContextFunction;
 }
 
 void SearchFilterByModuleListAndCU::Dump(Stream *s) const {}
Index: lldb/source/Breakpoint/BreakpointResolverName.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -260,6 +260,8 @@
   SymbolContextList func_list;
   bool filter_by_cu =
   (filter.GetFilterRequiredItems() & eSymbolContex

[Lldb-commits] [lldb] d5c26f8 - [lldb/unittests] Better error messages when creating sockets fails

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-04-16T11:59:37+02:00
New Revision: d5c26f871b7ee81e7bc6cc17cfddc9d08befe971

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

LOG: [lldb/unittests] Better error messages when creating sockets fails

We get failures in SocketTestUtilities on the pre-merge bots. This
might give us a clue as to what's wrong.

Added: 


Modified: 
lldb/unittests/Host/SocketTestUtilities.cpp

Removed: 




diff  --git a/lldb/unittests/Host/SocketTestUtilities.cpp 
b/lldb/unittests/Host/SocketTestUtilities.cpp
index 858d64f9b4fc..ab883531bdf2 100644
--- a/lldb/unittests/Host/SocketTestUtilities.cpp
+++ b/lldb/unittests/Host/SocketTestUtilities.cpp
@@ -33,10 +33,10 @@ void lldb_private::CreateConnectedSockets(
   Status error;
   std::unique_ptr listen_socket_up(
   new SocketType(true, child_processes_inherit));
-  EXPECT_FALSE(error.Fail());
+  ASSERT_THAT_ERROR(error.ToError(), llvm::Succeeded());
   error = listen_socket_up->Listen(listen_remote_address, 5);
-  EXPECT_FALSE(error.Fail());
-  EXPECT_TRUE(listen_socket_up->IsValid());
+  ASSERT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  ASSERT_TRUE(listen_socket_up->IsValid());
 
   Status accept_error;
   Socket *accept_socket;
@@ -47,21 +47,19 @@ void lldb_private::CreateConnectedSockets(
   std::string connect_remote_address = get_connect_addr(*listen_socket_up);
   std::unique_ptr connect_socket_up(
   new SocketType(true, child_processes_inherit));
-  EXPECT_FALSE(error.Fail());
+  ASSERT_THAT_ERROR(error.ToError(), llvm::Succeeded());
   error = connect_socket_up->Connect(connect_remote_address);
-  EXPECT_FALSE(error.Fail());
-  EXPECT_TRUE(connect_socket_up->IsValid());
+  ASSERT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  ASSERT_TRUE(connect_socket_up->IsValid());
 
   a_up->swap(connect_socket_up);
-  EXPECT_TRUE(error.Success());
-  EXPECT_NE(nullptr, a_up->get());
-  EXPECT_TRUE((*a_up)->IsValid());
+  ASSERT_TRUE((*a_up)->IsValid());
 
   accept_thread.join();
   b_up->reset(static_cast(accept_socket));
-  EXPECT_TRUE(accept_error.Success());
-  EXPECT_NE(nullptr, b_up->get());
-  EXPECT_TRUE((*b_up)->IsValid());
+  ASSERT_THAT_ERROR(accept_error.ToError(), llvm::Succeeded());
+  ASSERT_NE(nullptr, b_up->get());
+  ASSERT_TRUE((*b_up)->IsValid());
 
   listen_socket_up.reset();
 }



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

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

[Pavel Labath via Phabricator via lldb-commits]

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

Sorry, this got burried in my inbox.  I think this looks good. I don't believe 
this test will pass on windows, because of the different path styles. I don't 
think it's possible to meaningfully test this on windows, as it requires the 
path in the core file to match a host path. So, I think you can just slap 
`@skipIfWindows` on the test.

Do you have commit access?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529



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

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Another interesting aspect of the "shell" tests is that there you can often get 
away with a broken/incomplete binary, whereas the "api" tests almost always 
require a fully functional executable. This enables testing of (some) aspects 
of handling of binaries with non-native architectures or operating systems. I 
see this as a big advantage of the shell tests. And a bit of a missed 
opportunity too -- we don't have many tests like this right now, which means is 
fairly hard to contribute something to lldb without having access to mac, 
linux, and windows machines (arm boxes are useful too, at times)...


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

https://reviews.llvm.org/D78242



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

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

[Yuri Per via Phabricator via lldb-commits]

yuri added a comment.

@labath, please take a look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529



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

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

I think this is a really good writeup. Thanks for doing this.




Comment at: lldb/docs/resources/test.rst:156-157
+of variants. It's very tempting to add more variants because it's an easy way
+to increase test coverage. It doesn't scale. It's cheap in developer time, but
+extremely expensive in terms of runtime and maintenance time because it
+multiplies the overhead.

As we usually maintain our own tests, I would say "maintenance time" is a 
subset of "developer time". I guess you wanted to say something like that it's 
easy to set up, but increases the runtime of the tests and has a large ongoing 
cost.



Comment at: lldb/docs/resources/test.rst:160
+
+The key take away is that the different variants don't prevent the need for
+focused tests. So relying on it to test say DWARF5 is a really bad idea.

s/prevent/remove (obviate) ?


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

https://reviews.llvm.org/D78242



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

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The thing which bugs me here (besides the duplication, which I think we now 
know how to handle) is that the definition of the behavior for the char*+size 
functions is spread out over REGISTER and RECORD macros -- one define the 
behavior of active replay, one for passive.

I sort of get that this is because the two modes are fairly different, but I 
think it still would be nice to centralize that, and I think that with some 
thought, that might be possible. As I understand it, tight now, the "active" 
redirection works by passing a pointer which completely replaces the behavior 
of the called method. In reality the replacing function will eventually call 
the replaced function anyway, with some fixups applied. What if, instead of 
replacing the function completely, we passed a pair of callbacks which would 
run before+after the original function. The first callback could apply any 
necessary fixups to the deserialized arguments, and the second one could (if in 
passive mode) translate the modifications done by the function into the "real" 
arguments. That way, we wouldn't need to do anything special in the RECORD 
macro...

Other ideas may be possible too...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77759



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

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:305
 if (is_trivially_serializable::value)
-  return;
+  return const_cast(t);
 // We need to make a copy as the original object might go out of scope.

JDevlieghere wrote:
> labath wrote:
> > What's up with the `const_cast` ? Should this maybe take a `T &t` argument 
> > and let `T` be deduced as `const U` when needed?
> I need to decode the template instantiation error for the details, but we 
> need to return a non-const T. 
It would be good to at least know the reason for that, because const_casts 
smell..



Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:829-834
+  &lldb_private::repro::construct::record,
+  args...);
+  recorder.RecordResult(c, false);
+} else if (Deserializer *deserializer = data.GetDeserializer()) {
+  if (recorder.ShouldCapture()) {
+lldb_private::repro::construct::replay(

are you sure that the `lldb_private::repro::construct::` 
qualifications are necessary here?



Comment at: lldb/source/API/SBReproducerPrivate.h:80-85
+  FileSpec file = l->GetFile();
+  static auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
+  if (!error_or_file)
+return {};
+  static ReplayData r((*error_or_file)->getBuffer());
+  return {r.GetDeserializer(), r.GetRegistry()};

Could this be done in the initialization code somewhere (inside 
`Reproducer::Initialize`, I guess)? We could avoid static variables and get 
better error handling that way...



Comment at: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp:91-95
+return TestInstrumentationDataRAII(os);
+  }
+
+  static TestInstrumentationDataRAII GetReplayData(llvm::StringRef buffer) {
+return TestInstrumentationDataRAII(buffer);

This works only accidentally because the compiler NRVO-s the temporary object. 
But that's not guaranteed to happen and could cause the 
`TestInstrumentationDataRAII` destructor to run twice. The simplest way to 
ensure the behavior you want is to return a `unique_ptr` to a constructed 
object (as well as delete all copy operations)...



Comment at: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp:1044
+  }
+}

Maybe an EXPECT_DEATH test case for what happens when the replayed function 
calls don't line up?


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

https://reviews.llvm.org/D77602



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

[Lldb-commits] [PATCH] D78141: [lldb/Reproducers] Simplify LLDB_RECORD macros

[Pavel Labath via Phabricator via lldb-commits]

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

Very nice.


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

https://reviews.llvm.org/D78141



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