[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't know much about IRMemoryMap myself, but this does seem uncontroversial. 
Nonetheless, I did manage to find something to rip into. :D




Comment at: source/Expression/IRMemoryMap.cpp:312-316
+// Round up the requested size to an aligned value, if needed.
+if (size & alignment_mask)
+  allocation_size = ((size + alignment) & (~alignment_mask));
+else
+  allocation_size = size;

`allocation_size = llvm::alignTo(size, alignment)`



Comment at: source/Expression/IRMemoryMap.cpp:324-325
+// reduce internal fragmentation.
+if (alignment > 1)
+  allocation_size += alignment;
+  }

I think  this should be just `allocation_size += alignment -1`. The subsequent 
realignment cannot eat more than `alignment-1` bytes.


https://reviews.llvm.org/D47551



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

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Pavel Labath via Phabricator via lldb-commits]

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

shrink_to_fit should be fine. I wouldn't expect to see any difference as it is 
implemented (in libstdc++ at least) using the swap trick too.


https://reviews.llvm.org/D47492



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

[Lldb-commits] [lldb] r333636 - Simplify DWARFUnit::m_die_array swap() to use shrink_to_fit()

[Jan Kratochvil via lldb-commits]

Author: jankratochvil
Date: Thu May 31 01:55:40 2018
New Revision: 333636

URL: http://llvm.org/viewvc/llvm-project?rev=333636&view=rev
Log:
Simplify DWARFUnit::m_die_array swap() to use shrink_to_fit()

rL145086 introduced m_die_array.shrink_to_fit() implemented by
exact_size_die_array.swap, it was before LLVM became written in C++11.

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp?rev=333636&r1=333635&r2=333636&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp Thu May 31 
01:55:40 2018
@@ -180,15 +180,7 @@ bool DWARFUnit::ExtractDIEsIfNeeded() {
 m_first_die = m_die_array.front();
   }
 
-  // Since std::vector objects will double their size, we really need to make a
-  // new array with the perfect size so we don't end up wasting space. So here
-  // we copy and swap to make sure we don't have any extra memory taken up.
-
-  if (m_die_array.size() < m_die_array.capacity()) {
-DWARFDebugInfoEntry::collection exact_size_die_array(m_die_array.begin(),
- m_die_array.end());
-exact_size_die_array.swap(m_die_array);
-  }
+  m_die_array.shrink_to_fit();
 
   ExtractDIEsEndCheck(offset);
 


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

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333636: Simplify DWARFUnit::m_die_array swap() to use 
shrink_to_fit() (authored by jankratochvil, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47492?vs=148964&id=149250#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47492

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -180,15 +180,7 @@
 m_first_die = m_die_array.front();
   }
 
-  // Since std::vector objects will double their size, we really need to make a
-  // new array with the perfect size so we don't end up wasting space. So here
-  // we copy and swap to make sure we don't have any extra memory taken up.
-
-  if (m_die_array.size() < m_die_array.capacity()) {
-DWARFDebugInfoEntry::collection exact_size_die_array(m_die_array.begin(),
- m_die_array.end());
-exact_size_die_array.swap(m_die_array);
-  }
+  m_die_array.shrink_to_fit();
 
   ExtractDIEsEndCheck(offset);
 


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -180,15 +180,7 @@
 m_first_die = m_die_array.front();
   }
 
-  // Since std::vector objects will double their size, we really need to make a
-  // new array with the perfect size so we don't end up wasting space. So here
-  // we copy and swap to make sure we don't have any extra memory taken up.
-
-  if (m_die_array.size() < m_die_array.capacity()) {
-DWARFDebugInfoEntry::collection exact_size_die_array(m_die_array.begin(),
- m_die_array.end());
-exact_size_die_array.swap(m_die_array);
-  }
+  m_die_array.shrink_to_fit();
 
   ExtractDIEsEndCheck(offset);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In https://reviews.llvm.org/D47492#1117350, @labath wrote:

> it is implemented (in libstdc++ at least) using the swap trick too.


Thanks for the heads up, I expected it does `realloc()`: implementation 
absolutely cannot rely on realloc ... because realloc, if it cannot shrink 
in-place, will either leave the memory alone (no-op case) or make a bitwise 
copy (and miss the opportunity for readjusting pointers, etc. that the proper 
C++ copying/moving constructors would give) 



Repository:
  rL LLVM

https://reviews.llvm.org/D47492



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

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Well, if a type is trivially copyable (which DWARFDebugInfo **should** be, 
though I haven't checked), it should be able to do the copy via a single 
`memcpy` instead of a bunch of constructor calls. That should be more-or-less 
equivalent to realloc.

If this does not end up being implented as a memcpy here, we have an easy 
opportunity to improve things.


Repository:
  rL LLVM

https://reviews.llvm.org/D47492



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

[Lldb-commits] [PATCH] D47539: [Platform] Accept arbitrary kext variants

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D47539#1117114, @jasonmolenda wrote:

> LGTM.  If we added more knowledge specifically about kext bundle layouts, we 
> could restrict which files we test to see if they are valid binaries - but 
> we'd need to parse the Info.plist at the top (to get the CFBundleExecutable 
> name, and look for variations on that prefix) and we'd need to handle 
> shallow/deep kext bundles.  Given how few files are in kext bundles besides 
> the kexts themselves (a couple of plists), this is code is much simpler than 
> encoding all that extra specifics about kexts.


Thanks Jason!

Currently there are no kext  test currently. Jason and I had a brief discussion 
yesterday on how we could do this in the future, but while it is certainly 
possible (and something I want to do) it's going to be too much work for this 
small change. Unless anyone has a practical idea for a test I'm going to land 
this as is, awaiting the aforementioned kext test infrastructure.


https://reviews.llvm.org/D47539



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

[Lldb-commits] Support of MSVC function-level linking

[Aleksandr Urakov via lldb-commits]

 Hello!

I'm Aleksandr from JetBrains. We are working on improving support of
MSVC-compiled binaries in lldb. We have made several fixes and would like
to upstream them.

The first patch adds support of function-level linking feature. The
SymbolFilePDB::ParseCompileUnitLineTable function relies on the fact that
ranges of compiled source files in the binary are continuous and don't
intersect with each other. ParseCompileUnitLineTable creates LineSequence
for each file and inserts it into LineTable, and the implementation of
LineTable relies on continuity of the sequence. But it's not always true
when function-level linking is enabled, e.g. in the attached input test
file test-pdb-function-level-linking.exe there is xstring's
std__basic_string_char_std__char_traits_char__std__allocator_char_max_size
(.00454820) between test-pdb-function-level-linking.cpp's foo (.00454770)
and main (.004548F0). The source is compiled with Microsoft C/C++ compiler
version 19.14.26429.4 for x86.

To fix the problem we propose to renew the sequence on each address gap.

The link to the patch and related files is:
https://drive.google.com/open?id=1ozp06jyqugjLGT-6wuJKS1UhRuXFsixf

Thanks!

-- 
Aleksandr Urakov
Software Developer
JetBrains
http://www.jetbrains.com
The Drive to Develop
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r303907 - Fix bug #28898

[Pavel Labath via lldb-commits]

I hate to resurrect an old thread, but there has been a new spurt of
this discussion about this patch here
.

I think I have an idea on how to improve things slightly for us here,
but as I know very little about this issue, I'd like someone to take a
look at this first.

Christos, can you check whether my proposal makes sense to you? I am
including it in this email for your convenience:


===
I've re-read the last years discussions, and I think I have an idea on
how to improve the situation here somewhat. The way I understand it,
we have three scenarios to worry about:

1. old-libedit --enable-widec
  - el_gets -> narrow_read_function
  - el_wgets -> wide_read_function
2. new-libedit --enable-widec
  - el_gets -> wide_read_function
  - el_wgets -> wide_read_function
3. libedit --disable-widec
  - I don't actually know what happens here but my proposal does not
change our behavior in this case.

As I understand it, the problem is that we are not able to distinguish
between (1) and (2) and build time, so we have to just pick one and
hope it works. This means we work correctly for (1), we fail for (2)
and something unknown happens for (3).

However, we **are** able to distinguish between (1+2) and (3) at build
time (just search for `el_wgets` symbol). And, if I understand it
correctly, when we use el_wgets and friends we are always correct, no
matter the libedit version. This would mean are always correct for
both (1) **and** (2), while the situation remains unchanged for (3).
This should be a strict improvement over status quo.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

If it is trivially copyable then IMO libstdc++ could use even that realloc() 
(that is for a shrink without any copy).


Repository:
  rL LLVM

https://reviews.llvm.org/D47492



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

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 149253.
labath added a comment.

Implement recursive search in IsMethod()


https://reviews.llvm.org/D47470

Files:
  lit/SymbolFile/DWARF/find-method-local-struct.cpp
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -288,22 +288,7 @@
   }
   // If we have a mangled name, then the DW_AT_name attribute is
   // usually the method name without the class or any parameters
-  const DWARFDebugInfoEntry *parent = die.GetParent();
-  bool is_method = false;
-  if (parent) {
-DWARFDIE parent_die(&unit, parent);
-if (parent_die.IsStructClassOrUnion())
-  is_method = true;
-else {
-  if (specification_die_form.IsValid()) {
-DWARFDIE specification_die =
-unit.GetSymbolFileDWARF()->DebugInfo()->GetDIE(
-DIERef(specification_die_form));
-if (specification_die.GetParent().IsStructClassOrUnion())
-  is_method = true;
-  }
-}
-  }
+  bool is_method = DWARFDIE(&unit, &die).IsMethod();
 
   if (is_method)
 set.function_methods.Insert(ConstString(name),
Index: source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -19,7 +19,9 @@
   //--
   // Tests
   //--
-  bool IsStructClassOrUnion() const;
+  bool IsStructUnionOrClass() const;
+
+  bool IsMethod() const;
 
   //--
   // Accessors
Index: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -10,12 +10,12 @@
 #include "DWARFDIE.h"
 
 #include "DWARFASTParser.h"
-#include "DWARFUnit.h"
 #include "DWARFDIECollection.h"
 #include "DWARFDebugInfo.h"
-#include "DWARFDeclContext.h"
-
 #include "DWARFDebugInfoEntry.h"
+#include "DWARFDeclContext.h"
+#include "DWARFUnit.h"
+#include "llvm/ADT/SmallSet.h"
 
 using namespace lldb_private;
 
@@ -209,12 +209,37 @@
 return DWARFDIE();
 }
 
