[Lldb-commits] [lldb] r353149 - [Expressions] Add support of expressions evaluation in some object's context

[Aleksandr Urakov via lldb-commits]

Author: aleksandr.urakov
Date: Tue Feb  5 01:14:36 2019
New Revision: 353149

URL: http://llvm.org/viewvc/llvm-project?rev=353149&view=rev
Log:
[Expressions] Add support of expressions evaluation in some object's context

Summary:
This patch adds support of expression evaluation in a context of some object.
Consider the following example:
```
struct S {
  int a = 11;
  int b = 12;
};

int main() {
  S s;
  int a = 1;
  int b = 2;
  // We have stopped here
  return 0;
}
```
This patch allows to do something like that:
```
lldb.frame.FindVariable("s").EvaluateExpression("a + b")
```
and the result will be `33` (not `3`) because fields `a` and `b` of `s` will be
used (not locals `a` and `b`).

This is achieved by replacing of `this` type and object for the expression. This
has some limitations: an expression can be evaluated only for values located in
the debuggee process memory (they must have an address of `eAddressTypeLoad`
type).

Reviewers: teemperor, clayborg, jingham, zturner, labath, davide, spyffe, 
serge-sans-paille

Reviewed By: jingham

Subscribers: abidh, lldb-commits, leonid.mashinskiy

Tags: #lldb

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

Added:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object/

lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object-objc/

lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object-objc/Makefile

lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object-objc/TestContextObjectObjc.py

lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object-objc/main.m

lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object/Makefile

lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object/TestContextObject.py

lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object/main.cpp
Modified:
lldb/trunk/include/lldb/API/SBValue.h
lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h
lldb/trunk/include/lldb/Expression/UserExpression.h
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/include/lldb/Symbol/TypeSystem.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/scripts/interface/SBValue.i
lldb/trunk/source/API/SBValue.cpp
lldb/trunk/source/Breakpoint/BreakpointLocation.cpp
lldb/trunk/source/Breakpoint/Watchpoint.cpp
lldb/trunk/source/Commands/CommandObjectExpression.cpp
lldb/trunk/source/Expression/ExpressionSourceCode.cpp
lldb/trunk/source/Expression/UserExpression.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/API/SBValue.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBValue.h?rev=353149&r1=353148&r2=353149&view=diff
==
--- lldb/trunk/include/lldb/API/SBValue.h (original)
+++ lldb/trunk/include/lldb/API/SBValue.h Tue Feb  5 01:14:36 2019
@@ -306,6 +306,13 @@ public:
   bool GetExpressionPath(lldb::SBStream &description,
  bool qualify_cxx_base_classes);
 
+  lldb::SBValue EvaluateExpression(const char *expr) const;
+  lldb::SBValue EvaluateExpression(const char *expr,
+   const SBExpressionOptions &options) const;
+  lldb::SBValue EvaluateExpression(const char *expr,
+   const SBExpressionOptions &options,
+   const char *name) const;
+
   SBValue(const lldb::ValueObjectSP &value_sp);
 
   //--

Modified: lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h?rev=353149&r1=353148&r2=353149&view=diff
==
--- lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h (original)
+++ lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h Tue Feb  5 
01:14:36 2019
@@ -36,7 +36,8 @@ public:
   const char *GetName() const { return m_name.c_str(); }
 
   bool GetText(std::string &text, lldb::LanguageType wrapping_language,
-   bool static_method, ExecutionContext &exe_ctx) const;
+   bool static_method, ExecutionContext &exe_ctx,
+   bool add_locals) const;
 
   // Given a string returned by GetText, find the beginning and end of the body
   // passed to CreateWrapped

[Lldb-commits] [PATCH] D55318: [Expressions] Add support of expressions evaluation in some object's context

[Aleksandr Urakov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353149: [Expressions] Add support of expressions evaluation 
in some object's context (authored by aleksandr.urakov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55318?vs=183816&id=185252#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55318

Files:
  lldb/trunk/include/lldb/API/SBValue.h
  lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h
  lldb/trunk/include/lldb/Expression/UserExpression.h
  lldb/trunk/include/lldb/Symbol/ClangASTContext.h
  lldb/trunk/include/lldb/Symbol/TypeSystem.h
  lldb/trunk/include/lldb/Target/Target.h
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object-objc/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object-objc/TestContextObjectObjc.py
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object-objc/main.m
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object/TestContextObject.py
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/context-object/main.cpp
  lldb/trunk/scripts/interface/SBValue.i
  lldb/trunk/source/API/SBValue.cpp
  lldb/trunk/source/Breakpoint/BreakpointLocation.cpp
  lldb/trunk/source/Breakpoint/Watchpoint.cpp
  lldb/trunk/source/Commands/CommandObjectExpression.cpp
  lldb/trunk/source/Expression/ExpressionSourceCode.cpp
  lldb/trunk/source/Expression/UserExpression.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp
  lldb/trunk/source/Target/Target.cpp

Index: lldb/trunk/scripts/interface/SBValue.i
===
--- lldb/trunk/scripts/interface/SBValue.i
+++ lldb/trunk/scripts/interface/SBValue.i
@@ -448,7 +448,19 @@
 ) GetExpressionPath;
 bool
 GetExpressionPath (lldb::SBStream &description, bool qualify_cxx_base_classes);
-
+
+lldb::SBValue
+EvaluateExpression(const char *expr) const;
+
+lldb::SBValue
+EvaluateExpression(const char *expr,
+   const SBExpressionOptions &options) const;
+
+lldb::SBValue
+EvaluateExpression(const char *expr,
+   const SBExpressionOptions &options,
+   const char *name) const;
+
 %pythoncode %{
 def __get_dynamic__ (self):
 '''Helper function for the "SBValue.dynamic" property.'''
Index: lldb/trunk/include/lldb/Expression/UserExpression.h
===
--- lldb/trunk/include/lldb/Expression/UserExpression.h
+++ lldb/trunk/include/lldb/Expression/UserExpression.h
@@ -272,6 +272,16 @@
   /// @param[out] jit_module_sp_ptr
   /// If non-nullptr, used to persist the generated IR module.
   ///
+  /// @param[in] ctx_obj
+  /// If specified, then the expression will be evaluated in the context of
+  /// this object. It means that the context object's address will be
+  /// treated as `this` for the expression (the expression will be
+  /// evaluated as if it was inside of a method of the context object's
+  /// class, and its `this` parameter were pointing to the context object).
+  /// The parameter makes sense for class and union types only.
+  /// Currently there is a limitation: the context object must be located
+  /// in the debuggee process' memory (and have the load address).
+  ///
   /// @result
   ///  A Process::ExpressionResults value.  eExpressionCompleted for
   ///  success.
@@ -281,7 +291,8 @@
llvm::StringRef expr_cstr, llvm::StringRef expr_prefix,
lldb::ValueObjectSP &result_valobj_sp, Status &error,
uint32_t line_offset = 0, std::string *fixed_expression = nullptr,
-   lldb::ModuleSP *jit_module_sp_ptr = nullptr);
+   lldb::ModuleSP *jit_module_sp_ptr = nullptr,
+   ValueObject *ctx_obj = nullptr);
 
   static const Status::ValueType kNoResult =
   0x1001; ///< ValueObject::GetError() returns this if there is no result
Index: lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h
===
--- lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h
+++ lldb/trunk/include/lldb/Expression/ExpressionSourceCode.h
@@ -36,7 +36,8 @@
   const char *GetName() const { return m_name.c_str(); 

[Lldb-commits] [PATCH] D55318: [Expressions] Add support of expressions evaluation in some object's context

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Thank you! I've updated the comments in the commit.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55318



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

[Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

[Pavel Labath via Phabricator via lldb-commits]

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

If Greg is happy, then LGTM from me too.




Comment at: source/Target/PathMappingList.cpp:223
+  
+  if (prefix_ref == ".") {
+prefix_is_relative = true;

jingham wrote:
> clayborg wrote:
> > We are we finding a "." in any path now? I thought we normalized those all 
> > out? Can a PathMappingList contain non normalized paths? if so, can't we 
> > just normalize them as they go into the list?
> That's the incoming path from the path maps.  You can't normalize just a "." 
> from the path mappings or it would become "" and we don't really know what 
> that means.  These are currently left as ".".  I think that's right.
FileSpec::IsValid() considers "" to be an invalid path. Most utilities/APIs do 
the same. The llvm api are a bit conflicted here though as sometimes they do 
consider "" to be invalid, but otoh they will happilly normalize "." to "". 

Bottom line: I also think that the current FileSpec behavior is correct.


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

https://reviews.llvm.org/D57552



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

[Lldb-commits] [PATCH] D56904: [NativePDB] Process virtual bases in the correct order

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Thanks for the reply! It seems that after your hint I've figured it out. It's 
the environment variable `VCINSTALLDIR`. I've added it to the `defaultenv` list 
of the `_get_visual_studio_environment` function and the test passes now. What 
do you think about this solution?

And do I understand right, it means, that clang-cl needs MSVC installation to 
work with these types?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56904



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

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Pavel Labath via Phabricator via lldb-commits]

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

OK, sounds good to me then. Just please make sure the update the test 
decorators before committing this (I expect you'll need to remove @xfailwindows 
from TestQuoting and add it to the `dir c:\` test (I forgot the name).


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

https://reviews.llvm.org/D56230



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

[Lldb-commits] [PATCH] D57714: [Reproducers] Instrumentation Framework: Serialization

[Pavel Labath via Phabricator via lldb-commits]

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

LGTM.




Comment at: unittests/Utility/ReproducerInstrumentationTest.cpp:1
+//===-- ReproducerTest.cpp *- C++ -*-===//
+//

Outdated file name.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57714



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

[Lldb-commits] [PATCH] D57742: [Expressions] Fix -Wreorder warning from r353149

[Krasimir Georgiev via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB353161: [Expressions] Fix -Wreorder warning from r353149 
(authored by krasimir, committed by ).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57742?vs=185270&id=185273#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57742

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


Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -65,12 +65,11 @@
 ClangExpressionDeclMap::ClangExpressionDeclMap(
 bool keep_result_in_memory,
 Materializer::PersistentVariableDelegate *result_delegate,
-ExecutionContext &exe_ctx,
-ValueObject *ctx_obj)
+ExecutionContext &exe_ctx, ValueObject *ctx_obj)
 : ClangASTSource(exe_ctx.GetTargetSP()), m_found_entities(),
   m_struct_members(), m_keep_result_in_memory(keep_result_in_memory),
-  m_result_delegate(result_delegate), m_parser_vars(), m_struct_vars(),
-  m_ctx_obj(ctx_obj) {
+  m_result_delegate(result_delegate), m_ctx_obj(ctx_obj), m_parser_vars(),
+  m_struct_vars() {
   EnableStructVars();
 }
 


Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -65,12 +65,11 @@
 ClangExpressionDeclMap::ClangExpressionDeclMap(
 bool keep_result_in_memory,
 Materializer::PersistentVariableDelegate *result_delegate,
-ExecutionContext &exe_ctx,
-ValueObject *ctx_obj)
+ExecutionContext &exe_ctx, ValueObject *ctx_obj)
 : ClangASTSource(exe_ctx.GetTargetSP()), m_found_entities(),
   m_struct_members(), m_keep_result_in_memory(keep_result_in_memory),
-  m_result_delegate(result_delegate), m_parser_vars(), m_struct_vars(),
-  m_ctx_obj(ctx_obj) {
+  m_result_delegate(result_delegate), m_ctx_obj(ctx_obj), m_parser_vars(),
+  m_struct_vars() {
   EnableStructVars();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57689: Adds property to force enabling of GDB JIT loader for MacOS

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Do we need both this and the `enable-jit-breakpoint` setting? My impression was 
that the latter was meant to be used for disabling the gdb jit feature. Is the 
gdb plugin useful for anything if it does not set the breakpoint (i.e. enable = 
on, but enable-jit-bkpt = off). If it isn't, then could we just remove the 
latter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57689



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

[Lldb-commits] [PATCH] D57745: [x64] Process the B field of the REX prefix correctly for the PUSH and POP instructions

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: jasonmolenda, labath.
aleksandr.urakov added a project: LLDB.
Herald added subscribers: lldb-commits, abidh.

This patch makes `x86AssemblyInspectionEngine` to process zero value of the `B` 
field of the `REX` prefix in a correct way for `PUSH` and `POP` instructions. 
MSVC sometimes emits `pushq %rbp` instruction as `0x40 0x55`, and it was not 
parsed correctly before.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D57745

Files:
  source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp

Index: unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
===
--- unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
+++ unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
@@ -325,27 +325,27 @@
   // compiled 'clang -fomit-frame-pointer -Os' for x86_64-apple-macosx
 
   uint8_t data[] = {
-  0x55,   // offset 0  -- pushq %rbp
-  0x41, 0x57, // offset 1  -- pushq %r15
-  0x41, 0x56, // offset 3  -- pushq %r14
-  0x41, 0x55, // offset 5  -- pushq %r13
-  0x41, 0x54, // offset 7  -- pushq %r12
-  0x53,   // offset 9  -- pushq %rbx
+  0x40, 0x55, // offset 0  -- pushq %rbp
+  0x41, 0x57, // offset 2  -- pushq %r15
+  0x41, 0x56, // offset 4  -- pushq %r14
+  0x41, 0x55, // offset 6  -- pushq %r13
+  0x41, 0x54, // offset 8  -- pushq %r12
+  0x53,   // offset 10 -- pushq %rbx
   0x48, 0x81, 0xec, 0x68, 0x38, 0x00,
-  0x00, // offset 10 -- subq $0x3868, %rsp
+  0x00, // offset 11 -- subq $0x3868, %rsp
 
   // 
 
   0x48, 0x81, 0xc4, 0x68, 0x38, 0x00,
-  0x00,// offset 17 -- addq $0x3868, %rsp
-  0x5b,// offset 24 -- popq %rbx
-  0x41, 0x5c,  // offset 25 -- popq %r12
-  0x41, 0x5d,  // offset 27 -- popq %r13
-  0x41, 0x5e,  // offset 29 -- popq %r14
-  0x41, 0x5f,  // offset 31 -- popq %r15
-  0x5d,// offset 33 -- popq %rbp
-  0xc3,// offset 34 -- retq
-  0xe8, 0x12, 0x34, 0x56, 0x78 // offset 35 -- callq whatever
+  0x00,// offset 18 -- addq $0x3868, %rsp
+  0x5b,// offset 25 -- popq %rbx
+  0x41, 0x5c,  // offset 26 -- popq %r12
+  0x41, 0x5d,  // offset 28 -- popq %r13
+  0x41, 0x5e,  // offset 30 -- popq %r14
+  0x41, 0x5f,  // offset 32 -- popq %r15
+  0x40, 0x5d,  // offset 34 -- popq %rbp
+  0xc3,// offset 36 -- retq
+  0xe8, 0x12, 0x34, 0x56, 0x78 // offset 37 -- callq whatever
   };
 
   AddressRange sample_range(0x1000, sizeof(data));
@@ -356,40 +356,40 @@
 
   // Unwind rules should look like
   // 0: CFA=rsp +8 => rsp=CFA+0 rip=[CFA-8]
-  // 1: CFA=rsp+16 => rbp=[CFA-16] rsp=CFA+0 rip=[CFA-8]
-  // 3: CFA=rsp+24 => rbp=[CFA-16] rsp=CFA+0 r15=[CFA-24] rip=[CFA-8]
-  // 5: CFA=rsp+32 => rbp=[CFA-16] rsp=CFA+0 r14=[CFA-32] r15=[CFA-24]
+  // 2: CFA=rsp+16 => rbp=[CFA-16] rsp=CFA+0 rip=[CFA-8]
+  // 4: CFA=rsp+24 => rbp=[CFA-16] rsp=CFA+0 r15=[CFA-24] rip=[CFA-8]
+  // 6: CFA=rsp+32 => rbp=[CFA-16] rsp=CFA+0 r14=[CFA-32] r15=[CFA-24]
   // rip=[CFA-8
-  // 7: CFA=rsp+40 => rbp=[CFA-16] rsp=CFA+0 r13=[CFA-40] r14=[CFA-32]
+  // 8: CFA=rsp+40 => rbp=[CFA-16] rsp=CFA+0 r13=[CFA-40] r14=[CFA-32]
   // r15=[CFA-24] rip=[CFA-8]
-  // 9: CFA=rsp+48 => rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48] r13=[CFA-40]
+  // 10: CFA=rsp+48 => rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48] r13=[CFA-40]
   // r14=[CFA-32] r15=[CFA-24] rip=[CFA-8]
-  // 10: CFA=rsp+56 => rbx=[CFA-56] rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48]
+  // 11: CFA=rsp+56 => rbx=[CFA-56] rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48]
   // r13=[CFA-40] r14=[CFA-32] r15=[CFA-24] rip=[CFA-8]
-  // 17: CFA=rsp+14496 => rbx=[CFA-56] rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48]
+  // 18: CFA=rsp+14496 => rbx=[CFA-56] rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48]
   // r13=[CFA-40] r14=[CFA-32] r15=[CFA-24] rip=[CFA-8]
 
-  // 24: CFA=rsp+56 => rbx=[CFA-56] rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48]
+  // 25: CFA=rsp+56 => rbx=[CFA-56] rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48]
   // r13=[CFA-40] r14=[CFA-32] r15=[CFA-24] rip=[CFA-8]
-  // 25: CFA=rsp+48 => rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48] r13=[CFA-40]
+  // 26: CFA=rsp+48 => rbp=[CFA-16] rsp=CFA+0 r12=[CFA-48] r13=[CFA-40]
   // r14=[CFA-32] r15=[CFA-24] rip=[CFA-8]
-  // 27: CFA=rsp+40 => rbp=[CFA-16] rsp=CFA+0 r13=[CFA-40] r14=[CFA-32]
+  // 28: CFA=rsp+40 => rbp=[CFA-16] rsp=CFA+0 r13=[CFA-40] r14=[CFA-32]
   // r15=[CFA-24] rip=[CFA-8]
-  // 29: CFA=rsp+32 => rbp=[CFA-16] rsp=CFA+0 r14=[CFA-32] r15=[CFA-24]
+  // 30: CFA=rsp+32 => rbp=[CFA-16] rsp=CFA+0 r14=[CFA-32] 

[Lldb-commits] [PATCH] D56322: [Reproducers] SBReproducer framework

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thank you. I like the new test, but see my comment about avoiding duplicating 
macros in the test. Besides returning objects, I can think of a couple of more 
interesting scenarios to cover:

- recursive calls (to exercise the g_global_boundary thingy)
- having two objects with identical `this` pointers (you can use placement 
`new` to guarantee that)
- calling the "custom" function during replay




Comment at: unittests/Utility/ReproducerInstrumentationTest.cpp:60-62
+template  bool Equals(T LHS, T RHS) {
+  return std::fabs(RHS - LHS) < std::numeric_limits::epsilon();
+}

googletest already has apis for this: EXPECT_FLOAT/DOUBLE_EQ or EXPECT_NEAR.



Comment at: unittests/Utility/ReproducerInstrumentationTest.cpp:67-122
+#define SB_RECORD_CONSTRUCTOR(Class, Signature, ...)   
\
+  lldb_private::repro::Recorder sb_recorder(g_serializer, g_registry,  
\
+LLVM_PRETTY_FUNCTION); 
\
+  sb_recorder.Record(&lldb_private::repro::construct::doit,   
\
+ __VA_ARGS__); 
\
+  sb_recorder.RecordResult(this);
+

Since these macros are the actual "public" interface of the instrumentation 
framework, and they do contain non-trivial logic, I think it would be nice to 
test the real macros, instead of redefining them here. (It would also be nice 
to simply avoid duplicating code.)

Would something like this work:
- move the macros to ReproducerInstrumentation.h (maybe also rename to 
`LLDB_RECORD_blah_blah`).
- parameterize them on a macro for getting the instrumentation classes. So, 
something like:
```

#define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...)  
 \
  if (InstrumentationData data = LLDB_GET_INSTRUMENTATION_DATA()) {  \
lldb_private::repro::Recorder recorder( \
data.GetSerializer(),   \
data.GetRegistry(), \
LLVM_PRETTY_FUNCTION); \
sb_recorder.Record(&lldb_private::repro::construct::doit, \
   __VA_ARGS__);   \
sb_recorder.RecordResult(this);\
  }
```
where `InstrumentationData` is essentially `std::pair` with a suitable `operator bool`. Then the tests could define 
`LLDB_GET_INSTRUMENTATION_DATA` simply to `InstrumentationData(&g_serializer, 
&g_registry)` and the "real" code could do something like
```
#define LLDB_GET_INSTRUMENTATION_DATA 
lldb_private::repro::GetInstrumentationData()

InstrumentationData GetInstrumentationData() {
  auto *g = lldb_private::repro::Reproducer::Instance().GetGenerator();
  if (!g) return {};
  auto &p = g->GetOrCreate();
  return {p.GetSerializer(), p.GetRegistry();
}
```



Comment at: unittests/Utility/ReproducerInstrumentationTest.cpp:143-148
+EXPECT_EQ(g_a, 0);
+EXPECT_EQ(g_b, 0);
+EXPECT_EQ(g_c, 0);
+EXPECT_EQ(g_d, "");
+EXPECT_EQ(g_e, 0);
+EXPECT_EQ(g_f, false);

This seems rather pointless, as you've just set the variables to those values. 
What are you testing here? The compiler?



Comment at: unittests/Utility/ReproducerInstrumentationTest.cpp:384
+  llvm::raw_string_ostream os(str);
+  g_serializer = Serializer(os);
+

Make g_serializer `llvm:Optional`, and then you can avoid needing it be 
copyable (I think making the Serializer class copyable is a bad idea for the 
same reason than making raw_ostream copyable would be a bad idea)



Comment at: unittests/Utility/ReproducerInstrumentationTest.cpp:399-404
+  EXPECT_EQ(foo.g_a, 100);
+  EXPECT_EQ(foo.g_b, 200);
+  EXPECT_TRUE(Equals(foo.g_c, c));
+  EXPECT_EQ(foo.g_d, "bar");
+  EXPECT_TRUE(Equals(foo.g_e, e));
+  EXPECT_EQ(foo.g_f, true);

access variables via the class (InstrumentedFoo::g_a) to make it clear that 
they are static.


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

https://reviews.llvm.org/D56322



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

[Lldb-commits] [PATCH] D56322: [Reproducers] SBReproducer framework

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: unittests/Utility/ReproducerInstrumentationTest.cpp:399-404
+  EXPECT_EQ(foo.g_a, 100);
+  EXPECT_EQ(foo.g_b, 200);
+  EXPECT_TRUE(Equals(foo.g_c, c));
+  EXPECT_EQ(foo.g_d, "bar");
+  EXPECT_TRUE(Equals(foo.g_e, e));
+  EXPECT_EQ(foo.g_f, true);

labath wrote:
> access variables via the class (InstrumentedFoo::g_a) to make it clear that 
> they are static.
Actually, what do you think about this pattern:
- have the variables be member variables
- add a `Validate` method to the InstrumentedFoo class. This method would also 
be instrumented, and it would check that the members are set to the correct 
value
- call the method during recording and it will be automatically called during 
replay. Then all you need to do in the test is check that `Validate` was called.

I am hoping that this will make things more manageable once you have multiple 
instrumented objects floating around in the test. Also, this would implicitly 
test that the replay framework calls the member functions on the right object 
(e.g., right now this would succeed if the framework just created a fresh new 
object for each call).


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

https://reviews.llvm.org/D56322



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

[Lldb-commits] [PATCH] D57745: [x64] Process the B field of the REX prefix correctly for the PUSH and POP instructions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The change makes sense, but I'd just create a new test with the instructions 
you're interested in instead of modifying an existing one. The comment above 
the code claims this assembly was produced by clang, which is clearly not true 
after your modifications.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57745



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

[Lldb-commits] [PATCH] D57745: [x64] Process the B field of the REX prefix correctly for the PUSH and POP instructions

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov updated this revision to Diff 185296.
aleksandr.urakov added a comment.

Yes, you are right, thank you! I've updated the patch.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57745

Files:
  source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
  unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp


Index: unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
===
--- unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
+++ unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
@@ -1415,6 +1415,34 @@
   EXPECT_EQ(-8, regloc.GetOffset());
 }
 
+TEST_F(Testx86AssemblyInspectionEngine, TestPushRBPWithREX) {
+  UnwindPlan::Row::RegisterLocation regloc;
+  UnwindPlan::RowSP row_sp;
+
+  uint8_t data[] = {
+  0x40, 0x55, // pushq %rbp
+  0x90  // nop
+  };
+
+  AddressRange sample_range(0x1000, sizeof(data));
+  UnwindPlan unwind_plan(eRegisterKindLLDB);
+
+  std::unique_ptr engine64 = Getx86_64Inspector();
+  EXPECT_TRUE(engine64->GetNonCallSiteUnwindPlanFromAssembly(
+  data, sizeof(data), sample_range, unwind_plan));
+
+  row_sp = unwind_plan.GetRowForFunctionOffset(2);
+
+  EXPECT_EQ(2ull, row_sp->GetOffset());
+  EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_rsp);
+  EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
+  EXPECT_EQ(16, row_sp->GetCFAValue().GetOffset());
+
+  EXPECT_TRUE(row_sp->GetRegisterInfo(k_rbp, regloc));
+  EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+  EXPECT_EQ(-16, regloc.GetOffset());
+}
+
 TEST_F(Testx86AssemblyInspectionEngine, TestPushESI) {
   UnwindPlan::Row::RegisterLocation regloc;
   UnwindPlan::RowSP row_sp;
@@ -1913,6 +1941,32 @@
   EXPECT_FALSE(row_sp->GetRegisterInfo(k_ebp, regloc));
 }
 
+TEST_F(Testx86AssemblyInspectionEngine, TestPopRBPWithREX) {
+  UnwindPlan::Row::RegisterLocation regloc;
+  UnwindPlan::RowSP row_sp;
+  AddressRange sample_range;
+  UnwindPlan unwind_plan(eRegisterKindLLDB);
+  std::unique_ptr engine = Getx86_64Inspector();
+
+  uint8_t data[] = {
+  0x41, 0x55, // pushq %rbp
+  0x41, 0x5d, // popq %rbp
+  0x90// nop
+  };
+
+  sample_range = AddressRange(0x1000, sizeof(data));
+
+  EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
+  data, sizeof(data), sample_range, unwind_plan));
+
+  row_sp = unwind_plan.GetRowForFunctionOffset(4);
+  EXPECT_EQ(4ull, row_sp->GetOffset());
+  EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_rsp);
+  EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
+  EXPECT_EQ(8, row_sp->GetCFAValue().GetOffset());
+  EXPECT_FALSE(row_sp->GetRegisterInfo(k_rbp, regloc));
+}
+
 TEST_F(Testx86AssemblyInspectionEngine, TestPopESI) {
   UnwindPlan::Row::RegisterLocation regloc;
   UnwindPlan::RowSP row_sp;
Index: source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
===
--- source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
+++ source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
@@ -364,8 +364,8 @@
   uint8_t *p = m_cur_insn;
   int regno_prefix_bit = 0;
   // If we have a rex prefix byte, check to see if a B bit is set
-  if (m_wordsize == 8 && *p == 0x41) {
-regno_prefix_bit = 1 << 3;
+  if (m_wordsize == 8 && (*p & 0xfe) == 0x40) {
+regno_prefix_bit = (*p & 1) << 3;
 p++;
   }
   if (*p >= 0x50 && *p <= 0x57) {
@@ -562,8 +562,8 @@
   uint8_t *p = m_cur_insn;
   int regno_prefix_bit = 0;
   // If we have a rex prefix byte, check to see if a B bit is set
-  if (m_wordsize == 8 && *p == 0x41) {
-regno_prefix_bit = 1 << 3;
+  if (m_wordsize == 8 && (*p & 0xfe) == 0x40) {
+regno_prefix_bit = (*p & 1) << 3;
 p++;
   }
   if (*p >= 0x58 && *p <= 0x5f) {


Index: unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
===
--- unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
+++ unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
@@ -1415,6 +1415,34 @@
   EXPECT_EQ(-8, regloc.GetOffset());
 }
 
+TEST_F(Testx86AssemblyInspectionEngine, TestPushRBPWithREX) {
+  UnwindPlan::Row::RegisterLocation regloc;
+  UnwindPlan::RowSP row_sp;
+
+  uint8_t data[] = {
+  0x40, 0x55, // pushq %rbp
+  0x90  // nop
+  };
+
+  AddressRange sample_range(0x1000, sizeof(data));
+  UnwindPlan unwind_plan(eRegisterKindLLDB);
+
+  std::unique_ptr engine64 = Getx86_64Inspector();
+  EXPECT_TRUE(engine64->GetNonCallSiteUnwindPlanFromAssembly(
+  data, sizeof(data), sample_range, unwind_plan));
+
+  row_sp = unwind_plan.GetRowForFunctionOffset(2);
+
+  EXPECT_EQ(2ull, row_sp->GetOffset());
+  EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_rsp);
+  EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);

[Lldb-commits] [PATCH] D57750: [CMake] Don't add `cxx` to `LLDB_TEST_DEPS` if it doesn't exist.

[Dan Liew via Phabricator via lldb-commits]

delcypher created this revision.
delcypher added reviewers: sgraenitz, JDevlieghere.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.

If we forget to build `libcxx` then previously the CMake configure
would have errors like:

  CMake Error at
  
/Users/dan/data/dev/llvm/upstream_monorepo/master/llvm/llvm/cmake/modules/AddLLVM.cmake:1374
  (add_dependencies):
The dependency target "cxx" of target
  "check-lldb-tools-lldb-mi-data-inputs" does not exist.
  Call Stack (most recent call first):

/Users/dan/data/dev/llvm/upstream_monorepo/master/llvm/llvm/cmake/modules/AddLLVM.cmake:1426
(add_lit_target)
  
/Users/dan/data/dev/llvm/upstream_monorepo/master/llvm/lldb/lit/CMakeLists.txt:76
  (add_lit_testsuites)

To avoid error this check the `cxx` target exists before adding it to
`LLDB_TEST_DEPS`. If it doesn't exist emit a warning similar to the
code above.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57750

Files:
  lldb/CMakeLists.txt


Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -111,7 +111,11 @@
   message(WARNING "LLDB test suite requires libc++ in 
llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
 endif()
   else()
-list(APPEND LLDB_TEST_DEPS cxx)
+if (NOT TARGET cxx)
+  message(WARNING "LLDB test suite requires libc++")
+else()
+  list(APPEND LLDB_TEST_DEPS cxx)
+endif()
   endif()
 endif()
   endif()


Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -111,7 +111,11 @@
   message(WARNING "LLDB test suite requires libc++ in llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
 endif()
   else()
-list(APPEND LLDB_TEST_DEPS cxx)
+if (NOT TARGET cxx)
+  message(WARNING "LLDB test suite requires libc++")
+else()
+  list(APPEND LLDB_TEST_DEPS cxx)
+endif()
   endif()
 endif()
   endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57750: [CMake] Don't add `cxx` to `LLDB_TEST_DEPS` if it doesn't exist.

[Dan Liew via Phabricator via lldb-commits]

delcypher added a comment.

Wow phabricator has made a right mess of displaying this patch. It's easier to 
look at as.

  diff --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
  index fdd84a6e7b7..6b26adb3383 100644
  --- a/lldb/CMakeLists.txt
  +++ b/lldb/CMakeLists.txt
  @@ -111,7 +111,11 @@ if(LLDB_INCLUDE_TESTS)
 message(WARNING "LLDB test suite requires libc++ in 
llvm/projects/libcxx or an existing build symlinked to ${cxx_dir}")
   endif()
 else()
  -list(APPEND LLDB_TEST_DEPS cxx)
  +if (NOT TARGET cxx)
  +  message(WARNING "LLDB test suite requires libc++")
  +else()
  +  list(APPEND LLDB_TEST_DEPS cxx)
  +endif()
 endif()
   endif()
 endif()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57750



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

[Lldb-commits] [lldb] r353174 - Update Xcode project after r353047

[Stefan Granitz via lldb-commits]

Author: stefan.graenitz
Date: Tue Feb  5 06:41:26 2019
New Revision: 353174

URL: http://llvm.org/viewvc/llvm-project?rev=353174&view=rev
Log:
Update Xcode project after r353047

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=353174&r1=353173&r2=353174&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Feb  5 06:41:26 2019
@@ -1890,8 +1890,8 @@
4984BA171B979C08008658D4 /* ExpressionVariable.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
ExpressionVariable.h; path = include/lldb/Expression/ExpressionVariable.h; 
sourceTree = ""; };
260C6EA213011581005E16B0 /* File.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = File.cpp; sourceTree = ""; };
260C6EA013011578005E16B0 /* File.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
File.h; path = include/lldb/Host/File.h; sourceTree = ""; };
-   3FDFDDBC199C3A06009756A7 /* FileAction.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = FileAction.cpp; path = source/Target/FileAction.cpp; sourceTree = 
""; };
-   3FDFD6C3199C396E009756A7 /* FileAction.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
FileAction.h; path = include/lldb/Target/FileAction.h; sourceTree = ""; 
};
+   3FDFDDBC199C3A06009756A7 /* FileAction.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = FileAction.cpp; path = FileAction.cpp; sourceTree = ""; };
+   3FDFD6C3199C396E009756A7 /* FileAction.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
FileAction.h; path = include/lldb/Host/FileAction.h; sourceTree = ""; };
3FDFDDBE199D345E009756A7 /* FileCache.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = FileCache.cpp; path = source/Host/common/FileCache.cpp; sourceTree = 
""; };
3FDFDDC0199D34E2009756A7 /* FileCache.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = FileCache.h; path 
= include/lldb/Host/FileCache.h; sourceTree = ""; };
DD8F277D22011CC9004ED75B /* FileCollector.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = FileCollector.cpp; path = source/Utility/FileCollector.cpp; sourceTree = 
""; };
@@ -2527,14 +2527,14 @@
2618EE601315B29C001D6D71 /* ProcessGDBRemote.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
ProcessGDBRemote.h; sourceTree = ""; };
2618EE611315B29C001D6D71 /* ProcessGDBRemoteLog.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = ProcessGDBRemoteLog.cpp; sourceTree = ""; };
2618EE621315B29C001D6D71 /* ProcessGDBRemoteLog.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
ProcessGDBRemoteLog.h; sourceTree = ""; };
-   233B007B1960C9E60090E598 /* ProcessInfo.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = ProcessInfo.cpp; path = source/Target/ProcessInfo.cpp; sourceTree = 
""; };
-   233B007A1960A0440090E598 /* ProcessInfo.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ProcessInfo.h; 
path = include/lldb/Target/ProcessInfo.h; sourceTree = ""; };
+   233B007B1960C9E60090E598 /* ProcessInfo.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = ProcessInfo.cpp; path = ProcessInfo.cpp; sourceTree = ""; };
+   233B007A1960A0440090E598 /* ProcessInfo.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ProcessInfo.h; 
path = include/lldb/Host/ProcessInfo.h; sourceTree = ""; };
2642FBAA13D003B400ED6808 /* ProcessKDP.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = ProcessKDP.cpp; sourceTree = ""; };
2642FBAB13D003B400ED6808 /* ProcessKDP.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
ProcessKDP.h; sourceTree = ""; };
2642FBAC13D003B400ED6808 /* ProcessKDPLog.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = ProcessKDPLog.cpp; sourceTree = ""; };
2642FBAD13D003B400ED6808 /* ProcessKDPLog.h */ = {isa = 
PB

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, lemo, amccarth.

The reason this wasn't working was that ProcessMinidump was creating odd
object-file-less modules, and SymbolFileBreakpad required the module to
have an associated object file because it needed to get its base
address.

This fixes that by introducing a PlaceholderObjectFile to serve as a
dummy object file. The general idea for this is taken from D55142 
, but
I've reworked it a bit to avoid the need for the PlaceholderModule
class. Now that we have an object file, our modules are sufficiently
similar to regular modules that we can use the regular Module class
almost out of the box -- the only thing I needed to tweak was the
Module::CreateModuleFromObjectFile functon to set the module's FileSpec
in addition to it's architecture. This wasn't needed for ObjectFileJIT
(the other user of CreateModuleFromObjectFile), but it shouldn't hurt it
either, and the change seems like a straightforward extension of this
function.


https://reviews.llvm.org/D57751

Files:
  include/lldb/Core/Module.h
  lit/Minidump/Inputs/linux-x86_64.dmp
  lit/Minidump/Inputs/linux-x86_64.syms
  lit/Minidump/breakpad-symbols.test
  source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -41,51 +41,80 @@
 using namespace lldb_private;
 using namespace minidump;
 
-//--
-/// A placeholder module used for minidumps, where the original
-/// object files may not be available (so we can't parse the object
-/// files to extract the set of sections/segments)
-///
-/// This placeholder module has a single synthetic section (.module_image)
-/// which represents the module memory range covering the whole module.
-//--
-class PlaceholderModule : public Module {
+namespace {
+
+/// A minimal ObjectFile implementation providing a dummy object file for the
+/// cases when the real module binary is not available. This allows the module
+/// to show up in "image list" and symbols to be added to it.
+class PlaceholderObjectFile : public ObjectFile {
 public:
-  PlaceholderModule(const ModuleSpec &module_spec) :
-Module(module_spec.GetFileSpec(), module_spec.GetArchitecture()) {
-if (module_spec.GetUUID().IsValid())
-  SetUUID(module_spec.GetUUID());
-  }
-
-  // Creates a synthetic module section covering the whole module image (and
-  // sets the section load address as well)
-  void CreateImageSection(const MinidumpModule *module, Target& target) {
-const ConstString section_name(".module_image");
-lldb::SectionSP section_sp(new Section(
-shared_from_this(), // Module to which this section belongs.
-nullptr,// ObjectFile
-0,  // Section ID.
-section_name,   // Section name.
-eSectionTypeContainer,  // Section type.
-module->base_of_image,  // VM address.
-module->size_of_image,  // VM size in bytes of this section.
-0,  // Offset of this section in the file.
-module->size_of_image,  // Size of the section as found in the file.
-12, // Alignment of the section (log2)
-0,  // Flags for this section.
-1));// Number of host bytes per target byte
-section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);
-GetSectionList()->AddSection(section_sp);
-target.GetSectionLoadList().SetSectionLoadAddress(
-section_sp, module->base_of_image);
+  PlaceholderObjectFile(const lldb::ModuleSP &module_sp,
+const ModuleSpec &module_spec, lldb::offset_t base,
+lldb::offset_t size)
+  : ObjectFile(module_sp, &module_spec.GetFileSpec(), /*file_offset*/ 0,
+   /*length*/ 0, /*data_sp*/ nullptr, /*data_offset*/ 0),
+m_arch(module_spec.GetArchitecture()), m_uuid(module_spec.GetUUID()),
+m_base(base), m_size(size) {
+m_symtab_ap = llvm::make_unique(this);
+  }
+
+  ConstString GetPluginName() override { return ConstString("placeholder"); }
+  uint32_t GetPluginVersion() override { return 1; }
+  bool ParseHeader() override { return true; }
+  Type CalculateType() override { return eTypeUnknown; }
+  Strata CalculateStrata() override { return eStrataUnknown; }
+  void Dump(Stream *s) override {}
+  uint32_t GetDependentModules(FileSpecList &file_list) override { return 0; }
+  bool IsExecutable() const override { return false; }
+  ArchSpec GetArchitecture() override { return m_arch; }
+  Symtab *GetSymtab() override { return m_symtab_ap.get(); }
+ 

[Lldb-commits] [PATCH] D57745: [x64] Process the B field of the REX prefix correctly for the PUSH and POP instructions

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good to me.

I don't expect Jason to have any problems with this, but it might be nice to 
wait a day or two to give him a chance to prove me wrong. :)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57745



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

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Greg, what do you think about the new approach in this patch?


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

https://reviews.llvm.org/D56595



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

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I like the way you did the compile units and the line tables and support file 
list. It would be nice to change this to do things more lazily. Right now we 
are parsing all compile unit data into CompUnitData structures and then passing 
their info along to the lldb_private::CompileUnit when we need to. We can be 
more lazy, see inlined comments.




Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:279
+  llvm::Optional func;
+  for (LineIterator It(*m_obj_file, Record::Func), End(*m_obj_file); It != End;
+   ++It) {

Do we need to iterate over the file multiple times here? We do it once here, 
and then once on line 260. 



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:281-300
+if (auto record = FuncRecord::parse(*It)) {
+  id = It.GetBookmark();
+  func = record;
+} else if (!id)
+  continue;
+else if (auto record = LineRecord::parse(*It)) {
+  FileSpec spec;

Seems like we should just populate the m_compile_units data with address range 
to character file offset here? When we are asked to create a compile unit, we 
do this work by going to the "lldb_private::CompileUnit::GetID()" which will 
return the file offset and we just head there and start parsing?



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:314
 
 CompUnitSP SymbolFileBreakpad::ParseCompileUnitAtIndex(uint32_t index) {
+  CompUnitSP cu = m_comp_units.GetEntryRef(index).data.GetCompUnit();

This seems like where we would do the heavy parsing code that are in the 
initialize object function. .Get the character file offset from m_compile_units 
and just parse what we need here? It will cause less work for us in the 
initialize object call then and we can be lazier



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:326
 bool SymbolFileBreakpad::ParseLineTable(CompileUnit &comp_unit) {
-  // TODO
-  return 0;
+  CompUnitData &data = m_comp_units.GetEntryRef(comp_unit.GetID()).data;
+  comp_unit.SetLineTable(data.ReleaseLineTable(*m_obj_file, 
m_files).release());

From the compile unit, if GetID() returns the character file offset to the FUNC 
or first LINE, then we don't need the preparsed CompUnitData? We can just parse 
the line table here if and only if we need to



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:332
+   FileSpecList &support_files) {
+  CompUnitData &data = m_comp_units.GetEntryRef(comp_unit.GetID()).data;
+  support_files = data.ReleaseSupportFiles(*m_obj_file, m_files);

Ditto above



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h:151-154
+  struct Bookmark {
+uint32_t section;
+size_t offset;
+  };

Why do we need more than just a file character offset here?



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h:182-188
+CompUnitData &operator=(const CompUnitData &rhs) {
+  m_comp_unit = rhs.m_comp_unit;
+  m_bookmark = rhs.m_bookmark;
+  m_support_files.reset();
+  m_line_table_up.reset();
+  return *this;
+}

Seems like if we just pick the file character offset of the function or the 
function's line entries as the lldb_private::CompileUnit user ID 
(lldb::user_id_t) then we don't really need this class? We just create the 
compile unit as a lldb_private::CompileUnit and our symbol file parser will 
fill in the rest? Any reason we need this CompUnitData class?



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h:212
+
+  using CompUnitMap = RangeDataVector;
+

Could this just be:

```
using CompUnitMap = RangeDataVector;
```
Where offset_t is the character fie offset for the first line of the FUNC 
entry? Any reason to use CompUnitData instead of just creating 
lldb_private::CompileUnit objects?



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h:214
+
+  std::vector m_files;
+  CompUnitMap m_comp_units;

Use FileSpecList?


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

https://reviews.llvm.org/D56595



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

Re: [Lldb-commits] [PATCH] D56904: [NativePDB] Process virtual bases in the correct order

[Zachary Turner via lldb-commits]

Yes, clang tries to find the visual studio installation, because that is
how it can find headers and libs.  Can you make a separate patch with the
changes to build.py and upload that?

On Tue, Feb 5, 2019 at 2:34 AM Aleksandr Urakov via Phabricator <
revi...@reviews.llvm.org> wrote:

> aleksandr.urakov added a comment.
>
> Thanks for the reply! It seems that after your hint I've figured it out.
> It's the environment variable `VCINSTALLDIR`. I've added it to the
> `defaultenv` list of the `_get_visual_studio_environment` function and the
> test passes now. What do you think about this solution?
>
> And do I understand right, it means, that clang-cl needs MSVC installation
> to work with these types?
>
>
> Repository:
>   rLLDB LLDB
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D56904/new/
>
> https://reviews.llvm.org/D56904
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57750: [CMake] Don't add `cxx` to `LLDB_TEST_DEPS` if it doesn't exist.

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Not sure if we want to change the behavior this way. The purpose of this code 
was to make the dependency to libc++ explicit because it's specific to macOS 
and it's missed quite often. In my experience warnings like the proposed one 
are hard to recognize in the load of CMake output.

Note that you can configure with `LLDB_INCLUDE_TESTS=OFF` to avoid the libc++ 
dependency. We could have an explicit error message that points this out. What 
do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57750



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

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for the review Greg. See my responses inline. I'm going to try 
incorporating the changes tomorrow.




Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:279
+  llvm::Optional func;
+  for (LineIterator It(*m_obj_file, Record::Func), End(*m_obj_file); It != End;
+   ++It) {

clayborg wrote:
> Do we need to iterate over the file multiple times here? We do it once here, 
> and then once on line 260. 
The two loops iterate over different parts of the file. The first one goes 
through the FILE records, and this one does the FUNC records. So the iteration 
here is efficient because we already know where different kinds of records are 
located in the file.

(of course, to figure out where these records are located, we've had to go 
through it once already (in ObjectFileBreakpad), so we still have to make two 
passes over this data in general. However, that is pretty much unavoidable if 
we want to do lazy (i.e. random access) into the file as it doesn't have any 
kind of index to start with.)



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:281-300
+if (auto record = FuncRecord::parse(*It)) {
+  id = It.GetBookmark();
+  func = record;
+} else if (!id)
+  continue;
+else if (auto record = LineRecord::parse(*It)) {
+  FileSpec spec;

clayborg wrote:
> Seems like we should just populate the m_compile_units data with address 
> range to character file offset here? When we are asked to create a compile 
> unit, we do this work by going to the "lldb_private::CompileUnit::GetID()" 
> which will return the file offset and we just head there and start parsing?
I think I could avoid creating the CompileUnit object here. However, I will 
still need to do the parsing here, as I will need to figure out the number of 
compile units first (best I might be able to achieve is to delay this until 
GetNumCompileUnits() time).



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:326
 bool SymbolFileBreakpad::ParseLineTable(CompileUnit &comp_unit) {
-  // TODO
-  return 0;
+  CompUnitData &data = m_comp_units.GetEntryRef(comp_unit.GetID()).data;
+  comp_unit.SetLineTable(data.ReleaseLineTable(*m_obj_file, 
m_files).release());

clayborg wrote:
> From the compile unit, if GetID() returns the character file offset to the 
> FUNC or first LINE, then we don't need the preparsed CompUnitData? We can 
> just parse the line table here if and only if we need to
That is pretty much what happens here. CompUnitData construct the line table 
(almost) lazily. It doesn't preparse. The reason I have this indirection, is 
that the creation of line tables is coupled with the creation of the support 
file list:
- in order to build the line table, I (obviously) need to go through the LINE 
records
- however, I also need to go through the LINE records in order to build the CU 
file list, because I need to know what files are actually used in this "CU"

It seemed like a good idea to me to avoid parsing the LINE records twice. So 
what I've done is that on the first call to 
(ReleaseLineTable|ReleaseSupportFiles), CompUnitData will parse both things. 
Then, the second call will return the already parsed data. That seems like a 
good tradeoff to me as these two items are generally used together (one is 
fairly useless without the other). 



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h:151-154
+  struct Bookmark {
+uint32_t section;
+size_t offset;
+  };

clayborg wrote:
> Why do we need more than just a file character offset here?
That's because ObjectFileBreakpad breaks down the file into sections (so all 
FILE records would go into one section, PUBLIC records into another, etc.). 
This means that we don't need any "bookmarks" when we want to jump straight to 
the PUBLIC records for instance, but it does mean we need two coordinates 
(section, offset) when we want to jump to a specific record within a section.



Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h:182-188
+CompUnitData &operator=(const CompUnitData &rhs) {
+  m_comp_unit = rhs.m_comp_unit;
+  m_bookmark = rhs.m_bookmark;
+  m_support_files.reset();
+  m_line_table_up.reset();
+  return *this;
+}

clayborg wrote:
> Seems like if we just pick the file character offset of the function or the 
> function's line entries as the lldb_private::CompileUnit user ID 
> (lldb::user_id_t) then we don't really need this class? We just create the 
> compile unit as a lldb_private::CompileUnit and our symbol file parser will 
> fill in the rest? Any reason we need this CompUnitData class?
I'd like the keep to enable the CompUnitData for conjugated parsing of line 
tables and support files. I think I can 

[Lldb-commits] [PATCH] D57402: build: remove custom variables

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.
Herald added a project: LLDB.

Hey sorry for the late reply, didn't see this earlier. Personally, I think the 
move away from the llvm-config approach is good, but I have no strong opinion 
about the solution.

Pro Zachary's proposal:

- It's good to have fewer ways to do things. The docs describe how to use 
`LLVM_DIR` to build against LLVM. If that's sufficient LLDB should do the same. 
https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project
- Clang also deprecated llvm-config, but has no `CLANG_PATH_TO_XY`. It's 
usually good advice to follow their model. 
https://github.com/llvm/llvm-project/blob/master/clang/CMakeLists.txt#L17

Pro Alex's code:

- It's a good match for out downstream code 
https://github.com/apple/swift-lldb/blob/upstream-with-swift/cmake/modules/LLDBStandalone.cmake
- It would allow to have individual build trees for LLVM and Clang right? I 
don't mind, but someone might?

Has anyone actually tried passing `Clang_DIR`?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57402



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

[Lldb-commits] [lldb] r353191 - [Obj-C] Fix undefined behaviour(s) in the new NSTaggedDate formatter.

[Davide Italiano via lldb-commits]

Author: davide
Date: Tue Feb  5 09:30:53 2019
New Revision: 353191

URL: http://llvm.org/viewvc/llvm-project?rev=353191&view=rev
Log:
[Obj-C] Fix undefined behaviour(s) in the new NSTaggedDate formatter.

Type punning through a union -> no good.
double to uint64 to double again -> no good either.

The nice side effect, other than silencing the sanitizer bot
is that it fixes the formatting of some dates, e.g. Jan 1st 1970.



Modified:
lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp

Modified: lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp?rev=353191&r1=353190&r2=353191&view=diff
==
--- lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp (original)
+++ lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp Tue Feb  5 09:30:53 2019
@@ -27,6 +27,7 @@
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/APInt.h"
+#include "llvm/ADT/bit.h"
 
 #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h"
 
@@ -749,24 +750,18 @@ bool lldb_private::formatters::NSURLSumm
 ///   distantFuture, except within about 1e-25 second of the reference date.
 const int TAGGED_DATE_EXPONENT_BIAS = 0x3ef;
 
-typedef union {
-  struct {
-uint64_t fraction:52;  // unsigned
-uint64_t exponent:11;  // signed
-uint64_t sign:1;
-  } repr;
-  uint64_t i;
-  double d;
-} DoubleBits;
-typedef union {
-  struct {
-uint64_t fraction:52;  // unsigned
-uint64_t exponent:7;   // signed
-uint64_t sign:1;
-uint64_t unused:4;  // placeholder for pointer tag bits
-  } repr;
-  uint64_t i;
-} TaggedDoubleBits;
+struct DoubleBits {
+  uint64_t fraction : 52; // unsigned
+  uint64_t exponent : 11; // signed
+  uint64_t sign : 1;
+};
+
+struct TaggedDoubleBits {
+  uint64_t fraction : 52; // unsigned
+  uint64_t exponent : 7;  // signed
+  uint64_t sign : 1;
+  uint64_t unused : 4; // placeholder for pointer tag bits
+};
 
 static uint64_t decodeExponent(uint64_t exp) {
   // Tagged exponent field is 7-bit signed. Sign-extend the value to 64 bits
@@ -774,24 +769,24 @@ static uint64_t decodeExponent(uint64_t
   return llvm::SignExtend64<7>(exp) + TAGGED_DATE_EXPONENT_BIAS;
 }
 
-static uint64_t decodeTaggedTimeInterval(uint64_t encodedTimeInterval) {
+static double decodeTaggedTimeInterval(uint64_t encodedTimeInterval) {
   if (encodedTimeInterval == 0)
 return 0.0;
   if (encodedTimeInterval == std::numeric_limits::max())
 return (uint64_t)-0.0;
 
-  TaggedDoubleBits encodedBits = {};
-  encodedBits.i = encodedTimeInterval;
-  DoubleBits decodedBits;
+  TaggedDoubleBits encodedBits =
+  llvm::bit_cast(encodedTimeInterval);
+  assert(encodedBits.unused == 0);
 
   // Sign and fraction are represented exactly.
   // Exponent is encoded.
-  assert(encodedBits.repr.unused == 0);
-  decodedBits.repr.sign = encodedBits.repr.sign;
-  decodedBits.repr.fraction = encodedBits.repr.fraction;
-  decodedBits.repr.exponent = decodeExponent(encodedBits.repr.exponent);
+  DoubleBits decodedBits;
+  decodedBits.sign = encodedBits.sign;
+  decodedBits.fraction = encodedBits.fraction;
+  decodedBits.exponent = decodeExponent(encodedBits.exponent);
 
-  return decodedBits.d;
+  return llvm::bit_cast(decodedBits);
 }
 
 bool lldb_private::formatters::NSDateSummaryProvider(
@@ -868,7 +863,8 @@ bool lldb_private::formatters::NSDateSum
 
   // Accomodate for the __NSTaggedDate format introduced in Foundation 1600.
   if (class_name == g___NSTaggedDate) {
-auto *runtime = 
llvm::dyn_cast_or_null(process_sp->GetObjCLanguageRuntime());
+auto *runtime = llvm::dyn_cast_or_null(
+process_sp->GetObjCLanguageRuntime());
 if (runtime && runtime->GetFoundationVersion() >= 1600)
   date_value = decodeTaggedTimeInterval(value_bits << 4);
   }


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

Re: [Lldb-commits] [lldb] r353191 - [Obj-C] Fix undefined behaviour(s) in the new NSTaggedDate formatter.

[Davide Italiano via lldb-commits]

Shafik, Zachary, I think you reported this a bit ago, but now I got
around to fix it (also because it was breaking something real).

Thanks,

--
Davide

On Tue, Feb 5, 2019 at 9:30 AM Davide Italiano via lldb-commits
 wrote:
>
> Author: davide
> Date: Tue Feb  5 09:30:53 2019
> New Revision: 353191
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353191&view=rev
> Log:
> [Obj-C] Fix undefined behaviour(s) in the new NSTaggedDate formatter.
>
> Type punning through a union -> no good.
> double to uint64 to double again -> no good either.
>
> The nice side effect, other than silencing the sanitizer bot
> is that it fixes the formatting of some dates, e.g. Jan 1st 1970.
>
> 
>
> Modified:
> lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp
>
> Modified: lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp?rev=353191&r1=353190&r2=353191&view=diff
> ==
> --- lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp (original)
> +++ lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp Tue Feb  5 09:30:53 2019
> @@ -27,6 +27,7 @@
>  #include "lldb/Utility/Stream.h"
>
>  #include "llvm/ADT/APInt.h"
> +#include "llvm/ADT/bit.h"
>
>  #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h"
>
> @@ -749,24 +750,18 @@ bool lldb_private::formatters::NSURLSumm
>  ///   distantFuture, except within about 1e-25 second of the reference date.
>  const int TAGGED_DATE_EXPONENT_BIAS = 0x3ef;
>
> -typedef union {
> -  struct {
> -uint64_t fraction:52;  // unsigned
> -uint64_t exponent:11;  // signed
> -uint64_t sign:1;
> -  } repr;
> -  uint64_t i;
> -  double d;
> -} DoubleBits;
> -typedef union {
> -  struct {
> -uint64_t fraction:52;  // unsigned
> -uint64_t exponent:7;   // signed
> -uint64_t sign:1;
> -uint64_t unused:4;  // placeholder for pointer tag bits
> -  } repr;
> -  uint64_t i;
> -} TaggedDoubleBits;
> +struct DoubleBits {
> +  uint64_t fraction : 52; // unsigned
> +  uint64_t exponent : 11; // signed
> +  uint64_t sign : 1;
> +};
> +
> +struct TaggedDoubleBits {
> +  uint64_t fraction : 52; // unsigned
> +  uint64_t exponent : 7;  // signed
> +  uint64_t sign : 1;
> +  uint64_t unused : 4; // placeholder for pointer tag bits
> +};
>
>  static uint64_t decodeExponent(uint64_t exp) {
>// Tagged exponent field is 7-bit signed. Sign-extend the value to 64 bits
> @@ -774,24 +769,24 @@ static uint64_t decodeExponent(uint64_t
>return llvm::SignExtend64<7>(exp) + TAGGED_DATE_EXPONENT_BIAS;
>  }
>
> -static uint64_t decodeTaggedTimeInterval(uint64_t encodedTimeInterval) {
> +static double decodeTaggedTimeInterval(uint64_t encodedTimeInterval) {
>if (encodedTimeInterval == 0)
>  return 0.0;
>if (encodedTimeInterval == std::numeric_limits::max())
>  return (uint64_t)-0.0;
>
> -  TaggedDoubleBits encodedBits = {};
> -  encodedBits.i = encodedTimeInterval;
> -  DoubleBits decodedBits;
> +  TaggedDoubleBits encodedBits =
> +  llvm::bit_cast(encodedTimeInterval);
> +  assert(encodedBits.unused == 0);
>
>// Sign and fraction are represented exactly.
>// Exponent is encoded.
> -  assert(encodedBits.repr.unused == 0);
> -  decodedBits.repr.sign = encodedBits.repr.sign;
> -  decodedBits.repr.fraction = encodedBits.repr.fraction;
> -  decodedBits.repr.exponent = decodeExponent(encodedBits.repr.exponent);
> +  DoubleBits decodedBits;
> +  decodedBits.sign = encodedBits.sign;
> +  decodedBits.fraction = encodedBits.fraction;
> +  decodedBits.exponent = decodeExponent(encodedBits.exponent);
>
> -  return decodedBits.d;
> +  return llvm::bit_cast(decodedBits);
>  }
>
>  bool lldb_private::formatters::NSDateSummaryProvider(
> @@ -868,7 +863,8 @@ bool lldb_private::formatters::NSDateSum
>
>// Accomodate for the __NSTaggedDate format introduced in Foundation 1600.
>if (class_name == g___NSTaggedDate) {
> -auto *runtime = 
> llvm::dyn_cast_or_null(process_sp->GetObjCLanguageRuntime());
> +auto *runtime = llvm::dyn_cast_or_null(
> +process_sp->GetObjCLanguageRuntime());
>  if (runtime && runtime->GetFoundationVersion() >= 1600)
>date_value = decodeTaggedTimeInterval(value_bits << 4);
>}
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r353191 - [Obj-C] Fix undefined behaviour(s) in the new NSTaggedDate formatter.

[Davide Italiano via lldb-commits]

To clarify, for those listening at home, it was the second UB
(overflow) that exposed a real bug.
Type punning was innocuous here, but given it's technically UB, I
fixed it anyway.

On Tue, Feb 5, 2019 at 9:33 AM Davide Italiano  wrote:
>
> Shafik, Zachary, I think you reported this a bit ago, but now I got
> around to fix it (also because it was breaking something real).
>
> Thanks,
>
> --
> Davide
>
> On Tue, Feb 5, 2019 at 9:30 AM Davide Italiano via lldb-commits
>  wrote:
> >
> > Author: davide
> > Date: Tue Feb  5 09:30:53 2019
> > New Revision: 353191
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=353191&view=rev
> > Log:
> > [Obj-C] Fix undefined behaviour(s) in the new NSTaggedDate formatter.
> >
> > Type punning through a union -> no good.
> > double to uint64 to double again -> no good either.
> >
> > The nice side effect, other than silencing the sanitizer bot
> > is that it fixes the formatting of some dates, e.g. Jan 1st 1970.
> >
> > 
> >
> > Modified:
> > lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp
> >
> > Modified: lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp?rev=353191&r1=353190&r2=353191&view=diff
> > ==
> > --- lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp (original)
> > +++ lldb/trunk/source/Plugins/Language/ObjC/Cocoa.cpp Tue Feb  5 09:30:53 
> > 2019
> > @@ -27,6 +27,7 @@
> >  #include "lldb/Utility/Stream.h"
> >
> >  #include "llvm/ADT/APInt.h"
> > +#include "llvm/ADT/bit.h"
> >
> >  #include "Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h"
> >
> > @@ -749,24 +750,18 @@ bool lldb_private::formatters::NSURLSumm
> >  ///   distantFuture, except within about 1e-25 second of the reference 
> > date.
> >  const int TAGGED_DATE_EXPONENT_BIAS = 0x3ef;
> >
> > -typedef union {
> > -  struct {
> > -uint64_t fraction:52;  // unsigned
> > -uint64_t exponent:11;  // signed
> > -uint64_t sign:1;
> > -  } repr;
> > -  uint64_t i;
> > -  double d;
> > -} DoubleBits;
> > -typedef union {
> > -  struct {
> > -uint64_t fraction:52;  // unsigned
> > -uint64_t exponent:7;   // signed
> > -uint64_t sign:1;
> > -uint64_t unused:4;  // placeholder for pointer tag bits
> > -  } repr;
> > -  uint64_t i;
> > -} TaggedDoubleBits;
> > +struct DoubleBits {
> > +  uint64_t fraction : 52; // unsigned
> > +  uint64_t exponent : 11; // signed
> > +  uint64_t sign : 1;
> > +};
> > +
> > +struct TaggedDoubleBits {
> > +  uint64_t fraction : 52; // unsigned
> > +  uint64_t exponent : 7;  // signed
> > +  uint64_t sign : 1;
> > +  uint64_t unused : 4; // placeholder for pointer tag bits
> > +};
> >
> >  static uint64_t decodeExponent(uint64_t exp) {
> >// Tagged exponent field is 7-bit signed. Sign-extend the value to 64 
> > bits
> > @@ -774,24 +769,24 @@ static uint64_t decodeExponent(uint64_t
> >return llvm::SignExtend64<7>(exp) + TAGGED_DATE_EXPONENT_BIAS;
> >  }
> >
> > -static uint64_t decodeTaggedTimeInterval(uint64_t encodedTimeInterval) {
> > +static double decodeTaggedTimeInterval(uint64_t encodedTimeInterval) {
> >if (encodedTimeInterval == 0)
> >  return 0.0;
> >if (encodedTimeInterval == std::numeric_limits::max())
> >  return (uint64_t)-0.0;
> >
> > -  TaggedDoubleBits encodedBits = {};
> > -  encodedBits.i = encodedTimeInterval;
> > -  DoubleBits decodedBits;
> > +  TaggedDoubleBits encodedBits =
> > +  llvm::bit_cast(encodedTimeInterval);
> > +  assert(encodedBits.unused == 0);
> >
> >// Sign and fraction are represented exactly.
> >// Exponent is encoded.
> > -  assert(encodedBits.repr.unused == 0);
> > -  decodedBits.repr.sign = encodedBits.repr.sign;
> > -  decodedBits.repr.fraction = encodedBits.repr.fraction;
> > -  decodedBits.repr.exponent = decodeExponent(encodedBits.repr.exponent);
> > +  DoubleBits decodedBits;
> > +  decodedBits.sign = encodedBits.sign;
> > +  decodedBits.fraction = encodedBits.fraction;
> > +  decodedBits.exponent = decodeExponent(encodedBits.exponent);
> >
> > -  return decodedBits.d;
> > +  return llvm::bit_cast(decodedBits);
> >  }
> >
> >  bool lldb_private::formatters::NSDateSummaryProvider(
> > @@ -868,7 +863,8 @@ bool lldb_private::formatters::NSDateSum
> >
> >// Accomodate for the __NSTaggedDate format introduced in Foundation 
> > 1600.
> >if (class_name == g___NSTaggedDate) {
> > -auto *runtime = 
> > llvm::dyn_cast_or_null(process_sp->GetObjCLanguageRuntime());
> > +auto *runtime = llvm::dyn_cast_or_null(
> > +process_sp->GetObjCLanguageRuntime());
> >  if (runtime && runtime->GetFoundationVersion() >= 1600)
> >date_value = decodeTaggedTimeInterval(value_bits << 4);
> >}
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-c

[Lldb-commits] [lldb] r353195 - [Reproducers] Instrumentation Framework: Serialization

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb  5 10:46:36 2019
New Revision: 353195

URL: http://llvm.org/viewvc/llvm-project?rev=353195&view=rev
Log:
[Reproducers] Instrumentation Framework: Serialization

This is the is serialization/deserialization part of the reproducer
instrumentation framework.

For all the details refer to the RFC on the mailing list:
http://lists.llvm.org/pipermail/lldb-dev/2019-January/014530.html

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

Added:
lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
lldb/trunk/source/Utility/ReproducerInstrumentation.cpp
lldb/trunk/unittests/Utility/ReproducerInstrumentationTest.cpp
Modified:
lldb/trunk/source/Utility/CMakeLists.txt
lldb/trunk/unittests/Utility/CMakeLists.txt

Added: lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h?rev=353195&view=auto
==
--- lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h (added)
+++ lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h Tue Feb  5 
10:46:36 2019
@@ -0,0 +1,302 @@
+//===-- ReproducerInstrumentation.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UTILITY_REPRODUCER_INSTRUMENTATION_H
+#define LLDB_UTILITY_REPRODUCER_INSTRUMENTATION_H
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Logging.h"
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorHandling.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+namespace repro {
+
+/// Mapping between serialized indices and their corresponding objects.
+///
+/// This class is used during replay to map indices back to in-memory objects.
+///
+/// When objects are constructed, they are added to this mapping using
+/// AddObjectForIndex.
+///
+/// When an object is passed to a function, its index is deserialized and
+/// AddObjectForIndex returns the corresponding object. If there is no object
+/// for the given index, a nullptr is returend. The latter is valid when custom
+/// replay code is in place and the actual object is ignored.
+class IndexToObject {
+public:
+  /// Returns an object as a pointer for the given index or nullptr if not
+  /// present in the map.
+  template  T *GetObjectForIndex(unsigned idx) {
+assert(idx != 0 && "Cannot get object for sentinel");
+void *object = GetObjectForIndexImpl(idx);
+return static_cast(object);
+  }
+
+  /// Adds a pointer to an object to the mapping for the given index.
+  template  void AddObjectForIndex(unsigned idx, T *object) {
+AddObjectForIndexImpl(
+idx, static_cast(
+ const_cast::type *>(object)));
+  }
+
+  /// Adds a reference to an object to the mapping for the given index.
+  template  void AddObjectForIndex(unsigned idx, T &object) {
+AddObjectForIndexImpl(
+idx, static_cast(
+ const_cast::type *>(&object)));
+  }
+
+private:
+  /// Helper method that does the actual lookup. The void* result is later cast
+  /// by the caller.
+  void *GetObjectForIndexImpl(unsigned idx);
+
+  /// Helper method that does the actual insertion.
+  void AddObjectForIndexImpl(unsigned idx, void *object);
+
+  /// Keeps a mapping between indices and their corresponding object.
+  llvm::DenseMap m_mapping;
+};
+
+/// We need to differentiate between pointers to fundamental and
+/// non-fundamental types. See the corresponding Deserializer::Read method
+/// for the reason why.
+struct PointerTag {};
+struct ReferenceTag {};
+struct ValueTag {};
+struct FundamentalPointerTag {};
+struct FundamentalReferenceTag {};
+
+/// Return the deserialization tag for the given type T.
+template  struct serializer_tag { typedef ValueTag type; };
+template  struct serializer_tag {
+  typedef
+  typename std::conditional::value,
+FundamentalPointerTag, PointerTag>::type type;
+};
+template  struct serializer_tag {
+  typedef typename std::conditional::value,
+FundamentalReferenceTag, 
ReferenceTag>::type
+  type;
+};
+
+/// Deserializes data from a buffer. It is used to deserialize function indices
+/// to replay, their arguments and return values.
+///
+/// Fundamental types and strings are read by value. Objects are read by their
+/// index, which get translated by the IndexToObject mapping maintained in
+/// this class.
+///
+/// Additional bookkeeping with regards to the IndexToObject is required to
+/// deserialize objects. When a constructor is run or an object is 

[Lldb-commits] [lldb] r353196 - [unittests] Fix warning

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb  5 10:46:37 2019
New Revision: 353196

URL: http://llvm.org/viewvc/llvm-project?rev=353196&view=rev
Log:
[unittests] Fix warning

warning: comparison of integers of different signs: 'const int' and
'const unsigned long' [-Wsign-compare]

Modified:

lldb/trunk/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp

Modified: 
lldb/trunk/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp?rev=353196&r1=353195&r2=353196&view=diff
==
--- 
lldb/trunk/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
 (original)
+++ 
lldb/trunk/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
 Tue Feb  5 10:46:37 2019
@@ -115,7 +115,7 @@ CheckInvalidProgramTranslation(llvm::Str
   StreamBuffer<32> stream(Stream::eBinary, address_size, byte_order);
   EXPECT_FALSE(TranslateFPOProgramToDWARFExpression(
   fpo_program, target_register_name, arch_type, stream));
-  EXPECT_EQ(0, stream.GetSize());
+  EXPECT_EQ((size_t)0, stream.GetSize());
 }
 
 TEST(PDBFPOProgramToDWARFExpressionTests, InvalidAssignmentSingle) {


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

[Lldb-commits] [PATCH] D57714: [Reproducers] Instrumentation Framework: Serialization

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353195: [Reproducers] Instrumentation Framework: 
Serialization (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57714?vs=185159&id=185349#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57714

Files:
  lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/trunk/source/Utility/CMakeLists.txt
  lldb/trunk/source/Utility/ReproducerInstrumentation.cpp
  lldb/trunk/unittests/Utility/CMakeLists.txt
  lldb/trunk/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
===
--- lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
+++ lldb/trunk/include/lldb/Utility/ReproducerInstrumentation.h
@@ -0,0 +1,302 @@
+//===-- ReproducerInstrumentation.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UTILITY_REPRODUCER_INSTRUMENTATION_H
+#define LLDB_UTILITY_REPRODUCER_INSTRUMENTATION_H
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Logging.h"
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorHandling.h"
+
+#include 
+#include 
+
+namespace lldb_private {
+namespace repro {
+
+/// Mapping between serialized indices and their corresponding objects.
+///
+/// This class is used during replay to map indices back to in-memory objects.
+///
+/// When objects are constructed, they are added to this mapping using
+/// AddObjectForIndex.
+///
+/// When an object is passed to a function, its index is deserialized and
+/// AddObjectForIndex returns the corresponding object. If there is no object
+/// for the given index, a nullptr is returend. The latter is valid when custom
+/// replay code is in place and the actual object is ignored.
+class IndexToObject {
+public:
+  /// Returns an object as a pointer for the given index or nullptr if not
+  /// present in the map.
+  template  T *GetObjectForIndex(unsigned idx) {
+assert(idx != 0 && "Cannot get object for sentinel");
+void *object = GetObjectForIndexImpl(idx);
+return static_cast(object);
+  }
+
+  /// Adds a pointer to an object to the mapping for the given index.
+  template  void AddObjectForIndex(unsigned idx, T *object) {
+AddObjectForIndexImpl(
+idx, static_cast(
+ const_cast::type *>(object)));
+  }
+
+  /// Adds a reference to an object to the mapping for the given index.
+  template  void AddObjectForIndex(unsigned idx, T &object) {
+AddObjectForIndexImpl(
+idx, static_cast(
+ const_cast::type *>(&object)));
+  }
+
+private:
+  /// Helper method that does the actual lookup. The void* result is later cast
+  /// by the caller.
+  void *GetObjectForIndexImpl(unsigned idx);
+
+  /// Helper method that does the actual insertion.
+  void AddObjectForIndexImpl(unsigned idx, void *object);
+
+  /// Keeps a mapping between indices and their corresponding object.
+  llvm::DenseMap m_mapping;
+};
+
+/// We need to differentiate between pointers to fundamental and
+/// non-fundamental types. See the corresponding Deserializer::Read method
+/// for the reason why.
+struct PointerTag {};
+struct ReferenceTag {};
+struct ValueTag {};
+struct FundamentalPointerTag {};
+struct FundamentalReferenceTag {};
+
+/// Return the deserialization tag for the given type T.
+template  struct serializer_tag { typedef ValueTag type; };
+template  struct serializer_tag {
+  typedef
+  typename std::conditional::value,
+FundamentalPointerTag, PointerTag>::type type;
+};
+template  struct serializer_tag {
+  typedef typename std::conditional::value,
+FundamentalReferenceTag, ReferenceTag>::type
+  type;
+};
+
+/// Deserializes data from a buffer. It is used to deserialize function indices
+/// to replay, their arguments and return values.
+///
+/// Fundamental types and strings are read by value. Objects are read by their
+/// index, which get translated by the IndexToObject mapping maintained in
+/// this class.
+///
+/// Additional bookkeeping with regards to the IndexToObject is required to
+/// deserialize objects. When a constructor is run or an object is returned by
+/// value, we need to capture the object and add it to the index together with
+/// its index. This is the job of HandleReplayResult(Void).
+class Deserializer {
+public:
+ 

[Lldb-commits] [PATCH] D57750: [CMake] Don't add `cxx` to `LLDB_TEST_DEPS` if it doesn't exist.

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Agreed with Stefan, I don't think this should be a warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57750



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

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

[Adrian McCarthy via Phabricator via lldb-commits]

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

Nice.


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

https://reviews.llvm.org/D57751



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

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Check the comment about m_platform_file




Comment at: include/lldb/Core/Module.h:176-177
+
+// Also update the module FileSpec.
+module_sp->m_file = module_sp->m_objfile_sp->GetFileSpec();
+return std::move(module_sp);

Why do we need to update the m_file? m_platform_file is the path of the module 
as it is known the the remote system, so if m_file differs from the object 
file's file spec, we might want to shuffle, m_file into m_platform_file first 
and then overwrite it?


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

https://reviews.llvm.org/D57751



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

[Lldb-commits] [PATCH] D57402: build: remove custom variables

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D57402#1385422 , @sgraenitz wrote:

> Pro Saleem's proposal:
>
> - It's good to have fewer ways to do things. The docs describe how to use 
> `LLVM_DIR` to build against LLVM. If that's sufficient LLDB should do the 
> same. https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project


Agreed.

> - Clang also deprecated llvm-config, but has no `CLANG_PATH_TO_XY`. It's 
> usually good advice to follow their model. 
> https://github.com/llvm/llvm-project/blob/master/clang/CMakeLists.txt#L17

I also agree with this.

> Pro Alex's code:
> 
> - It would allow to have individual build trees for LLVM and Clang right? I 
> don't mind, but someone might?

I think that this patch doesn't change that because you can set LLVM_DIR and 
Clang_DIR to different build trees if you want, much like setting 
LLDB_PATH_TO_LLVM_BUILD and LLDB_PATH_TO_CLANG_BUILD.

> - For us it's a good match for our downstream code 
> https://github.com/apple/swift-lldb/blob/upstream-with-swift/cmake/modules/LLDBStandalone.cmake

I don't think it's too difficult to change how it works downstream in 
lldb-swift. I actually already have an open PR trying to make swift-lldb closer 
to upstream in this particular file: 
https://github.com/apple/swift-lldb/pull/1252
This change would be a cherry-pick on top of that, I think.

> Has anyone actually tried passing `Clang_DIR`?

The consequence of this is that you would be required to specify Clang_DIR 
unless a clang install is in your `CMAKE_MODULE_PATH`. I assume @compnerd 
specified Clang_DIR when he tested this patch.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57402



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

[Lldb-commits] [PATCH] D56595: SymbolFileBreakpad: Add line table support

[Leonard Mosescu via Phabricator via lldb-commits]

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

The latest version looks good to me. Please update the description (it still 
says it uses the one CU per symbols file)




Comment at: include/lldb/Core/FileSpecList.h:72
+  /// Move-assignment operator.
+  FileSpecList &operator=(FileSpecList &&rhs) = default;
 

nit: if you have move assignment-operator also add the move constructor


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

https://reviews.llvm.org/D56595



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

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

[Leonard Mosescu via Phabricator via lldb-commits]

lemo accepted this revision.
lemo added a comment.

Looks good - a few comments inline.




Comment at: include/lldb/Core/Module.h:178
+module_sp->m_file = module_sp->m_objfile_sp->GetFileSpec();
+return std::move(module_sp);
   }

nit: return std::move(x) is almost never a good idea. It's not needed, verbose 
and most importantly it actually inhibits NRVO so it's likely less efficient. 



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:51
 public:
-  PlaceholderModule(const ModuleSpec &module_spec) :
-Module(module_spec.GetFileSpec(), module_spec.GetArchitecture()) {
-if (module_spec.GetUUID().IsValid())
-  SetUUID(module_spec.GetUUID());
-  }
-
-  // Creates a synthetic module section covering the whole module image (and
-  // sets the section load address as well)
-  void CreateImageSection(const MinidumpModule *module, Target& target) {
-const ConstString section_name(".module_image");
-lldb::SectionSP section_sp(new Section(
-shared_from_this(), // Module to which this section belongs.
-nullptr,// ObjectFile
-0,  // Section ID.
-section_name,   // Section name.
-eSectionTypeContainer,  // Section type.
-module->base_of_image,  // VM address.
-module->size_of_image,  // VM size in bytes of this section.
-0,  // Offset of this section in the file.
-module->size_of_image,  // Size of the section as found in the file.
-12, // Alignment of the section (log2)
-0,  // Flags for this section.
-1));// Number of host bytes per target byte
-section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);
-GetSectionList()->AddSection(section_sp);
-target.GetSectionLoadList().SetSectionLoadAddress(
-section_sp, module->base_of_image);
+  PlaceholderObjectFile(const lldb::ModuleSP &module_sp,
+const ModuleSpec &module_spec, lldb::offset_t base,

Can we avoid passing both the module_sp and module_spec? I try to avoid 
interfaces which allow for inconsistent states, ex: what if module_sp and 
module_spec describe completely different modules?

At very least I'd add a few asserts, but if we can change the interface to 
avoid it, even better.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:52
-//--
-class PlaceholderModule : public Module {
 public:

Nice - so we can do without this in all scenarios? What about PDBs?



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:66
+  Strata CalculateStrata() override { return eStrataUnknown; }
+  void Dump(Stream *s) override {}
+  uint32_t GetDependentModules(FileSpecList &file_list) override { return 0; }

should we dump something meaningful? I don't remember the exact command but I 
think this surfaces in user output


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

https://reviews.llvm.org/D57751



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

[Lldb-commits] [PATCH] D57780: Don't include UnixSignals.h from Host

[Zachary Turner via Phabricator via lldb-commits]

zturner created this revision.
zturner added a reviewer: labath.
Herald added a reviewer: jfb.

This file lives in Target, which makes sense since it is supposed to represent 
the various types of signals that can occur on any platform (since you might be 
remote debugging a platform with different signals than the host).

However, Host had a method called `GetUnixSignals` which just returned an 
instance of this class initialized for the Host's signals.  Rather than have 
this functionality in Host, we can just provide a static factory method on 
`UnixSignals` called `CreateForHost`.  This brings us one step closer to 
breaking the Target -> Host -> Target cycle.


https://reviews.llvm.org/D57780

Files:
  lldb/include/lldb/Host/Host.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/UnixSignals.h
  lldb/source/Host/common/Host.cpp
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/UnixSignals.cpp

Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -11,6 +11,7 @@
 #include "Plugins/Process/Utility/LinuxSignals.h"
 #include "Plugins/Process/Utility/MipsLinuxSignals.h"
 #include "Plugins/Process/Utility/NetBSDSignals.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Host/StringConvert.h"
 #include "lldb/Utility/ArchSpec.h"
 
@@ -50,6 +51,12 @@
   }
 }
 
+lldb::UnixSignalsSP UnixSignals::CreateForHost() {
+  static lldb::UnixSignalsSP s_unix_signals_sp =
+  Create(HostInfo::GetArchitecture());
+  return s_unix_signals_sp;
+}
+
 //--
 // UnixSignals constructor
 //--
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1739,9 +1739,9 @@
   return s_default_unix_signals_sp;
 }
 
-const UnixSignalsSP &Platform::GetUnixSignals() {
+UnixSignalsSP Platform::GetUnixSignals() {
   if (IsHost())
-return Host::GetUnixSignals();
+return UnixSignals::CreateForHost();
   return GetRemoteUnixSignals();
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -398,7 +398,7 @@
 StringExtractorGDBRemote &packet) {
   StructuredData::Array signal_array;
 
-  const auto &signals = Host::GetUnixSignals();
+  lldb::UnixSignalsSP signals = UnixSignals::CreateForHost();
   for (auto signo = signals->GetFirstSignalNumber();
signo != LLDB_INVALID_SIGNAL_NUMBER;
signo = signals->GetNextSignalNumber(signo)) {
Index: lldb/source/Host/common/Host.cpp
===
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -56,7 +56,6 @@
 #include "lldb/Host/ProcessLauncher.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
-#include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/DataBufferLLVM.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
@@ -613,12 +612,6 @@
 
 #endif
 
-const UnixSignalsSP &Host::GetUnixSignals() {
-  static const auto s_unix_signals_sp =
-  UnixSignals::Create(HostInfo::GetArchitecture());
-  return s_unix_signals_sp;
-}
-
 std::unique_ptr Host::CreateDefaultConnection(llvm::StringRef url) {
 #if defined(_WIN32)
   if (url.startswith("file://"))
Index: lldb/include/lldb/Target/UnixSignals.h
===
--- lldb/include/lldb/Target/UnixSignals.h
+++ lldb/include/lldb/Target/UnixSignals.h
@@ -22,6 +22,7 @@
 class UnixSignals {
 public:
   static lldb::UnixSignalsSP Create(const ArchSpec &arch);
+  static lldb::UnixSignalsSP CreateForHost();
 
   //--
   // Constructors and Destructors
Index: lldb/include/lldb/Target/Platform.h
===
--- lldb/include/lldb/Target/Platform.h
+++ lldb/include/lldb/Target/Platform.h
@@ -689,7 +689,7 @@
 
   virtual const lldb::UnixSignalsSP &GetRemoteUnixSignals();
 
-  const lldb::UnixSignalsSP &GetUnixSignals();
+  lldb::UnixSignalsSP GetUnixSignals();
 
   //--
   /// Locate a queue name given a thread's qaddr
Index: lldb/include/lldb/Host/Host.h
===
--- lldb/include/lldb/Host/Host.h
+++ lldb/include/lldb/Host/Host.h
@@ -196,8 +1

[Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Thanks for the answers. LGTM


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

https://reviews.llvm.org/D57552



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

[Lldb-commits] [PATCH] D57781: Fix strlen() of unbound array undefined behavior

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added a project: LLDB.
Herald added subscribers: abidh, aprantl.

LLDB testsuite fails when built by GCC8 on:

  LLDB :: SymbolFile/DWARF/find-basic-namespace.cpp

This is because this code in LLDB codebase has undefined behavior:

  #include 
  #include 
  // lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1731
  static struct section_64 {
char sectname[16];
char segname[16];
  } sect64 = { 
{'_','_','a','p','p','l','e','_','n','a','m','e','s','p','a','c'}, "__DWARF" };
  int main() {
return std::min(strlen(sect64.sectname), sizeof(sect64.sectname));
  }

It has been discussed as a (false) bugreport to GCC: wrong-code: LLDB testcase 
fails: SymbolFile/DWARF/find-basic-namespace.cpp 


I will check it in if there are no comments as it looks obvious enough to me.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D57781

Files:
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Utility/ConstString.cpp


Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -143,7 +143,7 @@
   const char *GetConstTrimmedCStringWithLength(const char *cstr,
size_t cstr_len) {
 if (cstr != nullptr) {
-  const size_t trimmed_len = std::min(strlen(cstr), cstr_len);
+  const size_t trimmed_len = strnlen(cstr, cstr_len);
   return GetConstCStringWithLength(cstr, trimmed_len);
 }
 return nullptr;
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1609,8 +1609,7 @@
   bool add_section = true;
   bool add_to_unified = true;
   ConstString const_segname(
-  load_cmd.segname,
-  std::min(strlen(load_cmd.segname), sizeof(load_cmd.segname)));
+  load_cmd.segname, strnlen(load_cmd.segname, sizeof(load_cmd.segname)));
 
   SectionSP unified_section_sp(
   context.UnifiedList.FindSectionByName(const_segname));
@@ -1729,8 +1728,7 @@
 
 if (add_section) {
   ConstString section_name(
-  sect64.sectname,
-  std::min(strlen(sect64.sectname), sizeof(sect64.sectname)));
+  sect64.sectname, strnlen(sect64.sectname, sizeof(sect64.sectname)));
   if (!const_segname) {
 // We have a segment with no name so we need to conjure up segments
 // that correspond to the section's segname if there isn't already such


Index: lldb/source/Utility/ConstString.cpp
===
--- lldb/source/Utility/ConstString.cpp
+++ lldb/source/Utility/ConstString.cpp
@@ -143,7 +143,7 @@
   const char *GetConstTrimmedCStringWithLength(const char *cstr,
size_t cstr_len) {
 if (cstr != nullptr) {
-  const size_t trimmed_len = std::min(strlen(cstr), cstr_len);
+  const size_t trimmed_len = strnlen(cstr, cstr_len);
   return GetConstCStringWithLength(cstr, trimmed_len);
 }
 return nullptr;
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1609,8 +1609,7 @@
   bool add_section = true;
   bool add_to_unified = true;
   ConstString const_segname(
-  load_cmd.segname,
-  std::min(strlen(load_cmd.segname), sizeof(load_cmd.segname)));
+  load_cmd.segname, strnlen(load_cmd.segname, sizeof(load_cmd.segname)));
 
   SectionSP unified_section_sp(
   context.UnifiedList.FindSectionByName(const_segname));
@@ -1729,8 +1728,7 @@
 
 if (add_section) {
   ConstString section_name(
-  sect64.sectname,
-  std::min(strlen(sect64.sectname), sizeof(sect64.sectname)));
+  sect64.sectname, strnlen(sect64.sectname, sizeof(sect64.sectname)));
   if (!const_segname) {
 // We have a segment with no name so we need to conjure up segments
 // that correspond to the section's segname if there isn't already such
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D57781: Fix strlen() of unbound array undefined behavior

[Adrian Prantl via Phabricator via lldb-commits]

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

Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57781



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

[Lldb-commits] [lldb] r353226 - [Py3/TestAppleOSSimulator] Another byte<->str interoperability issue.

[Davide Italiano via lldb-commits]

Author: davide
Date: Tue Feb  5 14:24:53 2019
New Revision: 353226

URL: http://llvm.org/viewvc/llvm-project?rev=353226&view=rev
Log:
[Py3/TestAppleOSSimulator] Another byte<->str interoperability issue.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py?rev=353226&r1=353225&r2=353226&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py
 Tue Feb  5 14:24:53 2019
@@ -15,7 +15,7 @@ class TestAppleSimulatorOSType(gdbremote
 
 def check_simulator_ostype(self, sdk, platform, arch='x86_64'):
 sim_devices_str = subprocess.check_output(['xcrun', 'simctl', 'list',
-   '-j', 'devices'])
+   '-j', 
'devices']).decode("utf-8")
 sim_devices = json.loads(sim_devices_str)['devices']
 # Find an available simulator for the requested platform
 deviceUDID = None


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

[Lldb-commits] [lldb] r353239 - Remove accidentally commited file in xcshareddata

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb  5 15:35:01 2019
New Revision: 353239

URL: http://llvm.org/viewvc/llvm-project?rev=353239&view=rev
Log:
Remove accidentally commited file in xcshareddata

Removed:
lldb/trunk/lldb.xcworkspace/xcshareddata/

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

[Lldb-commits] [lldb] r353243 - Fix PathMappingList::FindFile to handle relative incoming FileSpecs.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Tue Feb  5 15:48:10 2019
New Revision: 353243

URL: http://llvm.org/viewvc/llvm-project?rev=353243&view=rev
Log:
Fix PathMappingList::FindFile to handle relative incoming FileSpecs.

An equivalent change was made to RemapPaths, but it needed to be made 
here as well.  Also added a test for this and made the setup a little
more complex to avoid false successes.



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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/a.yaml
lldb/trunk/source/Target/PathMappingList.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py?rev=353243&r1=353242&r2=353243&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py
 Tue Feb  5 15:48:10 2019
@@ -15,16 +15,9 @@ class TestTargetSourceMap(TestBase):
 src_path = os.path.join(src_dir, "main.c")
 yaml_path = os.path.join(src_dir, "a.yaml")
 yaml_base, ext = os.path.splitext(yaml_path)
-obj_path = self.getBuildArtifact(yaml_base)
+obj_path = self.getBuildArtifact("main.o")
 self.yaml2obj(yaml_path, obj_path)
 
-def cleanup():
-if os.path.exists(obj_path):
-os.unlink(obj_path)
-
-# Execute the cleanup function during test case tear down.
-self.addTearDownHook(cleanup)
-
 # Create a target with the object file we just created from YAML
 target = self.dbg.CreateTarget(obj_path)
 
@@ -32,10 +25,19 @@ class TestTargetSourceMap(TestBase):
 bp = target.BreakpointCreateByLocation(src_path, 2)
 self.assertTrue(bp.GetNumLocations() == 0,
 "make sure no breakpoints were resolved without map")
-src_map_cmd = 'settings set target.source-map ./ "%s"' % (src_dir)
+src_map_cmd = 'settings set target.source-map . "%s"' % (src_dir)
 self.dbg.HandleCommand(src_map_cmd)
 
 # Set a breakpoint after we remap source and verify that it succeeds
 bp = target.BreakpointCreateByLocation(src_path, 2)
 self.assertTrue(bp.GetNumLocations() == 1,
 "make sure breakpoint was resolved with map")
+
+# Now make sure that we can actually FIND the source file using this
+# remapping:
+retval = lldb.SBCommandReturnObject()
+self.dbg.GetCommandInterpreter().HandleCommand("source list -f main.c 
-l 2", retval)
+self.assertTrue(retval.Succeeded(), "source list didn't succeed.")
+self.assertTrue(retval.GetOutput() != None, "We got no ouput from 
source list")
+self.assertTrue("return" in retval.GetOutput(), "We didn't find the 
source file...")
+

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/a.yaml
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/a.yaml?rev=353243&r1=353242&r2=353243&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/a.yaml 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/a.yaml 
Tue Feb  5 15:48:10 2019
@@ -1,51 +1,31 @@
 --- !mach-o
-FileHeader:
+FileHeader:  
   magic:   0xFEEDFACF
   cputype: 0x0107
   cpusubtype:  0x0003
-  filetype:0x000A
-  ncmds:   6
-  sizeofcmds:  1376
-  flags:   0x
+  filetype:0x0001
+  ncmds:   4
+  sizeofcmds:  1160
+  flags:   0x2000
   reserved:0x
-LoadCommands:
-  - cmd: LC_UUID
-cmdsize: 24
-uuid:D37CC773-C218-3F97-99C9-CE4E77DDF2CE
-  - cmd: LC_SYMTAB
-cmdsize: 24
-symoff:  4096
-nsyms:   2
-stroff:  4128
-strsize: 28
+LoadCommands:
   - cmd: LC_SEGMENT_64
-cmdsize: 72
-segname: __PAGEZERO
+cmdsize: 1032
+segname: ''
 vmaddr:  0
-vmsize:  4294967296
-fileoff: 0
-filesize:0
-maxprot: 0
-initprot:0
-nsects:  0
-flags:   0
-  - cmd: LC_SEGMENT_64
-cmdsize: 232
-segname: __TEXT
-vmaddr:  4294967296
-vmsize:  4096
-fileoff: 0
-filesize:0
+vmsize:

[Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353243: Fix PathMappingList::FindFile to handle relative 
incoming FileSpecs. (authored by jingham, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57552?vs=184790&id=185427#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57552

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py
  lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/a.yaml
  lldb/trunk/source/Target/PathMappingList.cpp

Index: lldb/trunk/source/Target/PathMappingList.cpp
===
--- lldb/trunk/source/Target/PathMappingList.cpp
+++ lldb/trunk/source/Target/PathMappingList.cpp
@@ -201,26 +201,46 @@
 
 bool PathMappingList::FindFile(const FileSpec &orig_spec,
FileSpec &new_spec) const {
-  if (!m_pairs.empty()) {
-char orig_path[PATH_MAX];
-const size_t orig_path_len =
-orig_spec.GetPath(orig_path, sizeof(orig_path));
-if (orig_path_len > 0) {
-  const_iterator pos, end = m_pairs.end();
-  for (pos = m_pairs.begin(); pos != end; ++pos) {
-const size_t prefix_len = pos->first.GetLength();
-
-if (orig_path_len >= prefix_len) {
-  if (::strncmp(pos->first.GetCString(), orig_path, prefix_len) == 0) {
-new_spec.SetFile(pos->second.GetCString(), FileSpec::Style::native);
-new_spec.AppendPathComponent(orig_path + prefix_len);
-if (FileSystem::Instance().Exists(new_spec))
-  return true;
-  }
-}
+  if (m_pairs.empty())
+return false;
+  
+  std::string orig_path = orig_spec.GetPath();
+
+  if (orig_path.empty())
+return false;
+  
+  bool orig_is_relative = orig_spec.IsRelative();
+
+  const_iterator pos, end = m_pairs.end();
+  for (pos = m_pairs.begin(); pos != end; ++pos) {
+llvm::StringRef orig_ref(orig_path);
+llvm::StringRef prefix_ref = pos->first.GetStringRef();
+if (orig_ref.size() >= prefix_ref.size()) {
+  // We consider a relative prefix or one of just "." to
+  // mean "only apply to relative paths".
+  bool prefix_is_relative = false;
+  
+  if (prefix_ref == ".") {
+prefix_is_relative = true;
+// Remove the "." since it will have been removed from the
+// FileSpec paths already.
+prefix_ref = prefix_ref.drop_front();
+  } else {
+FileSpec prefix_spec(prefix_ref, FileSpec::Style::native);
+prefix_is_relative = prefix_spec.IsRelative();
+  }
+  if (prefix_is_relative != orig_is_relative)
+continue;
+
+  if (orig_ref.consume_front(prefix_ref)) {
+new_spec.SetFile(pos->second.GetCString(), FileSpec::Style::native);
+new_spec.AppendPathComponent(orig_ref);
+if (FileSystem::Instance().Exists(new_spec))
+  return true;
   }
 }
   }
+  
   new_spec.Clear();
   return false;
 }
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/TestTargetSourceMap.py
@@ -15,16 +15,9 @@
 src_path = os.path.join(src_dir, "main.c")
 yaml_path = os.path.join(src_dir, "a.yaml")
 yaml_base, ext = os.path.splitext(yaml_path)
-obj_path = self.getBuildArtifact(yaml_base)
+obj_path = self.getBuildArtifact("main.o")
 self.yaml2obj(yaml_path, obj_path)
 
-def cleanup():
-if os.path.exists(obj_path):
-os.unlink(obj_path)
-
-# Execute the cleanup function during test case tear down.
-self.addTearDownHook(cleanup)
-
 # Create a target with the object file we just created from YAML
 target = self.dbg.CreateTarget(obj_path)
 
@@ -32,10 +25,19 @@
 bp = target.BreakpointCreateByLocation(src_path, 2)
 self.assertTrue(bp.GetNumLocations() == 0,
 "make sure no breakpoints were resolved without map")
-src_map_cmd = 'settings set target.source-map ./ "%s"' % (src_dir)
+src_map_cmd = 'settings set target.source-map . "%s"' % (src_dir)
 self.dbg.HandleCommand(src_map_cmd)
 
 # Set a breakpoint after we remap source and verify that it succeeds
 bp = target.BreakpointCreateByLocation(src_path, 2)
 self.assertTrue(bp.GetNumLocations() == 1,
 "make sure breakpoint was resolved with map")
+
+# Now make sure that we can actually FIND the source file using this
+# remapping:
+retval = ll

[Lldb-commits] [PATCH] D56322: [Reproducers] SBReproducer framework

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 185431.

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

https://reviews.llvm.org/D56322

Files:
  include/lldb/API/SBReproducer.h
  include/lldb/Utility/ReproducerInstrumentation.h
  source/API/CMakeLists.txt
  source/API/SBReproducer.cpp
  source/API/SBReproducerPrivate.h
  source/Utility/ReproducerInstrumentation.cpp
  tools/driver/Driver.cpp
  unittests/Utility/ReproducerInstrumentationTest.cpp

Index: unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- unittests/Utility/ReproducerInstrumentationTest.cpp
+++ unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -9,12 +9,14 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include 
+#include 
+
 #include "lldb/Utility/ReproducerInstrumentation.h"
 
 using namespace lldb_private;
 using namespace lldb_private::repro;
 
-namespace {
 struct Foo {
   int m = 1;
 };
@@ -40,7 +42,194 @@
   unsigned long l = 8;
   unsigned short m = 9;
 };
-} // namespace
+class TestingRegistry : public Registry {
+public:
+  TestingRegistry();
+};
+
+template  bool Equals(T LHS, T RHS) {
+  return std::fabs(RHS - LHS) < std::numeric_limits::epsilon();
+}
+
+static llvm::Optional g_serializer;
+static llvm::Optional g_registry;
+
+#define LLDB_GET_INSTRUMENTATION_DATA  \
+  InstrumentationData(*g_serializer, *g_registry)
+
+class InstrumentedFoo {
+public:
+  ~InstrumentedFoo() { EXPECT_TRUE(m_validated); }
+
+  /// Instrumented methods.
+  /// {
+  InstrumentedFoo();
+  InstrumentedFoo(const InstrumentedFoo &foo);
+  InstrumentedFoo &operator=(const InstrumentedFoo &foo);
+  void A(int a);
+  void B(int &b) const;
+  int C(float *c);
+  int D(const char *d) const;
+  static void E(double e);
+  static int F();
+  void Validate();
+   }
+
+private:
+  int m_a = 0;
+  mutable int m_b = 0;
+  float m_c = 0;
+  mutable std::string m_d = {};
+  static double g_e;
+  static bool g_f;
+
+  mutable int m_called = 0;
+  mutable bool m_validated = false;
+};
+
+class InstrumentedBar {
+public:
+  ~InstrumentedBar() { EXPECT_TRUE(m_validated); }
+
+  /// Instrumented methods.
+  /// {
+  InstrumentedBar();
+  InstrumentedFoo GetInstrumentedFoo();
+  void SetInstrumentedFoo(InstrumentedFoo *foo);
+  void SetInstrumentedFoo(InstrumentedFoo &foo);
+  void Validate();
+  /// }
+
+private:
+  bool m_get_instrumend_foo_called = false;
+  bool m_set_instrumend_foo_ptr_called = false;
+  bool m_set_instrumend_foo_ref_called = false;
+
+  mutable bool m_validated = false;
+};
+
+double InstrumentedFoo::g_e = 0;
+bool InstrumentedFoo::g_f = false;
+
+InstrumentedFoo::InstrumentedFoo() {
+  LLDB_RECORD_CONSTRUCTOR_NO_ARGS(InstrumentedFoo);
+}
+
+InstrumentedFoo::InstrumentedFoo(const InstrumentedFoo &foo) {
+  LLDB_RECORD_CONSTRUCTOR(InstrumentedFoo, (const InstrumentedFoo &), foo);
+  foo.m_validated = true; // The framework's copy should not be validated.
+}
+
+InstrumentedFoo &InstrumentedFoo::operator=(const InstrumentedFoo &foo) {
+  LLDB_RECORD_METHOD(InstrumentedFoo &,
+ InstrumentedFoo, operator=,(const InstrumentedFoo &), foo);
+  foo.m_validated = true; // The framework's copy should not be validated.
+  return *this;
+}
+
+void InstrumentedFoo::A(int a) {
+  LLDB_RECORD_METHOD(void, InstrumentedFoo, A, (int), a);
+  B(a);
+  m_a = a;
+}
+
+void InstrumentedFoo::B(int &b) const {
+  LLDB_RECORD_METHOD_CONST(void, InstrumentedFoo, B, (int &), b);
+  m_called++;
+  m_b = b;
+}
+
+int InstrumentedFoo::C(float *c) {
+  LLDB_RECORD_METHOD(int, InstrumentedFoo, C, (float *), c);
+  m_c = *c;
+  return 1;
+}
+
+int InstrumentedFoo::D(const char *d) const {
+  LLDB_RECORD_METHOD_CONST(int, InstrumentedFoo, D, (const char *), d);
+  m_d = std::string(d);
+  return 2;
+}
+
+void InstrumentedFoo::E(double e) {
+  LLDB_RECORD_STATIC_METHOD(void, InstrumentedFoo, E, (double), e);
+  g_e = e;
+}
+
+int InstrumentedFoo::F() {
+  LLDB_RECORD_STATIC_METHOD_NO_ARGS(int, InstrumentedFoo, F);
+  g_f = true;
+  return 3;
+}
+
+void InstrumentedFoo::Validate() {
+  LLDB_RECORD_METHOD_NO_ARGS(void, InstrumentedFoo, Validate);
+  EXPECT_EQ(m_a, 100);
+  EXPECT_EQ(m_b, 200);
+  EXPECT_NEAR(m_c, 300.3, 0.01);
+  EXPECT_EQ(m_d, "bar");
+  EXPECT_NEAR(g_e, 400.4, 0.01);
+  EXPECT_EQ(g_f, true);
+  EXPECT_EQ(2, m_called);
+
+  m_validated = true;
+}
+
+InstrumentedBar::InstrumentedBar() {
+  LLDB_RECORD_CONSTRUCTOR_NO_ARGS(InstrumentedBar);
+}
+
+InstrumentedFoo InstrumentedBar::GetInstrumentedFoo() {
+  LLDB_RECORD_METHOD_NO_ARGS(InstrumentedFoo, InstrumentedBar,
+ GetInstrumentedFoo);
+  m_get_instrumend_foo_called = true;
+  return LLDB_RECORD_RESULT(InstrumentedFoo());
+}
+
+void InstrumentedBar::SetInstrumentedFoo(InstrumentedFoo *foo) {
+  LLDB_RECORD_METHOD(void, InstrumentedBar, SetInstrumentedFoo,
+ (InstrumentedFoo *), foo);
+  m_set_instrumend_foo_ptr_ca

[Lldb-commits] [PATCH] D57475: [Reproducers] Add SBReproducer macros

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 185432.

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

https://reviews.llvm.org/D57475

Files:
  source/API/SBAddress.cpp
  source/API/SBAttachInfo.cpp
  source/API/SBBlock.cpp
  source/API/SBBreakpoint.cpp
  source/API/SBBreakpointLocation.cpp
  source/API/SBBreakpointName.cpp
  source/API/SBBreakpointOptionCommon.cpp
  source/API/SBBroadcaster.cpp
  source/API/SBCommandInterpreter.cpp
  source/API/SBCommandReturnObject.cpp
  source/API/SBCommunication.cpp
  source/API/SBCompileUnit.cpp
  source/API/SBData.cpp
  source/API/SBDebugger.cpp
  source/API/SBDeclaration.cpp
  source/API/SBError.cpp
  source/API/SBEvent.cpp
  source/API/SBExecutionContext.cpp
  source/API/SBExpressionOptions.cpp
  source/API/SBFileSpec.cpp
  source/API/SBFileSpecList.cpp
  source/API/SBFrame.cpp
  source/API/SBFunction.cpp
  source/API/SBHostOS.cpp
  source/API/SBInitializerOptions.cpp
  source/API/SBInstruction.cpp
  source/API/SBInstructionList.cpp
  source/API/SBLanguageRuntime.cpp
  source/API/SBLaunchInfo.cpp
  source/API/SBLineEntry.cpp
  source/API/SBListener.cpp
  source/API/SBMemoryRegionInfo.cpp
  source/API/SBMemoryRegionInfoList.cpp
  source/API/SBModule.cpp
  source/API/SBModuleSpec.cpp
  source/API/SBPlatform.cpp
  source/API/SBProcess.cpp
  source/API/SBProcessInfo.cpp
  source/API/SBQueue.cpp
  source/API/SBQueueItem.cpp
  source/API/SBReproducer.cpp
  source/API/SBSection.cpp
  source/API/SBSourceManager.cpp
  source/API/SBStream.cpp
  source/API/SBStringList.cpp
  source/API/SBStructuredData.cpp
  source/API/SBSymbol.cpp
  source/API/SBSymbolContext.cpp
  source/API/SBSymbolContextList.cpp
  source/API/SBTarget.cpp
  source/API/SBThread.cpp
  source/API/SBThreadCollection.cpp
  source/API/SBThreadPlan.cpp
  source/API/SBTrace.cpp
  source/API/SBTraceOptions.cpp
  source/API/SBType.cpp
  source/API/SBTypeCategory.cpp
  source/API/SBTypeEnumMember.cpp
  source/API/SBTypeFilter.cpp
  source/API/SBTypeFormat.cpp
  source/API/SBTypeNameSpecifier.cpp
  source/API/SBTypeSummary.cpp
  source/API/SBTypeSynthetic.cpp
  source/API/SBUnixSignals.cpp
  source/API/SBValue.cpp
  source/API/SBValueList.cpp
  source/API/SBVariablesOptions.cpp
  source/API/SBWatchpoint.cpp
  source/Utility/ReproducerInstrumentation.cpp



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

[Lldb-commits] [PATCH] D57745: [x64] Process the B field of the REX prefix correctly for the PUSH and POP instructions

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda accepted this revision.
jasonmolenda added a comment.

The change looks good to me, thanks for fixing this.  I'm not sure you're 
testing what you meant to test in TestPopRBPWithREX because you're 
pushing/popping r13.  You could get the unwind state at byte offset 2 (after 
the push r13 has executed) and see that you can GetRegisterInfo() r13, and the 
unwind state at byte offset 4 and verify that you can't GetRegisterInfo() r13.  
That's a good test to make sure we handle the B bit correctly.




Comment at: 
unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp:1953
+  0x41, 0x55, // pushq %rbp
+  0x41, 0x5d, // popq %rbp
+  0x90// nop

These are pushing/popping r13 aren't they?  0x40 0x55 gives us register 5 
(rbp), but 0x41 0x55 gives us register 13 (r13) if I'm reading the intel 
manuals right (volume 2a, section 2.2.1.2 "More on REX Prefix Fields").


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57745



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

[Lldb-commits] [PATCH] D56822: [Reproducers] Tool to insert SBReproducer macros

[Jonas Devlieghere via Phabricator via lldb-commits]

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

- Use new LLDB_ prefix
- Insert two newlines at the end of the macro, so clang-format fixes it up 
consistently.


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

https://reviews.llvm.org/D56822

Files:
  tools/CMakeLists.txt
  tools/sbrepro/CMakeLists.txt
  tools/sbrepro/SBRepro.cpp

Index: tools/sbrepro/SBRepro.cpp
===
--- /dev/null
+++ tools/sbrepro/SBRepro.cpp
@@ -0,0 +1,287 @@
+#include "clang/AST/AST.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/ASTConsumers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::driver;
+using namespace clang::tooling;
+
+static llvm::cl::OptionCategory ReproCategory("SB Reproducer");
+
+/// Get the macro name for recording method calls.
+///
+/// LLDB_RECORD_METHOD
+/// LLDB_RECORD_METHOD_CONST
+/// LLDB_RECORD_METHOD_NO_ARGS
+/// LLDB_RECORD_METHOD_CONST_NO_ARGS
+/// LLDB_RECORD_STATIC_METHOD
+/// LLDB_RECORD_STATIC_METHOD_NO_ARGS
+static std::string GetRecordMethodMacroName(bool Static, bool Const,
+bool NoArgs) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << "LLDB_RECORD";
+  if (Static)
+OS << "_STATIC";
+  OS << "_METHOD";
+  if (Const)
+OS << "_CONST";
+  if (NoArgs)
+OS << "_NO_ARGS";
+
+  return OS.str();
+}
+
+/// Get the macro name for register methods.
+///
+/// LLDB_REGISTER_CONSTRUCTOR
+/// LLDB_REGISTER_METHOD
+/// LLDB_REGISTER_METHOD_CONST
+/// LLDB_REGISTER_STATIC_METHOD
+static std::string GetRegisterMethodMacroName(bool Static, bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << "LLDB_REGISTER";
+  if (Static)
+OS << "_STATIC";
+  OS << "_METHOD";
+  if (Const)
+OS << "_CONST";
+
+  return OS.str();
+}
+
+static std::string GetRecordMethodMacro(StringRef Result, StringRef Class,
+StringRef Method, StringRef Signature,
+StringRef Values, bool Static,
+bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << GetRecordMethodMacroName(Static, Const, Values.empty());
+  OS << "(" << Result << ", " << Class << ", " << Method;
+
+  if (!Values.empty()) {
+OS << ", (" << Signature << "), " << Values << ");\n\n";
+  } else {
+OS << ");\n\n";
+  }
+
+  return OS.str();
+}
+
+static std::string GetRecordConstructorMacro(StringRef Class,
+ StringRef Signature,
+ StringRef Values) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  if (!Values.empty()) {
+OS << "LLDB_RECORD_CONSTRUCTOR(" << Class << ", (" << Signature << "), "
+   << Values << ");\n\n";
+  } else {
+OS << "LLDB_RECORD_CONSTRUCTOR_NO_ARGS(" << Class << ");\n\n";
+  }
+  return OS.str();
+}
+
+static std::string GetRegisterConstructorMacro(StringRef Class,
+   StringRef Signature) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  OS << "LLDB_REGISTER_CONSTRUCTOR(" << Class << ", (" << Signature
+ << "));\n\n";
+  return OS.str();
+}
+
+static std::string GetRegisterMethodMacro(StringRef Result, StringRef Class,
+  StringRef Method, StringRef Signature,
+  bool Static, bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  OS << GetRegisterMethodMacroName(Static, Const);
+  OS << "(" << Result << ", " << Class << ", " << Method << ", (" << Signature
+ << "));\n";
+  return OS.str();
+}
+
+class SBVisitor : public RecursiveASTVisitor {
+public:
+  SBVisitor(Rewriter &R, ASTContext &Context)
+  : MyRewriter(R), Context(Context) {}
+
+  bool VisitCXXMethodDecl(CXXMethodDecl *Decl) {
+// Not all decls should be registered for the reproducer. Please refer to
+// that method's comment for details.
+if (ShouldSkip(Decl))
+  return false;
+
+// Print 'bool' instead of '_Bool'.
+PrintingPolicy Policy(Context.getLangOpts());
+Policy.Bool = true;
+
+// Collect the functions parameter types and names.
+std::vector ParamTypes;
+std::vector ParamNames;
+for (auto *P : Decl->parameters()) {
+  QualType T = P->getType();
+
+  // Currently we don't support functions that have function pointers as an
+  // argument.
+  if (T->isFun

[Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

This is failing on the windows bot:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/1379


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57552



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

Re: [Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

[Jim Ingham via lldb-commits]

I don't have the ability to debug this and the test output isn't very helpful.  
 There's very little chance what this is testing was working on Windows before 
this patch, as the relative case was unhandled...  So I don't think this patch 
makes things worse.  If somebody who has a windows machine available wants to 
try to debug the windows side of this, that would be great!

Give me just a minute and I'll split the test into a part that does the path 
remap and a part that tests we can find the file and xfail the latter on 
windows. 

Jim


> On Feb 5, 2019, at 4:30 PM, Stella Stamenova via Phabricator 
>  wrote:
> 
> stella.stamenova added a comment.
> 
> This is failing on the windows bot:
> 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/1379
> 
> 
> Repository:
>  rL LLVM
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D57552/new/
> 
> https://reviews.llvm.org/D57552
> 
> 
> 

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

[Lldb-commits] [lldb] r353251 - Add the source directory for https://reviews.llvm.org/D57552.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Tue Feb  5 16:52:04 2019
New Revision: 353251

URL: http://llvm.org/viewvc/llvm-project?rev=353251&view=rev
Log:
Add the source directory for https://reviews.llvm.org/D57552.

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/Trivial/

lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/Trivial/main.c

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/Trivial/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/Trivial/main.c?rev=353251&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/Trivial/main.c
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/source-map/Trivial/main.c
 Tue Feb  5 16:52:04 2019
@@ -0,0 +1,7 @@
+int main()
+{
+  return 0;
+}
+int main () {
+  return 0;
+}


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

Re: [Lldb-commits] [PATCH] D57552: Handle "." in target.source-map in PathMapListing::FindFiles

[Jim Ingham via lldb-commits]

Oh, wait.  I forgot to svn add the directory in which I was hiding the source 
file.  I did that in r353251.  Let's see if that works any better!

Jim


> On Feb 5, 2019, at 4:42 PM, Jim Ingham  wrote:
> 
> I don't have the ability to debug this and the test output isn't very 
> helpful.   There's very little chance what this is testing was working on 
> Windows before this patch, as the relative case was unhandled...  So I don't 
> think this patch makes things worse.  If somebody who has a windows machine 
> available wants to try to debug the windows side of this, that would be great!
> 
> Give me just a minute and I'll split the test into a part that does the path 
> remap and a part that tests we can find the file and xfail the latter on 
> windows. 
> 
> Jim
> 
> 
>> On Feb 5, 2019, at 4:30 PM, Stella Stamenova via Phabricator 
>>  wrote:
>> 
>> stella.stamenova added a comment.
>> 
>> This is failing on the windows bot:
>> 
>> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/1379
>> 
>> 
>> Repository:
>> rL LLVM
>> 
>> CHANGES SINCE LAST ACTION
>> https://reviews.llvm.org/D57552/new/
>> 
>> https://reviews.llvm.org/D57552
>> 
>> 
>> 
> 

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

[Lldb-commits] [lldb] r353257 - Little more cleanup on https://reviews.llvm.org/D57552

[Jim Ingham via lldb-commits]

Author: jingham
Date: Tue Feb  5 17:27:45 2019
New Revision: 353257

URL: http://llvm.org/viewvc/llvm-project?rev=353257&view=rev
Log:
Little more cleanup on https://reviews.llvm.org/D57552
Thanks Jonas...  One more early continue and using
a range where we had an iterator.

NFC

Modified:
lldb/trunk/source/Target/PathMappingList.cpp

Modified: lldb/trunk/source/Target/PathMappingList.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/PathMappingList.cpp?rev=353257&r1=353256&r2=353257&view=diff
==
--- lldb/trunk/source/Target/PathMappingList.cpp (original)
+++ lldb/trunk/source/Target/PathMappingList.cpp Tue Feb  5 17:27:45 2019
@@ -211,33 +211,32 @@ bool PathMappingList::FindFile(const Fil
   
   bool orig_is_relative = orig_spec.IsRelative();
 
-  const_iterator pos, end = m_pairs.end();
-  for (pos = m_pairs.begin(); pos != end; ++pos) {
+  for (auto entry : m_pairs) {
 llvm::StringRef orig_ref(orig_path);
-llvm::StringRef prefix_ref = pos->first.GetStringRef();
-if (orig_ref.size() >= prefix_ref.size()) {
-  // We consider a relative prefix or one of just "." to
-  // mean "only apply to relative paths".
-  bool prefix_is_relative = false;
-  
-  if (prefix_ref == ".") {
-prefix_is_relative = true;
-// Remove the "." since it will have been removed from the
-// FileSpec paths already.
-prefix_ref = prefix_ref.drop_front();
-  } else {
-FileSpec prefix_spec(prefix_ref, FileSpec::Style::native);
-prefix_is_relative = prefix_spec.IsRelative();
-  }
-  if (prefix_is_relative != orig_is_relative)
-continue;
+llvm::StringRef prefix_ref = entry.first.GetStringRef();
+if (orig_ref.size() < prefix_ref.size())
+  continue;
+// We consider a relative prefix or one of just "." to
+// mean "only apply to relative paths".
+bool prefix_is_relative = false;
+
+if (prefix_ref == ".") {
+  prefix_is_relative = true;
+  // Remove the "." since it will have been removed from the
+  // FileSpec paths already.
+  prefix_ref = prefix_ref.drop_front();
+} else {
+  FileSpec prefix_spec(prefix_ref, FileSpec::Style::native);
+  prefix_is_relative = prefix_spec.IsRelative();
+}
+if (prefix_is_relative != orig_is_relative)
+  continue;
 
-  if (orig_ref.consume_front(prefix_ref)) {
-new_spec.SetFile(pos->second.GetCString(), FileSpec::Style::native);
-new_spec.AppendPathComponent(orig_ref);
-if (FileSystem::Instance().Exists(new_spec))
-  return true;
-  }
+if (orig_ref.consume_front(prefix_ref)) {
+  new_spec.SetFile(entry.second.GetCString(), FileSpec::Style::native);
+  new_spec.AppendPathComponent(orig_ref);
+  if (FileSystem::Instance().Exists(new_spec))
+return true;
 }
   }
   


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

[Lldb-commits] [lldb] r353258 - Sort Xcode project

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb  5 17:30:45 2019
New Revision: 353258

URL: http://llvm.org/viewvc/llvm-project?rev=353258&view=rev
Log:
Sort Xcode project

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=353258&r1=353257&r2=353258&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Feb  5 17:30:45 2019
@@ -602,8 +602,8 @@
268900EE13353E6F00698AC0 /* PathMappingList.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 495BBACB119A0DBE00418BEA /* PathMappingList.cpp 
*/; };
2668A2EE20AF417D00D94111 /* PathMappingListTest.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 2668A2ED20AF417D00D94111 /* 
PathMappingListTest.cpp */; };
AF815DF921C855B400023A34 /* PdbAstBuilder.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF815DF721C855B400023A34 /* PdbAstBuilder.cpp 
*/; };
-   5A6424952204D05000C3D9DB /* PdbFPOProgramToDWARFExpression.h in 
Headers */ = {isa = PBXBuildFile; fileRef = 5A6424912204D04F00C3D9DB /* 
PdbFPOProgramToDWARFExpression.h */; };
5A6424972204D05000C3D9DB /* PdbFPOProgramToDWARFExpression.cpp 
in Sources */ = {isa = PBXBuildFile; fileRef = 5A6424932204D04F00C3D9DB /* 
PdbFPOProgramToDWARFExpression.cpp */; };
+   5A6424952204D05000C3D9DB /* PdbFPOProgramToDWARFExpression.h in 
Headers */ = {isa = PBXBuildFile; fileRef = 5A6424912204D04F00C3D9DB /* 
PdbFPOProgramToDWARFExpression.h */; };
AFD966BA217140B6006714AC /* PdbIndex.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = AFD966B6217140B6006714AC /* PdbIndex.cpp */; };
AF0F459E219FA1C800C1E612 /* PdbSymUid.cpp in Sources */ = {isa 
= PBXBuildFile; fileRef = AF0F459D219FA1C800C1E612 /* PdbSymUid.cpp */; };
AFD966B9217140B6006714AC /* PdbUtil.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = AFD966B5217140B6006714AC /* PdbUtil.cpp */; };
@@ -1620,8 +1620,8 @@
7F94D7172040A13A006EE3EA /* CleanUpTest.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = CleanUpTest.cpp; sourceTree = ""; };
949EEDA11BA76571008C63CF /* Cocoa.cpp */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = Cocoa.cpp; 
path = Language/ObjC/Cocoa.cpp; sourceTree = ""; };
949EEDA21BA76571008C63CF /* Cocoa.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = Cocoa.h; path = 
Language/ObjC/Cocoa.h; sourceTree = ""; };
-   5A6424942204D05000C3D9DB /* CodeViewRegisterMapping.h */ = {isa 
= PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name 
= CodeViewRegisterMapping.h; path = 
source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.h; sourceTree = 
SOURCE_ROOT; };
5A6424922204D04F00C3D9DB /* CodeViewRegisterMapping.cpp */ = 
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = 
sourcecode.cpp.cpp; name = CodeViewRegisterMapping.cpp; path = 
source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp; sourceTree = 
SOURCE_ROOT; };
+   5A6424942204D05000C3D9DB /* CodeViewRegisterMapping.h */ = {isa 
= PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name 
= CodeViewRegisterMapping.h; path = 
source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.h; sourceTree = 
SOURCE_ROOT; };
9441816D1C8F5EC900E5A8D9 /* CommandAlias.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = CommandAlias.cpp; path = source/Interpreter/CommandAlias.cpp; sourceTree 
= ""; };
9441816B1C8F5EB000E5A8D9 /* CommandAlias.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; name = CommandAlias.h; 
path = include/lldb/Interpreter/CommandAlias.h; sourceTree = ""; };
4C09CB74116BD98B00C7A725 /* CommandCompletions.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = CommandCompletions.cpp; path = source/Commands/CommandCompletions.cpp; 
sourceTree = ""; };
@@ -2441,8 +2441,6 @@
4C639EC321FA684700A7B957 /* OptionsWithRawTest.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = OptionsWithRawTest.cpp; sourceTree = ""; };
4C562CC21CC07DDD00C52EAC /* PDBASTParser.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = PDBASTParser.cpp; path = PDB/PDBASTParser.cpp; sourceTree = ""; };
4C562CC31CC07DDD00C52EAC /* PDBASTParser.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
PDBASTParser.h; path =

[Lldb-commits] [lldb] r353259 - [Reproducers] Add instrumentation to Xcode project

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb  5 17:31:00 2019
New Revision: 353259

URL: http://llvm.org/viewvc/llvm-project?rev=353259&view=rev
Log:
[Reproducers] Add instrumentation to Xcode project

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=353259&r1=353258&r2=353259&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Feb  5 17:31:00 2019
@@ -738,6 +738,8 @@
9485545A1DCBAE3B00345FF5 /* RenderScriptScriptGroup.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 948554591DCBAE3B00345FF5 /* 
RenderScriptScriptGroup.cpp */; };
23D065911D4A7BEE0008EDE6 /* RenderScriptx86ABIFixups.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 23D065861D4A7BDA0008EDE6 /* 
RenderScriptx86ABIFixups.cpp */; };
AFCB1D5F219CD5EA00730AD5 /* Reproducer.cpp in Sources */ = {isa 
= PBXBuildFile; fileRef = AFCB1D5E219CD5EA00730AD5 /* Reproducer.cpp */; };
+   DD6C13BB220A6F22005C2AE8 /* ReproducerInstrumentation.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = DD6C13BA220A6F21005C2AE8 /* 
ReproducerInstrumentation.cpp */; };
+   DD6C13BD220A6F6A005C2AE8 /* ReproducerInstrumentationTest.cpp 
in Sources */ = {isa = PBXBuildFile; fileRef = DD6C13BC220A6F6A005C2AE8 /* 
ReproducerInstrumentationTest.cpp */; };
4C639ECF21FA684900A7B957 /* ReproducerTest.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 4C639EC121FA684700A7B957 /* ReproducerTest.cpp 
*/; };
4FBC04ED211A06200015A814 /* RichManglingContext.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 4FBC04EC211A06200015A814 /* 
RichManglingContext.cpp */; };
4FBC04EF211A06820015A814 /* RichManglingContext.h in Headers */ 
= {isa = PBXBuildFile; fileRef = 4FBC04EE211A06820015A814 /* 
RichManglingContext.h */; };
@@ -2719,6 +2721,8 @@
23D065861D4A7BDA0008EDE6 /* RenderScriptx86ABIFixups.cpp */ = 
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = 
sourcecode.cpp.cpp; path = RenderScriptx86ABIFixups.cpp; sourceTree = 
""; };
23D065871D4A7BDA0008EDE6 /* RenderScriptx86ABIFixups.h */ = 
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; 
path = RenderScriptx86ABIFixups.h; sourceTree = ""; };
AFCB1D5E219CD5EA00730AD5 /* Reproducer.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = Reproducer.cpp; path = source/Utility/Reproducer.cpp; sourceTree = 
""; };
+   DD6C13BA220A6F21005C2AE8 /* ReproducerInstrumentation.cpp */ = 
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = 
sourcecode.cpp.cpp; name = ReproducerInstrumentation.cpp; path = 
source/Utility/ReproducerInstrumentation.cpp; sourceTree = ""; };
+   DD6C13BC220A6F6A005C2AE8 /* ReproducerInstrumentationTest.cpp 
*/ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = 
sourcecode.cpp.cpp; path = ReproducerInstrumentationTest.cpp; sourceTree = 
""; };
4C639EC121FA684700A7B957 /* ReproducerTest.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = ReproducerTest.cpp; sourceTree = ""; };
4FBC04EC211A06200015A814 /* RichManglingContext.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = RichManglingContext.cpp; path = source/Core/RichManglingContext.cpp; 
sourceTree = ""; };
4FBC04EE211A06820015A814 /* RichManglingContext.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
RichManglingContext.h; path = include/lldb/Core/RichManglingContext.h; 
sourceTree = ""; };
@@ -3656,6 +3660,7 @@
4C639EC021FA684700A7B957 /* PredicateTest.cpp 
*/,
4C639EC421FA684800A7B957 /* 
RegisterValueTest.cpp */,
4C639EC121FA684700A7B957 /* ReproducerTest.cpp 
*/,
+   DD6C13BC220A6F6A005C2AE8 /* 
ReproducerInstrumentationTest.cpp */,
23CB14E91D66CC0E00EDDDE1 /* ScalarTest.cpp */,
9A3D43E21F3237D500EB767C /* StateTest.cpp */,
9A3D43C61F3150D200EB767C /* StatusTest.cpp */,
@@ -4702,6 +4707,7 @@
26764C9C1E48F516008D3573 /* RegularExpression.h 
*/,
26764C9F1E48F528008D3573 /* 
RegularExpression.cpp */,
AFCB1D5E219CD5EA00730AD5 /* Reproducer.cpp */,
+   DD6C13BA220A6F21005C2AE8 /* 
ReproducerInstrumentation.cpp */,
26BC7D7410F

Re: [Lldb-commits] [lldb] r353257 - Little more cleanup on https://reviews.llvm.org/D57552

[Jonas Devlieghere via lldb-commits]

Thanks Jim!

On Tue, Feb 5, 2019 at 5:27 PM Jim Ingham via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: jingham
> Date: Tue Feb  5 17:27:45 2019
> New Revision: 353257
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353257&view=rev
> Log:
> Little more cleanup on https://reviews.llvm.org/D57552
> Thanks Jonas...  One more early continue and using
> a range where we had an iterator.
>
> NFC
>
> Modified:
> lldb/trunk/source/Target/PathMappingList.cpp
>
> Modified: lldb/trunk/source/Target/PathMappingList.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/PathMappingList.cpp?rev=353257&r1=353256&r2=353257&view=diff
>
> ==
> --- lldb/trunk/source/Target/PathMappingList.cpp (original)
> +++ lldb/trunk/source/Target/PathMappingList.cpp Tue Feb  5 17:27:45 2019
> @@ -211,33 +211,32 @@ bool PathMappingList::FindFile(const Fil
>
>bool orig_is_relative = orig_spec.IsRelative();
>
> -  const_iterator pos, end = m_pairs.end();
> -  for (pos = m_pairs.begin(); pos != end; ++pos) {
> +  for (auto entry : m_pairs) {
>  llvm::StringRef orig_ref(orig_path);
> -llvm::StringRef prefix_ref = pos->first.GetStringRef();
> -if (orig_ref.size() >= prefix_ref.size()) {
> -  // We consider a relative prefix or one of just "." to
> -  // mean "only apply to relative paths".
> -  bool prefix_is_relative = false;
> -
> -  if (prefix_ref == ".") {
> -prefix_is_relative = true;
> -// Remove the "." since it will have been removed from the
> -// FileSpec paths already.
> -prefix_ref = prefix_ref.drop_front();
> -  } else {
> -FileSpec prefix_spec(prefix_ref, FileSpec::Style::native);
> -prefix_is_relative = prefix_spec.IsRelative();
> -  }
> -  if (prefix_is_relative != orig_is_relative)
> -continue;
> +llvm::StringRef prefix_ref = entry.first.GetStringRef();
> +if (orig_ref.size() < prefix_ref.size())
> +  continue;
> +// We consider a relative prefix or one of just "." to
> +// mean "only apply to relative paths".
> +bool prefix_is_relative = false;
> +
> +if (prefix_ref == ".") {
> +  prefix_is_relative = true;
> +  // Remove the "." since it will have been removed from the
> +  // FileSpec paths already.
> +  prefix_ref = prefix_ref.drop_front();
> +} else {
> +  FileSpec prefix_spec(prefix_ref, FileSpec::Style::native);
> +  prefix_is_relative = prefix_spec.IsRelative();
> +}
> +if (prefix_is_relative != orig_is_relative)
> +  continue;
>
> -  if (orig_ref.consume_front(prefix_ref)) {
> -new_spec.SetFile(pos->second.GetCString(),
> FileSpec::Style::native);
> -new_spec.AppendPathComponent(orig_ref);
> -if (FileSystem::Instance().Exists(new_spec))
> -  return true;
> -  }
> +if (orig_ref.consume_front(prefix_ref)) {
> +  new_spec.SetFile(entry.second.GetCString(),
> FileSpec::Style::native);
> +  new_spec.AppendPathComponent(orig_ref);
> +  if (FileSystem::Instance().Exists(new_spec))
> +return true;
>  }
>}
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r353268 - [CMake] Unify scripts for generating VCS headers

[Petr Hosek via lldb-commits]

Author: phosek
Date: Tue Feb  5 19:51:00 2019
New Revision: 353268

URL: http://llvm.org/viewvc/llvm-project?rev=353268&view=rev
Log:
[CMake] Unify scripts for generating VCS headers

Previously, there were two different scripts for generating VCS headers:
one used by LLVM and one used by Clang and lldb. They were both similar,
but different. They were both broken in their own ways, for example the
one used by Clang didn't properly handle monorepo resulting in an
incorrect version information reported by Clang.

This change unifies two the scripts by introducing a new script that's
used from both LLVM, Clang and lldb, ensures that the new script
supports both monorepo and standalone SVN and Git setups, and removes
the old scripts.

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

Modified:
lldb/trunk/source/CMakeLists.txt
lldb/trunk/source/lldb.cpp

Modified: lldb/trunk/source/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/CMakeLists.txt?rev=353268&r1=353267&r2=353268&view=diff
==
--- lldb/trunk/source/CMakeLists.txt (original)
+++ lldb/trunk/source/CMakeLists.txt Tue Feb  5 19:51:00 2019
@@ -15,30 +15,30 @@ foreach(file
   endif()
 endforeach()
 
-if(DEFINED lldb_vc)
-  set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/SVNVersion.inc")
-  set(get_svn_script "${LLVM_CMAKE_PATH}/GetSVN.cmake")
-
-  # Create custom target to generate the VC revision include.
-  add_custom_command(OUTPUT "${version_inc}"
-DEPENDS "${lldb_vc}" "${get_svn_script}"
-COMMAND
-${CMAKE_COMMAND} "-DFIRST_SOURCE_DIR=${LLDB_SOURCE_DIR}"
- "-DFIRST_NAME=LLDB"
- "-DHEADER_FILE=${version_inc}"
- -P "${get_svn_script}")
-
-  # Mark the generated header as being generated.
-  set_source_files_properties("${version_inc}"
-PROPERTIES GENERATED TRUE
-   HEADER_FILE_ONLY TRUE)
-
-  # Tell Version.cpp that it needs to build with -DHAVE_SVN_VERSION_INC.
-  set_property(SOURCE lldb.cpp APPEND PROPERTY 
-   COMPILE_DEFINITIONS "HAVE_SVN_VERSION_INC")
-  list(APPEND lldbBase_SOURCES ${version_inc})
+set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
+set(generate_vcs_version_script 
"${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
+
+if(lldb_vc)
+  set(lldb_source_dir ${LLDB_SOURCE_DIR})
 endif()
 
+add_custom_command(OUTPUT "${version_inc}"
+  DEPENDS "${lldb_vc}" "${generate_vcs_version_script}"
+  COMMAND ${CMAKE_COMMAND} "-DNAMES=LLDB"
+   "-DLLDB_SOURCE_DIR=${LLDB_SOURCE_DIR}"
+   "-DHEADER_FILE=${version_inc}"
+   -P "${generate_vcs_version_script}")
+
+# Mark the generated header as being generated.
+set_source_files_properties("${version_inc}"
+  PROPERTIES GENERATED TRUE
+ HEADER_FILE_ONLY TRUE)
+
+set_property(SOURCE lldb.cpp APPEND PROPERTY
+ COMPILE_DEFINITIONS "HAVE_VCS_VERSION_INC")
+
+list(APPEND lldbBase_SOURCES ${version_inc})
+
 if(APPLE)
   set(apple_version_inc "${CMAKE_CURRENT_BINARY_DIR}/AppleVersion.inc")
   set(apple_version_script 
"${LLDB_SOURCE_DIR}/cmake/modules/EmbedAppleVersion.cmake")

Modified: lldb/trunk/source/lldb.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/lldb.cpp?rev=353268&r1=353267&r2=353268&view=diff
==
--- lldb/trunk/source/lldb.cpp (original)
+++ lldb/trunk/source/lldb.cpp Tue Feb  5 19:51:00 2019
@@ -13,8 +13,8 @@ using namespace lldb_private;
 
 #include "clang/Basic/Version.h"
 
-#ifdef HAVE_SVN_VERSION_INC
-#include "SVNVersion.inc"
+#ifdef HAVE_VCS_VERSION_INC
+#include "VCSVersion.inc"
 #endif
 
 #ifdef HAVE_APPLE_VERSION_INC


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

[Lldb-commits] [lldb] r353269 - Add a warning to GDBRemoteRegisterContext (if packet logging enabled)

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Feb  5 20:08:09 2019
New Revision: 353269

URL: http://llvm.org/viewvc/llvm-project?rev=353269&view=rev
Log:
Add a warning to GDBRemoteRegisterContext (if packet logging enabled)
if the size of the g packet response was smaller than expected and is
going to be ignored.


Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp?rev=353269&r1=353268&r2=353269&view=diff
==
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp 
Tue Feb  5 20:08:09 2019
@@ -205,6 +205,14 @@ bool GDBRemoteRegisterContext::ReadRegis
 if (buffer_sp->GetByteSize() >= m_reg_data.GetByteSize()) {
   SetAllRegisterValid(true);
   return true;
+} else {
+  Log 
*log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_THREAD |
+
GDBR_LOG_PACKETS));
+  if (log)
+log->Printf ("error: GDBRemoteRegisterContext::ReadRegisterBytes 
tried to read the "
+"entire register context at once, expected at least %" 
PRId64 " bytes "
+"but only got %" PRId64 " bytes.", 
m_reg_data.GetByteSize(),
+buffer_sp->GetByteSize());
 }
   }
   return false;


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

[Lldb-commits] [lldb] r353271 - [Reproducers] lldb-instr: tool to generate instrumentation macros.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Tue Feb  5 20:33:14 2019
New Revision: 353271

URL: http://llvm.org/viewvc/llvm-project?rev=353271&view=rev
Log:
[Reproducers] lldb-instr: tool to generate instrumentation macros.

This patch introduces a new tool called 'lldb-instr'. It automates the
workflow of inserting LLDB_RECORD and LLDB_REGSITER macros for
instrumentation.

Because the tool won't be part of the build process, I didn't want to
over-complicate it. SB_RECORD macros are inserted in place, while
SB_REGISTER macros are printed to stdout, and have to be manually copied
into the Registry's constructor. Additionally, the utility makes no
attempt to properly format the inserted macros. Please use clang-format
to format the changes after running the tool.

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

Added:
lldb/trunk/lit/Reproducer/Inputs/foo.cpp
lldb/trunk/lit/Reproducer/TestInstrumentationRecord.test
lldb/trunk/lit/Reproducer/TestInstrumentationRegister.test
lldb/trunk/tools/lldb-instr/
lldb/trunk/tools/lldb-instr/CMakeLists.txt
lldb/trunk/tools/lldb-instr/Instrument.cpp
Modified:
lldb/trunk/lit/helper/toolchain.py
lldb/trunk/tools/CMakeLists.txt

Added: lldb/trunk/lit/Reproducer/Inputs/foo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/Inputs/foo.cpp?rev=353271&view=auto
==
--- lldb/trunk/lit/Reproducer/Inputs/foo.cpp (added)
+++ lldb/trunk/lit/Reproducer/Inputs/foo.cpp Tue Feb  5 20:33:14 2019
@@ -0,0 +1,18 @@
+struct Foo {
+  Foo();
+  Foo(int i);
+
+  void A();
+  void B(int i);
+  int C(int i);
+  int D(bool b) const;
+  static void E();
+  static int F(int i);
+};
+
+void Foo::A() {}
+void Foo::B(int i) {}
+int Foo::C(int i) { return i; }
+int Foo::D(bool b) const { return 1; }
+void Foo::E() {}
+int Foo::F(int i) { return i; }

Added: lldb/trunk/lit/Reproducer/TestInstrumentationRecord.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/TestInstrumentationRecord.test?rev=353271&view=auto
==
--- lldb/trunk/lit/Reproducer/TestInstrumentationRecord.test (added)
+++ lldb/trunk/lit/Reproducer/TestInstrumentationRecord.test Tue Feb  5 
20:33:14 2019
@@ -0,0 +1,10 @@
+# RUN: cp %p/Inputs/foo.cpp %t.cpp
+# RUN: lldb-instr %t.cpp
+# RUN: cat %t.cpp | FileCheck %s
+
+# CHECK: LLDB_RECORD_METHOD_NO_ARGS(void, Foo, A);
+# CHECK: LLDB_RECORD_METHOD(void, Foo, B, (int), i);
+# CHECK: LLDB_RECORD_METHOD(int, Foo, C, (int), i);
+# CHECK: LLDB_RECORD_METHOD_CONST(int, Foo, D, (bool), b);
+# CHECK: LLDB_RECORD_STATIC_METHOD_NO_ARGS(void, Foo, E);
+# CHECK: LLDB_RECORD_STATIC_METHOD(int, Foo, F, (int), i);

Added: lldb/trunk/lit/Reproducer/TestInstrumentationRegister.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/TestInstrumentationRegister.test?rev=353271&view=auto
==
--- lldb/trunk/lit/Reproducer/TestInstrumentationRegister.test (added)
+++ lldb/trunk/lit/Reproducer/TestInstrumentationRegister.test Tue Feb  5 
20:33:14 2019
@@ -0,0 +1,9 @@
+# RUN: cp %p/Inputs/foo.cpp %t.cpp
+# RUN: lldb-instr %t.cpp | FileCheck %s
+
+# CHECK: LLDB_REGISTER_METHOD(void, Foo, A, ());
+# CHECK: LLDB_REGISTER_METHOD(void, Foo, B, (int));
+# CHECK: LLDB_REGISTER_METHOD(int, Foo, C, (int));
+# CHECK: LLDB_REGISTER_METHOD_CONST(int, Foo, D, (bool));
+# CHECK: LLDB_REGISTER_STATIC_METHOD(void, Foo, E, ());
+# CHECK: LLDB_REGISTER_STATIC_METHOD(int, Foo, F, (int));

Modified: lldb/trunk/lit/helper/toolchain.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/helper/toolchain.py?rev=353271&r1=353270&r2=353271&view=diff
==
--- lldb/trunk/lit/helper/toolchain.py (original)
+++ lldb/trunk/lit/helper/toolchain.py Tue Feb  5 20:33:14 2019
@@ -24,7 +24,7 @@ def use_lldb_substitutions(config):
 
 build_script = os.path.dirname(__file__)
 build_script = os.path.join(build_script, 'build.py')
-build_script_args = [build_script, 
+build_script_args = [build_script,
 '--compiler=any', # Default to best compiler
 '--arch=' + str(config.lldb_bitness)]
 if config.lldb_lit_tools_dir:
@@ -44,6 +44,7 @@ def use_lldb_substitutions(config):
   extra_args=dsargs,
   unresolved='ignore'),
 'lldb-test',
+'lldb-instr',
 ToolSubst('%build',
   command="'" + sys.executable + "'",
   extra_args=build_script_args)

Modified: lldb/trunk/tools/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/CMakeLists.txt?rev=353271&r1=353270&r2=353271&view=diff
==
--- lldb/trunk/tools/CMakeLists.txt (original)
+++ lldb/tr

[Lldb-commits] [PATCH] D56822: [Reproducers] Tool to insert SBReproducer macros

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB353271: [Reproducers] lldb-instr: tool to generate 
instrumentation macros. (authored by JDevlieghere, committed by ).
Herald added a reviewer: serge-sans-paille.
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D56822?vs=185434&id=185486#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56822

Files:
  lit/Reproducer/Inputs/foo.cpp
  lit/Reproducer/TestInstrumentationRecord.test
  lit/Reproducer/TestInstrumentationRegister.test
  lit/helper/toolchain.py
  tools/CMakeLists.txt
  tools/lldb-instr/CMakeLists.txt
  tools/lldb-instr/Instrument.cpp

Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
+add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
 add_subdirectory(lldb-vscode)
Index: tools/lldb-instr/CMakeLists.txt
===
--- tools/lldb-instr/CMakeLists.txt
+++ tools/lldb-instr/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_lldb_tool(lldb-instr
+  Instrument.cpp
+
+  LINK_LIBS
+clangAST
+clangBasic
+clangFrontend
+clangTooling
+
+  LINK_COMPONENTS
+Support
+  )
Index: tools/lldb-instr/Instrument.cpp
===
--- tools/lldb-instr/Instrument.cpp
+++ tools/lldb-instr/Instrument.cpp
@@ -0,0 +1,285 @@
+#include "clang/AST/AST.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/ASTConsumers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+#include 
+
+using namespace clang;
+using namespace clang::driver;
+using namespace clang::tooling;
+
+static llvm::cl::OptionCategory InstrCategory("LLDB Instrumentation Generator");
+
+/// Get the macro name for recording method calls.
+///
+/// LLDB_RECORD_METHOD
+/// LLDB_RECORD_METHOD_CONST
+/// LLDB_RECORD_METHOD_NO_ARGS
+/// LLDB_RECORD_METHOD_CONST_NO_ARGS
+/// LLDB_RECORD_STATIC_METHOD
+/// LLDB_RECORD_STATIC_METHOD_NO_ARGS
+static std::string GetRecordMethodMacroName(bool Static, bool Const,
+bool NoArgs) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << "LLDB_RECORD";
+  if (Static)
+OS << "_STATIC";
+  OS << "_METHOD";
+  if (Const)
+OS << "_CONST";
+  if (NoArgs)
+OS << "_NO_ARGS";
+
+  return OS.str();
+}
+
+/// Get the macro name for register methods.
+///
+/// LLDB_REGISTER_CONSTRUCTOR
+/// LLDB_REGISTER_METHOD
+/// LLDB_REGISTER_METHOD_CONST
+/// LLDB_REGISTER_STATIC_METHOD
+static std::string GetRegisterMethodMacroName(bool Static, bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << "LLDB_REGISTER";
+  if (Static)
+OS << "_STATIC";
+  OS << "_METHOD";
+  if (Const)
+OS << "_CONST";
+
+  return OS.str();
+}
+
+static std::string GetRecordMethodMacro(StringRef Result, StringRef Class,
+StringRef Method, StringRef Signature,
+StringRef Values, bool Static,
+bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << GetRecordMethodMacroName(Static, Const, Values.empty());
+  OS << "(" << Result << ", " << Class << ", " << Method;
+
+  if (!Values.empty()) {
+OS << ", (" << Signature << "), " << Values << ");\n\n";
+  } else {
+OS << ");\n\n";
+  }
+
+  return OS.str();
+}
+
+static std::string GetRecordConstructorMacro(StringRef Class,
+ StringRef Signature,
+ StringRef Values) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  if (!Values.empty()) {
+OS << "LLDB_RECORD_CONSTRUCTOR(" << Class << ", (" << Signature << "), "
+   << Values << ");\n\n";
+  } else {
+OS << "LLDB_RECORD_CONSTRUCTOR_NO_ARGS(" << Class << ");\n\n";
+  }
+  return OS.str();
+}
+
+static std::string GetRegisterConstructorMacro(StringRef Class,
+   StringRef Signature) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  OS << "LLDB_REGISTER_CONSTRUCTOR(" << Class << ", (" << Signature
+ << "));\n\n";
+  return OS.str();
+}
+
+static std::string GetRegisterMethodMacro(StringRef Result, StringRef Class,
+  S