[Lldb-commits] [lldb] r326552 - [testsuite] Remove workaround for categories and inline tests.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Fri Mar  2 02:38:11 2018
New Revision: 326552

URL: http://llvm.org/viewvc/llvm-project?rev=326552&view=rev
Log:
[testsuite] Remove workaround for categories and inline tests.

Adding categories to inline tests does not work because the attribute
is set at the function level. For methods, this means it applies to all
instances of that particular class. While this is what we want in most
cases, it's not for inline tests, where different instances correspond
to different tests.

With the workaround in place, assigning a category to one test resulted
in the category applied to *all* inline tests.

This patch removes the workaround and throws an exception with an
informative error message, to prevent this from happening in the future.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/decorators.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/decorators.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/decorators.py?rev=326552&r1=326551&r2=326552&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/decorators.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/decorators.py Fri Mar  2 02:38:11 
2018
@@ -304,15 +304,12 @@ def add_test_categories(cat):
 if isinstance(func, type) and issubclass(func, unittest2.TestCase):
 raise Exception(
 "@add_test_categories can only be used to decorate a test 
method")
-
-# Update or set the categories attribute. For instance methods, the
-# attribute must be set on the actual function.
-func_for_attr = func
-if inspect.ismethod(func_for_attr):
-func_for_attr = func.__func__
-if hasattr(func_for_attr, "categories"):
-cat.extend(func_for_attr.categories)
-setattr(func_for_attr, "categories", cat)
+try:
+if hasattr(func, "categories"):
+cat.extend(func.categories)
+setattr(func, "categories", cat)
+except AttributeError:
+raise Exception('Cannot assign categories to inline tests.')
 
 return func
 


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

[Lldb-commits] [PATCH] D44015: Fix std unique pointer not printing.

[Alexandre Yukio Yamashita via Phabricator via lldb-commits]

alexandreyy created this revision.
alexandreyy added reviewers: labath, clayborg.

Std unique pointers were not being printed. The unique pointer layout was 
changed in libstdc++ 6.0.23.


https://reviews.llvm.org/D44015

Files:
  source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp


Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -64,6 +64,16 @@
 
   ValueObjectSP tuple_sp =
   valobj_sp->GetChildMemberWithName(ConstString("_M_t"), true);
+
+  ValueObjectSP tuple_sp_child  =
+  tuple_sp->GetChildMemberWithName(ConstString("_M_t"), true);
+
+  /* if there is a _M_t child, the pointers are found in the
+   * tuple_sp_child (for libstdc++ 6.0.23). */
+  if (tuple_sp_child) {
+tuple_sp = tuple_sp_child;
+  }
+
   if (!tuple_sp)
 return false;
 


Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -64,6 +64,16 @@
 
   ValueObjectSP tuple_sp =
   valobj_sp->GetChildMemberWithName(ConstString("_M_t"), true);
+
+  ValueObjectSP tuple_sp_child  =
+  tuple_sp->GetChildMemberWithName(ConstString("_M_t"), true);
+
+  /* if there is a _M_t child, the pointers are found in the
+   * tuple_sp_child (for libstdc++ 6.0.23). */
+  if (tuple_sp_child) {
+tuple_sp = tuple_sp_child;
+  }
+
   if (!tuple_sp)
 return false;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks. The logging stuff looks good to me. I know very little about the 
TypeFromUser class and friends, so someone else should probably ack that.


https://reviews.llvm.org/D43912



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

[Lldb-commits] [PATCH] D43984: Make the clang module cache setting available without a target

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 136803.
aprantl added a comment.

Added a testcase.


https://reviews.llvm.org/D43984

Files:
  include/lldb/Core/ModuleList.h
  include/lldb/Target/Target.h
  packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
  
packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py
  packages/Python/lldbsuite/test/lang/objc/modules-cache/f.h
  packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m
  packages/Python/lldbsuite/test/lang/objc/modules-cache/module.modulemap
  packages/Python/lldbsuite/test/lldbtest.py
  source/Core/Debugger.cpp
  source/Core/ModuleList.cpp
  source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -3509,9 +3509,6 @@
  OptionValue::eTypeString, nullptr, nullptr,
  "A list of trap handler function names, e.g. a common Unix user process "
  "one is _sigtramp."},
-{"clang-modules-cache-path",
- OptionValue::eTypeFileSpec, false, 0, nullptr, nullptr,
- "The path to the clang modules cache directory (-fmodules-cache-path)."},
 {"display-runtime-support-values", OptionValue::eTypeBoolean, false, false,
  nullptr, nullptr, "If true, LLDB will show variables that are meant to "
"support the operation of a language's runtime "
@@ -3561,7 +3558,6 @@
   ePropertyMemoryModuleLoadLevel,
   ePropertyDisplayExpressionsInCrashlogs,
   ePropertyTrapHandlerNames,
-  ePropertyClangModulesCachePath,
   ePropertyDisplayRuntimeSupportValues,
   ePropertyNonStopModeEnabled,
   ePropertyExperimental
@@ -3941,15 +3937,6 @@
   return option_value->GetCurrentValue();
 }
 
-FileSpec &TargetProperties::GetClangModulesCachePath() {
-  const uint32_t idx = ePropertyClangModulesCachePath;
-  OptionValueFileSpec *option_value =
-  m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
-   idx);
-  assert(option_value);
-  return option_value->GetCurrentValue();
-}
-
 FileSpecList &TargetProperties::GetClangModuleSearchPaths() {
   const uint32_t idx = ePropertyClangModuleSearchPaths;
   OptionValueFileSpecList *option_value =
Index: source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -13,7 +13,6 @@
 
 // Other libraries and framework includes
 #include "clang/Basic/TargetInfo.h"
-#include "clang/Driver/Driver.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Preprocessor.h"
@@ -28,6 +27,7 @@
 // Project includes
 #include "ClangModulesDeclVendor.h"
 
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/CompileUnit.h"
@@ -591,12 +591,11 @@
   // Add additional search paths with { "-I", path } or { "-F", path } here.
 
   {
-llvm::SmallString<128> Path;
-target.GetClangModulesCachePath().GetPath(Path);
-if (Path.empty())
-  clang::driver::Driver::getDefaultModuleCachePath(Path);
+llvm::SmallString<128> path;
+auto props = ModuleList::GetGlobalModuleListProperties();
+props.GetClangModulesCachePath().GetPath(path);
 std::string module_cache_argument("-fmodules-cache-path=");
-module_cache_argument.append(Path.str());
+module_cache_argument.append(path.str());
 compiler_invocation_arguments.push_back(module_cache_argument);
   }
 
Index: source/Core/ModuleList.cpp
===
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -13,6 +13,9 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Interpreter/OptionValueProperties.h"
+#include "lldb/Interpreter/OptionValueFileSpec.h"
+#include "lldb/Interpreter/Property.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h" // for SymbolContextList, SymbolCon...
 #include "lldb/Symbol/VariableList.h"
@@ -31,6 +34,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h" // for fs
+#include "clang/Driver/Driver.h"
 
 #include  // for operator!=, time_point
 #include  // for shared_ptr
@@ -60,6 +64,40 @@
 using namespace lldb;
 using namespace lldb_private;
 
+namespace {
+
+PropertyDefinition g_properties[] = {
+{"modules-cache-path", OptionValue::eTypeFileSpec, true, 0, nullptr,
+ nullptr,
+ "The path to the clang modules cache directory (-fmodules-cache-path)."},
+{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, nullptr, nullptr}};
+
+enum { 

Re: [Lldb-commits] [lldb] r326552 - [testsuite] Remove workaround for categories and inline tests.

[Vedant Kumar via lldb-commits]

Thanks for taking care of this!

vedant

> On Mar 2, 2018, at 2:38 AM, Jonas Devlieghere via lldb-commits 
>  wrote:
> 
> Author: jdevlieghere
> Date: Fri Mar  2 02:38:11 2018
> New Revision: 326552
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=326552&view=rev
> Log:
> [testsuite] Remove workaround for categories and inline tests.
> 
> Adding categories to inline tests does not work because the attribute
> is set at the function level. For methods, this means it applies to all
> instances of that particular class. While this is what we want in most
> cases, it's not for inline tests, where different instances correspond
> to different tests.
> 
> With the workaround in place, assigning a category to one test resulted
> in the category applied to *all* inline tests.
> 
> This patch removes the workaround and throws an exception with an
> informative error message, to prevent this from happening in the future.
> 
> Modified:
>lldb/trunk/packages/Python/lldbsuite/test/decorators.py
> 
> Modified: lldb/trunk/packages/Python/lldbsuite/test/decorators.py
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/decorators.py?rev=326552&r1=326551&r2=326552&view=diff
> ==
> --- lldb/trunk/packages/Python/lldbsuite/test/decorators.py (original)
> +++ lldb/trunk/packages/Python/lldbsuite/test/decorators.py Fri Mar  2 
> 02:38:11 2018
> @@ -304,15 +304,12 @@ def add_test_categories(cat):
> if isinstance(func, type) and issubclass(func, unittest2.TestCase):
> raise Exception(
> "@add_test_categories can only be used to decorate a test 
> method")
> -
> -# Update or set the categories attribute. For instance methods, the
> -# attribute must be set on the actual function.
> -func_for_attr = func
> -if inspect.ismethod(func_for_attr):
> -func_for_attr = func.__func__
> -if hasattr(func_for_attr, "categories"):
> -cat.extend(func_for_attr.categories)
> -setattr(func_for_attr, "categories", cat)
> +try:
> +if hasattr(func, "categories"):
> +cat.extend(func.categories)
> +setattr(func, "categories", cat)
> +except AttributeError:
> +raise Exception('Cannot assign categories to inline tests.')
> 
> return func
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D44041: Only replace object file sections when non-empty

[Francis Ricci via Phabricator via lldb-commits]

fjricci created this revision.
fjricci added reviewers: clayborg, zturner, labath.
Herald added subscribers: JDevlieghere, arichardson, emaste.

In order to allow some sections to exist either in split debug-info or in the 
main binary,
don't replace non-empty sections with empty sections.


https://reviews.llvm.org/D44041

Files:
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp


Index: source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
===
--- source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -141,7 +141,7 @@
 SectionType section_type = g_sections[idx];
 SectionSP section_sp(
 objfile_section_list->FindSectionByType(section_type, true));
-if (section_sp) {
+if (section_sp && section_sp->GetFileSize()) {
   SectionSP module_section_sp(
   module_section_list->FindSectionByType(section_type, true));
   if (module_section_sp)
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2004,7 +2004,7 @@
 SectionType section_type = g_sections[idx];
 SectionSP section_sp(
 elf_section_list->FindSectionByType(section_type, true));
-if (section_sp) {
+if (section_sp && section_sp->GetFileSize()) {
   SectionSP module_section_sp(
   unified_section_list.FindSectionByType(section_type, true));
   if (module_section_sp)


Index: source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
===
--- source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -141,7 +141,7 @@
 SectionType section_type = g_sections[idx];
 SectionSP section_sp(
 objfile_section_list->FindSectionByType(section_type, true));
-if (section_sp) {
+if (section_sp && section_sp->GetFileSize()) {
   SectionSP module_section_sp(
   module_section_list->FindSectionByType(section_type, true));
   if (module_section_sp)
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2004,7 +2004,7 @@
 SectionType section_type = g_sections[idx];
 SectionSP section_sp(
 elf_section_list->FindSectionByType(section_type, true));
-if (section_sp) {
+if (section_sp && section_sp->GetFileSize()) {
   SectionSP module_section_sp(
   unified_section_list.FindSectionByType(section_type, true));
   if (module_section_sp)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44042: Ensure that trailing characters aren't included in PECOFF section names

[Francis Ricci via Phabricator via lldb-commits]

fjricci created this revision.
fjricci added reviewers: clayborg, zturner, wallace.

If a section name is exactly 8 characters (the maximum section name length),
and the next item in the section header struct contains a non-zero value,
we would append garbage data to the end of the section name string due to the
lack of null-termination. Ensure that we don't construct the section name
with more than sizeof(sect.name) characters.


https://reviews.llvm.org/D44042

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -505,7 +505,10 @@
 
 return false;
   }
-  sect_name = sect.name;
+
+  // The section name has a max length of 8 characters, but isn't
+  // necessarily null-terminated
+  sect_name = std::string(sect.name, sizeof(sect.name));
   return true;
 }
 


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -505,7 +505,10 @@
 
 return false;
   }
-  sect_name = sect.name;
+
+  // The section name has a max length of 8 characters, but isn't
+  // necessarily null-terminated
+  sect_name = std::string(sect.name, sizeof(sect.name));
   return true;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44042: Ensure that trailing characters aren't included in PECOFF section names

[walter erquinigo via Phabricator via lldb-commits]

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

good catch


https://reviews.llvm.org/D44042



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

[Lldb-commits] [PATCH] D44041: Only replace object file sections when non-empty

[Pavel Labath via Phabricator via lldb-commits]

labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.

This looks like a perfect case for lldb-test. You will run into a couple of 
problems which will prevent this from working out of the box. I tried to fix 
those in https://reviews.llvm.org/D42955, but that turned into a major 
refactor. Until that CL lands, we should be able to make a quick hack in 
lldb-test which will make this testable:
The thing you will need to modify is to explicitly call 
module->GetSymbolVendor() before dumping out the sections (so that the symbol 
vendor populates these), and avoid setting the SymbolFileSpec on the ModuleSpec 
(so that the symbol vendor will find the external symbol file.


https://reviews.llvm.org/D44041



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

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Pavel Labath via Phabricator via lldb-commits]

labath requested review of this revision.
labath added a comment.

Explicitly requesting re-review to make sure this is on your radar.

We have a new patch that  would benefit from 
this, but if this is not ready to go in yet (it's a turned into a fairly big 
refactor), I'll probably add a workaround to lldb-test to make that patch 
testable.


https://reviews.llvm.org/D42955



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

[Lldb-commits] [PATCH] D44042: Ensure that trailing characters aren't included in PECOFF section names

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Is it possible to test this? Is it possible to make yaml2obj generate a file 
that would trigger this?


https://reviews.llvm.org/D44042



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

[Lldb-commits] [PATCH] D44042: Ensure that trailing characters aren't included in PECOFF section names

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Just compile something with /Z7 and you'll get a section called `.debug$S` in 
the object file, which is exactly 8 characters.  Then teach lldb-test to dump 
an object file's sections.


https://reviews.llvm.org/D44042



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

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Greg Clayton via Phabricator via lldb-commits]

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

Looks fine, a bit hard to tell exactly what is going on so I will accept as 
long as the following things are still true:

- each lldb_private::ObjectFile has its own section list that perfectly mirrors 
exactly what is in that object file and that file alone
- The lldb_private::Module hands out a unified section list that is populated 
by the symbol vendor where it uses one or more object files to create the 
unified section list


https://reviews.llvm.org/D42955



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

[Lldb-commits] [PATCH] D44042: Ensure that trailing characters aren't included in PECOFF section names

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Add test as well.


https://reviews.llvm.org/D44042



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

[Lldb-commits] [PATCH] D44042: Ensure that trailing characters aren't included in PECOFF section names

[Greg Clayton via Phabricator via lldb-commits]

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



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:511
+  // necessarily null-terminated
+  sect_name = std::string(sect.name, sizeof(sect.name));
   return true;

This is storing any extra NULL characters in sect_name. Might be better to do:

```
sect_name = std::string(sect.name, strnlen(sect.name, sizeof(sect.name)));
```


https://reviews.llvm.org/D44042



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

[Lldb-commits] [PATCH] D44042: Ensure that trailing characters aren't included in PECOFF section names

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Will we have the same problem with coff_symbol?


https://reviews.llvm.org/D44042



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

[Lldb-commits] [lldb] r326628 - Make the clang module cache setting available without a target

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Fri Mar  2 14:42:44 2018
New Revision: 326628

URL: http://llvm.org/viewvc/llvm-project?rev=326628&view=rev
Log:
Make the clang module cache setting available without a target

It turns out that setting the clang module cache after LLDB has a
Target can be too late. In particular, the Swift language plugin needs
to know the setting without having access to a Target. This patch
moves the setting into the *LLDB* module cache, where it is a global
setting that is available before any Target is created and more
importantly, is shared between all Targets.

rdar://problem/37944432

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

Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/f.h
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/module.modulemap
Modified:
lldb/trunk/include/lldb/Core/ModuleList.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/Core/ModuleList.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ModuleList.h?rev=326628&r1=326627&r2=326628&view=diff
==
--- lldb/trunk/include/lldb/Core/ModuleList.h (original)
+++ lldb/trunk/include/lldb/Core/ModuleList.h Fri Mar  2 14:42:44 2018
@@ -12,6 +12,7 @@
 
 #include "lldb/Core/Address.h" // for Address
 #include "lldb/Core/ModuleSpec.h"  // for ModuleSpec
+#include "lldb/Core/UserSettingsController.h"
 #include "lldb/Utility/FileSpec.h" // for FileSpec
 #include "lldb/Utility/Iterable.h"
 #include "lldb/Utility/Status.h" // for Status
@@ -74,6 +75,14 @@ class VariableList;
 
 namespace lldb_private {
 
+class ModuleListProperties : public Properties {
+public:
+  ModuleListProperties();
+
+  FileSpec GetClangModulesCachePath() const;
+  bool SetClangModulesCachePath(llvm::StringRef path);
+}; 
+
 //--
 /// @class ModuleList ModuleList.h "lldb/Core/ModuleList.h"
 /// @brief A collection class for Module objects.
@@ -534,6 +543,8 @@ public:
   Stream *feedback_stream = nullptr,
   bool continue_on_error = true);
 
+  static ModuleListProperties &GetGlobalModuleListProperties();
+
   static bool ModuleIsInCache(const Module *module_ptr);
 
   static Status GetSharedModule(const ModuleSpec &module_spec,
@@ -551,7 +562,7 @@ public:
   static size_t RemoveOrphanSharedModules(bool mandatory);
 
   static bool RemoveSharedModuleIfOrphaned(const Module *module_ptr);
-
+  
   void ForEach(std::function const
&callback) const;
 

Modified: lldb/trunk/include/lldb/Target/Target.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=326628&r1=326627&r2=326628&view=diff
==
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Fri Mar  2 14:42:44 2018
@@ -126,8 +126,6 @@ public:
 
   FileSpecList &GetDebugFileSearchPaths();
 
-  FileSpec &GetClangModulesCachePath();
-
   FileSpecList &GetClangModuleSearchPaths();
 
   bool GetEnableAutoImportClangModules() const;

Added: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile?rev=326628&view=auto
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile 
(added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile 
Fri Mar  2 14:42:44 2018
@@ -0,0 +1,4 @@
+LEVEL = ../../../make
+OBJC_SOURCES := main.m
+include $(LEVEL)/Makefile.rules
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(SRCDIR)

Added: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py?rev=326628&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc

[Lldb-commits] [PATCH] D43984: Make the clang module cache setting available without a target

[Phabricator via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL326628: Make the clang module cache setting available 
without a target (authored by adrian, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43984?vs=136803&id=136858#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43984

Files:
  lldb/trunk/include/lldb/Core/ModuleList.h
  lldb/trunk/include/lldb/Target/Target.h
  lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py
  lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/f.h
  lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m
  
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/module.modulemap
  lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
  lldb/trunk/source/Core/Debugger.cpp
  lldb/trunk/source/Core/ModuleList.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  lldb/trunk/source/Target/Target.cpp

Index: lldb/trunk/include/lldb/Core/ModuleList.h
===
--- lldb/trunk/include/lldb/Core/ModuleList.h
+++ lldb/trunk/include/lldb/Core/ModuleList.h
@@ -12,6 +12,7 @@
 
 #include "lldb/Core/Address.h" // for Address
 #include "lldb/Core/ModuleSpec.h"  // for ModuleSpec
+#include "lldb/Core/UserSettingsController.h"
 #include "lldb/Utility/FileSpec.h" // for FileSpec
 #include "lldb/Utility/Iterable.h"
 #include "lldb/Utility/Status.h" // for Status
@@ -74,6 +75,14 @@
 
 namespace lldb_private {
 
+class ModuleListProperties : public Properties {
+public:
+  ModuleListProperties();
+
+  FileSpec GetClangModulesCachePath() const;
+  bool SetClangModulesCachePath(llvm::StringRef path);
+}; 
+
 //--
 /// @class ModuleList ModuleList.h "lldb/Core/ModuleList.h"
 /// @brief A collection class for Module objects.
@@ -534,6 +543,8 @@
   Stream *feedback_stream = nullptr,
   bool continue_on_error = true);
 
+  static ModuleListProperties &GetGlobalModuleListProperties();
+
   static bool ModuleIsInCache(const Module *module_ptr);
 
   static Status GetSharedModule(const ModuleSpec &module_spec,
@@ -551,7 +562,7 @@
   static size_t RemoveOrphanSharedModules(bool mandatory);
 
   static bool RemoveSharedModuleIfOrphaned(const Module *module_ptr);
-
+  
   void ForEach(std::function const
&callback) const;
 
Index: lldb/trunk/include/lldb/Target/Target.h
===
--- lldb/trunk/include/lldb/Target/Target.h
+++ lldb/trunk/include/lldb/Target/Target.h
@@ -126,8 +126,6 @@
 
   FileSpecList &GetDebugFileSearchPaths();
 
-  FileSpec &GetClangModulesCachePath();
-
   FileSpecList &GetClangModuleSearchPaths();
 
   bool GetEnableAutoImportClangModules() const;
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -1921,8 +1921,8 @@
 if self.child:
 assert(self.getDebugInfo() == 'default')
 mod_cache = os.path.join(self.getBuildDir(), "module-cache")
-self.runCmd("settings set target.clang-modules-cache-path "
-+ mod_cache)
+self.runCmd('settings set clang.modules-cache-path "%s"'
+% mod_cache)
 
 
 if "LLDB_MAX_LAUNCH_COUNT" in os.environ:
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m
@@ -0,0 +1,5 @@
+@import Foo;
+int main() {
+  f(); // Set breakpoint here.
+  return 0;
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
@@ -0,0 +1,4 @@
+LEVEL = ../../../make
+OBJC_SOURCES := main.m
+include $(LEVEL)/Makefile.rules
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(SRCDIR)
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/f.h
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/f.h
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-c

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I can try to split the patch up a bit if you think it will make things easier.  
I'm not sure how much I will be able to do that, as the patch is not that big, 
it just needs to touch a bunch of files for the changed interfaces.

In https://reviews.llvm.org/D42955#1026061, @clayborg wrote:

> Looks fine, a bit hard to tell exactly what is going on so I will accept as 
> long as the following things are still true:
>
> - each lldb_private::ObjectFile has its own section list that perfectly 
> mirrors exactly what is in that object file and that file alone


This part is not completely true. It is true for ObjectFileELF/COFF/JIT, and is 
enforced by the fact that the ObjectFile::GetSections does not even have access 
to the unified section list. However, it is *not* true for ObjectFileMachO (but 
that is not because of this patch). This is the problem I've had when trying to 
refactor this (and it's the reason the ObjectFileMachO<->SymbolVendorMacOSX 
interface is a bit weird). It seems that the ObjectFileMachO sometimes takes a 
section from the unified list and adds it to it's own list (or at least it 
appears to be doing that). I don't know enough about MachO to understand why is 
it doing that, or how to fix it. I've highlighted the places in the code where 
I think this is happening. Any suggestions would be welcome here.

> - The lldb_private::Module hands out a unified section list that is populated 
> by the symbol vendor where it uses one or more object files to create the 
> unified section list

This part is true.




Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1415-1416
 
   SectionSP unified_section_sp(
   unified_section_list.FindSectionByName(const_segname));
   if (is_dsym && unified_section_sp) {

Here we are grabbing a section from the unified section list.



Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1562
   }
   m_sections_ap->AddSection(unified_section_sp);
 }

And here it gets added to the object file's section list.


https://reviews.llvm.org/D42955



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

[Lldb-commits] [lldb] r326634 - Don't compile testcase with clang modules enabled.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Fri Mar  2 15:15:04 2018
New Revision: 326634

URL: http://llvm.org/viewvc/llvm-project?rev=326634&view=rev
Log:
Don't compile testcase with clang modules enabled.

It isn't actually necessary for what we are testing here and should
fix the test on the Linux bots.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile?rev=326634&r1=326633&r2=326634&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile 
Fri Mar  2 15:15:04 2018
@@ -1,4 +1,3 @@
 LEVEL = ../../../make
 OBJC_SOURCES := main.m
 include $(LEVEL)/Makefile.rules
-CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(SRCDIR)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m?rev=326634&r1=326633&r2=326634&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m 
Fri Mar  2 15:15:04 2018
@@ -1,4 +1,4 @@
-@import Foo;
+#include "f.h"
 int main() {
   f(); // Set breakpoint here.
   return 0;


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

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D42955#1026116, @labath wrote:

> I can try to split the patch up a bit if you think it will make things 
> easier.  I'm not sure how much I will be able to do that, as the patch is not 
> that big, it just needs to touch a bunch of files for the changed interfaces.
>
> In https://reviews.llvm.org/D42955#1026061, @clayborg wrote:
>
> > Looks fine, a bit hard to tell exactly what is going on so I will accept as 
> > long as the following things are still true:
> >
> > - each lldb_private::ObjectFile has its own section list that perfectly 
> > mirrors exactly what is in that object file and that file alone
>
>
> This part is not completely true. It is true for ObjectFileELF/COFF/JIT, and 
> is enforced by the fact that the ObjectFile::GetSections does not even have 
> access to the unified section list. However, it is *not* true for 
> ObjectFileMachO (but that is not because of this patch). This is the problem 
> I've had when trying to refactor this (and it's the reason the 
> ObjectFileMachO<->SymbolVendorMacOSX interface is a bit weird). It seems that 
> the ObjectFileMachO sometimes takes a section from the unified list and adds 
> it to it's own list (or at least it appears to be doing that). I don't know 
> enough about MachO to understand why is it doing that, or how to fix it. I've 
> highlighted the places in the code where I think this is happening. Any 
> suggestions would be welcome here.


The dSYM file is a mach-o file that contains symbols only, It is because the 
dSYM file (stand alone debug info file) has all of the section definitions from 
the main executable, but no section content for everything except the DWARF 
debug info. The DWARF only exists in the dSYM file. The dSYM file also has all 
of the symbols as it gets made before the executable is stripped. Many symbols 
refer to a section by section index, so that is why we must have the section 
definitions in the dSYM file. So the dSYM wants the sections from the main 
executable so it can access __text and __data if needed since symbol refer to 
these and someone might ask a symbol from a dSYM file what its instructions 
are. So the dSYM uses the real sections from the main executable file instead 
of its own.

> 
> 
>> - The lldb_private::Module hands out a unified section list that is 
>> populated by the symbol vendor where it uses one or more object files to 
>> create the unified section list
> 
> This part is true.




https://reviews.llvm.org/D42955



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

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D42955#1026175, @clayborg wrote:

> The dSYM file is a mach-o file that contains symbols only, It is because the 
> dSYM file (stand alone debug info file) has all of the section definitions 
> from the main executable, but no section content for everything except the 
> DWARF debug info. The DWARF only exists in the dSYM file. The dSYM file also 
> has all of the symbols as it gets made before the executable is stripped. 
> Many symbols refer to a section by section index, so that is why we must have 
> the section definitions in the dSYM file. So the dSYM wants the sections from 
> the main executable so it can access __text and __data if needed since symbol 
> refer to these and someone might ask a symbol from a dSYM file what its 
> instructions are. So the dSYM uses the real sections from the main executable 
> file instead of its own.


I see, thanks for the explanation. The code makes more sense now.

But now I have another question. Is this an intentional design decision that we 
want to preserve; or just how things happen to work now, and should be cleaned 
up in the future (to achieve the "ObjectFile contains only own sections" 
invariant)?

I'm asking this, because if it's the former then this patch probably needs to 
be redone, as it tries to enforce that invariant (and then needs to jump 
through hoops to enable the ObjectFileMachO use case). OTOH, if it's the 
latter, then I think this is a step in the right direction, as it makes it 
obvious that ObjectFileMachO is doing something that it shouldn't (and at the 
same time it makes it harder for other ObjectFiles to do the same).

BTW, the same situation (symbol file containing empty section definitions for 
code&data) can happen on elf targets as well (if the symbol file is produced 
with `strip --only-keep-debug`), but everything seems to be working fine, 
presumably because all consumers are accessing the sections through the unified 
section list.


https://reviews.llvm.org/D42955



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

Re: [Lldb-commits] [lldb] r326634 - Don't compile testcase with clang modules enabled.

[Pavel Labath via lldb-commits]

That didn't seem to do the trick:

TBH, I'd be surprised if you can pull off compiling a fully working
objc executable on linux.

On 2 March 2018 at 15:15, Adrian Prantl via lldb-commits
 wrote:
> Author: adrian
> Date: Fri Mar  2 15:15:04 2018
> New Revision: 326634
>
> URL: http://llvm.org/viewvc/llvm-project?rev=326634&view=rev
> Log:
> Don't compile testcase with clang modules enabled.
>
> It isn't actually necessary for what we are testing here and should
> fix the test on the Linux bots.
>
> Modified:
> lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
> lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile?rev=326634&r1=326633&r2=326634&view=diff
> ==
> --- 
> lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile 
> (original)
> +++ 
> lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile 
> Fri Mar  2 15:15:04 2018
> @@ -1,4 +1,3 @@
>  LEVEL = ../../../make
>  OBJC_SOURCES := main.m
>  include $(LEVEL)/Makefile.rules
> -CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(SRCDIR)
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m?rev=326634&r1=326633&r2=326634&view=diff
> ==
> --- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m 
> (original)
> +++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/main.m 
> Fri Mar  2 15:15:04 2018
> @@ -1,4 +1,4 @@
> -@import Foo;
> +#include "f.h"
>  int main() {
>f(); // Set breakpoint here.
>return 0;
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In https://reviews.llvm.org/D42955#1026197, @labath wrote:

> In https://reviews.llvm.org/D42955#1026175, @clayborg wrote:
>
> > The dSYM file is a mach-o file that contains symbols only, It is because 
> > the dSYM file (stand alone debug info file) has all of the section 
> > definitions from the main executable, but no section content for everything 
> > except the DWARF debug info. The DWARF only exists in the dSYM file. The 
> > dSYM file also has all of the symbols as it gets made before the executable 
> > is stripped. Many symbols refer to a section by section index, so that is 
> > why we must have the section definitions in the dSYM file. So the dSYM 
> > wants the sections from the main executable so it can access __text and 
> > __data if needed since symbol refer to these and someone might ask a symbol 
> > from a dSYM file what its instructions are. So the dSYM uses the real 
> > sections from the main executable file instead of its own.
>
>
> I see, thanks for the explanation. The code makes more sense now.
>
> But now I have another question. Is this an intentional design decision that 
> we want to preserve; or just how things happen to work now, and should be 
> cleaned up in the future (to achieve the "ObjectFile contains only own 
> sections" invariant)?


Intentional, but it is up to the SymbolVendor subclass to do all of this. We 
want the data from the dSYM to be useful and since the dSYM really is a 
companion debug info file, it needs info from elsewhere to be complete.

> I'm asking this, because if it's the former then this patch probably needs to 
> be redone, as it tries to enforce that invariant (and then needs to jump 
> through hoops to enable the ObjectFileMachO use case). OTOH, if it's the 
> latter, then I think this is a step in the right direction, as it makes it 
> obvious that ObjectFileMachO is doing something that it shouldn't (and at the 
> same time it makes it harder for other ObjectFiles to do the same).

It should be doing it. Again, the symbol vendor is the one that is trying to 
make everything fit together and "just work".

> BTW, the same situation (symbol file containing empty section definitions for 
> code&data) can happen on elf targets as well (if the symbol file is produced 
> with `strip --only-keep-debug`), but everything seems to be working fine, 
> presumably because all consumers are accessing the sections through the 
> unified section list.

Exactly. I see any good debug info format not needing to duplicate things that 
might be in another file. The symbol vendor under the covers, creates a unified 
experience for all consumers of the Module that contains the symbol vendor, so 
it is something that we want to encourage and allow. So ObjectFileMachO isn't 
doing anything wrong. Feel free to update the design if needed?


https://reviews.llvm.org/D42955



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

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

AS for the ELF example where only debug info is around, it might not be running 
into issues if it doesn't contain a symbol table. If it does contain a symbol 
table, hopefully it is using the unified section table, otherwise if might have 
symbol's whose addresses have sections from the stripped ELF file and it might 
not have the section contents that it needs to back it. In that case we might 
fail to disassemble the symbol's code.


https://reviews.llvm.org/D42955



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

[Lldb-commits] [lldb] r326640 - Mark ObjC testcase as skipUnlessDarwin and fix a typo in test function.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Fri Mar  2 15:57:09 2018
New Revision: 326640

URL: http://llvm.org/viewvc/llvm-project?rev=326640&view=rev
Log:
Mark ObjC testcase as skipUnlessDarwin and fix a typo in test function.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py?rev=326640&r1=326639&r2=326640&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-cache/TestClangModulesCache.py
 Fri Mar  2 15:57:09 2018
@@ -22,6 +22,7 @@ class ObjCModulesTestCase(TestBase):
 def setUp(self):
 TestBase.setUp(self)
 
+@skipUnlessDarwin
 def test_expr(self):
 self.build()
 self.main_source_file = lldb.SBFileSpec("main.m")
@@ -36,5 +37,5 @@ class ObjCModulesTestCase(TestBase):
 % self.getSourceDir())
 (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
 self, "Set breakpoint here", self.main_source_file)
-self.runCmd("expr @import Darwin")
+self.runCmd("expr @import Foo")
 self.assertTrue(os.path.isdir(mod_cache), "module cache exists")


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

[Lldb-commits] [PATCH] D44055: [lldb] Fix "requires global constructor" warning in g_range_specifiers

[Kuba (Brecka) Mracek via Phabricator via lldb-commits]

kubamracek created this revision.
kubamracek added reviewers: jingham, davide, jasonmolenda, aprantl.

https://reviews.llvm.org/D44055

Files:
  source/Breakpoint/BreakpointID.cpp


Index: source/Breakpoint/BreakpointID.cpp
===
--- source/Breakpoint/BreakpointID.cpp
+++ source/Breakpoint/BreakpointID.cpp
@@ -26,16 +26,14 @@
 
 BreakpointID::~BreakpointID() = default;
 
-static llvm::StringRef g_range_specifiers[] = {"-", "to", "To", "TO"};
-
 // Tells whether or not STR is valid to use between two strings representing
 // breakpoint IDs, to
 // indicate a range of breakpoint IDs.  This is broken out into a separate
 // function so that we can
 // easily change or add to the format for specifying ID ranges at a later date.
 
 bool BreakpointID::IsRangeIdentifier(llvm::StringRef str) {
-  for (auto spec : g_range_specifiers) {
+  for (auto spec : GetRangeSpecifiers()) {
 if (spec == str)
   return true;
   }
@@ -48,6 +46,7 @@
 }
 
 llvm::ArrayRef BreakpointID::GetRangeSpecifiers() {
+  static llvm::StringRef g_range_specifiers[] = {"-", "to", "To", "TO"};
   return llvm::makeArrayRef(g_range_specifiers);
 }
 


Index: source/Breakpoint/BreakpointID.cpp
===
--- source/Breakpoint/BreakpointID.cpp
+++ source/Breakpoint/BreakpointID.cpp
@@ -26,16 +26,14 @@
 
 BreakpointID::~BreakpointID() = default;
 
-static llvm::StringRef g_range_specifiers[] = {"-", "to", "To", "TO"};
-
 // Tells whether or not STR is valid to use between two strings representing
 // breakpoint IDs, to
 // indicate a range of breakpoint IDs.  This is broken out into a separate
 // function so that we can
 // easily change or add to the format for specifying ID ranges at a later date.
 
 bool BreakpointID::IsRangeIdentifier(llvm::StringRef str) {
-  for (auto spec : g_range_specifiers) {
+  for (auto spec : GetRangeSpecifiers()) {
 if (spec == str)
   return true;
   }
@@ -48,6 +46,7 @@
 }
 
 llvm::ArrayRef BreakpointID::GetRangeSpecifiers() {
+  static llvm::StringRef g_range_specifiers[] = {"-", "to", "To", "TO"};
   return llvm::makeArrayRef(g_range_specifiers);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D42955#1026216, @clayborg wrote:

> AS for the ELF example where only debug info is around, it might not be 
> running into issues if it doesn't contain a symbol table. If it does contain 
> a symbol table, hopefully it is using the unified section table, otherwise if 
> might have symbol's whose addresses have sections from the stripped ELF file 
> and it might not have the section contents that it needs to back it. In that 
> case we might fail to disassemble the symbol's code.


I made a trivial experiment:

  g++ a.cc
  objcopy a.out --only-keep-debug a.sym #a.sym contains a symtab
  strip a.out # a.out does not contain a symtab
  (lldb) target create "a.out"
  Current executable set to 'a.out' (x86_64).
  (lldb) b main
  Breakpoint 1: no locations (pending).
  WARNING:  Unable to resolve breakpoint to any actual locations.
  (lldb) target symbols add a.sym
  symbol file '/tmp/X/a.sym' has been added to '/tmp/X/a.out'
  1 location added to breakpoint 1
  #symbol resolution seems fine
  (lldb) disassemble -n main
  a.out`main:
  a.out[0x69b] <+0>:  pushq  %rbp
  a.out[0x69c] <+1>:  movq   %rsp, %rbp
  a.out[0x69f] <+4>:  callq  0x690 ; foo()
  a.out[0x6a4] <+9>:  addl   $0x2a, %eax
  a.out[0x6a7] <+12>: popq   %rbp
  a.out[0x6a8] <+13>: retq
  # so does disassembling

The part that is not clear to me is, if the dsym object file should absorb the 
sections from the main object file, then what is the purpose of the "unified 
section list" in the module? I can see why we need a unified list, and I think 
it's a good idea, but having an ObjectFile containing the merged list seems to 
defeat it. From a separation of concerns POV, it would be much clearer if each 
ObjectFile contained only it's own sections, and then some other entity 
(Module) held the combined data(sections) of the individual object files. Then, 
most clients should operate on the unified view of the module, but if anyone 
ever had a reason, he could always look directly at a specific ObjectFile, and 
see what's contained there.

Among other things, this could be very useful for lldb-server. lldb-server 
needs only lightweight access to the data in the object file -- it does not 
need the Module class and everything it pulls in (SymbolVendor, SymbolFile, 
...). If we could make the ObjectFile class completely standalone, we can avoid 
pulling all this stuff in and make the code more modular. It would also help 
with the migration to llvm's Object parsing library, as it knows nothing about 
unified sections. If we had a standalone ObjectFile implementation, then it 
would be easy to swap it out for llvm's and still keep the "section 
unification" code intact as it would be in a different layer.

So, could this be a direction we can move in? I can put this patch on ice and 
try to make the ObjectFileMachO standalone instead. I don't think it should be 
that hard, given that this is how things already work for elf. I'm hoping it 
will be a matter of changing some consumers to use module->GetSectionList() 
instead of objfile->GetSectionList()...


https://reviews.llvm.org/D42955



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

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

[Kuba (Brecka) Mracek via Phabricator via lldb-commits]

kubamracek created this revision.
kubamracek added reviewers: davide, jasonmolenda, jingham, aprantl.

https://reviews.llvm.org/D44056

Files:
  source/Host/posix/HostThreadPosix.cpp


Index: source/Host/posix/HostThreadPosix.cpp
===
--- source/Host/posix/HostThreadPosix.cpp
+++ source/Host/posix/HostThreadPosix.cpp
@@ -44,9 +44,10 @@
 #ifndef __ANDROID__
 #ifndef __FreeBSD__
 llvm_unreachable("someone is calling HostThread::Cancel()");
-#endif
+#else
 int err = ::pthread_cancel(m_thread);
 error.SetError(err, eErrorTypePOSIX);
+#endif
 #else
 error.SetErrorString("HostThreadPosix::Cancel() not supported on Android");
 #endif


Index: source/Host/posix/HostThreadPosix.cpp
===
--- source/Host/posix/HostThreadPosix.cpp
+++ source/Host/posix/HostThreadPosix.cpp
@@ -44,9 +44,10 @@
 #ifndef __ANDROID__
 #ifndef __FreeBSD__
 llvm_unreachable("someone is calling HostThread::Cancel()");
-#endif
+#else
 int err = ::pthread_cancel(m_thread);
 error.SetError(err, eErrorTypePOSIX);
+#endif
 #else
 error.SetErrorString("HostThreadPosix::Cancel() not supported on Android");
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44055: [lldb] Fix "requires global constructor" warning in g_range_specifiers

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Breakpoint/BreakpointID.cpp:49
 llvm::ArrayRef BreakpointID::GetRangeSpecifiers() {
+  static llvm::StringRef g_range_specifiers[] = {"-", "to", "To", "TO"};
   return llvm::makeArrayRef(g_range_specifiers);

You can probably make this `static constexpr llvm::StringLiteral` and avoid 
constructors altogether..


https://reviews.llvm.org/D44055



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

[Lldb-commits] [PATCH] D44058: [lldb] Fix "code unreachable" warning in DNBArchImplX86_64::SetRegisterValue

[Kuba (Brecka) Mracek via Phabricator via lldb-commits]

kubamracek created this revision.
kubamracek added reviewers: jingham, jasonmolenda, aprantl, davide.

Looks like an actual bug and a result of a bag merge.


https://reviews.llvm.org/D44058

Files:
  tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp


Index: tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -1917,9 +1917,9 @@
 success = true;
   }
   break;
+case e_regSetFPU:
   if (reg > fpu_xmm15 && !(CPUHasAVX() || FORCE_AVX_REGS))
 return false;
-case e_regSetFPU:
   switch (reg) {
   case fpu_fcw:
 *((uint16_t *)(&m_state.context.fpu.no_avx.__fpu_fcw)) =


Index: tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -1917,9 +1917,9 @@
 success = true;
   }
   break;
+case e_regSetFPU:
   if (reg > fpu_xmm15 && !(CPUHasAVX() || FORCE_AVX_REGS))
 return false;
-case e_regSetFPU:
   switch (reg) {
   case fpu_fcw:
 *((uint16_t *)(&m_state.context.fpu.no_avx.__fpu_fcw)) =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Host/posix/HostThreadPosix.cpp:44
   if (IsJoinable()) {
 #ifndef __ANDROID__
 #ifndef __FreeBSD__

What about:
```
#ifdef __ANDROID__
error.SetErrorString("HostThreadPosix::Cancel() not supported on Android");
#else
#ifdef __FreeBSD__
int err = ::pthread_cancel(m_thread);
error.SetError(err, eErrorTypePOSIX);
#else
llvm_unreachable("someone is calling HostThread::Cancel()");
#endif
#endif
  }
  return error;
}
```


https://reviews.llvm.org/D44056



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

[Lldb-commits] [PATCH] D44060: [lldb] Fix "code requires global destructor" warning in g_architecture_mutex

[Kuba (Brecka) Mracek via Phabricator via lldb-commits]

kubamracek created this revision.
kubamracek added reviewers: jingham, jasonmolenda, davide, aprantl.

https://reviews.llvm.org/D44060

Files:
  source/Core/PluginManager.cpp


Index: source/Core/PluginManager.cpp
===
--- source/Core/PluginManager.cpp
+++ source/Core/PluginManager.cpp
@@ -285,7 +285,10 @@
 
 typedef std::vector ArchitectureInstances;
 
-static std::mutex g_architecture_mutex;
+static std::mutex &GetArchitectureMutex() {
+static std::mutex g_architecture_mutex;
+return g_architecture_mutex;
+}
 
 static ArchitectureInstances &GetArchitectureInstances() {
   static ArchitectureInstances g_instances;
@@ -295,13 +298,13 @@
 void PluginManager::RegisterPlugin(const ConstString &name,
llvm::StringRef description,
ArchitectureCreateInstance create_callback) 
{
-  std::lock_guard guard(g_architecture_mutex);
+  std::lock_guard guard(GetArchitectureMutex());
   GetArchitectureInstances().push_back({name, description, create_callback});
 }
 
 void PluginManager::UnregisterPlugin(
 ArchitectureCreateInstance create_callback) {
-  std::lock_guard guard(g_architecture_mutex);
+  std::lock_guard guard(GetArchitectureMutex());
   auto &instances = GetArchitectureInstances();
 
   for (auto pos = instances.begin(), end = instances.end(); pos != end; ++pos) 
{
@@ -315,7 +318,7 @@
 
 std::unique_ptr
 PluginManager::CreateArchitectureInstance(const ArchSpec &arch) {
-  std::lock_guard guard(g_architecture_mutex);
+  std::lock_guard guard(GetArchitectureMutex());
   for (const auto &instances : GetArchitectureInstances()) {
 if (auto plugin_up = instances.create_callback(arch))
   return plugin_up;


Index: source/Core/PluginManager.cpp
===
--- source/Core/PluginManager.cpp
+++ source/Core/PluginManager.cpp
@@ -285,7 +285,10 @@
 
 typedef std::vector ArchitectureInstances;
 
-static std::mutex g_architecture_mutex;
+static std::mutex &GetArchitectureMutex() {
+static std::mutex g_architecture_mutex;
+return g_architecture_mutex;
+}
 
 static ArchitectureInstances &GetArchitectureInstances() {
   static ArchitectureInstances g_instances;
@@ -295,13 +298,13 @@
 void PluginManager::RegisterPlugin(const ConstString &name,
llvm::StringRef description,
ArchitectureCreateInstance create_callback) {
-  std::lock_guard guard(g_architecture_mutex);
+  std::lock_guard guard(GetArchitectureMutex());
   GetArchitectureInstances().push_back({name, description, create_callback});
 }
 
 void PluginManager::UnregisterPlugin(
 ArchitectureCreateInstance create_callback) {
-  std::lock_guard guard(g_architecture_mutex);
+  std::lock_guard guard(GetArchitectureMutex());
   auto &instances = GetArchitectureInstances();
 
   for (auto pos = instances.begin(), end = instances.end(); pos != end; ++pos) {
@@ -315,7 +318,7 @@
 
 std::unique_ptr
 PluginManager::CreateArchitectureInstance(const ArchSpec &arch) {
-  std::lock_guard guard(g_architecture_mutex);
+  std::lock_guard guard(GetArchitectureMutex());
   for (const auto &instances : GetArchitectureInstances()) {
 if (auto plugin_up = instances.create_callback(arch))
   return plugin_up;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

[Alex Langford via Phabricator via lldb-commits]

xiaobai added inline comments.



Comment at: source/Host/posix/HostThreadPosix.cpp:44
   if (IsJoinable()) {
 #ifndef __ANDROID__
 #ifndef __FreeBSD__

aprantl wrote:
> What about:
> ```
> #ifdef __ANDROID__
> error.SetErrorString("HostThreadPosix::Cancel() not supported on 
> Android");
> #else
> #ifdef __FreeBSD__
> int err = ::pthread_cancel(m_thread);
> error.SetError(err, eErrorTypePOSIX);
> #else
> llvm_unreachable("someone is calling HostThread::Cancel()");
> #endif
> #endif
>   }
>   return error;
> }
> ```
I agree with Adrian's suggestion, but I would add that you can remove one of 
the `#endif` if you use `#elif defined(__FreeBSD__)` instead of an `#else` + 
`#ifdef __FreeBSD__`.


https://reviews.llvm.org/D44056



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

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Host/posix/HostThreadPosix.cpp:44
   if (IsJoinable()) {
 #ifndef __ANDROID__
 #ifndef __FreeBSD__

xiaobai wrote:
> aprantl wrote:
> > What about:
> > ```
> > #ifdef __ANDROID__
> > error.SetErrorString("HostThreadPosix::Cancel() not supported on 
> > Android");
> > #else
> > #ifdef __FreeBSD__
> > int err = ::pthread_cancel(m_thread);
> > error.SetError(err, eErrorTypePOSIX);
> > #else
> > llvm_unreachable("someone is calling HostThread::Cancel()");
> > #endif
> > #endif
> >   }
> >   return error;
> > }
> > ```
> I agree with Adrian's suggestion, but I would add that you can remove one of 
> the `#endif` if you use `#elif defined(__FreeBSD__)` instead of an `#else` + 
> `#ifdef __FreeBSD__`.
I think we can just unify the __ANDROID__ and __FreeBSD__  cases (turn both 
into unreachable). We only run lldb-server on android, and there we're mostly 
single-threaded, so there shouldn't be any thread cancelling going on anyway...


https://reviews.llvm.org/D44056



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

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

`::pthread_cancel` is available for NetBSD as well.


https://reviews.llvm.org/D44056



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

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

[Alex Langford via Phabricator via lldb-commits]

xiaobai added inline comments.



Comment at: source/Host/posix/HostThreadPosix.cpp:44
   if (IsJoinable()) {
 #ifndef __ANDROID__
 #ifndef __FreeBSD__

labath wrote:
> xiaobai wrote:
> > aprantl wrote:
> > > What about:
> > > ```
> > > #ifdef __ANDROID__
> > > error.SetErrorString("HostThreadPosix::Cancel() not supported on 
> > > Android");
> > > #else
> > > #ifdef __FreeBSD__
> > > int err = ::pthread_cancel(m_thread);
> > > error.SetError(err, eErrorTypePOSIX);
> > > #else
> > > llvm_unreachable("someone is calling HostThread::Cancel()");
> > > #endif
> > > #endif
> > >   }
> > >   return error;
> > > }
> > > ```
> > I agree with Adrian's suggestion, but I would add that you can remove one 
> > of the `#endif` if you use `#elif defined(__FreeBSD__)` instead of an 
> > `#else` + `#ifdef __FreeBSD__`.
> I think we can just unify the __ANDROID__ and __FreeBSD__  cases (turn both 
> into unreachable). We only run lldb-server on android, and there we're mostly 
> single-threaded, so there shouldn't be any thread cancelling going on 
> anyway...
I don't think we can make the FreeBSD case unreachable (if I understand the 
code correctly) since that's the one case when `::pthread_cancel` is actually 
getting called.

If we can make the `__ANDROID__` case unreachable, this would very easily turn 
into
```
#if defined(__FreeBSD__) || defined(__NetBSD__)
int err = ::pthread_cancel(m_thread);
error.SetError(err, eErrorTypePOSIX);
#else
llvm_unreachable("Someone is calling HostThreadPosix::Cancel()");
#endif
```

I'm somewhat unfamiliar with how exactly this code is used on android. If I 
understand correctly, you're saying it's used in lldb-server and you meant that 
lldb-server is mostly single threaded so this code shouldn't get run there? 


https://reviews.llvm.org/D44056



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