[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Balázs Kéri via Phabricator via lldb-commits]

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:7738
+Expected ASTImporter::ImportInternal(Decl *FromD) {
+  // Import the declaration.
+  ASTNodeImporter Importer(*this);

Comment can be better: Import it using ASTNodeImporter.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:581
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+  // Returns a ImporterConstructor that constructs this class.

Is this `using` needed?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:583
+  // Returns a ImporterConstructor that constructs this class.
+  static ASTImporterOptionSpecificTestBase::ImporterConstructor
+  getConstructor() {

Is it possible to make this a static variable instead of function?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:588
+  bool MinimalImport, ASTImporterLookupTable *LookupTabl) {
+  return static_cast(
+  new RedirectingImporter(ToContext, ToFileManager, FromContext,

The `static_cast` should not be needed.


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

https://reviews.llvm.org/D59485



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

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

[Balázs Kéri via Phabricator via lldb-commits]

balazske added inline comments.



Comment at: lldb/source/Symbol/StdModuleHandler.cpp:220
+case TemplateArgument::Type: {
+  QualType our_type = m_importer.Import(arg.getAsType());
+  imported_args.push_back(TemplateArgument(our_type));

teemperor wrote:
> balazske wrote:
> > For long-term (but it should be not so long) the `ASTImporter::Import_New` 
> > should be used (later the Import_New will become the Import). By using the 
> > existing Import_New the code will be prepared for this change. For this to 
> > work the error handling (ignore error, return it from Import or something 
> > else) must be implemented here.
> Thanks, fixed! I currently just return nothing, which will bring us back to 
> LLDB's normal importing logic (which then maybe does better).
The `TemplateArgument::Integral` case is not fixed yet.



Comment at: lldb/source/Symbol/StdModuleHandler.cpp:212
+  if (!type)
+return {};
+  imported_args.push_back(TemplateArgument(*type));

This does not work correctly: `llvm::consumeError(type.takeError())` should be 
called to handle the error, otherwise it will trigger assertion.


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

https://reviews.llvm.org/D59537



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

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

> @martong It's not related to lookup or anything like that, it's just that we 
> don't have enough information in our debug info AST to allow people to use 
> meta programming. The custom logic we have in LLDB looks into the std C++ 
> module to fill out these gaps in the AST by hand when they are used in an 
> expression.
> 
> The remark about the alternative being slow just means that fixing all the 
> templates we have in our debug info AST seems like a costly approach. I'm 
> also not sure how well it would work, as I never tried mixing a C++ module in 
> an ASTContext that isn't currently being parsed by Clang.

Well, I still don't understand how LLDB synthesis the AST. 
So you have a C++ module in a .pcm file. This means you could create an AST 
from that with ASTReader from it's .clang_ast section, right? In that case the 
AST should be complete with all type information. If this was the case then I 
don't see why it is not possible to use clang::ASTImporter to import templates 
and specializations, since we do exactly that in CTU.

Or do you guys create the AST from the debug info, from the .debug_info section 
of .pcm module file? And this is why AST is incomplete? I've got this from 
https://www.youtube.com/watch?v=EgkZ8PTNSHQ&list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&index=2&t=5s
If this is the case, then comes my naiive quiestion: Why don't you use the 
ASTReader?


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

https://reviews.llvm.org/D59485



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

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 191859.
teemperor marked 4 inline comments as done.
teemperor added a comment.

- Addressed feedback.


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

https://reviews.llvm.org/D59485

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -304,6 +304,14 @@
   const char *const InputFileName = "input.cc";
   const char *const OutputFileName = "output.cc";
 
+public:
+  /// Allocates an ASTImporter (or one of its subclasses).
+  typedef std::function
+  ImporterConstructor;
+
+private:
   // Buffer for the To context, must live in the test scope.
   std::string ToCode;
 
@@ -316,22 +324,37 @@
 std::unique_ptr Unit;
 TranslationUnitDecl *TUDecl = nullptr;
 std::unique_ptr Importer;
-TU(StringRef Code, StringRef FileName, ArgVector Args)
+// The lambda that constructs the ASTImporter we use in this test.
+ImporterConstructor Creator;
+TU(StringRef Code, StringRef FileName, ArgVector Args,
+   ImporterConstructor C = ImporterConstructor())
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C) {
   Unit->enableSourceFileDiagnostics();
+
+  // If the test doesn't need a specific ASTImporter, we just create a
+  // normal ASTImporter with it.
+  if (!Creator)
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+  return new ASTImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, MinimalImport, LookupTable);
+};
+}
+
+void setImporter(std::unique_ptr I) {
+  Importer = std::move(I);
 }
 
 void lazyInitImporter(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST) {
   assert(ToAST);
-  if (!Importer) {
-Importer.reset(
-new ASTImporter(ToAST->getASTContext(), ToAST->getFileManager(),
-Unit->getASTContext(), Unit->getFileManager(),
-false, &LookupTable));
-  }
+  if (!Importer)
+Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(),
+   Unit->getASTContext(), Unit->getFileManager(),
+   false, &LookupTable));
   assert(&ToAST->getASTContext() == &Importer->getToContext());
   createVirtualFileIfNeeded(ToAST, FileName, Code);
 }
@@ -401,11 +424,12 @@
   // Must not be called more than once within the same test.
   std::tuple
   getImportedDecl(StringRef FromSrcCode, Language FromLang, StringRef ToSrcCode,
-  Language ToLang, StringRef Identifier = DeclToImportID) {
+  Language ToLang, StringRef Identifier = DeclToImportID,
+  ImporterConstructor Creator = ImporterConstructor()) {
 ArgVector FromArgs = getArgVectorForLanguage(FromLang),
   ToArgs = getArgVectorForLanguage(ToLang);
 
-FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs);
+FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs, Creator);
 TU &FromTU = FromTUs.back();
 
 assert(!ToAST);
@@ -455,6 +479,12 @@
 return ToAST->getASTContext().getTranslationUnitDecl();
   }
 
+  ASTImporter &getImporter(Decl *From, Language ToLang) {
+lazyInitToAST(ToLang, "", OutputFileName);
+TU *FromTU = findFromTU(From);
+return *FromTU->Importer;
+  }
+
   // Import the given Decl into the ToCtx.
   // May be called several times in a given test.
   // The different instances of the param From may have different ASTContext.
@@ -544,6 +574,75 @@
   EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2));
 }
 
+struct CustomImporter : ASTImporterOptionSpecificTestBase {};
+
+namespace {
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+  // Returns a ImporterConstructor that constructs this class.
+  static ASTImporterOptionSpecificTestBase::ImporterConstructor Constructor;
+
+protected:
+  llvm::Expected ImportInternal(Decl *FromD) override {
+auto *ND = dyn_cast(FromD);
+if (!ND)
+  return ASTImporter::ImportInternal(FromD);
+if (ND->getName() != "shouldNotBeImported")
+  return ASTImporter::ImportInternal(FromD);
+for (Decl *D : getToContext().getTranslationUnitDecl()->decls()) {
+  if (auto ND = dyn_cast(D))
+if (N

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

[Balázs Kéri via Phabricator via lldb-commits]

balazske added inline comments.



Comment at: lldb/source/Symbol/StdModuleHandler.cpp:245
+new_class_template->AddSpecialization(found_decl, InsertPos);
+new_class_template->dumpColor();
+if (new_class_template->isOutOfLine())

This dump is not needed?


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

https://reviews.llvm.org/D59537



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

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

> Well, I still don't understand how LLDB synthesis the AST. 
>  So you have a C++ module in a .pcm file. This means you could create an AST 
> from that with ASTReader from it's .clang_ast section, right? In that case 
> the AST should be complete with all type information. If this was the case 
> then I don't see why it is not possible to use clang::ASTImporter to import 
> templates and specializations, since we do exactly that in CTU.
> 
> Or do you guys create the AST from the debug info, from the .debug_info 
> section of .pcm module file? And this is why AST is incomplete? I've got this 
> from 
> https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
>  If this is the case, then comes my naiive quiestion: Why don't you use the 
> ASTReader?

There are no C++ modules involved in our use case beside the generic `std` 
module. No user-written code is read from a module and we don't have any PCM 
file beside the `std` module we build for the expression evaluator.

We use the ASTReader to directly read the `std` module contents into the 
expression evaluation ASTContext, but this doesn't give us the decls for the 
instantiations the user has made (e.g. `std::vector`). We only have these 
user instantiations in the 'normal' debug info where we have a rather minimal 
AST (again, no modules are not involved in building this debug info AST). The 
`InternalImport` in the LLDB patch just stitches the module AST and the debug 
info AST together when we import something that we also have (in better quality 
from the C++ module) in the target ASTContext.




Comment at: clang/unittests/AST/ASTImporterTest.cpp:581
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+  // Returns a ImporterConstructor that constructs this class.

balazske wrote:
> Is this `using` needed?
Yeah, otherwise we have to provide our own constructor that essentially just 
forwards to the ASTImporter constructor.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:588
+  bool MinimalImport, ASTImporterLookupTable *LookupTabl) {
+  return static_cast(
+  new RedirectingImporter(ToContext, ToFileManager, FromContext,

balazske wrote:
> The `static_cast` should not be needed.
Thanks!


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

https://reviews.llvm.org/D59485



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

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 191876.
teemperor added a comment.

- Removed dump() that was left in there from some debugging.
- Changed the second `Import` call I missed to `Import_New`
- No longer ignoring the Errors in Expected when importing template arguments.


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

https://reviews.llvm.org/D59537

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/include/lldb/Symbol/ClangASTImporter.h
  lldb/include/lldb/Symbol/StdModuleHandler.h
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/deque-basic/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/deque-basic/TestBasicDeque.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/deque-basic/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/deque-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/deque-dbg-info-content/TestDbgInfoContentDeque.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/deque-dbg-info-content/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/forward_list-basic/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/forward_list-basic/TestBasicForwardList.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/forward_list-basic/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/forward_list-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardList.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/forward_list-dbg-info-content/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/list-basic/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/list-basic/TestBasicList.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/list-basic/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/list-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/list-dbg-info-content/TestDbgInfoContentList.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/list-dbg-info-content/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/shared_ptr-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/shared_ptr-dbg-info-content/TestSharedPtrDbgInfoContent.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/shared_ptr-dbg-info-content/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/shared_ptr/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/shared_ptr/TestSharedPtr.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/shared_ptr/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/unique_ptr-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/unique_ptr-dbg-info-content/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/unique_ptr/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/unique_ptr/TestUniquePtr.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/unique_ptr/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-basic/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-basic/TestBasicVector.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-basic/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-bool/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-bool/TestBoolVector.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-bool/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-dbg-info-content/TestDbgInfoContentVector.py
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-dbg-info-content/main.cpp
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-of-vectors/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/vector-of-vectors/TestVectorOfVectors.py
  
lldb/packages/Pytho

[Lldb-commits] [lldb] r356751 - Extend r356573 (minidump UUID handling) to cover elf build-ids too

[Pavel Labath via lldb-commits]

Author: labath
Date: Fri Mar 22 07:03:59 2019
New Revision: 356751

URL: http://llvm.org/viewvc/llvm-project?rev=356751&view=rev
Log:
Extend r356573 (minidump UUID handling) to cover elf build-ids too

Breakpad (but not crashpad) will insert an empty (all-zero) build-id
record for modules which do not have a build-id. This tells lldb to
treat such records as empty/invalid uuids.

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-zero.dmp
Modified:

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

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=356751&r1=356750&r2=356751&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Fri Mar 22 07:03:59 2019
@@ -119,3 +119,16 @@ class MiniDumpUUIDTestCase(TestBase):
 self.assertEqual(2, len(modules))
 self.verify_module(modules[0], "/tmp/a", 
"01020304-0506-0708-090A-0B0C0D0E0F10-11121314")
 self.verify_module(modules[1], "/tmp/b", 
"0A141E28-323C-4650-5A64-6E78828C96A0-AAB4BEC8")
+
+def test_uuid_modules_elf_build_id_zero(self):
+"""
+Test multiple modules having a MINIDUMP_MODULE.CvRecord that is 
valid,
+and contains a ELF build ID whose value is all zero.
+"""
+self.dbg.CreateTarget(None)
+self.target = self.dbg.GetSelectedTarget()
+self.process = 
self.target.LoadCore("linux-arm-uuids-elf-build-id-zero.dmp")
+modules = self.target.modules
+self.assertEqual(2, len(modules))
+self.verify_module(modules[0], "/not/exist/a", None)
+self.verify_module(modules[1], "/not/exist/b", None)

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-zero.dmp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-zero.dmp?rev=356751&view=auto
==
Binary files 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-zero.dmp
 (added) and 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-zero.dmp
 Fri Mar 22 07:03:59 2019 differ

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=356751&r1=356750&r2=356751&view=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Fri Mar 22 
07:03:59 2019
@@ -204,7 +204,7 @@ UUID MinidumpParser::GetModuleUUID(const
 }
 return UUID::fromData(pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
   } else if (cv_signature == CvSignature::ElfBuildId)
-return UUID::fromData(cv_record);
+return UUID::fromOptionalData(cv_record);
 
   return UUID();
 }


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

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356753: [ObjectYAML] Add basic minidump generation support 
(authored by labath, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59482?vs=191653&id=191879#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59482

Files:
  llvm/trunk/include/llvm/ObjectYAML/MinidumpYAML.h
  llvm/trunk/include/llvm/ObjectYAML/ObjectYAML.h
  llvm/trunk/lib/ObjectYAML/CMakeLists.txt
  llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/trunk/lib/ObjectYAML/ObjectYAML.cpp
  llvm/trunk/test/tools/yaml2obj/minidump-raw-stream-small-size.yaml
  llvm/trunk/test/tools/yaml2obj/minidump-systeminfo-other-long.yaml
  llvm/trunk/test/tools/yaml2obj/minidump-systeminfo-other-not-hex.yaml
  llvm/trunk/test/tools/yaml2obj/minidump-systeminfo-other-short.yaml
  llvm/trunk/test/tools/yaml2obj/minidump-systeminfo-x86-long.yaml
  llvm/trunk/test/tools/yaml2obj/minidump-systeminfo-x86-short.yaml
  llvm/trunk/tools/yaml2obj/CMakeLists.txt
  llvm/trunk/tools/yaml2obj/yaml2minidump.cpp
  llvm/trunk/tools/yaml2obj/yaml2obj.cpp
  llvm/trunk/tools/yaml2obj/yaml2obj.h
  llvm/trunk/unittests/ObjectYAML/CMakeLists.txt
  llvm/trunk/unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
===
--- llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
+++ llvm/trunk/lib/ObjectYAML/MinidumpYAML.cpp
@@ -0,0 +1,385 @@
+//===- MinidumpYAML.cpp - Minidump YAMLIO implementation --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/ObjectYAML/MinidumpYAML.h"
+
+using namespace llvm;
+using namespace llvm::MinidumpYAML;
+using namespace llvm::minidump;
+
+namespace {
+class BlobAllocator {
+public:
+  size_t tell() const { return NextOffset; }
+
+  size_t AllocateCallback(size_t Size,
+  std::function Callback) {
+size_t Offset = NextOffset;
+NextOffset += Size;
+Callbacks.push_back(std::move(Callback));
+return Offset;
+  }
+
+  size_t AllocateBytes(ArrayRef Data) {
+return AllocateCallback(
+Data.size(), [Data](raw_ostream &OS) { OS << toStringRef(Data); });
+  }
+
+  template  size_t AllocateArray(ArrayRef Data) {
+return AllocateBytes({reinterpret_cast(Data.data()),
+  sizeof(T) * Data.size()});
+  }
+
+  template  size_t AllocateObject(const T &Data) {
+return AllocateArray(makeArrayRef(Data));
+  }
+
+  void writeTo(raw_ostream &OS) const;
+
+private:
+  size_t NextOffset = 0;
+
+  std::vector> Callbacks;
+};
+} // namespace
+
+void BlobAllocator::writeTo(raw_ostream &OS) const {
+  size_t BeginOffset = OS.tell();
+  for (const auto &Callback : Callbacks)
+Callback(OS);
+  assert(OS.tell() == BeginOffset + NextOffset &&
+ "Callbacks wrote an unexpected number of bytes.");
+  (void)BeginOffset;
+}
+
+/// Perform an optional yaml-mapping of an endian-aware type EndianType. The
+/// only purpose of this function is to avoid casting the Default value to the
+/// endian type;
+template 
+static inline void mapOptional(yaml::IO &IO, const char *Key, EndianType &Val,
+   typename EndianType::value_type Default) {
+  IO.mapOptional(Key, Val, EndianType(Default));
+}
+
+/// Yaml-map an endian-aware type EndianType as some other type MapType.
+template 
+static inline void mapRequiredAs(yaml::IO &IO, const char *Key,
+ EndianType &Val) {
+  MapType Mapped = static_cast(Val);
+  IO.mapRequired(Key, Mapped);
+  Val = static_cast(Mapped);
+}
+
+/// Perform an optional yaml-mapping of an endian-aware type EndianType as some
+/// other type MapType.
+template 
+static inline void mapOptionalAs(yaml::IO &IO, const char *Key, EndianType &Val,
+ MapType Default) {
+  MapType Mapped = static_cast(Val);
+  IO.mapOptional(Key, Mapped, Default);
+  Val = static_cast(Mapped);
+}
+
+namespace {
+/// Return the appropriate yaml Hex type for a given endian-aware type.
+template  struct HexType;
+template <> struct HexType { using type = yaml::Hex16; };
+template <> struct HexType { using type = yaml::Hex32; };
+template <> struct HexType { using type = yaml::Hex64; };
+} // namespace
+
+/// Yaml-map an endian-aware type as an appropriately-sized hex value.
+template 
+static inline void mapRequiredHex(yaml::IO &IO, const char *Key,
+  EndianType &Val) {
+  mapRequiredAs::type>(IO, Key, Val);
+}
+
+/// Perform an optional yaml-mapping of an endian-aware type as an
+/// appropriately-sized hex value.
+template 
+static inline vo

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

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

@rocallahan I find that people are discussing a generic approach in D59553 



Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54747



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

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Balázs Kéri via Phabricator via lldb-commits]

balazske added inline comments.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:583
+  // Returns a ImporterConstructor that constructs this class.
+  static ASTImporterOptionSpecificTestBase::ImporterConstructor
+  getConstructor() {

balazske wrote:
> Is it possible to make this a static variable instead of function?
Another small thing: The comment above is now not updated ("Returns").



Comment at: clang/unittests/AST/ASTImporterTest.cpp:348
+
+void setImporter(std::unique_ptr I) {
+  Importer = std::move(I);

This function is not needed?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:482
 
+  ASTImporter &getImporter(Decl *From, Language ToLang) {
+lazyInitToAST(ToLang, "", OutputFileName);

This is unused, can be removed.


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

https://reviews.llvm.org/D59485



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

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

In D59485#1439390 , @teemperor wrote:

> > Well, I still don't understand how LLDB synthesis the AST. 
> >  So you have a C++ module in a .pcm file. This means you could create an 
> > AST from that with ASTReader from it's .clang_ast section, right? In that 
> > case the AST should be complete with all type information. If this was the 
> > case then I don't see why it is not possible to use clang::ASTImporter to 
> > import templates and specializations, since we do exactly that in CTU.
> > 
> > Or do you guys create the AST from the debug info, from the .debug_info 
> > section of .pcm module file? And this is why AST is incomplete? I've got 
> > this from 
> > https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
> >  If this is the case, then comes my naiive quiestion: Why don't you use the 
> > ASTReader?
>
> There are no C++ modules involved in our use case beside the generic `std` 
> module. No user-written code is read from a module and we don't have any PCM 
> file beside the `std` module we build for the expression evaluator.
>
> We use the ASTReader to directly read the `std` module contents into the 
> expression evaluation ASTContext, but this doesn't give us the decls for the 
> instantiations the user has made (e.g. `std::vector`). We only have 
> these user instantiations in the 'normal' debug info where we have a rather 
> minimal AST (again, no modules are not involved in building this debug info 
> AST). The `InternalImport` in the LLDB patch just stitches the module AST and 
> the debug info AST together when we import something that we also have (in 
> better quality from the C++ module) in the target ASTContext.


Thank you for the explanation.
Now I understand you directly create specializations from the std module and 
intercept the import to avoid importing broken specializations from the debug 
info, this makes sense.


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

https://reviews.llvm.org/D59485



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

[Lldb-commits] [PATCH] D59667: Regression test to ensure that we handling importing of anonymous enums correctly

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: 
packages/Python/lldbsuite/test/expression_command/cast_int_to_anonymous_enum/main.cpp:1
+#include 
+

You can speed up compilation time of this test significantly by removing the 
printf and the include of cstdio.





Comment at: 
packages/Python/lldbsuite/test/expression_command/cast_int_to_anonymous_enum/main.cpp:8
+int main() {
+   flow_e f;
+

It looks like this variable is not actually used by the test?


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

https://reviews.llvm.org/D59667



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

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

[Gabor Marton via Phabricator via lldb-commits]

martong added inline comments.



Comment at: lldb/include/lldb/Symbol/StdModuleHandler.h:22
+/// process is setup to use C++ modules and has the 'std' module attached.
+class StdModuleHandler {
+  clang::ASTImporter *m_importer = nullptr;

I think it would worth to extend this description with something like this:  

`tryInstantiateStdTemplate` directly *creates* (or finds existing) 
specializations from the std module and intercept the import from the debug 
info AST to avoid having broken specializations in the expression evaluation 
context.



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

https://reviews.llvm.org/D59537



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

[Lldb-commits] [PATCH] D59681: Update the lldb driver to support the -O and -S options when passing --repl

[Adrian Prantl via Phabricator via lldb-commits]

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

Thanks! LGTM with one tiny change to the test.




Comment at: lldb/lit/Driver/TestRepl.test:2
+# RUN: echo ':quit' | %lldb -x --repl -O 'expr 42' -S %S/Inputs/Print2.in -o 
'expr something invalid' -s %s 2>&1 | FileCheck %s
+# CHECK: warning: commands specified to run after file load (via -o or -s) are 
ignored in REPL mode
+# CHECK: (int) $0 = 42

I think the REPL by default echoes all comments, so this may actually be 
matching the comment in the input file rather than the warning?

I think this should do the trick:

`# CHECK: {{w}}arning: commands specified to run after file load (via -o or -s) 
are ignored in REPL mode`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59681



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

[Lldb-commits] [lldb] r356773 - Revert "Move the rest of the sections over to DWARFContext."

[Pavel Labath via lldb-commits]

Author: labath
Date: Fri Mar 22 09:07:58 2019
New Revision: 356773

URL: http://llvm.org/viewvc/llvm-project?rev=356773&view=rev
Log:
Revert "Move the rest of the sections over to DWARFContext."

This reverts commit r356682 because it breaks the DWO flavours of some
tests:
lldb-Suite :: lang/c/const_variables/TestConstVariables.py
lldb-Suite :: lang/c/local_variables/TestLocalVariables.py
lldb-Suite :: lang/c/vla/TestVLA.py

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp?rev=356773&r1=356772&r2=356773&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp Fri Mar 22 
09:07:58 2019
@@ -15,18 +15,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
-DWARFCompileUnit::DWARFCompileUnit(SymbolFileDWARF *dwarf2Data,
-   DWARFContext &dwarf_context)
-: DWARFUnit(dwarf2Data, dwarf_context) {}
+DWARFCompileUnit::DWARFCompileUnit(SymbolFileDWARF *dwarf2Data)
+: DWARFUnit(dwarf2Data) {}
 
-llvm::Expected DWARFCompileUnit::extract(
-SymbolFileDWARF *dwarf2Data, DWARFContext &dwarf_context,
-const DWARFDataExtractor &debug_info, lldb::offset_t *offset_ptr) {
+llvm::Expected
+DWARFCompileUnit::extract(SymbolFileDWARF *dwarf2Data,
+  const DWARFDataExtractor &debug_info,
+  lldb::offset_t *offset_ptr) {
   assert(debug_info.ValidOffset(*offset_ptr));
 
   // std::make_shared would require the ctor to be public.
-  std::shared_ptr cu_sp(
-  new DWARFCompileUnit(dwarf2Data, dwarf_context));
+  std::shared_ptr cu_sp(new DWARFCompileUnit(dwarf2Data));
 
   cu_sp->m_offset = *offset_ptr;
 

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h?rev=356773&r1=356772&r2=356773&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h Fri Mar 22 
09:07:58 2019
@@ -12,15 +12,10 @@
 #include "DWARFUnit.h"
 #include "llvm/Support/Error.h"
 
-namespace lldb_private {
-class DWARFContext;
-}
-
 class DWARFCompileUnit : public DWARFUnit {
 public:
   static llvm::Expected
   extract(SymbolFileDWARF *dwarf2Data,
-  lldb_private::DWARFContext &dwarf_context,
   const lldb_private::DWARFDataExtractor &debug_info,
   lldb::offset_t *offset_ptr);
   void Dump(lldb_private::Stream *s) const override;
@@ -44,8 +39,7 @@ public:
   uint32_t GetHeaderByteSize() const override;
 
 private:
-  DWARFCompileUnit(SymbolFileDWARF *dwarf2Data,
-   lldb_private::DWARFContext &dwarf_context);
+  DWARFCompileUnit(SymbolFileDWARF *dwarf2Data);
   DISALLOW_COPY_AND_ASSIGN(DWARFCompileUnit);
 };
 

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp?rev=356773&r1=356772&r2=356773&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp Fri Mar 22 
09:07:58 2019
@@ -14,13 +14,6 @@ using namespace lldb;
 using namespace lldb_private;
 
 static const DWARFDataExtractor *
-GetPointerOrNull(const llvm::Optional &extractor) {
-  if (!extractor.hasValue())
-return nullptr;
-  return extractor.getPointer();
-}
-
-st

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

In D59485#1439570 , @martong wrote:

> In D59485#1439390 , @teemperor wrote:
>
> > > Well, I still don't understand how LLDB synthesis the AST. 
> > >  So you have a C++ module in a .pcm file. This means you could create an 
> > > AST from that with ASTReader from it's .clang_ast section, right? In that 
> > > case the AST should be complete with all type information. If this was 
> > > the case then I don't see why it is not possible to use 
> > > clang::ASTImporter to import templates and specializations, since we do 
> > > exactly that in CTU.
> > > 
> > > Or do you guys create the AST from the debug info, from the .debug_info 
> > > section of .pcm module file? And this is why AST is incomplete? I've got 
> > > this from 
> > > https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
> > >  If this is the case, then comes my naiive quiestion: Why don't you use 
> > > the ASTReader?
> >
> > There are no C++ modules involved in our use case beside the generic `std` 
> > module. No user-written code is read from a module and we don't have any 
> > PCM file beside the `std` module we build for the expression evaluator.
> >
> > We use the ASTReader to directly read the `std` module contents into the 
> > expression evaluation ASTContext, but this doesn't give us the decls for 
> > the instantiations the user has made (e.g. `std::vector`). We only 
> > have these user instantiations in the 'normal' debug info where we have a 
> > rather minimal AST (again, no modules are not involved in building this 
> > debug info AST). The `InternalImport` in the LLDB patch just stitches the 
> > module AST and the debug info AST together when we import something that we 
> > also have (in better quality from the C++ module) in the target ASTContext.
>
>
> Thank you for the explanation.
>  Now I understand you directly create specializations from the std module and 
> intercept the import to avoid importing broken specializations from the debug 
> info, this makes sense.


Really, just one last question to see the whole picture: If clang::ASTImporter 
were capable of handling a specialization/instantiation with missing info then 
we could use that. So the reason for this interception is that 
clang::ASTImporter::VisitClassTemplateSpecializationDecl cannot handle the 
specialization it receives because that or its dependent nodes has too many 
missing data, right?


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

https://reviews.llvm.org/D59485



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

[Lldb-commits] [PATCH] D59611: Move the rest of the section loading over to DWARFContext

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This broke the DWO flavours of some tests:

  lldb-Suite :: lang/c/const_variables/TestConstVariables.py
  lldb-Suite :: lang/c/local_variables/TestLocalVariables.py
  lldb-Suite :: lang/c/vla/TestVLA.py

I believe the problem is that DWARFContext::LoadOrGetSection does not take DWO 
files into account. It was modelled based on SymbolFileDWARF's notion of 
loading sections, but you probably missed the fact that SymbolFileDWARFDwo 
overrides `LoadSectionData` to load the data from a different object file 
https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp#L32.
 The DWARFContext does not have that, and so it always loads the sections from 
the main module object file, which results in things going south.

I think the fix for this should be relatively simple. We just need to give 
SymbolFileDWARFDwo the opportunity to initialize the DWARFContext with a 
different set of sections. However, since this will require changing it's 
constructor to not accept a Module object (since dwo files do not have one -- 
the next best thing might be to take a SectionList), and change the 
DWARFContext ownership model (so that subclasses can customize it's 
initialization), I did not want to try make a hasty unreviewed fix for that. 
Instead I just reverted this patch for now. (The problem was kind of already 
introduced by the previous patch in this series, but I believe that one is fine 
to keep because there is no DWO flavour of debug_aranges).

Let me know if you have trouble reproducing these failures on your side.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59611



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

[Lldb-commits] [PATCH] D59681: Update the lldb driver to support the -O and -S options when passing --repl

[Nathan Hawes via Phabricator via lldb-commits]

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



Comment at: lldb/lit/Driver/TestRepl.test:2
+# RUN: echo ':quit' | %lldb -x --repl -O 'expr 42' -S %S/Inputs/Print2.in -o 
'expr something invalid' -s %s 2>&1 | FileCheck %s
+# CHECK: warning: commands specified to run after file load (via -o or -s) are 
ignored in REPL mode
+# CHECK: (int) $0 = 42

aprantl wrote:
> I think the REPL by default echoes all comments, so this may actually be 
> matching the comment in the input file rather than the warning?
> 
> I think this should do the trick:
> 
> `# CHECK: {{w}}arning: commands specified to run after file load (via -o or 
> -s) are ignored in REPL mode`
Good catch! I think this patch actually changes that behavior because the %lldb 
expansion includes `-S path/to/lldb/lit/lit-lldb-init` which sets 
echo-comment-commands false and isn't ignored anymore, but if we ever regress 
and stop respecting `-S` this test would still pass...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59681



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

[Lldb-commits] [PATCH] D59681: Update the lldb driver to support the -O and -S options when passing --repl

[Nathan Hawes via Phabricator via lldb-commits]

nathawes updated this revision to Diff 191904.
nathawes added a comment.

Updated the test based on feedback from @aprantl and also to specifically check 
we don't see the output from the -o and -s commands.


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

https://reviews.llvm.org/D59681

Files:
  lldb/lit/Driver/TestRepl.test
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -609,47 +609,55 @@
   // are processed.
   WriteCommandsForSourcing(eCommandPlacementBeforeFile, commands_stream);
 
-  const size_t num_args = m_option_data.m_args.size();
-  if (num_args > 0) {
-char arch_name[64];
-if (lldb::SBDebugger::GetDefaultArchitecture(arch_name, sizeof(arch_name)))
-  commands_stream.Printf("target create --arch=%s %s", arch_name,
- EscapeString(m_option_data.m_args[0]).c_str());
-else
-  commands_stream.Printf("target create %s",
- EscapeString(m_option_data.m_args[0]).c_str());
-
-if (!m_option_data.m_core_file.empty()) {
-  commands_stream.Printf(" --core %s",
- EscapeString(m_option_data.m_core_file).c_str());
-}
-commands_stream.Printf("\n");
+  // If we're not in --repl mode, add the commands to process the file
+  // arguments, and the commands specified to run afterwards.
+  if (!m_option_data.m_repl) {
+const size_t num_args = m_option_data.m_args.size();
+if (num_args > 0) {
+  char arch_name[64];
+  if (lldb::SBDebugger::GetDefaultArchitecture(arch_name, sizeof(arch_name)))
+commands_stream.Printf("target create --arch=%s %s", arch_name,
+   EscapeString(m_option_data.m_args[0]).c_str());
+  else
+commands_stream.Printf("target create %s",
+   EscapeString(m_option_data.m_args[0]).c_str());
 
-if (num_args > 1) {
-  commands_stream.Printf("settings set -- target.run-args ");
-  for (size_t arg_idx = 1; arg_idx < num_args; ++arg_idx)
-commands_stream.Printf(
-" %s", EscapeString(m_option_data.m_args[arg_idx]).c_str());
+  if (!m_option_data.m_core_file.empty()) {
+commands_stream.Printf(" --core %s",
+   EscapeString(m_option_data.m_core_file).c_str());
+  }
   commands_stream.Printf("\n");
-}
-  } else if (!m_option_data.m_core_file.empty()) {
-commands_stream.Printf("target create --core %s\n",
-   EscapeString(m_option_data.m_core_file).c_str());
-  } else if (!m_option_data.m_process_name.empty()) {
-commands_stream.Printf("process attach --name %s",
-   EscapeString(m_option_data.m_process_name).c_str());
 
-if (m_option_data.m_wait_for)
-  commands_stream.Printf(" --waitfor");
+  if (num_args > 1) {
+commands_stream.Printf("settings set -- target.run-args ");
+for (size_t arg_idx = 1; arg_idx < num_args; ++arg_idx)
+  commands_stream.Printf(
+  " %s", EscapeString(m_option_data.m_args[arg_idx]).c_str());
+commands_stream.Printf("\n");
+  }
+} else if (!m_option_data.m_core_file.empty()) {
+  commands_stream.Printf("target create --core %s\n",
+ EscapeString(m_option_data.m_core_file).c_str());
+} else if (!m_option_data.m_process_name.empty()) {
+  commands_stream.Printf("process attach --name %s",
+ EscapeString(m_option_data.m_process_name).c_str());
+
+  if (m_option_data.m_wait_for)
+commands_stream.Printf(" --waitfor");
 
-commands_stream.Printf("\n");
+  commands_stream.Printf("\n");
 
-  } else if (LLDB_INVALID_PROCESS_ID != m_option_data.m_process_pid) {
-commands_stream.Printf("process attach --pid %" PRIu64 "\n",
-   m_option_data.m_process_pid);
-  }
+} else if (LLDB_INVALID_PROCESS_ID != m_option_data.m_process_pid) {
+  commands_stream.Printf("process attach --pid %" PRIu64 "\n",
+ m_option_data.m_process_pid);
+}
 
-  WriteCommandsForSourcing(eCommandPlacementAfterFile, commands_stream);
+WriteCommandsForSourcing(eCommandPlacementAfterFile, commands_stream);
+  } else if (!m_option_data.m_after_file_commands.empty()) {
+// We're in repl mode and after-file-load commands were specified.
+WithColor::warning() << "commands specified to run after file load (via -o "
+  "or -s) are ignored in REPL mode.\n";
+  }
 
   if (GetDebugMode()) {
 result.PutError(m_debugger.GetErrorFileHandle());
@@ -659,100 +667,101 @@
   bool handle_events = true;
   bool spawn_thread = false;
 
-  if (m_option_data.m_repl) {
-const char *repl_options = nullptr;
-if (!m_option_data.m_repl_options.empty())
-  repl_options = m_option_data.m_rep

[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: aprantl, davide, compnerd, JDevlieghere, jingham.

I'm adding this to reduce the difference between swift-lldb and
llvm.org's lldb.


https://reviews.llvm.org/D59708

Files:
  source/Plugins/ExpressionParser/Clang/ClangHost.cpp


Index: source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -45,6 +45,8 @@
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
 
+  // LLVM.org's build of LLDB uses the clang resource directory placed in
+  // $install_dir/lib/clang/$clang_version
   llvm::SmallString<256> clang_dir(parent_dir);
   llvm::SmallString<32> relative_path;
   llvm::sys::path::append(relative_path,
@@ -58,6 +60,18 @@
 return true;
   }
 
+  // swift-lldb uses the clang resource directory copied from swift, which by
+  // default is placed in $install_dir/lib/lldb/clang
+  clang_dir = parent_dir;
+  relative_path.clear();
+  llvm::sys::path::append(relative_path, "lib", "lldb", "clang");
+  llvm::sys::path::append(clang_dir, relative_path);
+  if (!verify || VerifyClangPath(clang_dir)) {
+file_spec.GetDirectory().SetString(clang_dir);
+FileSystem::Instance().Resolve(file_spec);
+return true;
+  }
+
   return HostInfo::ComputePathRelativeToLibrary(file_spec, relative_path);
 }
 


Index: source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -45,6 +45,8 @@
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
 
+  // LLVM.org's build of LLDB uses the clang resource directory placed in
+  // $install_dir/lib/clang/$clang_version
   llvm::SmallString<256> clang_dir(parent_dir);
   llvm::SmallString<32> relative_path;
   llvm::sys::path::append(relative_path,
@@ -58,6 +60,18 @@
 return true;
   }
 
+  // swift-lldb uses the clang resource directory copied from swift, which by
+  // default is placed in $install_dir/lib/lldb/clang
+  clang_dir = parent_dir;
+  relative_path.clear();
+  llvm::sys::path::append(relative_path, "lib", "lldb", "clang");
+  llvm::sys::path::append(clang_dir, relative_path);
+  if (!verify || VerifyClangPath(clang_dir)) {
+file_spec.GetDirectory().SetString(clang_dir);
+FileSystem::Instance().Resolve(file_spec);
+return true;
+  }
+
   return HostInfo::ComputePathRelativeToLibrary(file_spec, relative_path);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

The behavior when verify is false seems a little weird to me.  In that case you 
will just always get the $install_dir/lib/clang/$clang_version path.  That's 
fairly different from the verify = true case.  OTOH, I couldn't see anybody 
passing verify = false anywhere.  Do we need that?


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

https://reviews.llvm.org/D59708



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

[Lldb-commits] [lldb] r356806 - Revert minidump changes

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Fri Mar 22 13:46:46 2019
New Revision: 356806

URL: http://llvm.org/viewvc/llvm-project?rev=356806&view=rev
Log:
Revert minidump changes

This reverts the following two commits:

Revert "Extend r356573 (minidump UUID handling) to cover elf build-ids too"
Revert "Fix UUID decoding from minidump files"

Greg's original commit broke the sanitizer bot which has been red for
several days now.

http://green.lab.llvm.org/green/view/LLDB/job/lldb-sanitized/

Removed:

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

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-16.dmp

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-20.dmp

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-zero.dmp

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-no-age.dmp

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-with-age.dmp

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-zero-uuids.dmp

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/macos-arm-uuids-no-age.dmp
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp

Removed: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=356805&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (removed)
@@ -1,134 +0,0 @@
-"""
-Test basics of Minidump debugging.
-"""
-
-from __future__ import print_function
-from six import iteritems
-
-import shutil
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class MiniDumpUUIDTestCase(TestBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-NO_DEBUG_INFO_TESTCASE = True
-
-def setUp(self):
-super(MiniDumpUUIDTestCase, self).setUp()
-self._initial_platform = lldb.DBG.GetSelectedPlatform()
-
-def tearDown(self):
-lldb.DBG.SetSelectedPlatform(self._initial_platform)
-super(MiniDumpUUIDTestCase, self).tearDown()
-
-def verify_module(self, module, verify_path, verify_uuid):
-uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
-self.assertEqual(verify_uuid, uuid)
-
-def test_zero_uuid_modules(self):
-"""
-Test multiple modules having a MINIDUMP_MODULE.CvRecord that is 
valid,
-but contains a PDB70 value whose age is zero and whose UUID values 
are 
-all zero. Prior to a fix all such modules would be duplicated to 
the
-first one since the UUIDs claimed to be valid and all zeroes. Now 
we
-ensure that the UUID is not valid for each module and that we have
-each of the modules in the target after loading the core
-"""
-self.dbg.CreateTarget(None)
-self.target = self.dbg.GetSelectedTarget()
-self.process = self.target.LoadCore("linux-arm-zero-uuids.dmp")
-modules = self.target.modules
-self.assertEqual(2, len(modules))
-self.verify_module(modules[0], "/file/does/not/exist/a", None)
-self.verify_module(modules[1], "/file/does/not/exist/b", None)
-
-def test_uuid_modules_no_age(self):
-"""
-Test multiple modules having a MINIDUMP_MODULE.CvRecord that is 
valid,
-and contains a PDB70 value whose age is zero and whose UUID values 
are 
-valid. Ensure we decode the UUID and don't include the age field 
in the UUID.
-"""
-self.dbg.CreateTarget(None)
-self.target = self.dbg.GetSelectedTarget()
-self.process = self.target.LoadCore("linux-arm-uuids-no-age.dmp")
-modules = self.target.modules
-self.assertEqual(2, len(modules))
-self.verify_module(modules[0], "/tmp/a", 
"01020304-0506-0708-090A-0B0C0D0E0F10")
-self.verify_module(modules[1], "/tmp/b", 
"0A141E28-323C-4650-5A64-6E78828C96A0")
-
-def test_uuid_modules_no_age_apple(self):
-"""
-Test multiple modules having a MINIDUMP_MODULE.CvRecord that is 
valid,
-and contains a PDB70 value whose age is zero and whose UUID values 
are 
-valid. Ensure we decode the UUID and don't i

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 191946.
teemperor marked 5 inline comments as done.
teemperor added a comment.

- Removed unused functions.
- Fixed two comments that I missed to update.


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

https://reviews.llvm.org/D59485

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -304,6 +304,14 @@
   const char *const InputFileName = "input.cc";
   const char *const OutputFileName = "output.cc";
 
+public:
+  /// Allocates an ASTImporter (or one of its subclasses).
+  typedef std::function
+  ImporterConstructor;
+
+private:
   // Buffer for the To context, must live in the test scope.
   std::string ToCode;
 
@@ -316,22 +324,33 @@
 std::unique_ptr Unit;
 TranslationUnitDecl *TUDecl = nullptr;
 std::unique_ptr Importer;
-TU(StringRef Code, StringRef FileName, ArgVector Args)
+// The lambda that constructs the ASTImporter we use in this test.
+ImporterConstructor Creator;
+TU(StringRef Code, StringRef FileName, ArgVector Args,
+   ImporterConstructor C = ImporterConstructor())
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C) {
   Unit->enableSourceFileDiagnostics();
+
+  // If the test doesn't need a specific ASTImporter, we just create a
+  // normal ASTImporter with it.
+  if (!Creator)
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+  return new ASTImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, MinimalImport, LookupTable);
+};
 }
 
 void lazyInitImporter(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST) {
   assert(ToAST);
-  if (!Importer) {
-Importer.reset(
-new ASTImporter(ToAST->getASTContext(), ToAST->getFileManager(),
-Unit->getASTContext(), Unit->getFileManager(),
-false, &LookupTable));
-  }
+  if (!Importer)
+Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(),
+   Unit->getASTContext(), Unit->getFileManager(),
+   false, &LookupTable));
   assert(&ToAST->getASTContext() == &Importer->getToContext());
   createVirtualFileIfNeeded(ToAST, FileName, Code);
 }
@@ -401,11 +420,12 @@
   // Must not be called more than once within the same test.
   std::tuple
   getImportedDecl(StringRef FromSrcCode, Language FromLang, StringRef ToSrcCode,
-  Language ToLang, StringRef Identifier = DeclToImportID) {
+  Language ToLang, StringRef Identifier = DeclToImportID,
+  ImporterConstructor Creator = ImporterConstructor()) {
 ArgVector FromArgs = getArgVectorForLanguage(FromLang),
   ToArgs = getArgVectorForLanguage(ToLang);
 
-FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs);
+FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs, Creator);
 TU &FromTU = FromTUs.back();
 
 assert(!ToAST);
@@ -544,6 +564,75 @@
   EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2));
 }
 
+struct CustomImporter : ASTImporterOptionSpecificTestBase {};
+
+namespace {
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+  // ImporterConstructor that constructs this class.
+  static ASTImporterOptionSpecificTestBase::ImporterConstructor Constructor;
+
+protected:
+  llvm::Expected ImportInternal(Decl *FromD) override {
+auto *ND = dyn_cast(FromD);
+if (!ND)
+  return ASTImporter::ImportInternal(FromD);
+if (ND->getName() != "shouldNotBeImported")
+  return ASTImporter::ImportInternal(FromD);
+for (Decl *D : getToContext().getTranslationUnitDecl()->decls()) {
+  if (auto ND = dyn_cast(D))
+if (ND->getName() == "realDecl")
+  return ND;
+}
+return ASTImporter::ImportInternal(FromD);
+  }
+};
+
+ASTImporterOptionSpecificTestBase::ImporterConstructor
+RedirectingImporter::Constructor =
+[](ASTContext &ToContext, FileManager &ToFileManager,
+   ASTContext &FromContext, FileManager &FromFileManager,
+   bool MinimalImport, ASTImporterLookupTable *LookupTabl) {
+  return new RedirectingImporter(ToContext, ToFileManager, FromCo

[Lldb-commits] [PATCH] D59708: [ExpressionParser] Add swift-lldb case for finding clang resource dir

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:67
+  relative_path.clear();
+  llvm::sys::path::append(relative_path, "lib", "lldb", "clang");
+  llvm::sys::path::append(clang_dir, relative_path);

Does swift-lldb never honour `LIBDIR`?  That is, can it never end up in `lib64`?



Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:73
+return true;
+  }
+

I think it would be nicer if you could create a static list of the paths and 
loop over it:

```
static const StringRef kResourceDirSuffixes[] = {
  "lib" TO_STRING(CLANG_LIBDIR_SUFFIX) "/clang" CLANG_VERSION_STRING,
  "lib" TO_STRING(CLANG_LIBDIR_SUFFIX) "/lldb/clang",
};

for (const auto &Suffix : kResourceDirSuffixes) {
llvm::SmallString<256> clang_dir(parent_dir);
llvm::SmallString<32> relative_path(Suffix);
llvm::sys::path::native(relative_path);
llvm::sys::path::append(clang_dir, relative_path);
if (!verify || VerifyClangPath(clang_dir)) {
  file_spec.GetDirectory().SetString(clang_dir);
  FileSystem::Instance().Resolve(file_spec);
  return true;
}
}
```


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

https://reviews.llvm.org/D59708



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

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: jingham, friss, JDevlieghere, aprantl, jasonmolenda.
Herald added subscribers: llvm-commits, jdoerfert.
Herald added a project: LLVM.

For the only version of Python actually supported on Darwin.

rdar://problem/40961425


Repository:
  rL LLVM

https://reviews.llvm.org/D59719

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp


Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -176,6 +176,16 @@
 static char g_python_home[] = LLDB_PYTHON_HOME;
 #endif
 Py_SetPythonHome(g_python_home);
+#endif
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2
+// For Darwin, the only Python version supported is the one shipped in the 
OS
+// and linked with lldb. Other installation of Python may have higher 
priorities
+// in the path, overriding PYTHONHOME and causing 
problems/incompatibilities.
+// In order to avoid confusion, always hardcode the PythonHome to be right,
+// as it's not going to change.
+
Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+#endif
 #endif
   }
 


Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -176,6 +176,16 @@
 static char g_python_home[] = LLDB_PYTHON_HOME;
 #endif
 Py_SetPythonHome(g_python_home);
+#endif
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2
+// For Darwin, the only Python version supported is the one shipped in the OS
+// and linked with lldb. Other installation of Python may have higher priorities
+// in the path, overriding PYTHONHOME and causing problems/incompatibilities.
+// In order to avoid confusion, always hardcode the PythonHome to be right,
+// as it's not going to change.
+Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+#endif
 #endif
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

[Jonas Devlieghere via Phabricator via lldb-commits]

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

I think this is okay, although I would prefer if there was a way to deduct the 
correct python home from the Python library we link against. I guess if that 
was possible we didn't need to set the PythonHome in the first place. We also 
can't deduct it from the Python interpreter (`PYTHON_EXECUTABLE`) because as 
we've seen in the past, it's possible that the one in your path (e.g. when 
installed with HomeBrew or from python.org) is different from the system one we 
link against on Darwin.

TL;DR LGTM


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59719



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

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:181
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2
+// For Darwin, the only Python version supported is the one shipped in the 
OS

Just to don't break hypothetical users building their own LLDB on ancient 
versions of macOS let's also change the minor version.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59719



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

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

[Davide Italiano via Phabricator via lldb-commits]

davide updated this revision to Diff 191958.
davide added a comment.

Adrian's feedback


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59719

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp


Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -176,6 +176,16 @@
 static char g_python_home[] = LLDB_PYTHON_HOME;
 #endif
 Py_SetPythonHome(g_python_home);
+#endif
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2 && PY_MINOR_VERSION == 7
+// For Darwin, the only Python version supported is the one shipped in the 
OS
+// and linked with lldb. Other installation of Python may have higher 
priorities
+// in the path, overriding PYTHONHOME and causing 
problems/incompatibilities.
+// In order to avoid confusion, always hardcode the PythonHome to be right,
+// as it's not going to change.
+
Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+#endif
 #endif
   }
 


Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -176,6 +176,16 @@
 static char g_python_home[] = LLDB_PYTHON_HOME;
 #endif
 Py_SetPythonHome(g_python_home);
+#endif
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2 && PY_MINOR_VERSION == 7
+// For Darwin, the only Python version supported is the one shipped in the OS
+// and linked with lldb. Other installation of Python may have higher priorities
+// in the path, overriding PYTHONHOME and causing problems/incompatibilities.
+// In order to avoid confusion, always hardcode the PythonHome to be right,
+// as it's not going to change.
+Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+#endif
 #endif
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r356816 - [ScriptInterpreter] Make sure that PYTHONHOME is right.

[Davide Italiano via lldb-commits]

Author: davide
Date: Fri Mar 22 15:19:57 2019
New Revision: 356816

URL: http://llvm.org/viewvc/llvm-project?rev=356816&view=rev
Log:
[ScriptInterpreter] Make sure that PYTHONHOME is right.

Summary:
For the only version of Python actually supported on Darwin.



Reviewers: jingham, friss, JDevlieghere, aprantl, jasonmolenda

Subscribers: jdoerfert, llvm-commits, lldb-commits

Tags: #llvm

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

Modified:

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Modified: 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=356816&r1=356815&r2=356816&view=diff
==
--- 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
Fri Mar 22 15:19:57 2019
@@ -177,6 +177,16 @@ private:
 #endif
 Py_SetPythonHome(g_python_home);
 #endif
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2 && PY_MINOR_VERSION == 7
+// For Darwin, the only Python version supported is the one shipped in the 
OS
+// and linked with lldb. Other installation of Python may have higher 
priorities
+// in the path, overriding PYTHONHOME and causing 
problems/incompatibilities.
+// In order to avoid confusion, always hardcode the PythonHome to be right,
+// as it's not going to change.
+
Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+#endif
+#endif
   }
 
   void InitializeThreadsPrivate() {


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

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB356816: [ScriptInterpreter] Make sure that PYTHONHOME is 
right. (authored by davide, committed by ).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D59719?vs=191958&id=191959#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59719

Files:
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp


Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -177,6 +177,16 @@
 #endif
 Py_SetPythonHome(g_python_home);
 #endif
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2 && PY_MINOR_VERSION == 7
+// For Darwin, the only Python version supported is the one shipped in the 
OS
+// and linked with lldb. Other installation of Python may have higher 
priorities
+// in the path, overriding PYTHONHOME and causing 
problems/incompatibilities.
+// In order to avoid confusion, always hardcode the PythonHome to be right,
+// as it's not going to change.
+
Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+#endif
+#endif
   }
 
   void InitializeThreadsPrivate() {


Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -177,6 +177,16 @@
 #endif
 Py_SetPythonHome(g_python_home);
 #endif
+#else
+#if defined(__APPLE__) && PY_MAJOR_VERSION == 2 && PY_MINOR_VERSION == 7
+// For Darwin, the only Python version supported is the one shipped in the OS
+// and linked with lldb. Other installation of Python may have higher priorities
+// in the path, overriding PYTHONHOME and causing problems/incompatibilities.
+// In order to avoid confusion, always hardcode the PythonHome to be right,
+// as it's not going to change.
+Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+#endif
+#endif
   }
 
   void InitializeThreadsPrivate() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r356819 - [ScriptInterpreter] Remove a warning and reformat comments.

[Davide Italiano via lldb-commits]

Author: davide
Date: Fri Mar 22 15:38:49 2019
New Revision: 356819

URL: http://llvm.org/viewvc/llvm-project?rev=356819&view=rev
Log:
[ScriptInterpreter] Remove a warning and reformat comments.

Modified:

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Modified: 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=356819&r1=356818&r2=356819&view=diff
==
--- 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
Fri Mar 22 15:38:49 2019
@@ -176,15 +176,15 @@ private:
 static char g_python_home[] = LLDB_PYTHON_HOME;
 #endif
 Py_SetPythonHome(g_python_home);
-#endif
 #else
 #if defined(__APPLE__) && PY_MAJOR_VERSION == 2 && PY_MINOR_VERSION == 7
 // For Darwin, the only Python version supported is the one shipped in the 
OS
-// and linked with lldb. Other installation of Python may have higher 
priorities
-// in the path, overriding PYTHONHOME and causing 
problems/incompatibilities.
-// In order to avoid confusion, always hardcode the PythonHome to be right,
-// as it's not going to change.
-
Py_SetPythonHome("/System/Library/Frameworks/Python.framework/Versions/2.7");
+// OS and linked with lldb. Other installation of Python may have higher
+// priorities in the path, overriding PYTHONHOME and causing
+// problems/incompatibilities. In order to avoid confusion, always hardcode
+// the PythonHome to be right, as it's not going to change.
+char path[] = "/System/Library/Frameworks/Python.framework/Versions/2.7";
+Py_SetPythonHome(path);
 #endif
 #endif
   }


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

[Lldb-commits] [lldb] r356825 - [Reproducers] Fix GDB remote flakiness during replay

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Fri Mar 22 16:33:17 2019
New Revision: 356825

URL: http://llvm.org/viewvc/llvm-project?rev=356825&view=rev
Log:
[Reproducers] Fix GDB remote flakiness during replay

This fixes the flakiness of the GDB remote reproducer during replay. It
was caused by a combination sending one ACK to many from the replay
server and the code that "flushes" any queued GDB remote packets in
GDBRemoteCommunicationClient::HandshakeWithServer.

The spurious ACK was the result of combining both implicit and explicit
handling of ACKs in the replay server. The handshake consists of an ACK
followed by an QStartNoAckMode. As long as we haven't seen any
QStartNoAckMode, we were sending implicit acknowledgments. So the first
ACK got acknowledged twice, once implicitly, and once as part of the
replay.

The reason we didn't notice this was the code in HandshakeWithServer
that "waits for any responses that might have been queued up in the
remote GDB server and flush them all". A 10ms timeout is used to move on
when no packets are left. If the second ACK didn't make it within those
10ms, all packets were offset by one.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp?rev=356825&r1=356824&r2=356825&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 Fri Mar 22 16:33:17 2019
@@ -35,8 +35,6 @@ static bool unexpected(llvm::StringRef e
   // trailing checksum. The 'actual' string contains only the packet's content.
   if (expected.contains(actual))
 return false;
-  if (expected == "+" || actual == "+")
-return false;
   // Contains a PID which might be different.
   if (expected.contains("vAttach"))
 return false;
@@ -51,11 +49,10 @@ static bool unexpected(llvm::StringRef e
 }
 
 GDBRemoteCommunicationReplayServer::GDBRemoteCommunicationReplayServer()
-: GDBRemoteCommunication("gdb-remote.server",
- "gdb-remote.server.rx_packet"),
-  m_async_broadcaster(nullptr, "lldb.gdb-remote.server.async-broadcaster"),
+: GDBRemoteCommunication("gdb-replay", "gdb-replay.rx_packet"),
+  m_async_broadcaster(nullptr, "lldb.gdb-replay.async-broadcaster"),
   m_async_listener_sp(
-  Listener::MakeListener("lldb.gdb-remote.server.async-listener")),
+  Listener::MakeListener("lldb.gdb-replay.async-listener")),
   m_async_thread_state_mutex(), m_skip_acks(false) {
   m_async_broadcaster.SetEventName(eBroadcastBitAsyncContinue,
"async thread continue");
@@ -92,26 +89,21 @@ GDBRemoteCommunicationReplayServer::GetP
 
   m_async_broadcaster.BroadcastEvent(eBroadcastBitAsyncContinue);
 
-  if (m_skip_acks) {
-const StringExtractorGDBRemote::ServerPacketType packet_type =
-packet.GetServerPacketType();
-switch (packet_type) {
-case StringExtractorGDBRemote::eServerPacketType_nack:
-case StringExtractorGDBRemote::eServerPacketType_ack:
-  return PacketResult::Success;
-default:
-  break;
-}
-  } else if (packet.GetStringRef() == "QStartNoAckMode") {
-m_skip_acks = true;
+  // If m_send_acks is true, we're before the handshake phase. We've already
+  // acknowledge the '+' packet so we're done here.
+  if (m_send_acks && packet.GetStringRef() == "+")
+return PacketResult::Success;
+
+  // This completes the handshake. Since m_send_acks was true, we can unset it
+  // already.
+  if (packet.GetStringRef() == "QStartNoAckMode")
 m_send_acks = false;
-  }
 
   // A QEnvironment packet is sent for every environment variable. If the
   // number of environment variables is different during replay, the replies
   // become out of sync.
   if (packet.GetStringRef().find("QEnvironment") == 0) {
-return SendRawPacketNoLock("$OK#9a", true);
+return SendRawPacketNoLock("$OK#9a");
   }
 
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
@@ -120,13 +112,17 @@ GDBRemoteCommunicationReplayServer::GetP
 GDBRemoteCommunicationHistory::Entry entry = m_packet_history.back();
 m_packet_history.pop_back();
 
+// We're handled the handshake implicitly before. Skip the packet and move
+// on.
+if (entry.packet.data == "+")
+  continue;
+
 if (entry.type == GDBRemoteCommunicationHistory::ePacketTypeSend) {
   if (unexpected(entry.packet.data, packet.GetStringRef())) {
 LLDB_LOG(log,
  "GDBRemoteCommunicationReplayServer expected packet: '{0}'",
  entry.packet.data);
-LLDB_LOG(log,
-