-bool DWARFDIE::IsStructClassOrUnion() const {
+bool DWARFDIE::IsStructUnionOrClass() const {
   const dw_tag_t tag = Tag();
   return tag == DW_TAG_class_type || tag == DW_TAG_structure_type ||
  tag == DW_TAG_union_type;
 }
 
+bool DWARFDIE::IsMethod() const {
+  // Prevent infinite recursion by maintaining a list of seen DIEs. Container
+  // sizes are optimized for the case of following DW_AT_specification and
+  // DW_AT_abstract_origin just once.
+  llvm::SmallVector worklist(1, *this);
+  llvm::SmallSet seen;
+
+  while (!worklist.empty()) {
+DWARFDIE die = worklist.back();
+worklist.pop_back();
+if (!seen.insert(die.GetID()).second)
+  continue;
+
+DWARFDIE parent = die.GetParent();
+if (parent.IsStructUnionOrClass())
+  return true;
+
+if (DWARFDIE d = die.GetReferencedDIE(DW_AT_specification))
+  worklist.push_back(d);
+if (DWARFDIE d = die.GetReferencedDIE(DW_AT_abstract_origin))
+  worklist.push_back(d);
+  }
+  return false;
+}
+
 DWARFDIE
 DWARFDIE::GetContainingDWOModuleDIE() const {
   if (IsValid()) {
Index: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -146,6 +146,14 @@
 m_apple_namespaces_up->FindByName(name.GetStringRef(), offsets);
 }
 
+static bool KeepFunctionDIE(DWARFDIE die, uint32_t name_type_mask) {
+  bool looking_for_methods = name_type_mask & eFunctionNameTypeMethod;
+  bool looking_for_functions = name_type_mask & eFunctionNameTypeBase;
+  if (looking_for_methods && looking_for_functions)
+return true;
+  return looking_for_methods == die.IsMethod();
+}
+
 void AppleDWARFIndex::GetFunctions(
 ConstString name, DWARFDebugInfo &info,
 llvm::function_refGetType();
-
-if (type) {
-  CompilerDeclContext decl_ctx =
-  get_decl_context_containing_uid(type->GetID());
-  if (decl_ctx.IsStructUnionOrClass()) {
-if (name_type_mask & eFunctionNameType

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

[Pavel Labath via Phabricator via lldb-commits]

labath marked 4 inline comments as done.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:153
+return true;
+  bool looking_for_methods = name_type_mask & eFunctionNameTypeMethod;
+  return looking_for_methods == die.IsMethod();

clayborg wrote:
> move up to line 150 and use this variable in the if statement instead of 
> repeating "name_type_mask & eFunctionNameTypeMethod" twice?
Done. I've created variables for both enum values, as having just one of them 
in the if statement looked weird.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222
+return true;
+  return GetReferencedDIE(DW_AT_specification)
+  .GetParent()

clayborg wrote:
> labath wrote:
> > clayborg wrote:
> > > I can never remember when a DW_AT_abstract_origin might be used. Might be 
> > > nice to have a DWARFDIE method:
> > > 
> > > ```
> > > DWARFDIE DWARFDIE::GetAbstractOriginOrSpecification();
> > > ```
> > > this would return either the the DW_AT_specification or the 
> > > DW_AT_abstract_origin. If we go that route the this coce would become:
> > > 
> > > ```
> > > GetAbstractOriginOrSpecification().GetParent().IsStructUnionOrClass();
> > > ```
> > > 
> > How would this method know which DIE to return? In case of inlined methods 
> > you can have a chain of DIEs:
> > `DIE1 --- DW_AT_abstract_origin --> DIE2 --- DW_AT_specification --> DIE3`
> > Each of these dies will have a different parent.
> > 
> > The current function will check for the parent of DIE1 and DIE3 (this is 
> > what the manual index does) though to be fully correct maybe we should 
> > check all three of them (?) Or do you think checking the last DIE in that 
> > list should be always enough? That would seem to be the case for the dwarf 
> > I've seen, but I'm not sure if that is always correct..
> Yeah, unfortunately I can't elaborate too much without seeing a bad example. 
> If anything in the chain has a parent that is a struct/union or class might 
> be enough, so we could ask each DIE in the specification or abstract origin 
> chain if its parent is a struct/union/class and just return true if so?
I've reimplemented the function to do a proper recursive search for the parent, 
including a infinite-recursion guard.


https://reviews.llvm.org/D47470



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

[Lldb-commits] [PATCH] D46885: Remove append parameter to FindGlobalVariables

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for the patch. I've clang-formatted it and committed in r333639.


https://reviews.llvm.org/D46885



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

[Lldb-commits] [PATCH] D46885: Remove append parameter to FindGlobalVariables

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333639: Remove append parameter to FindGlobalVariables 
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46885?vs=146848&id=149254#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46885

Files:
  lldb/trunk/include/lldb/Core/Module.h
  lldb/trunk/include/lldb/Core/ModuleList.h
  lldb/trunk/include/lldb/Symbol/SymbolFile.h
  lldb/trunk/include/lldb/Symbol/SymbolVendor.h
  lldb/trunk/source/API/SBModule.cpp
  lldb/trunk/source/API/SBTarget.cpp
  lldb/trunk/source/Commands/CommandObjectTarget.cpp
  lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp
  lldb/trunk/source/Core/Module.cpp
  lldb/trunk/source/Core/ModuleList.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp
  lldb/trunk/source/Plugins/InstrumentationRuntime/TSan/TSanRuntime.cpp
  
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/trunk/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/trunk/source/Symbol/SymbolFile.cpp
  lldb/trunk/source/Symbol/SymbolVendor.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp

Index: lldb/trunk/include/lldb/Symbol/SymbolVendor.h
===
--- lldb/trunk/include/lldb/Symbol/SymbolVendor.h
+++ lldb/trunk/include/lldb/Symbol/SymbolVendor.h
@@ -81,11 +81,11 @@
 
   virtual size_t FindGlobalVariables(const ConstString &name,
  const CompilerDeclContext *parent_decl_ctx,
- bool append, size_t max_matches,
+ size_t max_matches,
  VariableList &variables);
 
   virtual size_t FindGlobalVariables(const RegularExpression ®ex,
- bool append, size_t max_matches,
+ size_t max_matches,
  VariableList &variables);
 
   virtual size_t FindFunctions(const ConstString &name,
Index: lldb/trunk/include/lldb/Symbol/SymbolFile.h
===
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h
@@ -148,10 +148,10 @@
 SymbolContextList &sc_list);
   virtual uint32_t
   FindGlobalVariables(const ConstString &name,
-  const CompilerDeclContext *parent_decl_ctx, bool append,
+  const CompilerDeclContext *parent_decl_ctx,
   uint32_t max_matches, VariableList &variables);
   virtual uint32_t FindGlobalVariables(const RegularExpression ®ex,
-   bool append, uint32_t max_matches,
+   uint32_t max_matches,
VariableList &variables);
   virtual uint32_t FindFunctions(const ConstString &name,
  const CompilerDeclContext *parent_decl_ctx,
Index: lldb/trunk/include/lldb/Core/ModuleList.h
===
--- lldb/trunk/include/lldb/Core/ModuleList.h
+++ lldb/trunk/include/lldb/Core/ModuleList.h
@@ -324,50 +324,36 @@
   /// The name of the global or static variable we are looking
   /// for.
   ///
-  /// @param[in] append
-  /// If \b true, any matches will be appended to \a
-  /// variable_list, else matches replace the contents of
-  /// \a variable_list.
-  ///
   /// @param[in] max_matches
   /// Allow the number of matches to be limited to \a
   /// max_matches. Specify UINT32_MAX to get all possible matches.
   ///
   /// @param[in] variable_list
-  /// A list of variables that gets the matches appended to (if
-  /// \a append it \b true), or replace (if \a append is \b false).
+  /// A list of variables that gets the matches appended to.
   ///
   /// @return
   /// The number of matches added to \a variable_list.
   //--
-  size_t FindGlobalVariables(const ConstString &name, bool append,
- size_t max_matches,
+  size_t FindGlobalVariables(const ConstString &name, size_t max_matches,
  VariableList &variable_list) const;
 
   //--

[Lldb-commits] [lldb] r333639 - Remove append parameter to FindGlobalVariables

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu May 31 02:46:26 2018
New Revision: 333639

URL: http://llvm.org/viewvc/llvm-project?rev=333639&view=rev
Log:
Remove append parameter to FindGlobalVariables

Summary:
As discussed in https://bugs.llvm.org/show_bug.cgi?id=37317,
FindGlobalVariables does not properly handle the case where
append=false.  As this doesn't seem to be used in the tree, this patch
removes the parameter entirely.

Reviewers: clayborg, jingham, labath

Reviewed By: clayborg

Subscribers: aprantl, lldb-commits, kubamracek, JDevlieghere

Differential Revision: https://reviews.llvm.org/D46885
Patch by Tom Tromey .

Modified:
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/include/lldb/Core/ModuleList.h
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/include/lldb/Symbol/SymbolVendor.h
lldb/trunk/source/API/SBModule.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp
lldb/trunk/source/Plugins/InstrumentationRuntime/TSan/TSanRuntime.cpp

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/trunk/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/trunk/source/Symbol/SymbolFile.cpp
lldb/trunk/source/Symbol/SymbolVendor.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=333639&r1=333638&r2=333639&view=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Thu May 31 02:46:26 2018
@@ -447,26 +447,19 @@ public:
   /// @param[in] parent_decl_ctx
   /// If valid, a decl context that results must exist within
   ///
-  /// @param[in] append
-  /// If \b true, any matches will be appended to \a
-  /// variable_list, else matches replace the contents of
-  /// \a variable_list.
-  ///
   /// @param[in] max_matches
   /// Allow the number of matches to be limited to \a
   /// max_matches. Specify UINT32_MAX to get all possible matches.
   ///
   /// @param[in] variable_list
-  /// A list of variables that gets the matches appended to (if
-  /// \a append it \b true), or replace (if \a append is \b false).
+  /// A list of variables that gets the matches appended to.
   ///
   /// @return
   /// The number of matches added to \a variable_list.
   //--
   size_t FindGlobalVariables(const ConstString &name,
  const CompilerDeclContext *parent_decl_ctx,
- bool append, size_t max_matches,
- VariableList &variable_list);
+ size_t max_matches, VariableList &variable_list);
 
   //--
   /// Find global and static variables by regular expression.
@@ -474,24 +467,18 @@ public:
   /// @param[in] regex
   /// A regular expression to use when matching the name.
   ///
-  /// @param[in] append
-  /// If \b true, any matches will be appended to \a
-  /// variable_list, else matches replace the contents of
-  /// \a variable_list.
-  ///
   /// @param[in] max_matches
   /// Allow the number of matches to be limited to \a
   /// max_matches. Specify UINT32_MAX to get all possible matches.
   ///
   /// @param[in] variable_list
-  /// A list of variables that gets the matches appended to (if
-  /// \a append it \b true), or replace (if \a append is \b false).
+  /// A list of variables that gets the matches appended to.
   ///
   /// @return
   /// The number of matches added to \a variable_list.
   //--
-  size_t FindGlobalVariables(const RegularExpression ®ex, bool append,
- size_t max_matches, VariableList &variable_list);
+  size_t FindGlobalVariables(const RegularExpression ®ex, size_t 
max_matches,
+ VariableList &variable_list);
 
   //--
   /// Find types by 

[Lldb-commits] [lldb] r333640 - Remove the TestSequenceFunctions "example" test

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu May 31 02:56:38 2018
New Revision: 333640

URL: http://llvm.org/viewvc/llvm-project?rev=333640&view=rev
Log:
Remove the TestSequenceFunctions "example" test

This test was using unittest (not unittest2) as the test framework, and
it worked with dotest only by accident. Remove it as we have a much more
realistic example test in test/testcases/sample_test.

Removed:
lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py

Removed: 
lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py?rev=333639&view=auto
==
--- lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py 
(removed)
@@ -1,34 +0,0 @@
-"""An example unittest copied from python tutorial."""
-
-import random
-import unittest
-import traceback
-
-
-class SequenceFunctionsTestCase(unittest.TestCase):
-
-def setUp(self):
-# traceback.print_stack()
-self.seq = list(range(10))
-
-def tearDown(self):
-# traceback.print_stack()
-pass
-
-def test_shuffle(self):
-# make sure the shuffled sequence does not lose any elements
-random.shuffle(self.seq)
-self.seq.sort()
-self.assertEqual(self.seq, list(range(10)))
-
-def test_choice(self):
-element = random.choice(self.seq)
-self.assertTrue(element in self.seq)
-
-def test_sample(self):
-self.assertRaises(ValueError, random.sample, self.seq, 20)
-for element in random.sample(self.seq, 5):
-self.assertTrue(element in self.seq)
-
-if __name__ == '__main__':
-unittest.main()


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

[Lldb-commits] [PATCH] D40470: Protect DWARFCompileUnit::m_die_array by a new mutex

[Jan Kratochvil via Phabricator via lldb-commits]

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



Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:181
+  // ExtractDIEsIfNeeded() will keep m_die_array populated forever.
+  uint32_t m_die_array_usecount = 0;
   // GetUnitDIEPtrOnly() needs to return pointer to the first DIE.

clayborg wrote:
> Why don't we expose the llvm::sys::RWMutex for BuildAddressRangeTable and 
> SymbolFileDWARF::Index? Then they can just grab the read lock?
Wouldn't you prefer an RAII lock (`DWARFUnit::ScopedExtractDIEs`) instead of 
the mutex exposure?

There are two mutexes (`m_die_array_mutex` and `m_die_array_scoped_mutex`), I 
tried it using a single mutex (as you IMO suggest) but then one needs to 
downgrade exclusive->shared lock there which requires a `retry` label there so 
I did not like the single lock ([[ 
https://people.redhat.com/jkratoch/mutex2.patch | mutex2.patch ]]).



https://reviews.llvm.org/D40470



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

[Lldb-commits] [PATCH] D40470: Protect DWARFCompileUnit::m_die_array by a new mutex

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 149255.
jankratochvil marked an inline comment as done.
jankratochvil added a comment.

RAII lock `DWARFUnit::ScopedExtractDIEs` with mutexes `m_die_array_mutex`, 
`m_die_array_scoped_mutex` and `m_first_die_mutex`.


https://reviews.llvm.org/D40470

Files:
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -38,23 +38,22 @@
 
   std::vector sets(num_compile_units);
 
-  // std::vector might be implemented using bit test-and-set, so use
-  // uint8_t instead.
-  std::vector clear_cu_dies(num_compile_units, false);
+  //--
+  // Keep memory down by clearing DIEs for any compile units if indexing
+  // caused us to load the compile unit's DIEs.
+  //--
+  std::vector>
+  clear_cu_dies(num_compile_units);
   auto parser_fn = [&](size_t cu_idx) {
 DWARFUnit *dwarf_cu = debug_info.GetCompileUnitAtIndex(cu_idx);
 if (dwarf_cu)
   IndexUnit(*dwarf_cu, sets[cu_idx]);
   };
 
   auto extract_fn = [&debug_info, &clear_cu_dies](size_t cu_idx) {
 DWARFUnit *dwarf_cu = debug_info.GetCompileUnitAtIndex(cu_idx);
-if (dwarf_cu) {
-  // dwarf_cu->ExtractDIEsIfNeeded() will return false if the DIEs
-  // for a compile unit have already been parsed.
-  if (dwarf_cu->ExtractDIEsIfNeeded())
-clear_cu_dies[cu_idx] = true;
-}
+if (dwarf_cu)
+  clear_cu_dies[cu_idx] = dwarf_cu->ExtractDIEsScoped();
   };
 
   // Create a task runner that extracts dies for each DWARF compile unit in a
@@ -89,15 +88,6 @@
  [&]() { finalize_fn(&IndexSet::globals); },
  [&]() { finalize_fn(&IndexSet::types); },
  [&]() { finalize_fn(&IndexSet::namespaces); });
-
-  //--
-  // Keep memory down by clearing DIEs for any compile units if indexing
-  // caused us to load the compile unit's DIEs.
-  //--
-  for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx) {
-if (clear_cu_dies[cu_idx])
-  debug_info.GetCompileUnitAtIndex(cu_idx)->ClearDIEs();
-  }
 }
 
 void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, IndexSet &set) {
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -13,6 +13,7 @@
 #include "DWARFDIE.h"
 #include "DWARFDebugInfoEntry.h"
 #include "lldb/lldb-enumerations.h"
+#include "llvm/Support/RWMutex.h"
 
 class DWARFUnit;
 class DWARFCompileUnit;
@@ -40,7 +41,20 @@
   virtual ~DWARFUnit();
 
   void ExtractUnitDIEIfNeeded();
-  bool ExtractDIEsIfNeeded();
+  void ExtractDIEsIfNeeded();
+
+  class ScopedExtractDIEs {
+DWARFUnit *m_cu;
+  public:
+bool m_clear_dies = false;
+ScopedExtractDIEs(DWARFUnit *cu);
+~ScopedExtractDIEs();
+DISALLOW_COPY_AND_ASSIGN(ScopedExtractDIEs);
+ScopedExtractDIEs(ScopedExtractDIEs &&rhs);
+ScopedExtractDIEs &operator=(ScopedExtractDIEs &&rhs);
+  };
+  ScopedExtractDIEs ExtractDIEsScoped();
+
   DWARFDIE LookupAddress(const dw_addr_t address);
   size_t AppendDIEsWithTag(const dw_tag_t tag,
DWARFDIECollection &matching_dies,
@@ -100,7 +114,6 @@
   dw_addr_t GetRangesBase() const { return m_ranges_base; }
   void SetAddrBase(dw_addr_t addr_base, dw_addr_t ranges_base,
dw_offset_t base_obj_offset);
-  void ClearDIEs();
   void BuildAddressRangeTable(SymbolFileDWARF *dwarf,
   DWARFDebugAranges *debug_aranges);
 
@@ -172,10 +185,17 @@
   void *m_user_data = nullptr;
   // The compile unit debug information entry item
   DWARFDebugInfoEntry::collection m_die_array;
+  mutable llvm::sys::RWMutex m_die_array_mutex;
+  // It is used for tracking of ScopedExtractDIEs instances.
+  mutable llvm::sys::RWMutex m_die_array_scoped_mutex;
+  // ScopedExtractDIEs instances should not call ClearDIEsRWLocked()
+  // as someone called ExtractDIEsIfNeeded().
+  std::atomic m_cancel_scopes;
   // GetUnitDIEPtrOnly() needs to return pointer to the first DIE.
   // But the first element of m_die_array after ExtractUnitDIEIfNeeded()
   // would possibly move in memory after later ExtractDIEsIfNeeded().
   DWARFDebugInfoEntry m_first_die;
+  llvm::sys::RWMutex m_first_die_mutex;
   // A table similar to the .debug_aranges table, but this one points to the
   // exact DW_TAG_subprogram DIEs
   st

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222
+return true;
+  return GetReferencedDIE(DW_AT_specification)
+  .GetParent()

labath wrote:
> clayborg wrote:
> > labath wrote:
> > > clayborg wrote:
> > > > I can never remember when a DW_AT_abstract_origin might be used. Might 
> > > > be nice to have a DWARFDIE method:
> > > > 
> > > > ```
> > > > DWARFDIE DWARFDIE::GetAbstractOriginOrSpecification();
> > > > ```
> > > > this would return either the the DW_AT_specification or the 
> > > > DW_AT_abstract_origin. If we go that route the this coce would become:
> > > > 
> > > > ```
> > > > GetAbstractOriginOrSpecification().GetParent().IsStructUnionOrClass();
> > > > ```
> > > > 
> > > How would this method know which DIE to return? In case of inlined 
> > > methods you can have a chain of DIEs:
> > > `DIE1 --- DW_AT_abstract_origin --> DIE2 --- DW_AT_specification --> DIE3`
> > > Each of these dies will have a different parent.
> > > 
> > > The current function will check for the parent of DIE1 and DIE3 (this is 
> > > what the manual index does) though to be fully correct maybe we should 
> > > check all three of them (?) Or do you think checking the last DIE in that 
> > > list should be always enough? That would seem to be the case for the 
> > > dwarf I've seen, but I'm not sure if that is always correct..
> > Yeah, unfortunately I can't elaborate too much without seeing a bad 
> > example. If anything in the chain has a parent that is a struct/union or 
> > class might be enough, so we could ask each DIE in the specification or 
> > abstract origin chain if its parent is a struct/union/class and just return 
> > true if so?
> I've reimplemented the function to do a proper recursive search for the 
> parent, including a infinite-recursion guard.
Looks good, we have almost the exact same implementation in `findRecursively` 
in llvm.


https://reviews.llvm.org/D47470



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

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

[Pavel Labath via Phabricator via lldb-commits]

labath marked 4 inline comments as done.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222
+return true;
+  return GetReferencedDIE(DW_AT_specification)
+  .GetParent()

JDevlieghere wrote:
> labath wrote:
> > clayborg wrote:
> > > labath wrote:
> > > > clayborg wrote:
> > > > > I can never remember when a DW_AT_abstract_origin might be used. 
> > > > > Might be nice to have a DWARFDIE method:
> > > > > 
> > > > > ```
> > > > > DWARFDIE DWARFDIE::GetAbstractOriginOrSpecification();
> > > > > ```
> > > > > this would return either the the DW_AT_specification or the 
> > > > > DW_AT_abstract_origin. If we go that route the this coce would become:
> > > > > 
> > > > > ```
> > > > > GetAbstractOriginOrSpecification().GetParent().IsStructUnionOrClass();
> > > > > ```
> > > > > 
> > > > How would this method know which DIE to return? In case of inlined 
> > > > methods you can have a chain of DIEs:
> > > > `DIE1 --- DW_AT_abstract_origin --> DIE2 --- DW_AT_specification --> 
> > > > DIE3`
> > > > Each of these dies will have a different parent.
> > > > 
> > > > The current function will check for the parent of DIE1 and DIE3 (this 
> > > > is what the manual index does) though to be fully correct maybe we 
> > > > should check all three of them (?) Or do you think checking the last 
> > > > DIE in that list should be always enough? That would seem to be the 
> > > > case for the dwarf I've seen, but I'm not sure if that is always 
> > > > correct..
> > > Yeah, unfortunately I can't elaborate too much without seeing a bad 
> > > example. If anything in the chain has a parent that is a struct/union or 
> > > class might be enough, so we could ask each DIE in the specification or 
> > > abstract origin chain if its parent is a struct/union/class and just 
> > > return true if so?
> > I've reimplemented the function to do a proper recursive search for the 
> > parent, including a infinite-recursion guard.
> Looks good, we have almost the exact same implementation in `findRecursively` 
> in llvm.
It should be. I copied it from there. :)


https://reviews.llvm.org/D47470



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

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, aprantl, tberghammer.
Herald added a subscriber: eraman.

This is split off from https://reviews.llvm.org/D47265 where I needed to be 
able to invoke every test
with -f. That patch is kinda dead now, but this part seems like a good
cleanup anyway.

The problem with inline tests was in the way we were adding methods to
the class, which left them with an incorrect __name__ property. This
prevented dotest from finding them with -f.

I fix this with (what I think is) the correct way of dynamically
creating classes -- passing the list of methods during type construction
instead of fixing up the class afterwards. Among other things this has
the advantage of not needing to do anything special for debug info
variants. As our test method will be visible to the metaclass, it will
automagically to the multiplication for us.


https://reviews.llvm.org/D47579

Files:
  packages/Python/lldbsuite/test/lldbinline.py


Index: packages/Python/lldbsuite/test/lldbinline.py
===
--- packages/Python/lldbsuite/test/lldbinline.py
+++ packages/Python/lldbsuite/test/lldbinline.py
@@ -135,37 +135,11 @@
 makefile.flush()
 makefile.close()
 
-@add_test_categories(["dsym"])
-def __test_with_dsym(self):
+def _test(self):
 self.using_dsym = True
 self.BuildMakefile()
 self.build()
 self.do_test()
-__test_with_dsym.debug_info = "dsym"
-
-@add_test_categories(["dwarf"])
-def __test_with_dwarf(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwarf.debug_info = "dwarf"
-
-@add_test_categories(["dwo"])
-def __test_with_dwo(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwo.debug_info = "dwo"
-
-@add_test_categories(["gmodules"])
-def __test_with_gmodules(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_gmodules.debug_info = "gmodules"
 
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
@@ -237,18 +211,11 @@
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
+
+test = ApplyDecoratorsToFunction(InlineTest._test, decorators)
 # Build the test case
-test = type(test_name, (InlineTest,), {'using_dsym': None})
-test.name = test_name
-
-test.test_with_dsym = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dsym, decorators)
-test.test_with_dwarf = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwarf, decorators)
-test.test_with_dwo = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwo, decorators)
-test.test_with_gmodules = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_gmodules, decorators)
+test = type(test_name, (InlineTest,), dict(using_dsym=None, test=test,
+   name=test_name))
 
 # Add the test case to the globals, and hide InlineTest
 __globals.update({test_name: test})


Index: packages/Python/lldbsuite/test/lldbinline.py
===
--- packages/Python/lldbsuite/test/lldbinline.py
+++ packages/Python/lldbsuite/test/lldbinline.py
@@ -135,37 +135,11 @@
 makefile.flush()
 makefile.close()
 
-@add_test_categories(["dsym"])
-def __test_with_dsym(self):
+def _test(self):
 self.using_dsym = True
 self.BuildMakefile()
 self.build()
 self.do_test()
-__test_with_dsym.debug_info = "dsym"
-
-@add_test_categories(["dwarf"])
-def __test_with_dwarf(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwarf.debug_info = "dwarf"
-
-@add_test_categories(["dwo"])
-def __test_with_dwo(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwo.debug_info = "dwo"
-
-@add_test_categories(["gmodules"])
-def __test_with_gmodules(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_gmodules.debug_info = "gmodules"
 
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
@@ -237,18 +211,11 @@
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
+
+test = ApplyDecoratorsToFunction(InlineTest._test, decorators)
 # Build the test case
-test = type(test_name, (InlineTest,), {'using_dsym': None})
-test.name = test_name
-
-test.test_with_dsym = ApplyDecoratorsToFunction(

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

If I correctly understand this change, this might make it possible to apply the 
`add_test_categories` decorator to an inline test. We had issues with this when 
introducing the swiftpr category because the methods were part of the inline 
test class. From the description I get the idea that now they become part of 
the individual instantiations? I **really** hope this is the case :-)

Regardless of my (potential) excitement, this looks good to me!




Comment at: packages/Python/lldbsuite/test/lldbinline.py:139
+def _test(self):
 self.using_dsym = True
 self.BuildMakefile()

Will the decorators ensure that this value is properly set (or ignored) for the 
non-dsym tests? 


https://reviews.llvm.org/D47579



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

Re: [Lldb-commits] [lldb] r303907 - Fix bug #28898

[Christos Zoulas via lldb-commits]

On May 31, 10:31am, lab...@google.com (Pavel Labath) wrote:
-- Subject: Re: [Lldb-commits] [lldb] r303907 - Fix bug #28898

| I hate to resurrect an old thread, but there has been a new spurt of
| this discussion about this patch here
| .
| 
| I think I have an idea on how to improve things slightly for us here,
| but as I know very little about this issue, I'd like someone to take a
| look at this first.
| 
| Christos, can you check whether my proposal makes sense to you? I am
| including it in this email for your convenience:
| 
| 
| ===
| I've re-read the last years discussions, and I think I have an idea on
| how to improve the situation here somewhat. The way I understand it,
| we have three scenarios to worry about:
| 
| 1. old-libedit --enable-widec
|   - el_gets -> narrow_read_function
|   - el_wgets -> wide_read_function
| 2. new-libedit --enable-widec
|   - el_gets -> wide_read_function
|   - el_wgets -> wide_read_function
| 3. libedit --disable-widec
|   - I don't actually know what happens here but my proposal does not
| change our behavior in this case.
| 
| As I understand it, the problem is that we are not able to distinguish
| between (1) and (2) and build time, so we have to just pick one and
| hope it works. This means we work correctly for (1), we fail for (2)
| and something unknown happens for (3).
| 
| However, we **are** able to distinguish between (1+2) and (3) at build
| time (just search for `el_wgets` symbol). And, if I understand it
| correctly, when we use el_wgets and friends we are always correct, no
| matter the libedit version. This would mean are always correct for
| both (1) **and** (2), while the situation remains unchanged for (3).
| This should be a strict improvement over status quo.

I think that you are right. Nevertheless what you propose it should
be an improvement over the status quo.

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

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D47579#1117457, @JDevlieghere wrote:

> If I correctly understand this change, this might make it possible to apply 
> the `add_test_categories` decorator to an inline test. We had issues with 
> this when introducing the swiftpr category because the methods were part of 
> the inline test class. From the description I get the idea that now they 
> become part of the individual instantiations? I **really** hope this is the 
> case :-)


I don't think this will have any affect on that as this still applies the 
decorators to the base class. However, I have an idea what might solve that.

Can you try inserting something like this instead of the 
`ApplyDecoratorsToFunction` line and see if your problems go away?

  @wraps(InlineTest._test)
  def test_func(*args, **kwargs):
  return InlineTest._test(*args, **kwargs)
  
  test_func = ApplyDecoratorsToFunction(test_func, decorators)

This should make sure each test class gets a fresh function object with an 
independent set of categories.




Comment at: packages/Python/lldbsuite/test/lldbinline.py:139
+def _test(self):
 self.using_dsym = True
 self.BuildMakefile()

JDevlieghere wrote:
> Will the decorators ensure that this value is properly set (or ignored) for 
> the non-dsym tests? 
Ah, good catch. No the decorators won't do that. However, this property is only 
used in getRerunArgs.
In fact it turns out that after this patch we don't even need to override 
getRerunArgs for inline tests as the default implementation (which just appends 
`-f class.method`) will work just fine for inline tests as well (or at least, 
it won't be any worse).

So I just obliterate every mention of this property.


https://reviews.llvm.org/D47579



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

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 149269.
labath added a comment.

- obliterate using_dsym property
- reduce confusion in the MakeInlineTest function (variable "test" used for 
multiple things)


https://reviews.llvm.org/D47579

Files:
  packages/Python/lldbsuite/test/lldbinline.py


Index: packages/Python/lldbsuite/test/lldbinline.py
===
--- packages/Python/lldbsuite/test/lldbinline.py
+++ packages/Python/lldbsuite/test/lldbinline.py
@@ -84,18 +84,6 @@
 class InlineTest(TestBase):
 # Internal implementation
 
-def getRerunArgs(self):
-# The -N option says to NOT run a if it matches the option argument, so
-# if we are using dSYM we say to NOT run dwarf (-N dwarf) and vice
-# versa.
-if self.using_dsym is None:
-# The test was skipped altogether.
-return ""
-elif self.using_dsym:
-return "-N dwarf " + self.mydir
-else:
-return "-N dsym " + self.mydir
-
 def BuildMakefile(self):
 makefilePath = self.getBuildArtifact("Makefile")
 if os.path.exists(makefilePath):
@@ -135,37 +123,10 @@
 makefile.flush()
 makefile.close()
 
-@add_test_categories(["dsym"])
-def __test_with_dsym(self):
-self.using_dsym = True
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dsym.debug_info = "dsym"
-
-@add_test_categories(["dwarf"])
-def __test_with_dwarf(self):
-self.using_dsym = False
+def _test(self):
 self.BuildMakefile()
 self.build()
 self.do_test()
-__test_with_dwarf.debug_info = "dwarf"
-
-@add_test_categories(["dwo"])
-def __test_with_dwo(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwo.debug_info = "dwo"
-
-@add_test_categories(["gmodules"])
-def __test_with_gmodules(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_gmodules.debug_info = "gmodules"
 
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
@@ -237,23 +198,15 @@
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
+
+test_func = ApplyDecoratorsToFunction(InlineTest._test, decorators)
 # Build the test case
-test = type(test_name, (InlineTest,), {'using_dsym': None})
-test.name = test_name
-
-test.test_with_dsym = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dsym, decorators)
-test.test_with_dwarf = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwarf, decorators)
-test.test_with_dwo = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwo, decorators)
-test.test_with_gmodules = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_gmodules, decorators)
+test_class = type(test_name, (InlineTest,), dict(test=test_func, 
name=test_name))
 
 # Add the test case to the globals, and hide InlineTest
-__globals.update({test_name: test})
+__globals.update({test_name: test_class})
 
 # Keep track of the original test filename so we report it
 # correctly in test results.
-test.test_filename = __file
-return test
+test_class.test_filename = __file
+return test_class


Index: packages/Python/lldbsuite/test/lldbinline.py
===
--- packages/Python/lldbsuite/test/lldbinline.py
+++ packages/Python/lldbsuite/test/lldbinline.py
@@ -84,18 +84,6 @@
 class InlineTest(TestBase):
 # Internal implementation
 
-def getRerunArgs(self):
-# The -N option says to NOT run a if it matches the option argument, so
-# if we are using dSYM we say to NOT run dwarf (-N dwarf) and vice
-# versa.
-if self.using_dsym is None:
-# The test was skipped altogether.
-return ""
-elif self.using_dsym:
-return "-N dwarf " + self.mydir
-else:
-return "-N dsym " + self.mydir
-
 def BuildMakefile(self):
 makefilePath = self.getBuildArtifact("Makefile")
 if os.path.exists(makefilePath):
@@ -135,37 +123,10 @@
 makefile.flush()
 makefile.close()
 
-@add_test_categories(["dsym"])
-def __test_with_dsym(self):
-self.using_dsym = True
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dsym.debug_info = "dsym"
-
-@add_test_categories(["dwarf"])
-def __test_with_dwarf(self):
-self.using_dsym = False
+def _test(self):
 self.BuildMakefile()
 self.build()
 self.do_test()
-__test_with_dwarf.debug_info = "dwarf"
-
-@add_test_categories(["dwo"])
-def __test_with_dwo(self):
-self.using_dsym

[Lldb-commits] [lldb] r333666 - Remove infinite recursion due to FileSpec change.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu May 31 09:28:29 2018
New Revision: 333666

URL: http://llvm.org/viewvc/llvm-project?rev=333666&view=rev
Log:
Remove infinite recursion due to FileSpec change.

Fixes infinite recursion due to change in how FileSpec deals with
removing the last path component.

Fixes timout for TestMiniDumpNew.py

Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp?rev=333666&r1=333665&r2=333666&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp Thu May 31 
09:28:29 2018
@@ -1763,12 +1763,10 @@ PlatformDarwin::FindBundleBinaryInExecSe
 
 FileSpec platform_pull_apart(platform_file);
 std::vector path_parts;
-ConstString unix_root_dir("/");
-while (true) {
+path_parts.push_back(
+platform_pull_apart.GetLastPathComponent().AsCString());
+while (platform_pull_apart.RemoveLastPathComponent()) {
   ConstString part = platform_pull_apart.GetLastPathComponent();
-  platform_pull_apart.RemoveLastPathComponent();
-  if (part.IsEmpty() || part == unix_root_dir)
-break;
   path_parts.push_back(part.AsCString());
 }
 const size_t path_parts_size = path_parts.size();


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

Re: [Lldb-commits] Support of MSVC function-level linking

[Greg Clayton via lldb-commits]

> On May 31, 2018, at 2:31 AM, Aleksandr Urakov via lldb-commits 
>  wrote:
> 
> Hello!
> 
> I'm Aleksandr from JetBrains. We are working on improving support of 
> MSVC-compiled binaries in lldb. We have made several fixes and would like to 
> upstream them.
> 
> The first patch adds support of function-level linking feature. The 
> SymbolFilePDB::ParseCompileUnitLineTable function relies on the fact that 
> ranges of compiled source files in the binary are continuous and don't 
> intersect with each other. ParseCompileUnitLineTable creates LineSequence for 
> each file and inserts it into LineTable, and the implementation of LineTable 
> relies on continuity of the sequence. But it's not always true when 
> function-level linking is enabled, e.g. in the attached input test file 
> test-pdb-function-level-linking.exe there is xstring's 
> std__basic_string_char_std__char_traits_char__std__allocator_char_max_size
>  (.00454820) between test-pdb-function-level-linking.cpp's foo (.00454770) 
> and main (.004548F0). The source is compiled with Microsoft C/C++ compiler 
> version 19.14.26429.4 for x86.
> 
> To fix the problem we propose to renew the sequence on each address gap.

That is what DWARF does as well. A line table can have many sequences where 
each sequence is a vector of rows whose addresses must always increase or stay 
the same. The line tables we have in LLDB mimic the DWARF line tables in many 
ways.


> 
> The link to the patch and related files is: 
> https://drive.google.com/open?id=1ozp06jyqugjLGT-6wuJKS1UhRuXFsixf 
> 
> 
> Thanks!
> 
> -- 
> Aleksandr Urakov
> Software Developer
> JetBrains
> http://www.jetbrains.com 
> The Drive to Develop
> ___
> 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] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Very nice


https://reviews.llvm.org/D47470



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

Re: [Lldb-commits] Support of MSVC function-level linking

[Leonard Mosescu via lldb-commits]

If anyone's working on this I'd suggest adding a test case for the "split
code" case as well (where even a single function is split into multiple
ranges). MSVC with PGO should help produce hot/cold cold split repros.

On Thu, May 31, 2018 at 10:24 AM, Greg Clayton via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
>
> On May 31, 2018, at 2:31 AM, Aleksandr Urakov via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
> Hello!
>
> I'm Aleksandr from JetBrains. We are working on improving support of
> MSVC-compiled binaries in lldb. We have made several fixes and would like
> to upstream them.
>
> The first patch adds support of function-level linking feature. The
> SymbolFilePDB::ParseCompileUnitLineTable function relies on the fact that
> ranges of compiled source files in the binary are continuous and don't
> intersect with each other. ParseCompileUnitLineTable creates LineSequence
> for each file and inserts it into LineTable, and the implementation of
> LineTable relies on continuity of the sequence. But it's not always true
> when function-level linking is enabled, e.g. in the attached input test
> file test-pdb-function-level-linking.exe there is xstring's
> std__basic_string_char_std__char_traits_char__std__allocator_char_max_size
> (.00454820) between test-pdb-function-level-linking.cpp's foo (.00454770)
> and main (.004548F0). The source is compiled with Microsoft C/C++ compiler
> version 19.14.26429.4 for x86.
>
> To fix the problem we propose to renew the sequence on each address gap.
>
>
> That is what DWARF does as well. A line table can have many sequences
> where each sequence is a vector of rows whose addresses must always
> increase or stay the same. The line tables we have in LLDB mimic the DWARF
> line tables in many ways.
>
>
>
> The link to the patch and related files is: https://drive.google.com/
> open?id=1ozp06jyqugjLGT-6wuJKS1UhRuXFsixf
>
> Thanks!
>
> --
> Aleksandr Urakov
> Software Developer
> JetBrains
> http://www.jetbrains.com
> The Drive to Develop
> ___
> 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 mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47481: Initialize FunctionCaller::m_struct_valid

[Greg Clayton via Phabricator via lldb-commits]

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

We need a clang warning that tells us when POD types are not initialized! I am 
sure we have other similar issues...


https://reviews.llvm.org/D47481



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

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

After seeing this, it might be nice to put the recursion that follows the 
DW_AT_specification and DW_AT_abstract_origin and avoids infinite recursion 
into a DWARFDIE function that takes a callback:

  typedef bool (*DIECallback)(DWARFDIE &die);
  void DWARFDIE::VisitAbstractOriginsAndSpecifications(DIECallback callback);

The contents would be very similar to the DWARFDIE::IsMethod() function, but it 
would listen to the "bool" return value from calling the "callback" function 
and stop recursing if true or false is returned so we can stop the recursion if 
we find what we need. Then we can re-use this function for more


https://reviews.llvm.org/D47470



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

Re: [Lldb-commits] Support of MSVC function-level linking

[Adrian McCarthy via lldb-commits]

Can you post your patch to https://reviews.llvm.org/ ?

On Thu, May 31, 2018 at 10:36 AM, Leonard Mosescu via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> If anyone's working on this I'd suggest adding a test case for the "split
> code" case as well (where even a single function is split into multiple
> ranges). MSVC with PGO should help produce hot/cold cold split repros.
>
> On Thu, May 31, 2018 at 10:24 AM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>>
>>
>> On May 31, 2018, at 2:31 AM, Aleksandr Urakov via lldb-commits <
>> lldb-commits@lists.llvm.org> wrote:
>>
>> Hello!
>>
>> I'm Aleksandr from JetBrains. We are working on improving support of
>> MSVC-compiled binaries in lldb. We have made several fixes and would like
>> to upstream them.
>>
>> The first patch adds support of function-level linking feature. The
>> SymbolFilePDB::ParseCompileUnitLineTable function relies on the fact
>> that ranges of compiled source files in the binary are continuous and don't
>> intersect with each other. ParseCompileUnitLineTable creates LineSequence
>> for each file and inserts it into LineTable, and the implementation of
>> LineTable relies on continuity of the sequence. But it's not always true
>> when function-level linking is enabled, e.g. in the attached input test
>> file test-pdb-function-level-linking.exe there is xstring's
>> std__basic_string_char_std__char_traits_char__std__allocator_char_max_size
>> (.00454820) between test-pdb-function-level-linking.cpp's foo
>> (.00454770) and main (.004548F0). The source is compiled with Microsoft
>> C/C++ compiler version 19.14.26429.4 for x86.
>>
>> To fix the problem we propose to renew the sequence on each address gap.
>>
>>
>> That is what DWARF does as well. A line table can have many sequences
>> where each sequence is a vector of rows whose addresses must always
>> increase or stay the same. The line tables we have in LLDB mimic the DWARF
>> line tables in many ways.
>>
>>
>>
>> The link to the patch and related files is: https://drive.google.com/o
>> pen?id=1ozp06jyqugjLGT-6wuJKS1UhRuXFsixf
>>
>> Thanks!
>>
>> --
>> Aleksandr Urakov
>> Software Developer
>> JetBrains
>> http://www.jetbrains.com
>> The Drive to Develop
>> ___
>> 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 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] D47481: Initialize FunctionCaller::m_struct_valid

[Tom Tromey via Phabricator via lldb-commits]

tromey added a comment.

I don't have write access so someone will have to land this for me.  Thanks.


https://reviews.llvm.org/D47481



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

[Lldb-commits] [lldb] r333690 - Set m_struct_valid to initial value in ctor.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Thu May 31 13:01:15 2018
New Revision: 333690

URL: http://llvm.org/viewvc/llvm-project?rev=333690&view=rev
Log:
Set m_struct_valid to initial value in ctor.  
Patch from Tom Tromey .

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

Modified:
lldb/trunk/source/Expression/FunctionCaller.cpp

Modified: lldb/trunk/source/Expression/FunctionCaller.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/FunctionCaller.cpp?rev=333690&r1=333689&r2=333690&view=diff
==
--- lldb/trunk/source/Expression/FunctionCaller.cpp (original)
+++ lldb/trunk/source/Expression/FunctionCaller.cpp Thu May 31 13:01:15 2018
@@ -48,7 +48,8 @@ FunctionCaller::FunctionCaller(Execution
   m_function_return_type(return_type),
   m_wrapper_function_name("__lldb_caller_function"),
   m_wrapper_struct_name("__lldb_caller_struct"), m_wrapper_args_addrs(),
-  m_arg_values(arg_value_list), m_compiled(false), m_JITted(false) {
+  m_struct_valid(false), m_arg_values(arg_value_list), m_compiled(false),
+  m_JITted(false) {
   m_jit_process_wp = lldb::ProcessWP(exe_scope.CalculateProcess());
   // Can't make a FunctionCaller without a process.
   assert(m_jit_process_wp.lock());


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

[Lldb-commits] [PATCH] D47481: Initialize FunctionCaller::m_struct_valid

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333690: Set m_struct_valid to initial value in ctor.   
(authored by jmolenda, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47481

Files:
  lldb/trunk/source/Expression/FunctionCaller.cpp


Index: lldb/trunk/source/Expression/FunctionCaller.cpp
===
--- lldb/trunk/source/Expression/FunctionCaller.cpp
+++ lldb/trunk/source/Expression/FunctionCaller.cpp
@@ -48,7 +48,8 @@
   m_function_return_type(return_type),
   m_wrapper_function_name("__lldb_caller_function"),
   m_wrapper_struct_name("__lldb_caller_struct"), m_wrapper_args_addrs(),
-  m_arg_values(arg_value_list), m_compiled(false), m_JITted(false) {
+  m_struct_valid(false), m_arg_values(arg_value_list), m_compiled(false),
+  m_JITted(false) {
   m_jit_process_wp = lldb::ProcessWP(exe_scope.CalculateProcess());
   // Can't make a FunctionCaller without a process.
   assert(m_jit_process_wp.lock());


Index: lldb/trunk/source/Expression/FunctionCaller.cpp
===
--- lldb/trunk/source/Expression/FunctionCaller.cpp
+++ lldb/trunk/source/Expression/FunctionCaller.cpp
@@ -48,7 +48,8 @@
   m_function_return_type(return_type),
   m_wrapper_function_name("__lldb_caller_function"),
   m_wrapper_struct_name("__lldb_caller_struct"), m_wrapper_args_addrs(),
-  m_arg_values(arg_value_list), m_compiled(false), m_JITted(false) {
+  m_struct_valid(false), m_arg_values(arg_value_list), m_compiled(false),
+  m_JITted(false) {
   m_jit_process_wp = lldb::ProcessWP(exe_scope.CalculateProcess());
   // Can't make a FunctionCaller without a process.
   assert(m_jit_process_wp.lock());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

In https://reviews.llvm.org/D47551#1117086, @lhames wrote:

> LGTM.
>
> I haven't looked at process memory management. How hard would your FIXME be 
> to implement?


After looking at this more carefully, I think the FIXME makes a bad 
prescription. It's based on the assumption that each platform-specific 
allocator makes use of an efficient API to make aligned allocations. This 
doesn't generally seem to be true, because the platform-specific allocators 
rely on mmap().

A better alternative might be to add a new API, 
`AllocatedMemoryCache::AllocateAlignedMemory(size, alignment)`. You could 
implement an alignment-aware allocator here, say, by splitting up free memory 
into a list of buckets (one list per alignment). Next, you could surface the 
API from `Process`, provided there's a fallback strategy when the allocated 
memory cache is disabled.

One caveat to all of this is that it might not be worth doing unless 
fragmentation-related overhead is identified as a bottleneck.

I'll remove the FIXME.


https://reviews.llvm.org/D47551



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

[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 149355.
vsk marked 2 inline comments as done.
vsk added a comment.

- Address Pavel's feedback, remove a questionable FIXME.


https://reviews.llvm.org/D47551

Files:
  lit/Expr/Inputs/ir-memory-map-basic.test
  lit/Expr/Inputs/ir-memory-map-overlap1.test
  lit/Expr/TestIRMemoryMap.test
  source/Expression/IRMemoryMap.cpp

Index: source/Expression/IRMemoryMap.cpp
===
--- source/Expression/IRMemoryMap.cpp
+++ source/Expression/IRMemoryMap.cpp
@@ -301,15 +301,21 @@
   lldb::addr_t allocation_address = LLDB_INVALID_ADDRESS;
   lldb::addr_t aligned_address = LLDB_INVALID_ADDRESS;
 
-  size_t alignment_mask = alignment - 1;
   size_t allocation_size;
 
-  if (size == 0)
+  if (size == 0) {
+// FIXME: Malloc(0) should either return an invalid address or assert, in
+// order to cut down on unnecessary allocations.
 allocation_size = alignment;
-  else
-allocation_size = (size & alignment_mask)
-  ? ((size + alignment) & (~alignment_mask))
-  : size;
+  } else {
+// Round up the requested size to an aligned value.
+allocation_size = llvm::alignTo(size, alignment);
+
+// The process page cache does not see the requested alignment. We can't
+// assume its result will be any more than 1-byte aligned. To work around
+// this, request `alignment - 1` additional bytes.
+allocation_size += alignment - 1;
+  }
 
   switch (policy) {
   default:
Index: lit/Expr/TestIRMemoryMap.test
===
--- lit/Expr/TestIRMemoryMap.test
+++ lit/Expr/TestIRMemoryMap.test
@@ -1,28 +1,3 @@
 # RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
-# RUN: lldb-test ir-memory-map %t %s
-
-malloc 0 1
-malloc 1 1
-
-malloc 2 1
-malloc 2 2
-malloc 2 4
-
-malloc 3 1
-malloc 3 2
-malloc 3 4
-
-malloc 128 1
-malloc 128 2
-malloc 128 4
-malloc 128 128
-
-malloc 2048 1
-malloc 2048 2
-malloc 2048 4
-
-malloc 3968 1
-malloc 3968 2
-malloc 3968 4
-
-malloc 0 1
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic.test
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1.test
Index: lit/Expr/Inputs/ir-memory-map-overlap1.test
===
--- /dev/null
+++ lit/Expr/Inputs/ir-memory-map-overlap1.test
@@ -0,0 +1,10 @@
+malloc 8 16
+malloc 16 8
+malloc 64 32
+malloc 1 8
+malloc 64 32
+malloc 64 8
+malloc 1024 32
+malloc 1 16
+malloc 8 16
+malloc 1024 16
\ No newline at end of file
Index: lit/Expr/Inputs/ir-memory-map-basic.test
===
--- /dev/null
+++ lit/Expr/Inputs/ir-memory-map-basic.test
@@ -0,0 +1,25 @@
+malloc 0 1
+malloc 1 1
+
+malloc 2 1
+malloc 2 2
+malloc 2 4
+
+malloc 3 1
+malloc 3 2
+malloc 3 4
+
+malloc 128 1
+malloc 128 2
+malloc 128 4
+malloc 128 128
+
+malloc 2048 1
+malloc 2048 2
+malloc 2048 4
+
+malloc 3968 1
+malloc 3968 2
+malloc 3968 4
+
+malloc 0 1
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r333701 - [lit] Remove the *.test suffix from two test inputs

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Thu May 31 15:09:01 2018
New Revision: 333701

URL: http://llvm.org/viewvc/llvm-project?rev=333701&view=rev
Log:
[lit] Remove the *.test suffix from two test inputs

This prevents the test inputs from being marked as unsupported tests,
due to their lack of RUN lines.

Added:
lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic
  - copied, changed from r333700, 
lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test
lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1
  - copied, changed from r333700, 
lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
Removed:
lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test
lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
Modified:
lldb/trunk/lit/Expr/TestIRMemoryMap.test

Copied: lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic (from r333700, 
lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test)
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic?p2=lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic&p1=lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test&r1=333700&r2=333701&rev=333701&view=diff
==
(empty)

Removed: lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test?rev=333700&view=auto
==
--- lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test (original)
+++ lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test (removed)
@@ -1,25 +0,0 @@
-malloc 0 1
-malloc 1 1
-
-malloc 2 1
-malloc 2 2
-malloc 2 4
-
-malloc 3 1
-malloc 3 2
-malloc 3 4
-
-malloc 128 1
-malloc 128 2
-malloc 128 4
-malloc 128 128
-
-malloc 2048 1
-malloc 2048 2
-malloc 2048 4
-
-malloc 3968 1
-malloc 3968 2
-malloc 3968 4
-
-malloc 0 1

Copied: lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1 (from r333700, 
lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test)
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1?p2=lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1&p1=lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test&r1=333700&r2=333701&rev=333701&view=diff
==
(empty)

Removed: lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test?rev=333700&view=auto
==
--- lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test (original)
+++ lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test (removed)
@@ -1,10 +0,0 @@
-malloc 8 16
-malloc 16 8
-malloc 64 32
-malloc 1 8
-malloc 64 32
-malloc 64 8
-malloc 1024 32
-malloc 1 16
-malloc 8 16
-malloc 1024 16
\ No newline at end of file

Modified: lldb/trunk/lit/Expr/TestIRMemoryMap.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestIRMemoryMap.test?rev=333701&r1=333700&r2=333701&view=diff
==
--- lldb/trunk/lit/Expr/TestIRMemoryMap.test (original)
+++ lldb/trunk/lit/Expr/TestIRMemoryMap.test Thu May 31 15:09:01 2018
@@ -1,10 +1,10 @@
 # RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
 
-# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic.test
-# RUN: lldb-test ir-memory-map -host-only %t %S/Inputs/ir-memory-map-basic.test
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic
+# RUN: lldb-test ir-memory-map -host-only %t %S/Inputs/ir-memory-map-basic
 
-# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1.test
-# RUN: lldb-test ir-memory-map -host-only %t 
%S/Inputs/ir-memory-map-overlap1.test
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1
+# RUN: lldb-test ir-memory-map -host-only %t %S/Inputs/ir-memory-map-overlap1
 
 # RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-mix-malloc-free
 # RUN: lldb-test ir-memory-map -host-only %t 
%S/Inputs/ir-memory-map-mix-malloc-free


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

[Lldb-commits] [lldb] r333700 - [IRMemoryMap] Test interleaved Mallocs and Frees

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Thu May 31 15:09:01 2018
New Revision: 333700

URL: http://llvm.org/viewvc/llvm-project?rev=333700&view=rev
Log:
[IRMemoryMap] Test interleaved Mallocs and Frees

This adds a new command to the ir-memory-map tester:

  free 

The argument to free is an index which identifies which live allocation
to free. Index 0 identifies the first live allocation in the address
space, index 1 identifies the second, etc. where the allocations are
sorted in increasing order.

For illustrative purposes, assume malloc returns monotonically
increasing addresses. Here are some examples of how free would work:

Example 1
-

malloc 16 1
malloc 32 1
free 1  //< Free the 32-byte allocation.
free 0  //< Next, free the 16-byte allocation.

Example 2
-

malloc 16 1
malloc 32 1
free 0  //< Free the 16-byte allocation.
free 0  //< Next, free the 32-byte allocation.

Added:
lldb/trunk/lit/Expr/Inputs/ir-memory-map-mix-malloc-free
Modified:
lldb/trunk/lit/Expr/TestIRMemoryMap.test
lldb/trunk/tools/lldb-test/lldb-test.cpp

Added: lldb/trunk/lit/Expr/Inputs/ir-memory-map-mix-malloc-free
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/ir-memory-map-mix-malloc-free?rev=333700&view=auto
==
--- lldb/trunk/lit/Expr/Inputs/ir-memory-map-mix-malloc-free (added)
+++ lldb/trunk/lit/Expr/Inputs/ir-memory-map-mix-malloc-free Thu May 31 
15:09:01 2018
@@ -0,0 +1,275 @@
+# This file was generated by a slightly extended version of a script
+# attached to https://reviews.llvm.org/D47508.
+
+malloc 2 4
+malloc 4095 128
+malloc 3 16
+malloc 8192 2
+malloc 1 128
+malloc 3 2
+free 3
+free 3
+malloc 32 2
+malloc 2 16
+free 1
+malloc 2048 2
+free 2
+malloc 2049 4
+malloc 4097 2
+malloc 2 16
+free 1
+free 6
+malloc 1 16
+malloc 33 128
+malloc 2 128
+malloc 2 16
+malloc 2 4
+malloc 2 4
+free 4
+malloc 4 4
+malloc 2 16
+free 3
+malloc 0 128
+free 6
+free 2
+malloc 33 2
+malloc 4095 2
+free 6
+malloc 2 4
+malloc 0 2
+free 1
+free 9
+free 5
+malloc 32 2
+malloc 4096 2
+free 2
+free 9
+free 6
+malloc 8192 16
+malloc 2 4
+malloc 4096 16
+free 10
+malloc 4 4
+free 7
+malloc 4 4
+malloc 8193 2
+malloc 1 2
+free 3
+free 7
+malloc 4096 2
+free 0
+malloc 4096 4
+free 3
+malloc 4097 128
+malloc 1 2
+malloc 4 2
+free 10
+free 8
+malloc 0 16
+malloc 2049 16
+free 9
+malloc 8193 16
+free 10
+free 1
+malloc 8193 2
+free 11
+malloc 3 16
+free 5
+malloc 33 128
+free 0
+malloc 2049 4
+malloc 1 128
+malloc 32 16
+free 1
+free 9
+malloc 2048 4
+free 0
+free 2
+malloc 5 4
+free 11
+malloc 2048 4
+malloc 4097 16
+malloc 8192 4
+free 2
+free 8
+free 11
+malloc 8192 4
+free 2
+malloc 8191 4
+malloc 32 128
+free 7
+malloc 4 16
+malloc 4096 128
+free 12
+malloc 2 4
+free 9
+malloc 8193 128
+malloc 4095 4
+malloc 2049 16
+malloc 2 4
+free 16
+malloc 0 128
+malloc 5 2
+malloc 2047 4
+malloc 2 2
+free 10
+malloc 0 128
+free 6
+malloc 2047 128
+free 11
+free 0
+free 9
+malloc 4 4
+malloc 3 2
+free 7
+malloc 1 128
+free 13
+malloc 8193 16
+malloc 4097 128
+free 15
+free 3
+malloc 1 2
+malloc 2049 4
+malloc 2048 2
+free 7
+malloc 31 4
+free 5
+free 14
+free 4
+free 10
+malloc 4 4
+free 6
+malloc 3 2
+malloc 1 128
+free 13
+malloc 4 16
+free 4
+free 8
+malloc 3 16
+free 13
+malloc 0 4
+free 8
+free 13
+malloc 1 2
+malloc 8192 128
+free 12
+malloc 2049 2
+malloc 1 16
+free 4
+free 7
+malloc 4 16
+malloc 4 128
+malloc 4096 16
+malloc 2048 16
+malloc 32 4
+malloc 8193 4
+free 2
+malloc 3 16
+malloc 8192 4
+free 1
+malloc 8191 2
+free 3
+malloc 8192 2
+malloc 8192 2
+free 3
+free 7
+malloc 31 4
+malloc 2049 2
+free 4
+free 3
+free 14
+free 1
+malloc 2048 4
+malloc 1 4
+malloc 1 4
+malloc 2 4
+malloc 4 16
+free 12
+free 14
+free 3
+free 0
+free 12
+free 10
+malloc 32 16
+free 5
+free 9
+free 4
+free 9
+free 3
+malloc 4096 16
+malloc 4 2
+free 12
+free 10
+free 3
+free 7
+malloc 4097 4
+malloc 4095 16
+free 3
+malloc 2047 16
+free 12
+malloc 8193 2
+free 2
+free 3
+malloc 0 4
+free 2
+free 9
+malloc 8192 16
+malloc 2 2
+malloc 4096 4
+malloc 5 128
+malloc 4095 4
+malloc 4095 4
+free 9
+malloc 32 4
+malloc 31 16
+free 15
+malloc 4097 16
+malloc 2048 128
+malloc 2048 2
+malloc 4096 128
+malloc 3 16
+free 2
+malloc 1 4
+malloc 3 4
+free 3
+malloc 31 2
+malloc 1 128
+free 21
+malloc 0 128
+free 8
+malloc 3 128
+free 5
+malloc 1 16
+free 9
+malloc 2048 4
+free 8
+free 4
+malloc 31 2
+malloc 1 16
+free 19
+malloc 2 4
+malloc 2 2
+free 8
+malloc 8193 16
+free 11
+free 8
+free 6
+free 12
+free 4
+free 0
+free 10
+free 1
+free 0
+free 5
+free 5
+free 3
+free 6
+free 1
+free 2
+free 6
+free 5
+free 4
+free 3
+free 1
+free 1
+free 0

Modified: lldb/trunk/lit/Expr/TestIRMemoryMap.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestIRMemoryMap.test?rev=333700&r1=333699&r2=333700&view=diff
==
--- lldb/trunk/lit/Expr/TestIRMemoryMap.test (original)
+++ lldb/tru

[Lldb-commits] [lldb] r333698 - [IRMemoryMap] Test host-side allocations

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Thu May 31 15:09:00 2018
New Revision: 333698

URL: http://llvm.org/viewvc/llvm-project?rev=333698&view=rev
Log:
[IRMemoryMap] Test host-side allocations

r333583 introduced testing for IRMemoryMap's process-side allocations
(eAllocationPolicyProcessOnly). This adds support for the host-side
variety (eAllocationPolicyHostOnly).

Modified:
lldb/trunk/lit/Expr/TestIRMemoryMap.test
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/lit/Expr/TestIRMemoryMap.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestIRMemoryMap.test?rev=333698&r1=333697&r2=333698&view=diff
==
--- lldb/trunk/lit/Expr/TestIRMemoryMap.test (original)
+++ lldb/trunk/lit/Expr/TestIRMemoryMap.test Thu May 31 15:09:00 2018
@@ -1,3 +1,5 @@
 # RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
 # RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic.test
+# RUN: lldb-test ir-memory-map -host-only %t %S/Inputs/ir-memory-map-basic.test
 # RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1.test
+# RUN: lldb-test ir-memory-map -host-only %t 
%S/Inputs/ir-memory-map-overlap1.test

Modified: lldb/trunk/tools/lldb-test/lldb-test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-test/lldb-test.cpp?rev=333698&r1=333697&r2=333698&view=diff
==
--- lldb/trunk/tools/lldb-test/lldb-test.cpp (original)
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp Thu May 31 15:09:00 2018
@@ -159,6 +159,10 @@ static cl::opt CommandFile(
 cl::desc(""),
 cl::init("-"),
 cl::sub(IRMemoryMapSubcommand));
+static cl::opt UseHostOnlyAllocationPolicy(
+"host-only", cl::desc("Use the host-only allocation policy"),
+cl::init(false), cl::sub(IRMemoryMapSubcommand));
+
 using AllocationT = std::pair;
 bool areAllocationsOverlapping(const AllocationT &L, const AllocationT &R);
 using AddrIntervalMap =
@@ -521,14 +525,16 @@ bool opts::irmemorymap::evalMalloc(IRMem
 exit(1);
   }
 
+  IRMemoryMap::AllocationPolicy AP =
+  UseHostOnlyAllocationPolicy ? IRMemoryMap::eAllocationPolicyHostOnly
+  : IRMemoryMap::eAllocationPolicyProcessOnly;
+
   // Issue the malloc in the target process with "-rw" permissions.
   const uint32_t Permissions = 0x3;
   const bool ZeroMemory = false;
-  IRMemoryMap::AllocationPolicy Policy =
-  IRMemoryMap::eAllocationPolicyProcessOnly;
   Status ST;
   addr_t Addr =
-  IRMemMap.Malloc(Size, Alignment, Permissions, Policy, ZeroMemory, ST);
+  IRMemMap.Malloc(Size, Alignment, Permissions, AP, ZeroMemory, ST);
   if (ST.Fail()) {
 outs() << formatv("Malloc error: {0}\n", ST);
 return true;


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

[Lldb-commits] [lldb] r333699 - [lldb-test] Make logging available to all subcommands

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Thu May 31 15:09:00 2018
New Revision: 333699

URL: http://llvm.org/viewvc/llvm-project?rev=333699&view=rev
Log:
[lldb-test] Make logging available to all subcommands

Modified:
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/tools/lldb-test/lldb-test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-test/lldb-test.cpp?rev=333699&r1=333698&r2=333699&view=diff
==
--- lldb/trunk/tools/lldb-test/lldb-test.cpp (original)
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp Thu May 31 15:09:00 2018
@@ -53,7 +53,10 @@ cl::SubCommand ModuleSubcommand("module-
 "Display LLDB Module Information");
 cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
 cl::SubCommand IRMemoryMapSubcommand("ir-memory-map", "Test IRMemoryMap");
+
 cl::opt Log("log", cl::desc("Path to a log file"), cl::init(""),
+ cl::sub(BreakpointSubcommand),
+ cl::sub(ModuleSubcommand), cl::sub(SymbolsSubcommand),
  cl::sub(IRMemoryMapSubcommand));
 
 /// Create a target using the file pointed to by \p Filename, or abort.


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

[Lldb-commits] [lldb] r333697 - [IRMemoryMap] Fix the alignment adjustment in Malloc

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Thu May 31 15:08:59 2018
New Revision: 333697

URL: http://llvm.org/viewvc/llvm-project?rev=333697&view=rev
Log:
[IRMemoryMap] Fix the alignment adjustment in Malloc

This prevents Malloc from allocating the same chunk of memory twice, as
a byproduct of an alignment adjustment which gave the client access to
unallocated memory.

Prior to this patch, the newly-added test failed with:

$ lldb-test ir-memory-map ... ir-memory-map-overlap1.test
...
Command: malloc(size=64, alignment=32)
Malloc: address = 0x1000cd080
Command: malloc(size=64, alignment=8)
Malloc: address = 0x1000cd0b0
Malloc error: overlapping allocation detected, previous allocation at 
[0x1000cd080, 0x1000cd0c0)

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

Added:
lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test
  - copied, changed from r333690, lldb/trunk/lit/Expr/TestIRMemoryMap.test
lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
Modified:
lldb/trunk/lit/Expr/TestIRMemoryMap.test
lldb/trunk/source/Expression/IRMemoryMap.cpp

Copied: lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test (from r333690, 
lldb/trunk/lit/Expr/TestIRMemoryMap.test)
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test?p2=lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test&p1=lldb/trunk/lit/Expr/TestIRMemoryMap.test&r1=333690&r2=333697&rev=333697&view=diff
==
--- lldb/trunk/lit/Expr/TestIRMemoryMap.test (original)
+++ lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test Thu May 31 15:08:59 2018
@@ -1,6 +1,3 @@
-# RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
-# RUN: lldb-test ir-memory-map %t %s
-
 malloc 0 1
 malloc 1 1
 

Added: lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test?rev=333697&view=auto
==
--- lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test (added)
+++ lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test Thu May 31 15:08:59 
2018
@@ -0,0 +1,10 @@
+malloc 8 16
+malloc 16 8
+malloc 64 32
+malloc 1 8
+malloc 64 32
+malloc 64 8
+malloc 1024 32
+malloc 1 16
+malloc 8 16
+malloc 1024 16
\ No newline at end of file

Modified: lldb/trunk/lit/Expr/TestIRMemoryMap.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestIRMemoryMap.test?rev=333697&r1=333696&r2=333697&view=diff
==
--- lldb/trunk/lit/Expr/TestIRMemoryMap.test (original)
+++ lldb/trunk/lit/Expr/TestIRMemoryMap.test Thu May 31 15:08:59 2018
@@ -1,28 +1,3 @@
 # RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
-# RUN: lldb-test ir-memory-map %t %s
-
-malloc 0 1
-malloc 1 1
-
-malloc 2 1
-malloc 2 2
-malloc 2 4
-
-malloc 3 1
-malloc 3 2
-malloc 3 4
-
-malloc 128 1
-malloc 128 2
-malloc 128 4
-malloc 128 128
-
-malloc 2048 1
-malloc 2048 2
-malloc 2048 4
-
-malloc 3968 1
-malloc 3968 2
-malloc 3968 4
-
-malloc 0 1
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic.test
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1.test

Modified: lldb/trunk/source/Expression/IRMemoryMap.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/IRMemoryMap.cpp?rev=333697&r1=333696&r2=333697&view=diff
==
--- lldb/trunk/source/Expression/IRMemoryMap.cpp (original)
+++ lldb/trunk/source/Expression/IRMemoryMap.cpp Thu May 31 15:08:59 2018
@@ -301,15 +301,21 @@ lldb::addr_t IRMemoryMap::Malloc(size_t
   lldb::addr_t allocation_address = LLDB_INVALID_ADDRESS;
   lldb::addr_t aligned_address = LLDB_INVALID_ADDRESS;
 
-  size_t alignment_mask = alignment - 1;
   size_t allocation_size;
 
-  if (size == 0)
+  if (size == 0) {
+// FIXME: Malloc(0) should either return an invalid address or assert, in
+// order to cut down on unnecessary allocations.
 allocation_size = alignment;
-  else
-allocation_size = (size & alignment_mask)
-  ? ((size + alignment) & (~alignment_mask))
-  : size;
+  } else {
+// Round up the requested size to an aligned value.
+allocation_size = llvm::alignTo(size, alignment);
+
+// The process page cache does not see the requested alignment. We can't
+// assume its result will be any more than 1-byte aligned. To work around
+// this, request `alignment - 1` additional bytes.
+allocation_size += alignment - 1;
+  }
 
   switch (policy) {
   default:


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

[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc

[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 rL333697: [IRMemoryMap] Fix the alignment adjustment in Malloc 
(authored by vedantk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47551?vs=149355&id=149366#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47551

Files:
  lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test
  lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
  lldb/trunk/lit/Expr/TestIRMemoryMap.test
  lldb/trunk/source/Expression/IRMemoryMap.cpp

Index: lldb/trunk/source/Expression/IRMemoryMap.cpp
===
--- lldb/trunk/source/Expression/IRMemoryMap.cpp
+++ lldb/trunk/source/Expression/IRMemoryMap.cpp
@@ -301,15 +301,21 @@
   lldb::addr_t allocation_address = LLDB_INVALID_ADDRESS;
   lldb::addr_t aligned_address = LLDB_INVALID_ADDRESS;
 
-  size_t alignment_mask = alignment - 1;
   size_t allocation_size;
 
-  if (size == 0)
+  if (size == 0) {
+// FIXME: Malloc(0) should either return an invalid address or assert, in
+// order to cut down on unnecessary allocations.
 allocation_size = alignment;
-  else
-allocation_size = (size & alignment_mask)
-  ? ((size + alignment) & (~alignment_mask))
-  : size;
+  } else {
+// Round up the requested size to an aligned value.
+allocation_size = llvm::alignTo(size, alignment);
+
+// The process page cache does not see the requested alignment. We can't
+// assume its result will be any more than 1-byte aligned. To work around
+// this, request `alignment - 1` additional bytes.
+allocation_size += alignment - 1;
+  }
 
   switch (policy) {
   default:
Index: lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
===
--- lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
+++ lldb/trunk/lit/Expr/Inputs/ir-memory-map-overlap1.test
@@ -0,0 +1,10 @@
+malloc 8 16
+malloc 16 8
+malloc 64 32
+malloc 1 8
+malloc 64 32
+malloc 64 8
+malloc 1024 32
+malloc 1 16
+malloc 8 16
+malloc 1024 16
\ No newline at end of file
Index: lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test
===
--- lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test
+++ lldb/trunk/lit/Expr/Inputs/ir-memory-map-basic.test
@@ -0,0 +1,25 @@
+malloc 0 1
+malloc 1 1
+
+malloc 2 1
+malloc 2 2
+malloc 2 4
+
+malloc 3 1
+malloc 3 2
+malloc 3 4
+
+malloc 128 1
+malloc 128 2
+malloc 128 4
+malloc 128 128
+
+malloc 2048 1
+malloc 2048 2
+malloc 2048 4
+
+malloc 3968 1
+malloc 3968 2
+malloc 3968 4
+
+malloc 0 1
Index: lldb/trunk/lit/Expr/TestIRMemoryMap.test
===
--- lldb/trunk/lit/Expr/TestIRMemoryMap.test
+++ lldb/trunk/lit/Expr/TestIRMemoryMap.test
@@ -1,28 +1,3 @@
 # RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
-# RUN: lldb-test ir-memory-map %t %s
-
-malloc 0 1
-malloc 1 1
-
-malloc 2 1
-malloc 2 2
-malloc 2 4
-
-malloc 3 1
-malloc 3 2
-malloc 3 4
-
-malloc 128 1
-malloc 128 2
-malloc 128 4
-malloc 128 128
-
-malloc 2048 1
-malloc 2048 2
-malloc 2048 4
-
-malloc 3968 1
-malloc 3968 2
-malloc 3968 4
-
-malloc 0 1
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic.test
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1.test
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47612: Add dependency on clang-headers when building LLDB.framework using CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: clayborg, labath, sas.
Herald added a subscriber: mgorny.

The LLDB.framework generated when building with CMake + Ninja/Make is
completely missing the clang headers. Although the code to copy them exists, we
don't even generate them unless we're building LLDB standalone.


https://reviews.llvm.org/D47612

Files:
  source/API/CMakeLists.txt


Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -187,6 +187,9 @@
 PUBLIC_HEADER "${framework_headers}")
 
   if(NOT IOS)
+if (NOT LLDB_BUILT_STANDALONE)
+  add_dependencies(liblldb clang-headers)
+endif()
 add_custom_command(TARGET liblldb POST_BUILD
   COMMAND ${CMAKE_COMMAND} -E copy_directory 
${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders $/Headers
   COMMAND ${CMAKE_COMMAND} -E create_symlink Versions/Current/Headers 
${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR}/LLDB.framework/Headers


Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -187,6 +187,9 @@
 PUBLIC_HEADER "${framework_headers}")
 
   if(NOT IOS)
+if (NOT LLDB_BUILT_STANDALONE)
+  add_dependencies(liblldb clang-headers)
+endif()
 add_custom_command(TARGET liblldb POST_BUILD
   COMMAND ${CMAKE_COMMAND} -E copy_directory ${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders $/Headers
   COMMAND ${CMAKE_COMMAND} -E create_symlink Versions/Current/Headers ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR}/LLDB.framework/Headers
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits