[Lldb-commits] [PATCH] D81423: 1/2: [nfc] [lldb] Reduce GetAttributes's depth parameter usage

2020-06-09 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:733
   DWARFAttributes attributes;
-  GetAttributes(cu, attributes);
+  GetAttributes(cu, attributes, 0 /* curr_depth */);
   return GetParentDeclContextDIE(cu, attributes);

I guess this could keep calling the public version of this function without the 
extra argument.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:783
   DWARFAttributes attributes;
-  GetAttributes(cu, attributes);
+  GetAttributes(cu, attributes, 0 /* curr_depth */);
   return GetQualifiedName(cu, attributes, storage);

and this too(?)



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:182
+   uint32_t curr_depth)
+  const; // "curr_depth" for internal use only, don't set this yourself!!!
 };

That comment is probably not useful anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81423



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


[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I think this looks good now. All that's now left is a test case. I think it 
should be possible to test this in a number of ways:

- CommandReturnObject unittest
- a shell test which forces `use-color` to true
- a python test which forces `use-color` to true
- a pexpect test




Comment at: lldb/include/lldb/Utility/Stream.h:402-403
 
+bool has_colors() const override { return true; }
+
 uint64_t current_pos() const override {

This is not consistent with the intend of this method (`This function 
determines if this stream is displayed and supports colors.`). One could argue 
whether this could be "true" for CommandObjectResult streams which are destined 
to be later printed to a terminal, but it's definitely not a good default.

However, all of that is irrelevant, as this should not be needed anymore. Is 
that correct?



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:212
 
-  CommandReturnObject result;
+  CommandReturnObject result(m_debugger.GetUseColor());
 

I'm wondering in how many places do we construct `CommandReturnObject`s. If 
it's not too many, it may make sense to remove the `=false` from the colors 
argument, to force the user to think about the right value...


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

https://reviews.llvm.org/D81058



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


[Lldb-commits] [PATCH] D81334: 2/2: [lldb] Fix DW_TAG_GNU_call_site-DW_AT_low_pc as produced by GCC

2020-06-09 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h:113
 
-  size_t GetAttributes(DWARFAttributes &attributes, uint32_t depth = 0) const;
+  size_t GetAttributes(DWARFAttributes &attributes, int32_t depth = 0) const;
 

jankratochvil wrote:
> labath wrote:
> > Using -1 to prevent recursion is pretty unobvious. I think it would be 
> > better to add a `bool recurse = true` argument between `attributes` and 
> > `depth`. In fact, I'd consider even deleting the `depth` argument -- it's 
> > an internal detail that noone except `DWARFDebugInfoEntry` should be using 
> > and the single usage there can be easily changed to 
> > `spec_die.GetDIE()->GetAttributes(spec_die.GetUnit(), attributes, recurse, 
> > depth+1)`
> I had this idea. :-) The problem is `bool recurse` is type-compatible with 
> `uint32_t depth` which leads to uncaught bugs. Given that non-recursive call 
> makes no sense with `depth>0` I find the special value `depth = -1` to be 
> suitable for non-recursive calls.
> Anyway implemented the `recurse` parameter in a safe way if it is worth it to 
> separate it.
> 
Right, I see. I don't think that a special tagged enum is really necessary here 
and flushing out the callers via two patches (like you did) would be enough. 
The only ones who that may impact are downstream users who merge in bulk, but: 
a) I doubt the downstream users are calling this function in their own code; b) 
we don't have to work extra hard to safeguard them.

However, given that you've already done it, I think it's fine to keep it -- 
though I'll probably change it back next time I work on this function.



Comment at: 
lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s:46
+   callb
+int3
+   ret

This is formatted strangely. Tab issues?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81334



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


[Lldb-commits] [lldb] 17798c6 - [lldb] Fix -Wmissing-field-initializers in StackFrameList

2020-06-09 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-06-09T11:58:08+02:00
New Revision: 17798c60bcc284e45529d6afde11bf59ffc549c8

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

LOG: [lldb] Fix -Wmissing-field-initializers in StackFrameList

The code is correct without these default values, but it is freaking the
compiler out.

Added: 


Modified: 
lldb/source/Target/StackFrameList.cpp

Removed: 




diff  --git a/lldb/source/Target/StackFrameList.cpp 
b/lldb/source/Target/StackFrameList.cpp
index 8549ac079b02..e4f5d3028366 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -241,8 +241,8 @@ void StackFrameList::GetOnlyConcreteFramesUpTo(uint32_t 
end_idx,
 /// callee.
 struct CallDescriptor {
   Function *func;
-  CallEdge::AddrType address_type;
-  addr_t address;
+  CallEdge::AddrType address_type = CallEdge::AddrType::Call;
+  addr_t address = LLDB_INVALID_ADDRESS;
 };
 using CallSequence = std::vector;
 



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


[Lldb-commits] [lldb] fd31e60 - [nfc] [lldb] Reduce GetAttributes's depth parameter usage

2020-06-09 Thread Jan Kratochvil via lldb-commits

Author: Jan Kratochvil
Date: 2020-06-09T13:41:06+02:00
New Revision: fd31e60b8ded16754f484b51c2e5f8da05ad8331

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

LOG: [nfc] [lldb] Reduce GetAttributes's depth parameter usage

Clean the code up a bit for D81334.

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
index 8bd21c65d28b..c330effc080b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
@@ -104,12 +104,10 @@ bool 
DWARFBaseDIE::Supports_DW_AT_APPLE_objc_complete_type() const {
   return IsValid() && 
GetDWARF()->Supports_DW_AT_APPLE_objc_complete_type(m_cu);
 }
 
-size_t DWARFBaseDIE::GetAttributes(DWARFAttributes &attributes,
-   uint32_t depth) const {
+size_t DWARFBaseDIE::GetAttributes(DWARFAttributes &attributes) const {
   if (IsValid())
-return m_die->GetAttributes(m_cu, attributes, depth);
-  if (depth == 0)
-attributes.Clear();
+return m_die->GetAttributes(m_cu, attributes);
+  attributes.Clear();
   return 0;
 }
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
index 48320c8b06de..0bad53ff4329 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
@@ -110,7 +110,7 @@ class DWARFBaseDIE {
   uint64_t GetAttributeValueAsAddress(const dw_attr_t attr,
   uint64_t fail_value) const;
 
-  size_t GetAttributes(DWARFAttributes &attributes, uint32_t depth = 0) const;
+  size_t GetAttributes(DWARFAttributes &attributes) const;
 
 protected:
   DWARFUnit *m_cu;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index 7b96c15bf3f9..a133ce126027 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -436,7 +436,8 @@ size_t DWARFDebugInfoEntry::GetAttributes(
 if (form_value.ExtractValue(data, &offset)) {
   DWARFDIE spec_die = form_value.Reference();
   if (spec_die)
-spec_die.GetAttributes(attributes, curr_depth + 1);
+spec_die.GetDIE()->GetAttributes(spec_die.GetCU(), attributes,
+ curr_depth + 1);
 }
   } else {
 llvm::Optional fixed_skip_size = 
DWARFFormValue::GetFixedSize(form, cu);

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index 3fb9c9135e85..7bcff312b1c5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -47,10 +47,9 @@ class DWARFDebugInfoEntry {
   bool Extract(const lldb_private::DWARFDataExtractor &data,
const DWARFUnit *cu, lldb::offset_t *offset_ptr);
 
-  size_t GetAttributes(const DWARFUnit *cu,
-   DWARFAttributes &attrs,
-   uint32_t curr_depth = 0)
-  const; // "curr_depth" for internal use only, don't set this yourself!!!
+  size_t GetAttributes(const DWARFUnit *cu, DWARFAttributes &attrs) const {
+return GetAttributes(cu, attrs, 0 /* curr_depth */);
+  }
 
   dw_offset_t
   GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
@@ -176,6 +175,10 @@ class DWARFDebugInfoEntry {
   /// A copy of the DW_TAG value so we don't have to go through the compile
   /// unit abbrev table
   dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
+
+private:
+  size_t GetAttributes(const DWARFUnit *cu, DWARFAttributes &attrs,
+   uint32_t curr_depth) const;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGINFOENTRY_H



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


[Lldb-commits] [lldb] 4515d35 - [lldb] Fix DW_TAG_GNU_call_site-DW_AT_low_pc as produced by GCC

2020-06-09 Thread Jan Kratochvil via lldb-commits

Author: Jan Kratochvil
Date: 2020-06-09T13:41:41+02:00
New Revision: 4515d35f5c9e2bdcdbcb9fe9028d4a84871958c5

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

LOG: [lldb] Fix DW_TAG_GNU_call_site-DW_AT_low_pc as produced by GCC

D80519 
added support for `DW_TAG_GNU_call_site` but
Bug 45886 
found one case did not work.

There is:

  0x00b1: DW_TAG_GNU_call_site
DW_AT_low_pc  (0x0040111e)
DW_AT_abstract_origin (0x00cc "a")
  ...
  0x00cc:   DW_TAG_subprogram
  DW_AT_name  ("a")
  DW_AT_prototyped(true)
  DW_AT_low_pc(0x00401109)
   - here it did overwrite the 'low_pc' variable 
containing value 0x40111e we wanted
  DW_AT_high_pc   (0x00401114)
  DW_AT_frame_base(DW_OP_call_frame_cfa)
  DW_AT_GNU_all_call_sites(true)

DW_TAG_GNU_call_site attributes order as produced by GCC:
0x00b1: DW_TAG_GNU_call_site
  DW_AT_low_pc  (0x0040111e)
  DW_AT_abstract_origin (0x00cc "a")

clang produces the attributes in opposite order:
0x0064: DW_TAG_GNU_call_site
  DW_AT_abstract_origin (0x002a "a")
  DW_AT_low_pc  (0x00401146)

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s

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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
index c330effc080b..fcb424029f5a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
@@ -104,9 +104,10 @@ bool 
DWARFBaseDIE::Supports_DW_AT_APPLE_objc_complete_type() const {
   return IsValid() && 
GetDWARF()->Supports_DW_AT_APPLE_objc_complete_type(m_cu);
 }
 
-size_t DWARFBaseDIE::GetAttributes(DWARFAttributes &attributes) const {
+size_t DWARFBaseDIE::GetAttributes(DWARFAttributes &attributes,
+   Recurse recurse) const {
   if (IsValid())
-return m_die->GetAttributes(m_cu, attributes);
+return m_die->GetAttributes(m_cu, attributes, recurse);
   attributes.Clear();
   return 0;
 }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
index 0bad53ff4329..059b84864be7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
@@ -110,7 +110,9 @@ class DWARFBaseDIE {
   uint64_t GetAttributeValueAsAddress(const dw_attr_t attr,
   uint64_t fail_value) const;
 
-  size_t GetAttributes(DWARFAttributes &attributes) const;
+  enum class Recurse : bool { no, yes };
+  size_t GetAttributes(DWARFAttributes &attributes,
+   Recurse recurse = Recurse::yes) const;
 
 protected:
   DWARFUnit *m_cu;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index a133ce126027..f6425a889da8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -399,9 +399,10 @@ bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
 // specification or abstract origin attributes and including those in the
 // results. Any duplicate attributes will have the first instance take
 // precedence (this can happen for declaration attributes).
-size_t DWARFDebugInfoEntry::GetAttributes(
-const DWARFUnit *cu, DWARFAttributes &attributes,
-uint32_t curr_depth) const {
+size_t DWARFDebugInfoEntry::GetAttributes(const DWARFUnit *cu,
+  DWARFAttributes &attributes,
+  Recurse recurse,
+  uint32_t curr_depth) const {
   const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
   if (abbrevDecl) {
 const DWARFDataExtractor &data = cu->GetData();
@@ -432,12 +433,13 @@ size_t DWARFDebugInfoEntry::GetAttributes(
 break;
   }
 
-  if ((attr == DW_AT_specification) || (attr == DW_AT_abstract_origin)) {
+  if (recurse == Recurs

[Lldb-commits] [PATCH] D81334: 2/2: [lldb] Fix DW_TAG_GNU_call_site-DW_AT_low_pc as produced by GCC

2020-06-09 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked 5 inline comments as done.
jankratochvil added inline comments.



Comment at: 
lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s:46
+   callb
+int3
+   ret

labath wrote:
> This is formatted strangely. Tab issues?
Yes, thanks. Some *.s files have tabs some spaces so I kept it with tabs (and 
fixed this one line).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81334



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


[Lldb-commits] [PATCH] D81334: 2/2: [lldb] Fix DW_TAG_GNU_call_site-DW_AT_low_pc as produced by GCC

2020-06-09 Thread Jan Kratochvil via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
jankratochvil marked an inline comment as done.
Closed by commit rG4515d35f5c9e: [lldb] Fix DW_TAG_GNU_call_site-DW_AT_low_pc 
as produced by GCC (authored by jankratochvil).

Changed prior to commit:
  https://reviews.llvm.org/D81334?vs=269328&id=269496#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81334

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s
@@ -0,0 +1,230 @@
+# This tests that lldb is compatible with DWARF-4 entry values GNU extension
+# with DW_TAG_GNU_call_site attributes order as produced by GCC:
+# 0x00b1: DW_TAG_GNU_call_site
+#   DW_AT_low_pc  (0x0040111e)
+#   DW_AT_abstract_origin (0x00cc "a")
+# clang produces the attributes in opposite order:
+# 0x0064: DW_TAG_GNU_call_site
+#   DW_AT_abstract_origin (0x002a "a")
+#   DW_AT_low_pc  (0x00401146)
+
+# REQUIRES: target-x86_64, system-linux, lld
+
+# RUN: %clang_host -o %t %s
+# RUN: %lldb %t -o r -o 'p p' -o exit | FileCheck %s
+
+# CHECK: (int) $0 = 1
+
+# The DWARF has been produced and modified from:
+# static __attribute__((noinline, noclone)) void b(int x) {
+#   asm("");
+# }
+# static __attribute__((noinline, noclone)) void a(int p) {
+#   b(2);
+# }
+# int main() {
+#   a(1);
+#   return 0;
+# }
+
+	.text
+.Ltext0:
+	.type	b, @function
+b:
+	.cfi_startproc
+	ret
+	.cfi_endproc
+	.size	b, .-b
+	.type	a, @function
+a:
+.LVL1:
+.LFB1:
+	.cfi_startproc
+	movl	$2, %edi
+.LVL2:
+	call	b
+	int3
+	ret
+	.cfi_endproc
+.LFE1:
+	.size	a, .-a
+	.globl	main
+	.type	main, @function
+main:
+.LFB2:
+	.cfi_startproc
+	movl	$1, %edi
+	call	a
+.LVL4:
+	movl	$0, %eax
+	ret
+	.cfi_endproc
+.LFE2:
+	.size	main, .-main
+.Letext0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	.Ldebuginfo_end - .Ldebuginfo_start	# Length of Compilation Unit Info
+.Ldebuginfo_start:
+	.value	0x4	# DWARF version number
+	.long	.Ldebug_abbrev0	# Offset Into Abbrev. Section
+	.byte	0x8	# Pointer Size (in bytes)
+	.uleb128 0x1	# (DIE (0xb) DW_TAG_compile_unit)
+	.asciz "GNU C17 10.1.1 20200507 (Red Hat 10.1.1-1) -mtune=generic -march=x86-64 -g -Og"	# DW_AT_producer: "GNU C17 10.1.1 20200507 (Red Hat 10.1.1-1) -mtune=generic -march=x86-64 -g -Og"
+	.byte	0xc	# DW_AT_language
+	.asciz "DW_TAG_GNU_call_site-DW_AT_low_pc.c"	# DW_AT_name
+	.asciz ""	# DW_AT_comp_dir: "/home/jkratoch/t"
+	.quad	.Ltext0	# DW_AT_low_pc
+	.quad	.Letext0-.Ltext0	# DW_AT_high_pc
+	.uleb128 0x2	# (DIE (0x2d) DW_TAG_subprogram)
+			# DW_AT_external
+	.asciz "main"	# DW_AT_name: "main"
+	.long	.Ltype_int	# DW_AT_type
+	.quad	.LFB2	# DW_AT_low_pc
+	.quad	.LFE2-.LFB2	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x3	# (DIE (0x4f) DW_TAG_GNU_call_site)
+	.quad	.LVL4	# DW_AT_low_pc
+	.long	.Lfunc_a	# DW_AT_abstract_origin
+	.uleb128 0x4	# (DIE (0x5c) DW_TAG_GNU_call_site_parameter)
+	.uleb128 0x1	# DW_AT_location
+	.byte	0x55	# DW_OP_reg5
+	.uleb128 0x1	# DW_AT_GNU_call_site_value
+	.byte	0x31	# DW_OP_lit1
+	.byte	0	# end of children of DIE 0x4f
+	.byte	0	# end of children of DIE 0x2d
+.Ltype_int:
+	.uleb128 0x5	# (DIE (0x63) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.asciz "int"	# DW_AT_name
+.Lfunc_a:
+	.uleb128 0x6	# (DIE (0x6a) DW_TAG_subprogram)
+	.asciz "a"	# DW_AT_name
+			# DW_AT_prototyped
+	.quad	.LFB1	# DW_AT_low_pc
+	.quad	.LFE1-.LFB1	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x7	# (DIE (0x86) DW_TAG_formal_parameter)
+	.asciz "p"	# DW_AT_name
+	.long	.Ltype_int	# DW_AT_type
+	.long	.LLST0	# DW_AT_location
+	.byte	0	# end of children of DIE 0x6a
+	.byte	0	# end of children of DIE 0xb
+.Ldebuginfo_end:
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	# (abbrev code)
+	.uleb128 0x11	# (TAG: DW_TAG_compile_unit)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x25	# (DW_AT_producer)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x13	# (DW_AT_language)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x1b	# (DW_AT_comp_dir)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (

[Lldb-commits] [PATCH] D81423: 1/2: [nfc] [lldb] Reduce GetAttributes's depth parameter usage

2020-06-09 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked 4 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:733
   DWARFAttributes attributes;
-  GetAttributes(cu, attributes);
+  GetAttributes(cu, attributes, 0 /* curr_depth */);
   return GetParentDeclContextDIE(cu, attributes);

labath wrote:
> I guess this could keep calling the public version of this function without 
> the extra argument.
It was intentional but in fact you are right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81423



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


[Lldb-commits] [PATCH] D81423: 1/2: [nfc] [lldb] Reduce GetAttributes's depth parameter usage

2020-06-09 Thread Jan Kratochvil via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
jankratochvil marked an inline comment as done.
Closed by commit rGfd31e60b8ded: [nfc] [lldb] Reduce GetAttributes's depth 
parameter usage (authored by jankratochvil).

Changed prior to commit:
  https://reviews.llvm.org/D81423?vs=269326&id=269495#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81423

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -47,10 +47,9 @@
   bool Extract(const lldb_private::DWARFDataExtractor &data,
const DWARFUnit *cu, lldb::offset_t *offset_ptr);
 
-  size_t GetAttributes(const DWARFUnit *cu,
-   DWARFAttributes &attrs,
-   uint32_t curr_depth = 0)
-  const; // "curr_depth" for internal use only, don't set this yourself!!!
+  size_t GetAttributes(const DWARFUnit *cu, DWARFAttributes &attrs) const {
+return GetAttributes(cu, attrs, 0 /* curr_depth */);
+  }
 
   dw_offset_t
   GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
@@ -176,6 +175,10 @@
   /// A copy of the DW_TAG value so we don't have to go through the compile
   /// unit abbrev table
   dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
+
+private:
+  size_t GetAttributes(const DWARFUnit *cu, DWARFAttributes &attrs,
+   uint32_t curr_depth) const;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDEBUGINFOENTRY_H
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -436,7 +436,8 @@
 if (form_value.ExtractValue(data, &offset)) {
   DWARFDIE spec_die = form_value.Reference();
   if (spec_die)
-spec_die.GetAttributes(attributes, curr_depth + 1);
+spec_die.GetDIE()->GetAttributes(spec_die.GetCU(), attributes,
+ curr_depth + 1);
 }
   } else {
 llvm::Optional fixed_skip_size = 
DWARFFormValue::GetFixedSize(form, cu);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
@@ -110,7 +110,7 @@
   uint64_t GetAttributeValueAsAddress(const dw_attr_t attr,
   uint64_t fail_value) const;
 
-  size_t GetAttributes(DWARFAttributes &attributes, uint32_t depth = 0) const;
+  size_t GetAttributes(DWARFAttributes &attributes) const;
 
 protected:
   DWARFUnit *m_cu;
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
@@ -104,12 +104,10 @@
   return IsValid() && 
GetDWARF()->Supports_DW_AT_APPLE_objc_complete_type(m_cu);
 }
 
-size_t DWARFBaseDIE::GetAttributes(DWARFAttributes &attributes,
-   uint32_t depth) const {
+size_t DWARFBaseDIE::GetAttributes(DWARFAttributes &attributes) const {
   if (IsValid())
-return m_die->GetAttributes(m_cu, attributes, depth);
-  if (depth == 0)
-attributes.Clear();
+return m_die->GetAttributes(m_cu, attributes);
+  attributes.Clear();
   return 0;
 }
 


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -47,10 +47,9 @@
   bool Extract(const lldb_private::DWARFDataExtractor &data,
const DWARFUnit *cu, lldb::offset_t *offset_ptr);
 
-  size_t GetAttributes(const DWARFUnit *cu,
-   DWARFAttributes &attrs,
-   uint32_t curr_depth = 0)
-  const; // "curr_depth" for internal use only, don't set this yourself!!!
+  size_t GetAttributes(const DWARFUnit *cu, DWARFAttributes &attrs) const {
+return GetAttributes(cu, attrs, 0 /* curr_depth */);
+  }
 
   dw_offset_t
   GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
@@ -176,6 +175,10 @@
   /// A copy of the DW_TAG value so we don't have to go through the compile
   /// unit abbrev table
   dw_tag_t m_tag = ll

[Lldb-commits] [PATCH] D81465: [lldb] Fix and enable Windows minidump tests

2020-06-09 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin created this revision.
jarin added a reviewer: labath.
jarin added a project: LLDB.
Herald added a subscriber: lldb-commits.
jarin edited the summary of this revision.

SBFileSpec.fullpath always uses the forward slash to join the directory with 
the base name. This causes mismatches when comparing Windows paths with 
backslashes in two of the minidump tests. To get around that we just compare 
the directory names separately from the filenames.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81465

Files:
  lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -20,7 +20,12 @@
 
 def verify_module(self, module, verify_path, verify_uuid):
 uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
+self.assertEqual(
+os.path.basename(verify_path),
+module.GetFileSpec().GetFilename())
+self.assertEqual(
+os.path.dirname(verify_path),
+module.GetFileSpec().GetDirectory() or "")
 self.assertEqual(verify_uuid, uuid)
 
 def get_minidump_modules(self, yaml_file):
@@ -130,7 +135,6 @@
 self.verify_module(modules[1], "/file/does/not/exist/b",
'11223344-1122-3344-1122-334411223344-11223344')
 
-@expectedFailureAll(oslist=["windows"])
 def test_partial_uuid_match(self):
 """
 Breakpad has been known to create minidump files using CvRecord in 
each
@@ -248,7 +252,6 @@
 "a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
 self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
 
-@expectedFailureAll(oslist=["windows"])
 def test_remove_placeholder_add_real_module(self):
 """
 Test that removing a placeholder module and adding back the real


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -20,7 +20,12 @@
 
 def verify_module(self, module, verify_path, verify_uuid):
 uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
+self.assertEqual(
+os.path.basename(verify_path),
+module.GetFileSpec().GetFilename())
+self.assertEqual(
+os.path.dirname(verify_path),
+module.GetFileSpec().GetDirectory() or "")
 self.assertEqual(verify_uuid, uuid)
 
 def get_minidump_modules(self, yaml_file):
@@ -130,7 +135,6 @@
 self.verify_module(modules[1], "/file/does/not/exist/b",
'11223344-1122-3344-1122-334411223344-11223344')
 
-@expectedFailureAll(oslist=["windows"])
 def test_partial_uuid_match(self):
 """
 Breakpad has been known to create minidump files using CvRecord in each
@@ -248,7 +252,6 @@
 "a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
 self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
 
-@expectedFailureAll(oslist=["windows"])
 def test_remove_placeholder_add_real_module(self):
 """
 Test that removing a placeholder module and adding back the real
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 70a2188 - [lldb] Test compatibility between a class type from a member function expr and its original version

2020-06-09 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-06-09T15:48:00+02:00
New Revision: 70a21887f7bd0b38e7c0676d8b25cc8a9980e009

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

LOG: [lldb] Test compatibility between a class type from a member function expr 
and its original version

Added: 
lldb/test/API/lang/cpp/this_class_type_mixing/Makefile
lldb/test/API/lang/cpp/this_class_type_mixing/TestThisClassTypeMixing.py
lldb/test/API/lang/cpp/this_class_type_mixing/main.cpp

Modified: 


Removed: 




diff  --git a/lldb/test/API/lang/cpp/this_class_type_mixing/Makefile 
b/lldb/test/API/lang/cpp/this_class_type_mixing/Makefile
new file mode 100644
index ..8b20bcb0
--- /dev/null
+++ b/lldb/test/API/lang/cpp/this_class_type_mixing/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lang/cpp/this_class_type_mixing/TestThisClassTypeMixing.py 
b/lldb/test/API/lang/cpp/this_class_type_mixing/TestThisClassTypeMixing.py
new file mode 100644
index ..8b09332af3f5
--- /dev/null
+++ b/lldb/test/API/lang/cpp/this_class_type_mixing/TestThisClassTypeMixing.py
@@ -0,0 +1,42 @@
+"""
+Tests using a class type that originated from a member function evaluation
+with the same class type outside a member function.
+
+When evaluating expressions in a class, LLDB modifies the type of the current
+class by injecting a "$__lldb_expr" member function into the class. This
+function should not cause the type to become incompatible with its original
+definition without the "$__lldb_expr" member.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break in member function",
+  lldb.SBFileSpec("main.cpp"))
+
+# Evaluate an expression in a member function. Store the type of the
+# 'this' pointer in a persistent variable.
+self.expect_expr("A $p = *this; $p", result_type="A")
+
+breakpoint = self.target().BreakpointCreateBySourceRegex(
+"// break in main", lldb.SBFileSpec("main.cpp"))
+self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)
+threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint)
+self.assertEqual(len(threads), 1)
+
+# Evaluate expressions in the main function. Use the persistent type
+# of "A" that came from an evaluation in a member function with a
+# normal "A" type and make sure that LLDB can still evaluate
+# expressions that reference both types at the same time.
+self.expect_expr("$p.i + a.i", result_type="int", result_value="2")
+self.expect_expr("a.i + $p.i", result_type="int", result_value="2")
+self.expect_expr("a = $p; a.i", result_type="int", result_value="1")

diff  --git a/lldb/test/API/lang/cpp/this_class_type_mixing/main.cpp 
b/lldb/test/API/lang/cpp/this_class_type_mixing/main.cpp
new file mode 100644
index ..d76b7cb29b1b
--- /dev/null
+++ b/lldb/test/API/lang/cpp/this_class_type_mixing/main.cpp
@@ -0,0 +1,11 @@
+struct A {
+  int i = 1;
+  int member_method() {
+return i; // break in member function
+  }
+};
+int main() {
+  A a;
+  int i = a.member_method();
+  return i; // break in main
+}



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


[Lldb-commits] [PATCH] D81465: [lldb] Fix and enable Windows minidump tests

2020-06-09 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Thanks for getting back to this. I think this is a good workaround for the 
problem (maybe it would be good to make a note of that in the comment). The 
`fullpath` implementation should be fixed use the correct path separator, but I 
haven't yet figured out a good way to do it -- it will probably involve 
reimplementing `fullpath` on top of `GetPath` and fiddling with swig typemaps 
so that `GetPath` is usable from python...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81465



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


[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 269573.
JDevlieghere added a comment.

- Remove default value for color in CommandReturnObject.
- Add test


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

https://reviews.llvm.org/D81058

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Utility/Stream.h
  lldb/include/lldb/Utility/StreamTee.h
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/Breakpoint/BreakpointOptions.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Interpreter/CommandAlias.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/Stream.cpp
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestUseColor.test

Index: lldb/test/Shell/Driver/TestUseColor.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestUseColor.test
@@ -0,0 +1,7 @@
+UNSUPPORTED: system-windows
+
+RUN: not %lldb -b -o 'settings set use-color true' -o 'settings show use-color' -o 'bogus' > %t 2>&1
+RUN: cat -e %t | FileCheck %s --check-prefix COLOR
+COLOR: use-color (boolean) = true
+# The [[ confuses FileCheck so regex match it.
+COLOR: {{.+}}0;1;31merror: {{.+}}0m'bogus' is not a valid command
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,3 @@
-# RUN: %lldb --no-use-colors -s %s | FileCheck %s
-settings show use-color
-# CHECK: use-color (boolean) = false
-q
+RUN: not %lldb -b --no-use-colors -o 'settings show use-color' -o 'bogus' 2>&1 | FileCheck %s --check-prefix NOCOLOR
+NOCOLOR: use-color (boolean) = false
+NOCOLOR: error: 'bogus' is not a valid command
Index: lldb/source/Utility/Stream.cpp
===
--- lldb/source/Utility/Stream.cpp
+++ lldb/source/Utility/Stream.cpp
@@ -22,13 +22,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order)
+Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order,
+   bool colors)
 : m_flags(flags), m_addr_size(addr_size), m_byte_order(byte_order),
-  m_indent_level(0), m_forwarder(*this) {}
+  m_indent_level(0), m_forwarder(*this, colors) {}
 
-Stream::Stream()
+Stream::Stream(bool colors)
 : m_flags(0), m_addr_size(4), m_byte_order(endian::InlHostByteOrder()),
-  m_indent_level(0), m_forwarder(*this) {}
+  m_indent_level(0), m_forwarder(*this, colors) {}
 
 // Destructor
 Stream::~Stream() {}
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2556,7 +2556,7 @@
   if (!any_active_hooks)
 return;
 
-  CommandReturnObject result;
+  CommandReturnObject result(m_debugger.GetUseColor());
 
   std::vector exc_ctx_with_reasons;
   std::vector sym_ctx_with_reasons;
Index: lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
===
--- lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
+++ lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
@@ -981,7 +981,7 @@
   EnableOptionsSP options_sp(new EnableOptions());
   options_sp->NotifyOptionParsingStarting(&exe_ctx);
 
-  CommandReturnObject result;
+  CommandReturnObject result(debugger.GetUseColor());
 
   // Parse the arguments.
   auto options_property_sp =
@@ -1036,7 +1036,7 @@
   }
 
   // Run the command.
-  CommandReturnObject return_object;
+  CommandReturnObject return_object(interpreter.GetDebugger().GetUseColor());
   interpreter.HandleCommand(command_stream.GetData(), eLazyBoolNo,
 return_object);
   return return_object.Succeeded();
Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -14,6 +14,18 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static llvm::raw_ostream &error(Stream &strm) {
+  return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Error,
+ llvm::ColorMode::Enable)
+ << "error: ";
+}
+
+static llvm::raw_ostream &warning(Stream &strm) {
+  return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Warning,
+ llvm::ColorMode::Enable)
+ << "warning: ";
+}
+

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked 4 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/Stream.h:402-403
 
+bool has_colors() const override { return true; }
+
 uint64_t current_pos() const override {

labath wrote:
> This is not consistent with the intend of this method (`This function 
> determines if this stream is displayed and supports colors.`). One could 
> argue whether this could be "true" for CommandObjectResult streams which are 
> destined to be later printed to a terminal, but it's definitely not a good 
> default.
> 
> However, all of that is irrelevant, as this should not be needed anymore. Is 
> that correct?
Correct!


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

https://reviews.llvm.org/D81058



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


[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 269576.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

- Update ctors in lldb-test and unit test


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

https://reviews.llvm.org/D81058

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Utility/Stream.h
  lldb/include/lldb/Utility/StreamTee.h
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/Breakpoint/BreakpointOptions.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Interpreter/CommandAlias.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/Stream.cpp
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestUseColor.test
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -54,7 +54,7 @@
   ASSERT_TRUE(debugger_sp);
 
   ScriptInterpreterLua script_interpreter(*debugger_sp);
-  CommandReturnObject result;
+  CommandReturnObject result(/*colors*/ false);
   EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", &result));
   EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", &result));
   EXPECT_TRUE(result.GetErrorData().startswith(
Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -380,7 +380,7 @@
 
 std::string Command = substitute(Line);
 P.formatLine("Command: {0}", Command);
-CommandReturnObject Result;
+CommandReturnObject Result(/*colors*/ false);
 if (!Dbg.GetCommandInterpreter().HandleCommand(
 Command.c_str(), /*add_to_history*/ eLazyBoolNo, Result)) {
   P.formatLine("Failed: {0}", Result.GetErrorData());
@@ -530,7 +530,7 @@
   LanguageSet languages;
   if (!Language.empty())
 languages.Insert(Language::GetLanguageTypeFromString(Language));
-  
+
   DenseSet SearchedFiles;
   TypeMap Map;
   if (!Name.empty())
@@ -1034,7 +1034,7 @@
 
   // Set up a Process. In order to allocate memory within a target, this
   // process must be alive and must support JIT'ing.
-  CommandReturnObject Result;
+  CommandReturnObject Result(/*colors*/ false);
   Dbg.SetAsyncExecution(false);
   CommandInterpreter &CI = Dbg.GetCommandInterpreter();
   auto IssueCmd = [&](const char *Cmd) -> bool {
@@ -1098,7 +1098,7 @@
 
   auto Dbg = lldb_private::Debugger::CreateInstance();
   ModuleList::GetGlobalModuleListProperties().SetEnableExternalLookup(false);
-  CommandReturnObject Result;
+  CommandReturnObject Result(/*colors*/ false);
   Dbg->GetCommandInterpreter().HandleCommand(
   "settings set plugin.process.gdb-remote.packet-timeout 60",
   /*add_to_history*/ eLazyBoolNo, Result);
Index: lldb/test/Shell/Driver/TestUseColor.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestUseColor.test
@@ -0,0 +1,7 @@
+UNSUPPORTED: system-windows
+
+RUN: not %lldb -b -o 'settings set use-color true' -o 'settings show use-color' -o 'bogus' > %t 2>&1
+RUN: cat -e %t | FileCheck %s --check-prefix COLOR
+COLOR: use-color (boolean) = true
+# The [[ confuses FileCheck so regex match it.
+COLOR: {{.+}}0;1;31merror: {{.+}}0m'bogus' is not a valid command
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,3 @@
-# RUN: %lldb --no-use-colors -s %s | FileCheck %s
-settings show use-color
-# CHECK: use-color (boolean) = false
-q
+RUN: not %lldb -b --no-use-colors -o 'settings show use-color' -o 'bogus' 2>&1 | FileCheck %s --check-prefix NOCOLOR
+NOCOLOR: use-color (boolean) = false
+NOCOLOR: error: 'bogus' is not a valid command
Index: lldb/source/Utility/Stream.cpp
===
--- lldb/source/Utility/Stream.cpp
+++ lldb/source/Utility/Stream.cpp
@@ -22,13 +22,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order)
+Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order,
+   bool colors)
 : m_flags(flags), m_addr_size(addr_size), m_byte_order(byte_order),
-  m_indent_level(0), m_forwarder(*this) {}
+ 

[Lldb-commits] [lldb] de019b8 - [lldb/Interpreter] Support color in CommandReturnObject

2020-06-09 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-06-09T10:45:45-07:00
New Revision: de019b88dd5804ec996fe8c12cddcc6feb13afa1

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

LOG: [lldb/Interpreter] Support color in CommandReturnObject

Color the error: and warning: part of the CommandReturnObject output,
similar to how an error is printed from the driver when colors are
enabled.

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

Added: 
lldb/test/Shell/Driver/TestUseColor.test

Modified: 
lldb/include/lldb/Interpreter/CommandReturnObject.h
lldb/include/lldb/Utility/Stream.h
lldb/include/lldb/Utility/StreamTee.h
lldb/source/API/SBCommandReturnObject.cpp
lldb/source/Breakpoint/BreakpointOptions.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/source/Expression/REPL.cpp
lldb/source/Interpreter/CommandAlias.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Interpreter/CommandReturnObject.cpp
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/source/Target/Target.cpp
lldb/source/Utility/Stream.cpp
lldb/test/Shell/Driver/TestNoUseColor.test
lldb/tools/lldb-test/lldb-test.cpp
lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h 
b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index ea072fd45c9e..6a3ec83a765f 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -16,6 +16,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/WithColor.h"
 
 #include 
 
@@ -23,7 +24,7 @@ namespace lldb_private {
 
 class CommandReturnObject {
 public:
-  CommandReturnObject();
+  CommandReturnObject(bool colors);
 
   ~CommandReturnObject();
 

diff  --git a/lldb/include/lldb/Utility/Stream.h 
b/lldb/include/lldb/Utility/Stream.h
index a042f5ef9bf6..e7f065a1fc7b 100644
--- a/lldb/include/lldb/Utility/Stream.h
+++ b/lldb/include/lldb/Utility/Stream.h
@@ -56,12 +56,13 @@ class Stream {
   ///
   /// Construct with dump flags \a flags and the default address size. \a
   /// flags can be any of the above enumeration logical OR'ed together.
-  Stream(uint32_t flags, uint32_t addr_size, lldb::ByteOrder byte_order);
+  Stream(uint32_t flags, uint32_t addr_size, lldb::ByteOrder byte_order,
+ bool colors = false);
 
   /// Construct a default Stream, not binary, host byte order and host addr
   /// size.
   ///
-  Stream();
+  Stream(bool colors = false);
 
   // FIXME: Streams should not be copyable.
   Stream(const Stream &other) : m_forwarder(*this) { (*this) = other; }
@@ -403,8 +404,10 @@ class Stream {
 }
 
   public:
-RawOstreamForward(Stream &target)
-: llvm::raw_ostream(/*unbuffered*/ true), m_target(target) {}
+RawOstreamForward(Stream &target, bool colors = false)
+: llvm::raw_ostream(/*unbuffered*/ true), m_target(target) {
+  enable_colors(colors);
+}
   };
   RawOstreamForward m_forwarder;
 };

diff  --git a/lldb/include/lldb/Utility/StreamTee.h 
b/lldb/include/lldb/Utility/StreamTee.h
index 2a9cd644ae2f..2995bc07f42a 100644
--- a/lldb/include/lldb/Utility/StreamTee.h
+++ b/lldb/include/lldb/Utility/StreamTee.h
@@ -19,7 +19,8 @@ namespace lldb_private {
 
 class StreamTee : public Stream {
 public:
-  StreamTee() : Stream(), m_streams_mutex(), m_streams() {}
+  StreamTee(bool colors = false)
+  : Stream(colors), m_streams_mutex(), m_streams() {}
 
   StreamTee(lldb::StreamSP &stream_sp)
   : Stream(), m_streams_mutex(), m_streams() {

diff  --git a/lldb/source/API/SBCommandReturnObject.cpp 
b/lldb/source/API/SBCommandReturnObject.cpp
index 102877eea2a6..fddf90b66481 100644
--- a/lldb/source/API/SBCommandReturnObject.cpp
+++ b/lldb/source/API/SBCommandReturnObject.cpp
@@ -22,7 +22,7 @@ using namespace lldb_private;
 class lldb_private::SBCommandReturnObjectImpl {
 public:
   SBCommandReturnObjectImpl()
-  : m_ptr(new CommandReturnObject()), m_owned(true) {}
+  : m_ptr(new CommandReturnObject(false)), m_owned(true) {}
   SBCommandReturnObjectImpl(CommandReturnObject &ref)
   : m_ptr(&ref), m_owned(false) {}
   SBCommandReturnObjectImpl(const SBCommandReturnObjectImpl &rhs)

diff  --git a/lldb/source/Breakpoint/BreakpointOptions.cpp 
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index db2a2f1fee86..3886d0329062 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -630,11 +630,11 @@ bool BreakpointOptions::BreakpointOptionsCallbackFunction(
 ExecutionCo

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGde019b88dd58: [lldb/Interpreter] Support color in 
CommandReturnObject (authored by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81058

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Utility/Stream.h
  lldb/include/lldb/Utility/StreamTee.h
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/Breakpoint/BreakpointOptions.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Interpreter/CommandAlias.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/Stream.cpp
  lldb/test/Shell/Driver/TestNoUseColor.test
  lldb/test/Shell/Driver/TestUseColor.test
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -54,7 +54,7 @@
   ASSERT_TRUE(debugger_sp);
 
   ScriptInterpreterLua script_interpreter(*debugger_sp);
-  CommandReturnObject result;
+  CommandReturnObject result(/*colors*/ false);
   EXPECT_TRUE(script_interpreter.ExecuteOneLine("foo = 1", &result));
   EXPECT_FALSE(script_interpreter.ExecuteOneLine("nil = foo", &result));
   EXPECT_TRUE(result.GetErrorData().startswith(
Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -380,7 +380,7 @@
 
 std::string Command = substitute(Line);
 P.formatLine("Command: {0}", Command);
-CommandReturnObject Result;
+CommandReturnObject Result(/*colors*/ false);
 if (!Dbg.GetCommandInterpreter().HandleCommand(
 Command.c_str(), /*add_to_history*/ eLazyBoolNo, Result)) {
   P.formatLine("Failed: {0}", Result.GetErrorData());
@@ -530,7 +530,7 @@
   LanguageSet languages;
   if (!Language.empty())
 languages.Insert(Language::GetLanguageTypeFromString(Language));
-  
+
   DenseSet SearchedFiles;
   TypeMap Map;
   if (!Name.empty())
@@ -1034,7 +1034,7 @@
 
   // Set up a Process. In order to allocate memory within a target, this
   // process must be alive and must support JIT'ing.
-  CommandReturnObject Result;
+  CommandReturnObject Result(/*colors*/ false);
   Dbg.SetAsyncExecution(false);
   CommandInterpreter &CI = Dbg.GetCommandInterpreter();
   auto IssueCmd = [&](const char *Cmd) -> bool {
@@ -1098,7 +1098,7 @@
 
   auto Dbg = lldb_private::Debugger::CreateInstance();
   ModuleList::GetGlobalModuleListProperties().SetEnableExternalLookup(false);
-  CommandReturnObject Result;
+  CommandReturnObject Result(/*colors*/ false);
   Dbg->GetCommandInterpreter().HandleCommand(
   "settings set plugin.process.gdb-remote.packet-timeout 60",
   /*add_to_history*/ eLazyBoolNo, Result);
Index: lldb/test/Shell/Driver/TestUseColor.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/TestUseColor.test
@@ -0,0 +1,7 @@
+UNSUPPORTED: system-windows
+
+RUN: not %lldb -b -o 'settings set use-color true' -o 'settings show use-color' -o 'bogus' > %t 2>&1
+RUN: cat -e %t | FileCheck %s --check-prefix COLOR
+COLOR: use-color (boolean) = true
+# The [[ confuses FileCheck so regex match it.
+COLOR: {{.+}}0;1;31merror: {{.+}}0m'bogus' is not a valid command
Index: lldb/test/Shell/Driver/TestNoUseColor.test
===
--- lldb/test/Shell/Driver/TestNoUseColor.test
+++ lldb/test/Shell/Driver/TestNoUseColor.test
@@ -1,4 +1,3 @@
-# RUN: %lldb --no-use-colors -s %s | FileCheck %s
-settings show use-color
-# CHECK: use-color (boolean) = false
-q
+RUN: not %lldb -b --no-use-colors -o 'settings show use-color' -o 'bogus' 2>&1 | FileCheck %s --check-prefix NOCOLOR
+NOCOLOR: use-color (boolean) = false
+NOCOLOR: error: 'bogus' is not a valid command
Index: lldb/source/Utility/Stream.cpp
===
--- lldb/source/Utility/Stream.cpp
+++ lldb/source/Utility/Stream.cpp
@@ -22,13 +22,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order)
+Stream::Stream(uint32_t flags, uint32_t addr_size, ByteOrder byte_order,
+   bool colors)
 : m_flags(flags), m_addr_size(addr_size), m_byte_order(byte

[Lldb-commits] [PATCH] D81465: [lldb] Fix and enable Windows minidump tests

2020-06-09 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin updated this revision to Diff 269609.
jarin added a comment.

Added a comment.


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

https://reviews.llvm.org/D81465

Files:
  lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -19,9 +19,16 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 def verify_module(self, module, verify_path, verify_uuid):
-uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
-self.assertEqual(verify_uuid, uuid)
+# Compare the filename and the directory separately. We are avoiding
+# SBFileSpec.fullpath because it causes a slash/backslash confusion
+# on Windows.
+self.assertEqual(
+os.path.basename(verify_path),
+module.GetFileSpec().GetFilename())
+self.assertEqual(
+os.path.dirname(verify_path),
+module.GetFileSpec().GetDirectory() or "")
+self.assertEqual(verify_uuid, module.GetUUIDString())
 
 def get_minidump_modules(self, yaml_file):
 minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + 
".dmp")
@@ -130,7 +137,6 @@
 self.verify_module(modules[1], "/file/does/not/exist/b",
'11223344-1122-3344-1122-334411223344-11223344')
 
-@expectedFailureAll(oslist=["windows"])
 def test_partial_uuid_match(self):
 """
 Breakpad has been known to create minidump files using CvRecord in 
each
@@ -248,7 +254,6 @@
 "a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
 self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
 
-@expectedFailureAll(oslist=["windows"])
 def test_remove_placeholder_add_real_module(self):
 """
 Test that removing a placeholder module and adding back the real


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -19,9 +19,16 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 def verify_module(self, module, verify_path, verify_uuid):
-uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
-self.assertEqual(verify_uuid, uuid)
+# Compare the filename and the directory separately. We are avoiding
+# SBFileSpec.fullpath because it causes a slash/backslash confusion
+# on Windows.
+self.assertEqual(
+os.path.basename(verify_path),
+module.GetFileSpec().GetFilename())
+self.assertEqual(
+os.path.dirname(verify_path),
+module.GetFileSpec().GetDirectory() or "")
+self.assertEqual(verify_uuid, module.GetUUIDString())
 
 def get_minidump_modules(self, yaml_file):
 minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp")
@@ -130,7 +137,6 @@
 self.verify_module(modules[1], "/file/does/not/exist/b",
'11223344-1122-3344-1122-334411223344-11223344')
 
-@expectedFailureAll(oslist=["windows"])
 def test_partial_uuid_match(self):
 """
 Breakpad has been known to create minidump files using CvRecord in each
@@ -248,7 +254,6 @@
 "a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
 self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
 
-@expectedFailureAll(oslist=["windows"])
 def test_remove_placeholder_add_real_module(self):
 """
 Test that removing a placeholder module and adding back the real
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 6c5c4a2 - [lldb/Reproducers] Also collect ::open and ::fopen

2020-06-09 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-06-09T11:59:02-07:00
New Revision: 6c5c4a2a50e1fcdd94c0288008af65c544a96452

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

LOG: [lldb/Reproducers] Also collect ::open  and ::fopen

Report files opened trough ::open and ::fopen to the FileCollector.

Added: 


Modified: 
lldb/source/Host/posix/FileSystemPosix.cpp
lldb/source/Host/windows/FileSystem.cpp

Removed: 




diff  --git a/lldb/source/Host/posix/FileSystemPosix.cpp 
b/lldb/source/Host/posix/FileSystemPosix.cpp
index 3660f67895a4..0aa34bc59435 100644
--- a/lldb/source/Host/posix/FileSystemPosix.cpp
+++ b/lldb/source/Host/posix/FileSystemPosix.cpp
@@ -72,9 +72,11 @@ Status FileSystem::ResolveSymbolicLink(const FileSpec &src, 
FileSpec &dst) {
 }
 
 FILE *FileSystem::Fopen(const char *path, const char *mode) {
+  Collect(path);
   return llvm::sys::RetryAfterSignal(nullptr, ::fopen, path, mode);
 }
 
 int FileSystem::Open(const char *path, int flags, int mode) {
+  Collect(path);
   return llvm::sys::RetryAfterSignal(-1, ::open, path, flags, mode);
 }

diff  --git a/lldb/source/Host/windows/FileSystem.cpp 
b/lldb/source/Host/windows/FileSystem.cpp
index cbd1915bdb44..94872c99b15e 100644
--- a/lldb/source/Host/windows/FileSystem.cpp
+++ b/lldb/source/Host/windows/FileSystem.cpp
@@ -86,6 +86,7 @@ Status FileSystem::ResolveSymbolicLink(const FileSpec &src, 
FileSpec &dst) {
 }
 
 FILE *FileSystem::Fopen(const char *path, const char *mode) {
+  Collect(path);
   std::wstring wpath, wmode;
   if (!llvm::ConvertUTF8toWide(path, wpath))
 return nullptr;
@@ -98,6 +99,7 @@ FILE *FileSystem::Fopen(const char *path, const char *mode) {
 }
 
 int FileSystem::Open(const char *path, int flags, int mode) {
+  Collect(path);
   std::wstring wpath;
   if (!llvm::ConvertUTF8toWide(path, wpath))
 return -1;



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


[Lldb-commits] [lldb] fac5d05 - [lldb] Fix and enable Windows minidump tests

2020-06-09 Thread Jaroslav Sevcik via lldb-commits

Author: Jaroslav Sevcik
Date: 2020-06-09T20:03:44Z
New Revision: fac5d05eb75fab163dc316859fa5e7d255b98f7c

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

LOG: [lldb] Fix and enable Windows minidump tests

SBFileSpec.fullpath always uses the forward slash to join the directory with the
base name. This causes mismatches when comparing Windows paths with backslashes
in two of the minidump tests. To get around that we just compare the directory
names separately from the filenames.

Reviewed By: labath

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

Added: 


Modified: 
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py 
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
index 601546b1dc8e..8cb7de9714e3 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -19,9 +19,14 @@ class MiniDumpUUIDTestCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 def verify_module(self, module, verify_path, verify_uuid):
-uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
-self.assertEqual(verify_uuid, uuid)
+# Compare the filename and the directory separately. We are avoiding
+# SBFileSpec.fullpath because it causes a slash/backslash confusion
+# on Windows.
+self.assertEqual(
+os.path.basename(verify_path), module.GetFileSpec().basename)
+self.assertEqual(
+os.path.dirname(verify_path), module.GetFileSpec().dirname or "")
+self.assertEqual(verify_uuid, module.GetUUIDString())
 
 def get_minidump_modules(self, yaml_file):
 minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + 
".dmp")
@@ -130,7 +135,6 @@ def test_uuid_modules_elf_build_id_same(self):
 self.verify_module(modules[1], "/file/does/not/exist/b",
'11223344-1122-3344-1122-334411223344-11223344')
 
-@expectedFailureAll(oslist=["windows"])
 def test_partial_uuid_match(self):
 """
 Breakpad has been known to create minidump files using CvRecord in 
each
@@ -248,7 +252,6 @@ def test_add_module_build_id_4(self):
 "a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
 self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
 
-@expectedFailureAll(oslist=["windows"])
 def test_remove_placeholder_add_real_module(self):
 """
 Test that removing a placeholder module and adding back the real



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


[Lldb-commits] [PATCH] D81499: [Debugger] Use FileSystem instead of calling llvm::sys::fs::openFileForWrite directly.

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jingham.

This replaces the (only) call to `llvm::sys::fs::openFileForWrite` with 
`FileSystem::Open`. This guarantees that we include log files in the 
reproducers.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D81499

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1154,17 +1154,22 @@
 if (pos != m_log_streams.end())
   log_stream_sp = pos->second.lock();
 if (!log_stream_sp) {
-  llvm::sys::fs::OpenFlags flags = llvm::sys::fs::OF_Text;
+  File::OpenOptions flags =
+  File::eOpenOptionWrite | File::eOpenOptionCanCreate;
   if (log_options & LLDB_LOG_OPTION_APPEND)
-flags |= llvm::sys::fs::OF_Append;
-  int FD;
-  if (std::error_code ec = llvm::sys::fs::openFileForWrite(
-  log_file, FD, llvm::sys::fs::CD_CreateAlways, flags)) {
-error_stream << "Unable to open log file: " << ec.message();
+flags |= File::eOpenOptionAppend;
+  else
+flags |= File::eOpenOptionTruncate;
+  auto file = FileSystem::Instance().Open(
+  FileSpec(log_file), flags, lldb::eFilePermissionsFileDefault, false);
+  if (!file) {
+// FIXME: This gets garbled when called from the log command.
+error_stream << "Unable to open log file: " << log_file;
 return false;
   }
-  log_stream_sp =
-  std::make_shared(FD, should_close, unbuffered);
+
+  log_stream_sp = std::make_shared(
+  (*file)->GetDescriptor(), should_close, unbuffered);
   m_log_streams[log_file] = log_stream_sp;
 }
   }


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1154,17 +1154,22 @@
 if (pos != m_log_streams.end())
   log_stream_sp = pos->second.lock();
 if (!log_stream_sp) {
-  llvm::sys::fs::OpenFlags flags = llvm::sys::fs::OF_Text;
+  File::OpenOptions flags =
+  File::eOpenOptionWrite | File::eOpenOptionCanCreate;
   if (log_options & LLDB_LOG_OPTION_APPEND)
-flags |= llvm::sys::fs::OF_Append;
-  int FD;
-  if (std::error_code ec = llvm::sys::fs::openFileForWrite(
-  log_file, FD, llvm::sys::fs::CD_CreateAlways, flags)) {
-error_stream << "Unable to open log file: " << ec.message();
+flags |= File::eOpenOptionAppend;
+  else
+flags |= File::eOpenOptionTruncate;
+  auto file = FileSystem::Instance().Open(
+  FileSpec(log_file), flags, lldb::eFilePermissionsFileDefault, false);
+  if (!file) {
+// FIXME: This gets garbled when called from the log command.
+error_stream << "Unable to open log file: " << log_file;
 return false;
   }
-  log_stream_sp =
-  std::make_shared(FD, should_close, unbuffered);
+
+  log_stream_sp = std::make_shared(
+  (*file)->GetDescriptor(), should_close, unbuffered);
   m_log_streams[log_file] = log_stream_sp;
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D81465: [lldb] Fix and enable Windows minidump tests

2020-06-09 Thread Jaroslav Sevcik via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfac5d05eb75f: [lldb] Fix and enable Windows minidump tests 
(authored by jarin).

Changed prior to commit:
  https://reviews.llvm.org/D81465?vs=269609&id=269649#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81465

Files:
  lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -19,9 +19,14 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 def verify_module(self, module, verify_path, verify_uuid):
-uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
-self.assertEqual(verify_uuid, uuid)
+# Compare the filename and the directory separately. We are avoiding
+# SBFileSpec.fullpath because it causes a slash/backslash confusion
+# on Windows.
+self.assertEqual(
+os.path.basename(verify_path), module.GetFileSpec().basename)
+self.assertEqual(
+os.path.dirname(verify_path), module.GetFileSpec().dirname or "")
+self.assertEqual(verify_uuid, module.GetUUIDString())
 
 def get_minidump_modules(self, yaml_file):
 minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + 
".dmp")
@@ -130,7 +135,6 @@
 self.verify_module(modules[1], "/file/does/not/exist/b",
'11223344-1122-3344-1122-334411223344-11223344')
 
-@expectedFailureAll(oslist=["windows"])
 def test_partial_uuid_match(self):
 """
 Breakpad has been known to create minidump files using CvRecord in 
each
@@ -248,7 +252,6 @@
 "a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
 self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
 
-@expectedFailureAll(oslist=["windows"])
 def test_remove_placeholder_add_real_module(self):
 """
 Test that removing a placeholder module and adding back the real


Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -19,9 +19,14 @@
 NO_DEBUG_INFO_TESTCASE = True
 
 def verify_module(self, module, verify_path, verify_uuid):
-uuid = module.GetUUIDString()
-self.assertEqual(verify_path, module.GetFileSpec().fullpath)
-self.assertEqual(verify_uuid, uuid)
+# Compare the filename and the directory separately. We are avoiding
+# SBFileSpec.fullpath because it causes a slash/backslash confusion
+# on Windows.
+self.assertEqual(
+os.path.basename(verify_path), module.GetFileSpec().basename)
+self.assertEqual(
+os.path.dirname(verify_path), module.GetFileSpec().dirname or "")
+self.assertEqual(verify_uuid, module.GetUUIDString())
 
 def get_minidump_modules(self, yaml_file):
 minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp")
@@ -130,7 +135,6 @@
 self.verify_module(modules[1], "/file/does/not/exist/b",
'11223344-1122-3344-1122-334411223344-11223344')
 
-@expectedFailureAll(oslist=["windows"])
 def test_partial_uuid_match(self):
 """
 Breakpad has been known to create minidump files using CvRecord in each
@@ -248,7 +252,6 @@
 "a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
 self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
 
-@expectedFailureAll(oslist=["windows"])
 def test_remove_placeholder_add_real_module(self):
 """
 Test that removing a placeholder module and adding back the real
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 2c0afac - [lldb/CMake] Add LLDB_PYTHON_VERSION to use Python 2 with CMake > 3.12

2020-06-09 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-06-09T14:11:11-07:00
New Revision: 2c0afacada0d1488bc88b1211203ea4fdb0a23e8

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

LOG: [lldb/CMake] Add LLDB_PYTHON_VERSION to use Python 2 with CMake > 3.12

In addition to having the default fallback from Python 3 to Python 2, it
should also be possible to build against Python 2 explicitly. This patch
makes that possible by setting LLDB_PYTHON_VERSION. The variable only
has effect with CMake 3.12 or later.

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

Added: 


Modified: 
lldb/cmake/modules/FindPythonInterpAndLibs.cmake

Removed: 




diff  --git a/lldb/cmake/modules/FindPythonInterpAndLibs.cmake 
b/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
index aae82a68bcfd..243e0463f48b 100644
--- a/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
+++ b/lldb/cmake/modules/FindPythonInterpAndLibs.cmake
@@ -4,54 +4,74 @@
 #
 # Find the python interpreter and libraries as a whole.
 
+macro(FindPython3)
+  # Use PYTHON_HOME as a hint to find Python 3.
+  set(Python3_ROOT_DIR "${PYTHON_HOME}")
+  find_package(Python3 COMPONENTS Interpreter Development)
+  if(Python3_FOUND AND Python3_Interpreter_FOUND)
+set(PYTHON_LIBRARIES ${Python3_LIBRARIES})
+set(PYTHON_INCLUDE_DIRS ${Python3_INCLUDE_DIRS})
+set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
+
+# The install name for the Python 3 framework in Xcode is relative to
+# the framework's location and not the dylib itself.
+#
+#   @rpath/Python3.framework/Versions/3.x/Python3
+#
+# This means that we need to compute the path to the Python3.framework
+# and use that as the RPATH instead of the usual dylib's directory.
+#
+# The check below shouldn't match Homebrew's Python framework as it is
+# called Python.framework instead of Python3.framework.
+if (APPLE AND Python3_LIBRARIES MATCHES "Python3.framework")
+  string(FIND "${Python3_LIBRARIES}" "Python3.framework" 
python_framework_pos)
+  string(SUBSTRING "${Python3_LIBRARIES}" "0" ${python_framework_pos} 
PYTHON_RPATH)
+endif()
+
+set(PYTHON3_FOUND TRUE)
+mark_as_advanced(
+  PYTHON_LIBRARIES
+  PYTHON_INCLUDE_DIRS
+  PYTHON_EXECUTABLE
+  PYTHON_RPATH
+  SWIG_EXECUTABLE)
+  endif()
+endmacro()
+
+macro(FindPython2)
+  # Use PYTHON_HOME as a hint to find Python 2.
+  set(Python2_ROOT_DIR "${PYTHON_HOME}")
+  find_package(Python2 COMPONENTS Interpreter Development)
+  if(Python2_FOUND AND Python2_Interpreter_FOUND)
+set(PYTHON_LIBRARIES ${Python2_LIBRARIES})
+set(PYTHON_INCLUDE_DIRS ${Python2_INCLUDE_DIRS})
+set(PYTHON_EXECUTABLE ${Python2_EXECUTABLE})
+
+set(PYTHON2_FOUND TRUE)
+mark_as_advanced(
+  PYTHON_LIBRARIES
+  PYTHON_INCLUDE_DIRS
+  PYTHON_EXECUTABLE
+  SWIG_EXECUTABLE)
+  endif()
+endmacro()
+
 if(PYTHON_LIBRARIES AND PYTHON_INCLUDE_DIRS AND PYTHON_EXECUTABLE AND 
SWIG_EXECUTABLE)
   set(PYTHONINTERPANDLIBS_FOUND TRUE)
 else()
   find_package(SWIG 2.0)
   if (SWIG_FOUND)
 if(NOT CMAKE_VERSION VERSION_LESS 3.12)
-  # Use PYTHON_HOME as a hint to find Python 3.
-  set(Python3_ROOT_DIR "${PYTHON_HOME}")
-  find_package(Python3 COMPONENTS Interpreter Development)
-  if(Python3_FOUND AND Python3_Interpreter_FOUND)
-set(PYTHON_LIBRARIES ${Python3_LIBRARIES})
-set(PYTHON_INCLUDE_DIRS ${Python3_INCLUDE_DIRS})
-set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
-
-# The install name for the Python 3 framework in Xcode is relative to
-# the framework's location and not the dylib itself.
-#
-#   @rpath/Python3.framework/Versions/3.x/Python3
-#
-# This means that we need to compute the path to the Python3.framework
-# and use that as the RPATH instead of the usual dylib's directory.
-#
-# The check below shouldn't match Homebrew's Python framework as it is
-# called Python.framework instead of Python3.framework.
-if (APPLE AND Python3_LIBRARIES MATCHES "Python3.framework")
-  string(FIND "${Python3_LIBRARIES}" "Python3.framework" 
python_framework_pos)
-  string(SUBSTRING "${Python3_LIBRARIES}" "0" ${python_framework_pos} 
PYTHON_RPATH)
+  if (LLDB_PYTHON_VERSION)
+if (LLDB_PYTHON_VERSION VERSION_EQUAL "2")
+  FindPython2()
+elseif(LLDB_PYTHON_VERSION VERSION_EQUAL "3")
+  FindPython3()
 endif()
-
-mark_as_advanced(
-  PYTHON_LIBRARIES
-  PYTHON_INCLUDE_DIRS
-  PYTHON_EXECUTABLE
-  PYTHON_RPATH
-  SWIG_EXECUTABLE)
-  elseif(NOT CMAKE_SYSTEM_NAME STREQUAL Windows)
-# Use PYTHON_HOME as a hint to find Python 2.
-set(Python2_ROOT_D

[Lldb-commits] [PATCH] D81501: [lldb/CMake] Make it possible to build against Python 2 with CMake > 3.12

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: compnerd, jingham, labath.
JDevlieghere added a project: LLDB.
Herald added a subscriber: mgorny.
compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

Would be nice to remove this entirely in favour of CMake's builtin support for 
Python Interpeter and Libraries.


JDevlieghere added a comment.

In D81501#2083359 , @compnerd wrote:

> Would be nice to remove this entirely in favour of CMake's builtin support 
> for Python Interpeter and Libraries.


It'll continue to exist to find Python and SWIG atomically, but at least once 
we bump the minimum requirement for CMake to 3.12 this file will be a lot 
simpler.


In addition to having the default fallback from Python 3 to Python 2, it should 
also be possible to build against Python 2 explicitly. This patch makes that 
possible by setting `LLDB_PYTHON_VERSION`.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D81501

Files:
  lldb/cmake/modules/FindPythonInterpAndLibs.cmake

Index: lldb/cmake/modules/FindPythonInterpAndLibs.cmake
===
--- lldb/cmake/modules/FindPythonInterpAndLibs.cmake
+++ lldb/cmake/modules/FindPythonInterpAndLibs.cmake
@@ -4,54 +4,74 @@
 #
 # Find the python interpreter and libraries as a whole.
 
+macro(FindPython3)
+  # Use PYTHON_HOME as a hint to find Python 3.
+  set(Python3_ROOT_DIR "${PYTHON_HOME}")
+  find_package(Python3 COMPONENTS Interpreter Development)
+  if(Python3_FOUND AND Python3_Interpreter_FOUND)
+set(PYTHON_LIBRARIES ${Python3_LIBRARIES})
+set(PYTHON_INCLUDE_DIRS ${Python3_INCLUDE_DIRS})
+set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
+
+# The install name for the Python 3 framework in Xcode is relative to
+# the framework's location and not the dylib itself.
+#
+#   @rpath/Python3.framework/Versions/3.x/Python3
+#
+# This means that we need to compute the path to the Python3.framework
+# and use that as the RPATH instead of the usual dylib's directory.
+#
+# The check below shouldn't match Homebrew's Python framework as it is
+# called Python.framework instead of Python3.framework.
+if (APPLE AND Python3_LIBRARIES MATCHES "Python3.framework")
+  string(FIND "${Python3_LIBRARIES}" "Python3.framework" python_framework_pos)
+  string(SUBSTRING "${Python3_LIBRARIES}" "0" ${python_framework_pos} PYTHON_RPATH)
+endif()
+
+set(PYTHON3_FOUND TRUE)
+mark_as_advanced(
+  PYTHON_LIBRARIES
+  PYTHON_INCLUDE_DIRS
+  PYTHON_EXECUTABLE
+  PYTHON_RPATH
+  SWIG_EXECUTABLE)
+  endif()
+endmacro()
+
+macro(FindPython2)
+  # Use PYTHON_HOME as a hint to find Python 2.
+  set(Python2_ROOT_DIR "${PYTHON_HOME}")
+  find_package(Python2 COMPONENTS Interpreter Development)
+  if(Python2_FOUND AND Python2_Interpreter_FOUND)
+set(PYTHON_LIBRARIES ${Python2_LIBRARIES})
+set(PYTHON_INCLUDE_DIRS ${Python2_INCLUDE_DIRS})
+set(PYTHON_EXECUTABLE ${Python2_EXECUTABLE})
+
+set(PYTHON2_FOUND TRUE)
+mark_as_advanced(
+  PYTHON_LIBRARIES
+  PYTHON_INCLUDE_DIRS
+  PYTHON_EXECUTABLE
+  SWIG_EXECUTABLE)
+  endif()
+endmacro()
+
 if(PYTHON_LIBRARIES AND PYTHON_INCLUDE_DIRS AND PYTHON_EXECUTABLE AND SWIG_EXECUTABLE)
   set(PYTHONINTERPANDLIBS_FOUND TRUE)
 else()
   find_package(SWIG 2.0)
   if (SWIG_FOUND)
 if(NOT CMAKE_VERSION VERSION_LESS 3.12)
-  # Use PYTHON_HOME as a hint to find Python 3.
-  set(Python3_ROOT_DIR "${PYTHON_HOME}")
-  find_package(Python3 COMPONENTS Interpreter Development)
-  if(Python3_FOUND AND Python3_Interpreter_FOUND)
-set(PYTHON_LIBRARIES ${Python3_LIBRARIES})
-set(PYTHON_INCLUDE_DIRS ${Python3_INCLUDE_DIRS})
-set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
-
-# The install name for the Python 3 framework in Xcode is relative to
-# the framework's location and not the dylib itself.
-#
-#   @rpath/Python3.framework/Versions/3.x/Python3
-#
-# This means that we need to compute the path to the Python3.framework
-# and use that as the RPATH instead of the usual dylib's directory.
-#
-# The check below shouldn't match Homebrew's Python framework as it is
-# called Python.framework instead of Python3.framework.
-if (APPLE AND Python3_LIBRARIES MATCHES "Python3.framework")
-  string(FIND "${Python3_LIBRARIES}" "Python3.framework" python_framework_pos)
-  string(SUBSTRING "${Python3_LIBRARIES}" "0" ${python_framework_pos} PYTHON_RPATH)
+  if (LLDB_PYTHON_VERSION)
+if (LLDB_PYTHON_VERSION VERSION_EQUAL "2")
+  FindPython2()
+elseif(LLDB_PYTHON_VERSION VERSION_EQUAL "3")
+  FindPython3()
 endif()
-
-mark_as_advanced(
-  PYTHON_LIBRARIES
-  PYTHON_INCLUDE_DIRS
-  PYTHON_EXE

[Lldb-commits] [PATCH] D81501: [lldb/CMake] Make it possible to build against Python 2 with CMake > 3.12

2020-06-09 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

Would be nice to remove this entirely in favour of CMake's builtin support for 
Python Interpeter and Libraries.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D81501



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


[Lldb-commits] [PATCH] D81501: [lldb/CMake] Make it possible to build against Python 2 with CMake > 3.12

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D81501#2083359 , @compnerd wrote:

> Would be nice to remove this entirely in favour of CMake's builtin support 
> for Python Interpeter and Libraries.


It'll continue to exist to find Python and SWIG atomically, but at least once 
we bump the minimum requirement for CMake to 3.12 this file will be a lot 
simpler.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D81501



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


[Lldb-commits] [PATCH] D81501: [lldb/CMake] Make it possible to build against Python 2 with CMake > 3.12

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c0afacada0d: [lldb/CMake] Add LLDB_PYTHON_VERSION to use 
Python 2 with CMake > 3.12 (authored by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81501

Files:
  lldb/cmake/modules/FindPythonInterpAndLibs.cmake

Index: lldb/cmake/modules/FindPythonInterpAndLibs.cmake
===
--- lldb/cmake/modules/FindPythonInterpAndLibs.cmake
+++ lldb/cmake/modules/FindPythonInterpAndLibs.cmake
@@ -4,54 +4,74 @@
 #
 # Find the python interpreter and libraries as a whole.
 
+macro(FindPython3)
+  # Use PYTHON_HOME as a hint to find Python 3.
+  set(Python3_ROOT_DIR "${PYTHON_HOME}")
+  find_package(Python3 COMPONENTS Interpreter Development)
+  if(Python3_FOUND AND Python3_Interpreter_FOUND)
+set(PYTHON_LIBRARIES ${Python3_LIBRARIES})
+set(PYTHON_INCLUDE_DIRS ${Python3_INCLUDE_DIRS})
+set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
+
+# The install name for the Python 3 framework in Xcode is relative to
+# the framework's location and not the dylib itself.
+#
+#   @rpath/Python3.framework/Versions/3.x/Python3
+#
+# This means that we need to compute the path to the Python3.framework
+# and use that as the RPATH instead of the usual dylib's directory.
+#
+# The check below shouldn't match Homebrew's Python framework as it is
+# called Python.framework instead of Python3.framework.
+if (APPLE AND Python3_LIBRARIES MATCHES "Python3.framework")
+  string(FIND "${Python3_LIBRARIES}" "Python3.framework" python_framework_pos)
+  string(SUBSTRING "${Python3_LIBRARIES}" "0" ${python_framework_pos} PYTHON_RPATH)
+endif()
+
+set(PYTHON3_FOUND TRUE)
+mark_as_advanced(
+  PYTHON_LIBRARIES
+  PYTHON_INCLUDE_DIRS
+  PYTHON_EXECUTABLE
+  PYTHON_RPATH
+  SWIG_EXECUTABLE)
+  endif()
+endmacro()
+
+macro(FindPython2)
+  # Use PYTHON_HOME as a hint to find Python 2.
+  set(Python2_ROOT_DIR "${PYTHON_HOME}")
+  find_package(Python2 COMPONENTS Interpreter Development)
+  if(Python2_FOUND AND Python2_Interpreter_FOUND)
+set(PYTHON_LIBRARIES ${Python2_LIBRARIES})
+set(PYTHON_INCLUDE_DIRS ${Python2_INCLUDE_DIRS})
+set(PYTHON_EXECUTABLE ${Python2_EXECUTABLE})
+
+set(PYTHON2_FOUND TRUE)
+mark_as_advanced(
+  PYTHON_LIBRARIES
+  PYTHON_INCLUDE_DIRS
+  PYTHON_EXECUTABLE
+  SWIG_EXECUTABLE)
+  endif()
+endmacro()
+
 if(PYTHON_LIBRARIES AND PYTHON_INCLUDE_DIRS AND PYTHON_EXECUTABLE AND SWIG_EXECUTABLE)
   set(PYTHONINTERPANDLIBS_FOUND TRUE)
 else()
   find_package(SWIG 2.0)
   if (SWIG_FOUND)
 if(NOT CMAKE_VERSION VERSION_LESS 3.12)
-  # Use PYTHON_HOME as a hint to find Python 3.
-  set(Python3_ROOT_DIR "${PYTHON_HOME}")
-  find_package(Python3 COMPONENTS Interpreter Development)
-  if(Python3_FOUND AND Python3_Interpreter_FOUND)
-set(PYTHON_LIBRARIES ${Python3_LIBRARIES})
-set(PYTHON_INCLUDE_DIRS ${Python3_INCLUDE_DIRS})
-set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
-
-# The install name for the Python 3 framework in Xcode is relative to
-# the framework's location and not the dylib itself.
-#
-#   @rpath/Python3.framework/Versions/3.x/Python3
-#
-# This means that we need to compute the path to the Python3.framework
-# and use that as the RPATH instead of the usual dylib's directory.
-#
-# The check below shouldn't match Homebrew's Python framework as it is
-# called Python.framework instead of Python3.framework.
-if (APPLE AND Python3_LIBRARIES MATCHES "Python3.framework")
-  string(FIND "${Python3_LIBRARIES}" "Python3.framework" python_framework_pos)
-  string(SUBSTRING "${Python3_LIBRARIES}" "0" ${python_framework_pos} PYTHON_RPATH)
+  if (LLDB_PYTHON_VERSION)
+if (LLDB_PYTHON_VERSION VERSION_EQUAL "2")
+  FindPython2()
+elseif(LLDB_PYTHON_VERSION VERSION_EQUAL "3")
+  FindPython3()
 endif()
-
-mark_as_advanced(
-  PYTHON_LIBRARIES
-  PYTHON_INCLUDE_DIRS
-  PYTHON_EXECUTABLE
-  PYTHON_RPATH
-  SWIG_EXECUTABLE)
-  elseif(NOT CMAKE_SYSTEM_NAME STREQUAL Windows)
-# Use PYTHON_HOME as a hint to find Python 2.
-set(Python2_ROOT_DIR "${PYTHON_HOME}")
-find_package(Python2 COMPONENTS Interpreter Development)
-if(Python2_FOUND AND Python2_Interpreter_FOUND)
-  set(PYTHON_LIBRARIES ${Python2_LIBRARIES})
-  set(PYTHON_INCLUDE_DIRS ${Python2_INCLUDE_DIRS})
-  set(PYTHON_EXECUTABLE ${Python2_EXECUTABLE})
-  mark_as_advanced(
-PYTHON_LIBRARIES
-PYTHON_INCLUDE_DIRS
-PYTHON_EXECUTABLE
-SWIG_EXECUTABLE)
+  else()
+FindPython3()

[Lldb-commits] [lldb] 7dd86c9 - [lldb/Reproducers] Skip test_remove_placeholder_add_real_module with reproducers

2020-06-09 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-06-09T16:17:53-07:00
New Revision: 7dd86c9e7caa8f226e48b59caf9f0a38789ce164

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

LOG: [lldb/Reproducers] Skip test_remove_placeholder_add_real_module with 
reproducers

Modules are not orphaned and it finds the existing module with the same
UUID from test_partial_uuid_match.

Added: 


Modified: 
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py 
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
index 8cb7de9714e3..cc6d6fb37cae 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -252,6 +252,7 @@ def test_add_module_build_id_4(self):
 "a", "", "01020304-0506-0708-090A-0B0C0D0E0F10").IsValid())
 self.assertFalse(self.target.AddModule("a", "", "01020305").IsValid())
 
+@skipIfReproducer # Modules are not orphaned and it finds the module with 
the same UUID from test_partial_uuid_match.
 def test_remove_placeholder_add_real_module(self):
 """
 Test that removing a placeholder module and adding back the real



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


[Lldb-commits] [PATCH] D81516: [lldb/Test] Ensure inline tests have a unique build directory

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, vsk.
Herald added a subscriber: aprantl.
JDevlieghere added a comment.

I discovered this because `TestBasicEntryValues.py` was failing with the 
reproducers. Because the paths to the binaries were the same, we'd end up with 
only the GNU variant of the binary in the VFS.


Inline tests have one method named 'test' which means that multiple inline 
tests in the same file end up sharing the same build directory per variant.

The output bellow illustrates that even though `BasicEntryValues_GNU` and 
`BasicEntryValues_V5` have unique names, the test method for the dwarf variant 
is called `test_dwarf` in both cases.

  PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_dwarf 
(lldbsuite.test.lldbtest.BasicEntryValues_GNU)
  PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_gmodules 
(lldbsuite.test.lldbtest.BasicEntryValues_GNU)
  PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_dsym 
(lldbsuite.test.lldbtest.BasicEntryValues_V5)
  PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_dwarf 
(lldbsuite.test.lldbtest.BasicEntryValues_V5)
  PASS: LLDB (/Users/jonas/llvm/build-ra/bin/clang-11-x86_64) :: test_gmodules 
(lldbsuite.test.lldbtest.BasicEntryValues_V5)

The '__inline_name__' attribute ensures that the (unique) test name is used 
instead.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D81516

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


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1743,6 +1743,15 @@
 if original_testcase.NO_DEBUG_INFO_TESTCASE:
 return original_testcase
 
+# Inline tests have one method named 'test' which means that multiple
+# inline tests in the same file end up sharing the same build directory
+# per variant. The '__inline_name__' attribute ensures that the
+# (unique) test name is used instead.
+inline_name = None
+for attrname, attrvalue in attrs.items():
+if attrname == "__inline_name__":
+inline_name = "test_" + attrvalue
+
 newattrs = {}
 for attrname, attrvalue in attrs.items():
 if attrname.startswith("test") and not getattr(
@@ -1766,7 +1775,8 @@
 def test_method(self, attrvalue=attrvalue):
 return attrvalue(self)
 
-method_name = attrname + "_" + cat
+test_name = inline_name if inline_name else attrname
+method_name = test_name + "_" + cat
 test_method.__name__ = method_name
 test_method.debug_info = cat
 newattrs[method_name] = test_method
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -206,7 +206,7 @@
 test_func = ApplyDecoratorsToFunction(InlineTest._test, decorators)
 # Build the test case
 test_class = type(name, (InlineTest,), dict(test=test_func,
-name=name, _build_dict=build_dict))
+name=name, __inline_name__=name, _build_dict=build_dict))
 
 # Add the test case to the globals, and hide InlineTest
 __globals.update({name: test_class})


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1743,6 +1743,15 @@
 if original_testcase.NO_DEBUG_INFO_TESTCASE:
 return original_testcase
 
+# Inline tests have one method named 'test' which means that multiple
+# inline tests in the same file end up sharing the same build directory
+# per variant. The '__inline_name__' attribute ensures that the
+# (unique) test name is used instead.
+inline_name = None
+for attrname, attrvalue in attrs.items():
+if attrname == "__inline_name__":
+inline_name = "test_" + attrvalue
+
 newattrs = {}
 for attrname, attrvalue in attrs.items():
 if attrname.startswith("test") and not getattr(
@@ -1766,7 +1775,8 @@
 def test_method(self, attrvalue=attrvalue):
 return attrvalue(self)
 
-method_name = attrname + "_" + cat
+test_name = inline_name if inline_name else attrname
+method_name = test_name + "_" + cat
 test_method.__name__ = method_name
 test_method.debug_inf

[Lldb-commits] [PATCH] D81516: [lldb/Test] Ensure inline tests have a unique build directory

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

I discovered this because `TestBasicEntryValues.py` was failing with the 
reproducers. Because the paths to the binaries were the same, we'd end up with 
only the GNU variant of the binary in the VFS.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D81516



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


[Lldb-commits] [PATCH] D81516: [lldb/Test] Ensure inline tests have a unique build directory

2020-06-09 Thread Vedant Kumar via Phabricator via lldb-commits
vsk added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1750
+# (unique) test name is used instead.
+inline_name = None
+for attrname, attrvalue in attrs.items():

Is `inline_suffix = attrs.get('__inline_name__')` no good?



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1778
 
-method_name = attrname + "_" + cat
+test_name = inline_name if inline_name else attrname
+method_name = test_name + "_" + cat

Would it be all right to unconditionally set `method_name = name + attrname + 
cat`?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D81516



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


[Lldb-commits] [PATCH] D81516: [lldb/Test] Ensure inline tests have a unique build directory

2020-06-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 269713.
JDevlieghere marked 2 inline comments as done.

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

https://reviews.llvm.org/D81516

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


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1743,6 +1743,12 @@
 if original_testcase.NO_DEBUG_INFO_TESTCASE:
 return original_testcase
 
+# Inline tests have one method named 'test' which means that multiple
+# inline tests in the same file end up sharing the same build directory
+# per variant. The '__inline_name__' attribute ensures that the
+# (unique) test name is used instead.
+inline_name = attrs.get('__inline_name__')
+
 newattrs = {}
 for attrname, attrvalue in attrs.items():
 if attrname.startswith("test") and not getattr(
@@ -1766,7 +1772,7 @@
 def test_method(self, attrvalue=attrvalue):
 return attrvalue(self)
 
-method_name = attrname + "_" + cat
+method_name = '_'.join(filter(None, [attrname, 
inline_name, cat]))
 test_method.__name__ = method_name
 test_method.debug_info = cat
 newattrs[method_name] = test_method
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -206,7 +206,7 @@
 test_func = ApplyDecoratorsToFunction(InlineTest._test, decorators)
 # Build the test case
 test_class = type(name, (InlineTest,), dict(test=test_func,
-name=name, _build_dict=build_dict))
+name=name, __inline_name__=name, _build_dict=build_dict))
 
 # Add the test case to the globals, and hide InlineTest
 __globals.update({name: test_class})


Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1743,6 +1743,12 @@
 if original_testcase.NO_DEBUG_INFO_TESTCASE:
 return original_testcase
 
+# Inline tests have one method named 'test' which means that multiple
+# inline tests in the same file end up sharing the same build directory
+# per variant. The '__inline_name__' attribute ensures that the
+# (unique) test name is used instead.
+inline_name = attrs.get('__inline_name__')
+
 newattrs = {}
 for attrname, attrvalue in attrs.items():
 if attrname.startswith("test") and not getattr(
@@ -1766,7 +1772,7 @@
 def test_method(self, attrvalue=attrvalue):
 return attrvalue(self)
 
-method_name = attrname + "_" + cat
+method_name = '_'.join(filter(None, [attrname, inline_name, cat]))
 test_method.__name__ = method_name
 test_method.debug_info = cat
 newattrs[method_name] = test_method
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -206,7 +206,7 @@
 test_func = ApplyDecoratorsToFunction(InlineTest._test, decorators)
 # Build the test case
 test_class = type(name, (InlineTest,), dict(test=test_func,
-name=name, _build_dict=build_dict))
+name=name, __inline_name__=name, _build_dict=build_dict))
 
 # Add the test case to the globals, and hide InlineTest
 __globals.update({name: test_class})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits